Skip to content

Commit ffa2e46

Browse files
tglsfdctglsfdc
committed
In libpq, always append new error messages to conn->errorMessage.
Previously, we had an undisciplined mish-mash of printfPQExpBuffer and appendPQExpBuffer calls to report errors within libpq. This commit establishes a uniform rule that appendPQExpBuffer[Str] should be used. conn->errorMessage is reset only at the start of an application request, and then accumulates messages till we're done. We can remove no less than three different ad-hoc mechanisms that were used to get the effect of concatenation of error messages within a sequence of operations. Although this makes things quite a bit cleaner conceptually, the main reason to do it is to make the world safer for the multiple-target-host feature that was added awhile back. Previously, there were many cases in which an error occurring during an individual host connection attempt would wipe out the record of what had happened during previous attempts. (The reporting is still inadequate, in that it can be hard to tell which host got the failure, but that seems like a matter for a separate commit.) Currently, lo_import and lo_export contain exceptions to the "never use printfPQExpBuffer" rule. If we changed them, we'd risk reporting an incidental lo_close failure before the actual read or write failure, which would be confusing, not least because lo_close happened after the main failure. We could improve this by inventing an internal version of lo_close that doesn't reset the errorMessage; but we'd also need a version of PQfn() that does that, and it didn't quite seem worth the trouble for now. Discussion: https://postgr.es/m/BN6PR05MB3492948E4FD76C156E747E8BC9160@BN6PR05MB3492.namprd05.prod.outlook.com
1 parent ce6a71f commit ffa2e46

14 files changed

+699
-774
lines changed

‎src/interfaces/libpq/fe-auth-scram.c

+67-57
Original file line numberDiff line numberDiff line change
@@ -208,14 +208,14 @@ pg_fe_scram_exchange(void *opaq, char *input, int inputlen,
208208
{
209209
if (inputlen == 0)
210210
{
211-
printfPQExpBuffer(&conn->errorMessage,
212-
libpq_gettext("malformed SCRAM message (empty message)\n"));
211+
appendPQExpBufferStr(&conn->errorMessage,
212+
libpq_gettext("malformed SCRAM message (empty message)\n"));
213213
goto error;
214214
}
215215
if (inputlen != strlen(input))
216216
{
217-
printfPQExpBuffer(&conn->errorMessage,
218-
libpq_gettext("malformed SCRAM message (length mismatch)\n"));
217+
appendPQExpBufferStr(&conn->errorMessage,
218+
libpq_gettext("malformed SCRAM message (length mismatch)\n"));
219219
goto error;
220220
}
221221
}
@@ -258,24 +258,24 @@ pg_fe_scram_exchange(void *opaq, char *input, int inputlen,
258258
*/
259259
if (!verify_server_signature(state, success))
260260
{
261-
printfPQExpBuffer(&conn->errorMessage,
262-
libpq_gettext("could not verify server signature\n"));
261+
appendPQExpBufferStr(&conn->errorMessage,
262+
libpq_gettext("could not verify server signature\n"));
263263
goto error;
264264
}
265265

266266
if (!*success)
267267
{
268-
printfPQExpBuffer(&conn->errorMessage,
269-
libpq_gettext("incorrect server signature\n"));
268+
appendPQExpBufferStr(&conn->errorMessage,
269+
libpq_gettext("incorrect server signature\n"));
270270
}
271271
*done = true;
272272
state->state = FE_SCRAM_FINISHED;
273273
break;
274274

275275
default:
276276
/* shouldn't happen */
277-
printfPQExpBuffer(&conn->errorMessage,
278-
libpq_gettext("invalid SCRAM exchange state\n"));
277+
appendPQExpBufferStr(&conn->errorMessage,
278+
libpq_gettext("invalid SCRAM exchange state\n"));
279279
goto error;
280280
}
281281
return;
@@ -287,6 +287,11 @@ pg_fe_scram_exchange(void *opaq, char *input, int inputlen,
287287

288288
/*
289289
* Read value for an attribute part of a SCRAM message.
290+
*
291+
* The buffer at **input is destructively modified, and *input is
292+
* advanced over the "attr=value" string and any following comma.
293+
*
294+
* On failure, append an error message to *errorMessage and return NULL.
290295
*/
291296
static char *
292297
read_attr_value(char **input, char attr, PQExpBuffer errorMessage)
@@ -296,7 +301,7 @@ read_attr_value(char **input, char attr, PQExpBuffer errorMessage)
296301

297302
if (*begin != attr)
298303
{
299-
printfPQExpBuffer(errorMessage,
304+
appendPQExpBuffer(errorMessage,
300305
libpq_gettext("malformed SCRAM message (attribute \"%c\" expected)\n"),
301306
attr);
302307
return NULL;
@@ -305,7 +310,7 @@ read_attr_value(char **input, char attr, PQExpBuffer errorMessage)
305310

306311
if (*begin != '=')
307312
{
308-
printfPQExpBuffer(errorMessage,
313+
appendPQExpBuffer(errorMessage,
309314
libpq_gettext("malformed SCRAM message (expected character \"=\" for attribute \"%c\")\n"),
310315
attr);
311316
return NULL;
@@ -346,8 +351,8 @@ build_client_first_message(fe_scram_state *state)
346351
*/
347352
if (!pg_strong_random(raw_nonce, SCRAM_RAW_NONCE_LEN))
348353
{
349-
printfPQExpBuffer(&conn->errorMessage,
350-
libpq_gettext("could not generate nonce\n"));
354+
appendPQExpBufferStr(&conn->errorMessage,
355+
libpq_gettext("could not generate nonce\n"));
351356
return NULL;
352357
}
353358

@@ -356,16 +361,16 @@ build_client_first_message(fe_scram_state *state)
356361
state->client_nonce = malloc(encoded_len + 1);
357362
if (state->client_nonce == NULL)
358363
{
359-
printfPQExpBuffer(&conn->errorMessage,
360-
libpq_gettext("out of memory\n"));
364+
appendPQExpBufferStr(&conn->errorMessage,
365+
libpq_gettext("out of memory\n"));
361366
return NULL;
362367
}
363368
encoded_len = pg_b64_encode(raw_nonce, SCRAM_RAW_NONCE_LEN,
364369
state->client_nonce, encoded_len);
365370
if (encoded_len < 0)
366371
{
367-
printfPQExpBuffer(&conn->errorMessage,
368-
libpq_gettext("could not encode nonce\n"));
372+
appendPQExpBufferStr(&conn->errorMessage,
373+
libpq_gettext("could not encode nonce\n"));
369374
return NULL;
370375
}
371376
state->client_nonce[encoded_len] = '\0';
@@ -431,8 +436,8 @@ build_client_first_message(fe_scram_state *state)
431436

432437
oom_error:
433438
termPQExpBuffer(&buf);
434-
printfPQExpBuffer(&conn->errorMessage,
435-
libpq_gettext("out of memory\n"));
439+
appendPQExpBufferStr(&conn->errorMessage,
440+
libpq_gettext("out of memory\n"));
436441
return NULL;
437442
}
438443

@@ -508,8 +513,8 @@ build_client_final_message(fe_scram_state *state)
508513
free(cbind_data);
509514
free(cbind_input);
510515
termPQExpBuffer(&buf);
511-
printfPQExpBuffer(&conn->errorMessage,
512-
"could not encode cbind data for channel binding\n");
516+
appendPQExpBufferStr(&conn->errorMessage,
517+
"could not encode cbind data for channel binding\n");
513518
return NULL;
514519
}
515520
buf.len += encoded_cbind_len;
@@ -523,8 +528,8 @@ build_client_final_message(fe_scram_state *state)
523528
* Shouldn't happen.
524529
*/
525530
termPQExpBuffer(&buf);
526-
printfPQExpBuffer(&conn->errorMessage,
527-
"channel binding not supported by this build\n");
531+
appendPQExpBufferStr(&conn->errorMessage,
532+
"channel binding not supported by this build\n");
528533
return NULL;
529534
#endif /* HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH */
530535
}
@@ -553,8 +558,8 @@ build_client_final_message(fe_scram_state *state)
553558
client_proof))
554559
{
555560
termPQExpBuffer(&buf);
556-
printfPQExpBuffer(&conn->errorMessage,
557-
libpq_gettext("could not calculate client proof\n"));
561+
appendPQExpBufferStr(&conn->errorMessage,
562+
libpq_gettext("could not calculate client proof\n"));
558563
return NULL;
559564
}
560565

@@ -569,8 +574,8 @@ build_client_final_message(fe_scram_state *state)
569574
if (encoded_len < 0)
570575
{
571576
termPQExpBuffer(&buf);
572-
printfPQExpBuffer(&conn->errorMessage,
573-
libpq_gettext("could not encode client proof\n"));
577+
appendPQExpBufferStr(&conn->errorMessage,
578+
libpq_gettext("could not encode client proof\n"));
574579
return NULL;
575580
}
576581
buf.len += encoded_len;
@@ -585,8 +590,8 @@ build_client_final_message(fe_scram_state *state)
585590

586591
oom_error:
587592
termPQExpBuffer(&buf);
588-
printfPQExpBuffer(&conn->errorMessage,
589-
libpq_gettext("out of memory\n"));
593+
appendPQExpBufferStr(&conn->errorMessage,
594+
libpq_gettext("out of memory\n"));
590595
return NULL;
591596
}
592597

@@ -606,8 +611,8 @@ read_server_first_message(fe_scram_state *state, char *input)
606611
state->server_first_message = strdup(input);
607612
if (state->server_first_message == NULL)
608613
{
609-
printfPQExpBuffer(&conn->errorMessage,
610-
libpq_gettext("out of memory\n"));
614+
appendPQExpBufferStr(&conn->errorMessage,
615+
libpq_gettext("out of memory\n"));
611616
return false;
612617
}
613618

@@ -616,39 +621,39 @@ read_server_first_message(fe_scram_state *state, char *input)
616621
&conn->errorMessage);
617622
if (nonce == NULL)
618623
{
619-
/* read_attr_value() has generated an error string */
624+
/* read_attr_value() has appended an error string */
620625
return false;
621626
}
622627

623628
/* Verify immediately that the server used our part of the nonce */
624629
if (strlen(nonce) < strlen(state->client_nonce) ||
625630
memcmp(nonce, state->client_nonce, strlen(state->client_nonce)) != 0)
626631
{
627-
printfPQExpBuffer(&conn->errorMessage,
628-
libpq_gettext("invalid SCRAM response (nonce mismatch)\n"));
632+
appendPQExpBufferStr(&conn->errorMessage,
633+
libpq_gettext("invalid SCRAM response (nonce mismatch)\n"));
629634
return false;
630635
}
631636

632637
state->nonce = strdup(nonce);
633638
if (state->nonce == NULL)
634639
{
635-
printfPQExpBuffer(&conn->errorMessage,
636-
libpq_gettext("out of memory\n"));
640+
appendPQExpBufferStr(&conn->errorMessage,
641+
libpq_gettext("out of memory\n"));
637642
return false;
638643
}
639644

640645
encoded_salt = read_attr_value(&input, 's', &conn->errorMessage);
641646
if (encoded_salt == NULL)
642647
{
643-
/* read_attr_value() has generated an error string */
648+
/* read_attr_value() has appended an error string */
644649
return false;
645650
}
646651
decoded_salt_len = pg_b64_dec_len(strlen(encoded_salt));
647652
state->salt = malloc(decoded_salt_len);
648653
if (state->salt == NULL)
649654
{
650-
printfPQExpBuffer(&conn->errorMessage,
651-
libpq_gettext("out of memory\n"));
655+
appendPQExpBufferStr(&conn->errorMessage,
656+
libpq_gettext("out of memory\n"));
652657
return false;
653658
}
654659
state->saltlen = pg_b64_decode(encoded_salt,
@@ -657,28 +662,28 @@ read_server_first_message(fe_scram_state *state, char *input)
657662
decoded_salt_len);
658663
if (state->saltlen < 0)
659664
{
660-
printfPQExpBuffer(&conn->errorMessage,
661-
libpq_gettext("malformed SCRAM message (invalid salt)\n"));
665+
appendPQExpBufferStr(&conn->errorMessage,
666+
libpq_gettext("malformed SCRAM message (invalid salt)\n"));
662667
return false;
663668
}
664669

665670
iterations_str = read_attr_value(&input, 'i', &conn->errorMessage);
666671
if (iterations_str == NULL)
667672
{
668-
/* read_attr_value() has generated an error string */
673+
/* read_attr_value() has appended an error string */
669674
return false;
670675
}
671676
state->iterations = strtol(iterations_str, &endptr, 10);
672677
if (*endptr != '\0' || state->iterations < 1)
673678
{
674-
printfPQExpBuffer(&conn->errorMessage,
675-
libpq_gettext("malformed SCRAM message (invalid iteration count)\n"));
679+
appendPQExpBufferStr(&conn->errorMessage,
680+
libpq_gettext("malformed SCRAM message (invalid iteration count)\n"));
676681
return false;
677682
}
678683

679684
if (*input != '\0')
680-
printfPQExpBuffer(&conn->errorMessage,
681-
libpq_gettext("malformed SCRAM message (garbage at end of server-first-message)\n"));
685+
appendPQExpBufferStr(&conn->errorMessage,
686+
libpq_gettext("malformed SCRAM message (garbage at end of server-first-message)\n"));
682687

683688
return true;
684689
}
@@ -697,8 +702,8 @@ read_server_final_message(fe_scram_state *state, char *input)
697702
state->server_final_message = strdup(input);
698703
if (!state->server_final_message)
699704
{
700-
printfPQExpBuffer(&conn->errorMessage,
701-
libpq_gettext("out of memory\n"));
705+
appendPQExpBufferStr(&conn->errorMessage,
706+
libpq_gettext("out of memory\n"));
702707
return false;
703708
}
704709

@@ -708,7 +713,12 @@ read_server_final_message(fe_scram_state *state, char *input)
708713
char *errmsg = read_attr_value(&input, 'e',
709714
&conn->errorMessage);
710715

711-
printfPQExpBuffer(&conn->errorMessage,
716+
if (errmsg == NULL)
717+
{
718+
/* read_attr_value() has appended an error message */
719+
return false;
720+
}
721+
appendPQExpBuffer(&conn->errorMessage,
712722
libpq_gettext("error received from server in SCRAM exchange: %s\n"),
713723
errmsg);
714724
return false;
@@ -719,20 +729,20 @@ read_server_final_message(fe_scram_state *state, char *input)
719729
&conn->errorMessage);
720730
if (encoded_server_signature == NULL)
721731
{
722-
/* read_attr_value() has generated an error message */
732+
/* read_attr_value() has appended an error message */
723733
return false;
724734
}
725735

726736
if (*input != '\0')
727-
printfPQExpBuffer(&conn->errorMessage,
728-
libpq_gettext("malformed SCRAM message (garbage at end of server-final-message)\n"));
737+
appendPQExpBufferStr(&conn->errorMessage,
738+
libpq_gettext("malformed SCRAM message (garbage at end of server-final-message)\n"));
729739

730740
server_signature_len = pg_b64_dec_len(strlen(encoded_server_signature));
731741
decoded_server_signature = malloc(server_signature_len);
732742
if (!decoded_server_signature)
733743
{
734-
printfPQExpBuffer(&conn->errorMessage,
735-
libpq_gettext("out of memory\n"));
744+
appendPQExpBufferStr(&conn->errorMessage,
745+
libpq_gettext("out of memory\n"));
736746
return false;
737747
}
738748

@@ -743,8 +753,8 @@ read_server_final_message(fe_scram_state *state, char *input)
743753
if (server_signature_len != SCRAM_KEY_LEN)
744754
{
745755
free(decoded_server_signature);
746-
printfPQExpBuffer(&conn->errorMessage,
747-
libpq_gettext("malformed SCRAM message (invalid server signature)\n"));
756+
appendPQExpBufferStr(&conn->errorMessage,
757+
libpq_gettext("malformed SCRAM message (invalid server signature)\n"));
748758
return false;
749759
}
750760
memcpy(state->ServerSignature, decoded_server_signature, SCRAM_KEY_LEN);

0 commit comments

Comments
 (0)