Обсуждение: libpq should append auth failures, not overwrite
I noticed that, although most error reports during libpq's connection
setup code append to conn->errorMessage, the ones in fe-auth.c and
fe-auth-scram.c don't: they're all printfPQExpBuffer() not
appendPQExpBuffer(). This seems wrong to me. It makes no difference
in simple cases with a single target server, but as soon as you have
multiple servers listed in "host", this coding makes it impossible
to tell which server rejected your login.
So I think we should basically s/printfPQExpBuffer/appendPQExpBuffer/g
anywhere those files touch conn->errorMessage, allowing any problems
with previous servers to be preserved in the eventually-reported message.
I won't bother posting an actual patch for that right now, but has anyone
got an objection?
regards, tom lane
On Thu, Aug 09, 2018 at 11:44:27AM -0400, Tom Lane wrote: > I noticed that, although most error reports during libpq's connection > setup code append to conn->errorMessage, the ones in fe-auth.c and > fe-auth-scram.c don't: they're all printfPQExpBuffer() not > appendPQExpBuffer(). This seems wrong to me. It makes no difference > in simple cases with a single target server, but as soon as you have > multiple servers listed in "host", this coding makes it impossible > to tell which server rejected your login. +1. -- Michael
Вложения
Michael Paquier <michael@paquier.xyz> writes:
> On Thu, Aug 09, 2018 at 11:44:27AM -0400, Tom Lane wrote:
>> I noticed that, although most error reports during libpq's connection
>> setup code append to conn->errorMessage, the ones in fe-auth.c and
>> fe-auth-scram.c don't: they're all printfPQExpBuffer() not
>> appendPQExpBuffer(). This seems wrong to me. It makes no difference
>> in simple cases with a single target server, but as soon as you have
>> multiple servers listed in "host", this coding makes it impossible
>> to tell which server rejected your login.
> +1.
So I thought this would be a trivial patch barely even worthy of posting,
but an awful lot of critters crawled out from under that rock. It's not
just fe-auth*.c at fault; low-level failures such as timeout errors were
also clearing conn->errorMessage, and if you got an actual server error
(like "role does not exist"), that did too. What I ended up doing was a
wholesale conversion of libpq's management of conn->errorMessage. Now,
there are a few identified points at connection start or query start that
explicitly clear errorMessage, and everyplace else in libpq is supposed to
append to it, never just printf onto it. (Interestingly, all of those
"identified points" already did clear errorMessage, meaning that a large
fraction of the existing printf's would always have started with an empty
errorMessage anyway.)
The first patch attached does that, and then the second one is a proposed
modification in the way we report failures for multi-host connection
attempts: let's explicitly label each section of conn->errorMessage with
the host we're trying to connect to. This can replace the ad-hoc labeling
that somebody thought would be a good idea for two errors associated with
the target_session_attrs=read-write feature. (That's not a bad idea in
itself, but doing it only for those two errors is.)
Here's some examples of the kind of connection failure reports the code
can produce with these patches:
$ psql "host=refusenik,dead,localhost user=readonly dbname=postgres connect_timeout=3 target_session_attrs=read-write"
psql: server "refusenik" port 5432:
could not connect to server: Connection refused
Is the server running on host "refusenik" (192.168.1.43) and accepting
TCP/IP connections on port 5432?
server "dead" port 5432:
timeout expired
server "localhost" port 5432:
could not open a read-write session
$ psql "host=refusenik,dead,localhost user=nobody dbname=postgres"
psql: server "refusenik" port 5432:
could not connect to server: Connection refused
Is the server running on host "refusenik" (192.168.1.43) and accepting
TCP/IP connections on port 5432?
server "dead" port 5432:
timeout expired
server "localhost" port 5432:
FATAL: role "nobody" does not exist
Before, you'd get a rather random subset of these messages depending on
which parts of the code decided to clear conn->errorMessage, and in
many cases it'd be really hard to tell which server was involved with
which message(s).
Some points for discussion and review:
1. The bulk of patch 0001 is indeed just
s/printfPQExpBuffer/appendPQExpBuffer/g, though I also made an attempt
to use appendPQExpBufferStr wherever possible. There are two categories
of printfPQExpBuffer calls I didn't change:
1a. Calls reporting an out-of-memory situation. There was already a
policy in some places that we'd intentionally printf not append such
messages, and I made that explicit. The idea is that we might not
have room to add more text to errorMessage, so resetting the buffer
provides more certainty of success. However, this makes for a pretty
weird inconsistency in the code; there are a lot of functions now in
which half of the messages are printf'd and half are append'd, so I'm
afraid that future patches will get it wrong as to which to use. Also,
in reality there often *would* be room to append "out of memory" without
enlarging errorMessage's buffer, so that this could just be pointless
destruction of useful error history. I didn't do anything about it here,
but I'm tempted to replace all the printf's for OOM messages with a
subroutine that will append if it can do so without enlarging the existing
buffer, else replace.
1b. There are a few places where it didn't seem worth changing anything
because it'd just result in needing to add a resetPQExpBuffer in the same
function or very nearby. Mostly in fe-lobj.c.
2. I had to drop some code (notably in pqPrepareAsyncResult) that
attempted to force conn->errorMessage to always have the same contents
as the current PGresult's PQresultErrorMessage; as it stood, server
errors such as "role does not exist" wiped out preceding contents of
errorMessage, breaking cases such as the second example above. This is
slightly annoying, but in the new dispensation it's possible that
conn->errorMessage contains a mix of libpq-generated errors and text
received from the server, so I'm not sure that preserving the old
equivalence is a useful goal anyway. We shouldn't cram pre-existing
errorMessage text back into a server-generated error; that would
invalidate the server's auxiliary error info such as the SQLSTATE.
I don't know if it's possible to do better here. One idea is to tweak
pqPrepareAsyncResult so that it does the message synchronization only
when in CONNECTION_OK state, allowing conn->errorMessage to be a
historical record only for connection processing. But that seems like
a hack.
3. In some places I failed to resist the temptation to copy-edit error
messages that didn't meet style guidelines, or to add libpq_gettext()
annotation where it seemed to have been missed.
4. connectDBComplete() cleared errorMessage after a successful
completion, but that's the wrong place to do it; we have to do that
at the success exits from PQconnectPoll, else applications that
use PQconnectPoll directly will see leftover garbage there.
5. There were places in PQconnectPoll that temporarily set
conn->status = CONNECTION_OK to prevent PQsendQueryStart from
complaining. A better idea is to leave the status alone and change
that test in PQsendQueryStart to be status != CONNECTION_BAD. This
not only simplifies PQconnectPoll, but allows PQsendQueryStart
to tell whether we're still in the connection sequence. I fixed
it to not clear errorMessage when we are, thus solving the problem
of PQsendQuery destroying prior connection error history. That allows
getting rid of saveErrorMessage/restoreErrorMessage, which were
(a) an incredibly ugly and inefficient hack, and (b) inadequate,
since there were other paths through PQconnectPoll that could reach
PQsendQueryStart but were not protected with calls to those.
6. In the 0002 patch, I have it printing the annotation only when
there's more than one connhost[], and only once per connhost not
once per IP address. The motivation for doing it like that was to
not change the behavior for simple cases with only one host name.
There is certainly room to argue that we should make an annotation
once per IP address, but that'd change the behavior for pretty common
cases like "localhost" resolving to both 127.0.0.1 and ::1, so I don't
know if it'd be a good idea or not. There's also an interaction to
consider with my patch at
https://www.postgresql.org/message-id/4913.1533827102%40sss.pgh.pa.us
namely that we have to be sure the labeling behaves sanely for
connhosts that fail to resolve any IP addresses.
I'll put this into the September commitfest.
regards, tom lane
diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c
index 1d9c937..0d2b590 100644
--- a/src/interfaces/libpq/fe-auth-scram.c
+++ b/src/interfaces/libpq/fe-auth-scram.c
@@ -185,14 +185,14 @@ pg_fe_scram_exchange(void *opaq, char *input, int inputlen,
{
if (inputlen == 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("malformed SCRAM message (empty message)\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("malformed SCRAM message (empty message)\n"));
goto error;
}
if (inputlen != strlen(input))
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("malformed SCRAM message (length mismatch)\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("malformed SCRAM message (length mismatch)\n"));
goto error;
}
}
@@ -240,17 +240,17 @@ pg_fe_scram_exchange(void *opaq, char *input, int inputlen,
else
{
*success = false;
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("incorrect server signature\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("incorrect server signature\n"));
}
*done = true;
state->state = FE_SCRAM_FINISHED;
break;
default:
- /* shouldn't happen */
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("invalid SCRAM exchange state\n"));
+ /* shouldn't happen, so we don't translate the msg */
+ appendPQExpBufferStr(&conn->errorMessage,
+ "invalid SCRAM exchange state\n");
goto error;
}
return;
@@ -272,7 +272,7 @@ read_attr_value(char **input, char attr, PQExpBuffer errorMessage)
if (*begin != attr)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("malformed SCRAM message (attribute \"%c\" expected)\n"),
attr);
return NULL;
@@ -281,7 +281,7 @@ read_attr_value(char **input, char attr, PQExpBuffer errorMessage)
if (*begin != '=')
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("malformed SCRAM message (expected character \"=\" for attribute \"%c\")\n"),
attr);
return NULL;
@@ -322,8 +322,8 @@ build_client_first_message(fe_scram_state *state)
*/
if (!pg_frontend_random(raw_nonce, SCRAM_RAW_NONCE_LEN))
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("could not generate nonce\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("could not generate nonce\n"));
return NULL;
}
@@ -471,13 +471,13 @@ build_client_final_message(fe_scram_state *state)
#else
/*
* Chose channel binding, but the SSL library doesn't support it.
- * Shouldn't happen.
+ * Shouldn't happen, so we don't translate the msg.
*/
termPQExpBuffer(&buf);
- printfPQExpBuffer(&conn->errorMessage,
- "channel binding not supported by this build\n");
+ appendPQExpBufferStr(&conn->errorMessage,
+ "channel binding not supported by this build\n");
return NULL;
-#endif /* HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH */
+#endif /* HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH */
}
#ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
else if (conn->ssl_in_use)
@@ -557,8 +557,8 @@ read_server_first_message(fe_scram_state *state, char *input)
if (strlen(nonce) < strlen(state->client_nonce) ||
memcmp(nonce, state->client_nonce, strlen(state->client_nonce)) != 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("invalid SCRAM response (nonce mismatch)\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("invalid SCRAM response (nonce mismatch)\n"));
return false;
}
@@ -596,14 +596,14 @@ read_server_first_message(fe_scram_state *state, char *input)
state->iterations = strtol(iterations_str, &endptr, 10);
if (*endptr != '\0' || state->iterations < 1)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("malformed SCRAM message (invalid iteration count)\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("malformed SCRAM message (invalid iteration count)\n"));
return false;
}
if (*input != '\0')
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("malformed SCRAM message (garbage at end of server-first-message)\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("malformed SCRAM message (garbage at end of server-first-message)\n"));
return true;
}
@@ -632,7 +632,7 @@ read_server_final_message(fe_scram_state *state, char *input)
char *errmsg = read_attr_value(&input, 'e',
&conn->errorMessage);
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("error received from server in SCRAM exchange: %s\n"),
errmsg);
return false;
@@ -648,16 +648,16 @@ read_server_final_message(fe_scram_state *state, char *input)
}
if (*input != '\0')
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("malformed SCRAM message (garbage at end of server-final-message)\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("malformed SCRAM message (garbage at end of server-final-message)\n"));
server_signature_len = pg_b64_decode(encoded_server_signature,
strlen(encoded_server_signature),
state->ServerSignature);
if (server_signature_len != SCRAM_KEY_LEN)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("malformed SCRAM message (invalid server signature)\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("malformed SCRAM message (invalid server signature)\n"));
return false;
}
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 540aba9..aa44cef 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -81,14 +81,12 @@ pg_GSS_error_int(PQExpBuffer str, const char *mprefix,
}
/*
- * GSSAPI errors contain two parts; put both into conn->errorMessage.
+ * GSSAPI errors contain two parts; append both to conn->errorMessage.
*/
static void
pg_GSS_error(const char *mprefix, PGconn *conn,
OM_uint32 maj_stat, OM_uint32 min_stat)
{
- resetPQExpBuffer(&conn->errorMessage);
-
/* Fetch major error codes */
pg_GSS_error_int(&conn->errorMessage, mprefix, maj_stat, GSS_C_GSS_CODE);
@@ -203,15 +201,15 @@ pg_GSS_startup(PGconn *conn, int payloadlen)
if (!(host && host[0] != '\0'))
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("host name must be specified\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("host name must be specified\n"));
return STATUS_ERROR;
}
if (conn->gctx)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("duplicate GSS authentication request\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("duplicate GSS authentication request\n"));
return STATUS_ERROR;
}
@@ -268,10 +266,10 @@ pg_SSPI_error(PGconn *conn, const char *mprefix, SECURITY_STATUS r)
FORMAT_MESSAGE_FROM_SYSTEM,
NULL, r, 0,
sysmsg, sizeof(sysmsg), NULL) == 0)
- printfPQExpBuffer(&conn->errorMessage, "%s: SSPI error %x\n",
+ appendPQExpBuffer(&conn->errorMessage, "%s: SSPI error %x\n",
mprefix, (unsigned int) r);
else
- printfPQExpBuffer(&conn->errorMessage, "%s: %s (%x)\n",
+ appendPQExpBuffer(&conn->errorMessage, "%s: %s (%x)\n",
mprefix, sysmsg, (unsigned int) r);
}
@@ -376,9 +374,11 @@ pg_SSPI_continue(PGconn *conn, int payloadlen)
/*
* This should never happen, at least not for Kerberos
* authentication. Keep check in case it shows up with other
- * authentication methods later.
+ * authentication methods later. We intentionally don't translate
+ * the message, since we aren't expecting it to happen.
*/
- printfPQExpBuffer(&conn->errorMessage, "SSPI returned invalid number of output buffers\n");
+ appendPQExpBufferStr(&conn->errorMessage,
+ "SSPI returned invalid number of output buffers\n");
return STATUS_ERROR;
}
@@ -418,8 +418,8 @@ pg_SSPI_startup(PGconn *conn, int use_negotiate, int payloadlen)
if (conn->sspictx)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("duplicate SSPI authentication request\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("duplicate SSPI authentication request\n"));
return STATUS_ERROR;
}
@@ -457,8 +457,8 @@ pg_SSPI_startup(PGconn *conn, int use_negotiate, int payloadlen)
*/
if (!(host && host[0] != '\0'))
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("host name must be specified\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("host name must be specified\n"));
return STATUS_ERROR;
}
conn->sspitarget = malloc(strlen(conn->krbsrvname) + strlen(host) + 2);
@@ -497,8 +497,8 @@ pg_SASL_init(PGconn *conn, int payloadlen)
if (conn->sasl_state)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("duplicate SASL authentication request\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("duplicate SASL authentication request\n"));
goto error;
}
@@ -513,8 +513,8 @@ pg_SASL_init(PGconn *conn, int payloadlen)
{
if (pqGets(&mechanism_buf, conn))
{
- printfPQExpBuffer(&conn->errorMessage,
- "fe_sendauth: invalid authentication request from server: invalid list of authentication
mechanisms\n");
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("invalid authentication request from server: invalid list of
authenticationmechanisms\n"));
goto error;
}
if (PQExpBufferDataBroken(mechanism_buf))
@@ -545,8 +545,8 @@ pg_SASL_init(PGconn *conn, int payloadlen)
* the client and server supported it. The SCRAM exchange
* checks for that, to prevent downgrade attacks.
*/
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("server offered SCRAM-SHA-256-PLUS authentication over a non-SSL
connection\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("server offered SCRAM-SHA-256-PLUS authentication over a non-SSL
connection\n"));
goto error;
}
}
@@ -557,8 +557,8 @@ pg_SASL_init(PGconn *conn, int payloadlen)
if (!selected_mechanism)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("none of the server's SASL authentication mechanisms are supported\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("none of the server's SASL authentication mechanisms are supported\n"));
goto error;
}
@@ -578,8 +578,9 @@ pg_SASL_init(PGconn *conn, int payloadlen)
password = conn->pgpass;
if (password == NULL || password[0] == '\0')
{
- printfPQExpBuffer(&conn->errorMessage,
- PQnoPasswordSupplied);
+ /* Intentionally not translated. */
+ appendPQExpBufferStr(&conn->errorMessage,
+ PQnoPasswordSupplied);
goto error;
}
@@ -688,8 +689,8 @@ pg_SASL_continue(PGconn *conn, int payloadlen, bool final)
if (outputlen != 0)
free(output);
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("AuthenticationSASLFinal received from server, but SASL authentication was not
completed\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("AuthenticationSASLFinal received from server, but SASL authentication was
notcompleted\n"));
return STATUS_ERROR;
}
if (outputlen != 0)
@@ -758,15 +759,15 @@ pg_local_sendauth(PGconn *conn)
{
char sebuf[256];
- printfPQExpBuffer(&conn->errorMessage,
- "pg_local_sendauth: sendmsg: %s\n",
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("could not send authentication message: %s\n"),
pqStrerror(errno, sebuf, sizeof(sebuf)));
return STATUS_ERROR;
}
return STATUS_OK;
#else
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SCM_CRED authentication method not supported\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("SCM_CRED authentication method not supported\n"));
return STATUS_ERROR;
#endif
}
@@ -856,13 +857,13 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
break;
case AUTH_REQ_KRB4:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("Kerberos 4 authentication not supported\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("Kerberos 4 authentication not supported\n"));
return STATUS_ERROR;
case AUTH_REQ_KRB5:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("Kerberos 5 authentication not supported\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("Kerberos 5 authentication not supported\n"));
return STATUS_ERROR;
#if defined(ENABLE_GSS) || defined(ENABLE_SSPI)
@@ -932,8 +933,8 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
/* No GSSAPI *or* SSPI support */
case AUTH_REQ_GSS:
case AUTH_REQ_GSS_CONT:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("GSSAPI authentication not supported\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("GSSAPI authentication not supported\n"));
return STATUS_ERROR;
#endif /* defined(ENABLE_GSS) || defined(ENABLE_SSPI) */
@@ -964,16 +965,16 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
*/
#if !defined(ENABLE_GSS)
case AUTH_REQ_SSPI:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SSPI authentication not supported\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("SSPI authentication not supported\n"));
return STATUS_ERROR;
#endif /* !define(ENABLE_GSSAPI) */
#endif /* ENABLE_SSPI */
case AUTH_REQ_CRYPT:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("Crypt authentication not supported\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("Crypt authentication not supported\n"));
return STATUS_ERROR;
case AUTH_REQ_MD5:
@@ -987,14 +988,15 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
password = conn->pgpass;
if (password == NULL || password[0] == '\0')
{
- printfPQExpBuffer(&conn->errorMessage,
- PQnoPasswordSupplied);
+ /* Intentionally not translated. */
+ appendPQExpBufferStr(&conn->errorMessage,
+ PQnoPasswordSupplied);
return STATUS_ERROR;
}
if (pg_password_sendauth(conn, password, areq) != STATUS_OK)
{
- printfPQExpBuffer(&conn->errorMessage,
- "fe_sendauth: error sending password authentication\n");
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("error sending password authentication\n"));
return STATUS_ERROR;
}
break;
@@ -1017,17 +1019,15 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
case AUTH_REQ_SASL_FIN:
if (conn->sasl_state == NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- "fe_sendauth: invalid authentication request from server: AUTH_REQ_SASL_CONT without
AUTH_REQ_SASL\n");
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("invalid authentication request from server: AUTH_REQ_SASL_CONT
withoutAUTH_REQ_SASL\n"));
return STATUS_ERROR;
}
if (pg_SASL_continue(conn, payloadlen,
(areq == AUTH_REQ_SASL_FIN)) != STATUS_OK)
{
- /* Use error message, if set already */
- if (conn->errorMessage.len == 0)
- printfPQExpBuffer(&conn->errorMessage,
- "fe_sendauth: error in SASL authentication\n");
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("error in SASL authentication\n"));
return STATUS_ERROR;
}
break;
@@ -1038,7 +1038,7 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
break;
default:
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("authentication method %u not supported\n"), areq);
return STATUS_ERROR;
}
@@ -1052,7 +1052,7 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
*
* Returns a pointer to malloc'd space containing whatever name the user
* has authenticated to the system. If there is an error, return NULL,
- * and put a suitable error message in *errorMessage if that's not NULL.
+ * and append a suitable error message to *errorMessage if that's not NULL.
*/
char *
pg_fe_getauthname(PQExpBuffer errorMessage)
@@ -1085,7 +1085,7 @@ pg_fe_getauthname(PQExpBuffer errorMessage)
if (GetUserName(username, &namesize))
name = username;
else if (errorMessage)
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("user name lookup failure: error code %lu\n"),
GetLastError());
#else
@@ -1095,12 +1095,12 @@ pg_fe_getauthname(PQExpBuffer errorMessage)
else if (errorMessage)
{
if (pwerr != 0)
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("could not look up local user ID %d: %s\n"),
(int) user_id,
pqStrerror(pwerr, pwdbuf, sizeof(pwdbuf)));
else
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("local user with ID %d does not exist\n"),
(int) user_id);
}
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 986f74f..9b6a065 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -1757,10 +1757,15 @@ connectDBStart(PGconn *conn)
conn->outCount = 0;
/*
- * Ensure errorMessage is empty, too. PQconnectPoll will append messages
- * to it in the process of scanning for a working server. Thus, if we
- * fail to connect to multiple hosts, the final error message will include
- * details about each failure.
+ * Ensure errorMessage is empty, too. PQconnectPoll and its subroutines
+ * will append messages to it in the process of scanning for a working
+ * server. Thus, if we fail to connect to multiple hosts, the final error
+ * message will include details about each failure.
+ *
+ * (The exception to the rule of "always append to conn->errorMessage" is
+ * "out of memory" complaints; for those it seems better to reset
+ * errorMessage, so as to minimize the risk of a further OOM failure while
+ * trying to report the condition.)
*/
resetPQExpBuffer(&conn->errorMessage);
@@ -1938,12 +1943,6 @@ connectDBComplete(PGconn *conn)
switch (flag)
{
case PGRES_POLLING_OK:
-
- /*
- * Reset stored error messages since we now have a working
- * connection
- */
- resetPQExpBuffer(&conn->errorMessage);
return 1; /* success! */
case PGRES_POLLING_READING:
@@ -1996,46 +1995,6 @@ connectDBComplete(PGconn *conn)
}
}
-/*
- * This subroutine saves conn->errorMessage, which will be restored back by
- * restoreErrorMessage subroutine. Returns false on OOM failure.
- */
-static bool
-saveErrorMessage(PGconn *conn, PQExpBuffer savedMessage)
-{
- initPQExpBuffer(savedMessage);
- appendPQExpBufferStr(savedMessage,
- conn->errorMessage.data);
- if (PQExpBufferBroken(savedMessage))
- {
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
- return false;
- }
- /* Clear whatever is in errorMessage now */
- resetPQExpBuffer(&conn->errorMessage);
- return true;
-}
-
-/*
- * Restores saved error messages back to conn->errorMessage, prepending them
- * to whatever is in conn->errorMessage already. (This does the right thing
- * if anything's been added to conn->errorMessage since saveErrorMessage.)
- */
-static void
-restoreErrorMessage(PGconn *conn, PQExpBuffer savedMessage)
-{
- appendPQExpBufferStr(savedMessage, conn->errorMessage.data);
- resetPQExpBuffer(&conn->errorMessage);
- appendPQExpBufferStr(&conn->errorMessage, savedMessage->data);
- /* If any step above hit OOM, just report that */
- if (PQExpBufferBroken(savedMessage) ||
- PQExpBufferBroken(&conn->errorMessage))
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
- termPQExpBuffer(savedMessage);
-}
-
/* ----------------
* PQconnectPoll
*
@@ -2071,7 +2030,6 @@ PQconnectPoll(PGconn *conn)
PGresult *res;
char sebuf[256];
int optval;
- PQExpBufferData savedMessage;
if (conn == NULL)
return PGRES_POLLING_FAILED;
@@ -2592,10 +2550,6 @@ keep_going: /* We will come back to here until there is
EnvironmentOptions);
if (!startpacket)
{
- /*
- * will not appendbuffer here, since it's likely to also
- * run out of memory
- */
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("out of memory\n"));
goto error_return;
@@ -2983,7 +2937,6 @@ keep_going: /* We will come back to here until there is
* avoid the Kerberos code doing a hostname look-up.
*/
res = pg_fe_sendauth(areq, msgLength, conn);
- conn->errorMessage.len = strlen(conn->errorMessage.data);
/* OK, we have processed the message; mark data consumed */
conn->inStart = conn->inCursor;
@@ -3103,31 +3056,19 @@ keep_going: /* We will come back to here until there is
conn->target_session_attrs != NULL &&
strcmp(conn->target_session_attrs, "read-write") == 0)
{
- /*
- * Save existing error messages across the PQsendQuery
- * attempt. This is necessary because PQsendQuery is
- * going to reset conn->errorMessage, so we would lose
- * error messages related to previous hosts we have tried
- * and failed to connect to.
- */
- if (!saveErrorMessage(conn, &savedMessage))
- goto error_return;
-
- conn->status = CONNECTION_OK;
if (!PQsendQuery(conn,
"SHOW transaction_read_only"))
- {
- restoreErrorMessage(conn, &savedMessage);
goto error_return;
- }
conn->status = CONNECTION_CHECK_WRITABLE;
- restoreErrorMessage(conn, &savedMessage);
return PGRES_POLLING_READING;
}
/* We can release the address lists now. */
release_all_addrinfo(conn);
+ /* Likewise drop any stored error messages. */
+ resetPQExpBuffer(&conn->errorMessage);
+
/* We are open for business! */
conn->status = CONNECTION_OK;
return PGRES_POLLING_OK;
@@ -3137,23 +3078,16 @@ keep_going: /* We will come back to here until there is
/*
* Do post-connection housekeeping (only needed in protocol 2.0).
- *
- * We pretend that the connection is OK for the duration of these
- * queries.
*/
- conn->status = CONNECTION_OK;
-
switch (pqSetenvPoll(conn))
{
case PGRES_POLLING_OK: /* Success */
break;
case PGRES_POLLING_READING: /* Still going */
- conn->status = CONNECTION_SETENV;
return PGRES_POLLING_READING;
case PGRES_POLLING_WRITING: /* Still going */
- conn->status = CONNECTION_SETENV;
return PGRES_POLLING_WRITING;
default:
@@ -3173,39 +3107,30 @@ keep_going: /* We will come back to here until there is
conn->target_session_attrs != NULL &&
strcmp(conn->target_session_attrs, "read-write") == 0)
{
- if (!saveErrorMessage(conn, &savedMessage))
- goto error_return;
-
- conn->status = CONNECTION_OK;
if (!PQsendQuery(conn,
"SHOW transaction_read_only"))
- {
- restoreErrorMessage(conn, &savedMessage);
goto error_return;
- }
conn->status = CONNECTION_CHECK_WRITABLE;
- restoreErrorMessage(conn, &savedMessage);
return PGRES_POLLING_READING;
}
/* We can release the address lists now. */
release_all_addrinfo(conn);
+ /* Likewise drop any stored error messages. */
+ resetPQExpBuffer(&conn->errorMessage);
+
/* We are open for business! */
conn->status = CONNECTION_OK;
return PGRES_POLLING_OK;
case CONNECTION_CONSUME:
{
- conn->status = CONNECTION_OK;
if (!PQconsumeInput(conn))
goto error_return;
if (PQisBusy(conn))
- {
- conn->status = CONNECTION_CONSUME;
return PGRES_POLLING_READING;
- }
/*
* Call PQgetResult() again to consume NULL result.
@@ -3214,10 +3139,15 @@ keep_going: /* We will come back to here until there is
if (res != NULL)
{
PQclear(res);
- conn->status = CONNECTION_CONSUME;
goto keep_going;
}
+ /* We can release the address lists now. */
+ release_all_addrinfo(conn);
+
+ /* Likewise drop any stored error messages. */
+ resetPQExpBuffer(&conn->errorMessage);
+
/* We are open for business! */
conn->status = CONNECTION_OK;
return PGRES_POLLING_OK;
@@ -3227,22 +3157,11 @@ keep_going: /* We will come back to here until there is
const char *displayed_host;
const char *displayed_port;
- if (!saveErrorMessage(conn, &savedMessage))
- goto error_return;
-
- conn->status = CONNECTION_OK;
if (!PQconsumeInput(conn))
- {
- restoreErrorMessage(conn, &savedMessage);
goto error_return;
- }
if (PQisBusy(conn))
- {
- conn->status = CONNECTION_CHECK_WRITABLE;
- restoreErrorMessage(conn, &savedMessage);
return PGRES_POLLING_READING;
- }
res = PQgetResult(conn);
if (res && (PQresultStatus(res) == PGRES_TUPLES_OK) &&
@@ -3258,7 +3177,6 @@ keep_going: /* We will come back to here until there is
const char *displayed_port;
PQclear(res);
- restoreErrorMessage(conn, &savedMessage);
/* Append error report to conn->errorMessage. */
if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS)
@@ -3289,10 +3207,6 @@ keep_going: /* We will come back to here until there is
/* Session is read-write, so we're good. */
PQclear(res);
- termPQExpBuffer(&savedMessage);
-
- /* We can release the address lists now. */
- release_all_addrinfo(conn);
/*
* Finish reading any remaining messages before being
@@ -3308,7 +3222,6 @@ keep_going: /* We will come back to here until there is
*/
if (res)
PQclear(res);
- restoreErrorMessage(conn, &savedMessage);
/* Append error report to conn->errorMessage. */
if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS)
@@ -3762,7 +3675,7 @@ PQreset(PGconn *conn)
conn->events[i].passThrough))
{
conn->status = CONNECTION_BAD;
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("PGEventProc \"%s\" failed during PGEVT_CONNRESET event\n"),
conn->events[i].name);
break;
@@ -3822,7 +3735,7 @@ PQresetPoll(PGconn *conn)
conn->events[i].passThrough))
{
conn->status = CONNECTION_BAD;
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("PGEventProc \"%s\" failed during PGEVT_CONNRESET event\n"),
conn->events[i].name);
return PGRES_POLLING_FAILED;
@@ -4169,8 +4082,9 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
if (pg_strncasecmp(url, LDAP_URL, strlen(LDAP_URL)) != 0)
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("invalid LDAP URL \"%s\": scheme must be ldap://\n"), purl);
+ appendPQExpBuffer(errorMessage,
+ libpq_gettext("invalid LDAP URL \"%s\": scheme must be ldap://\n"),
+ purl);
free(url);
return 3;
}
@@ -4184,8 +4098,9 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
p = strchr(url + strlen(LDAP_URL), '/');
if (p == NULL || *(p + 1) == '\0' || *(p + 1) == '?')
{
- printfPQExpBuffer(errorMessage, libpq_gettext(
- "invalid LDAP URL \"%s\": missing distinguished name\n"), purl);
+ appendPQExpBuffer(errorMessage,
+ libpq_gettext("invalid LDAP URL \"%s\": missing distinguished name\n"),
+ purl);
free(url);
return 3;
}
@@ -4195,8 +4110,9 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
/* attribute */
if ((p = strchr(dn, '?')) == NULL || *(p + 1) == '\0' || *(p + 1) == '?')
{
- printfPQExpBuffer(errorMessage, libpq_gettext(
- "invalid LDAP URL \"%s\": must have exactly one attribute\n"),
purl);
+ appendPQExpBuffer(errorMessage,
+ libpq_gettext("invalid LDAP URL \"%s\": must have exactly one attribute\n"),
+ purl);
free(url);
return 3;
}
@@ -4206,7 +4122,9 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
/* scope */
if ((p = strchr(attrs[0], '?')) == NULL || *(p + 1) == '\0' || *(p + 1) == '?')
{
- printfPQExpBuffer(errorMessage, libpq_gettext("invalid LDAP URL \"%s\": must have search scope
(base/one/sub)\n"),purl);
+ appendPQExpBuffer(errorMessage,
+ libpq_gettext("invalid LDAP URL \"%s\": must have search scope (base/one/sub)\n"),
+ purl);
free(url);
return 3;
}
@@ -4216,7 +4134,7 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
/* filter */
if ((p = strchr(scopestr, '?')) == NULL || *(p + 1) == '\0' || *(p + 1) == '?')
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("invalid LDAP URL \"%s\": no filter\n"), purl);
free(url);
return 3;
@@ -4237,8 +4155,9 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
lport = strtol(portstr, &endptr, 10);
if (*portstr == '\0' || *endptr != '\0' || errno || lport < 0 || lport > 65535)
{
- printfPQExpBuffer(errorMessage, libpq_gettext(
- "invalid LDAP URL \"%s\": invalid port number\n"), purl);
+ appendPQExpBuffer(errorMessage,
+ libpq_gettext("invalid LDAP URL \"%s\": invalid port number\n"),
+ purl);
free(url);
return 3;
}
@@ -4248,8 +4167,9 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
/* Allow only one attribute */
if (strchr(attrs[0], ',') != NULL)
{
- printfPQExpBuffer(errorMessage, libpq_gettext(
- "invalid LDAP URL \"%s\": must have exactly one attribute\n"),
purl);
+ appendPQExpBuffer(errorMessage,
+ libpq_gettext("invalid LDAP URL \"%s\": must have exactly one attribute\n"),
+ purl);
free(url);
return 3;
}
@@ -4263,7 +4183,9 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
scope = LDAP_SCOPE_SUBTREE;
else
{
- printfPQExpBuffer(errorMessage, libpq_gettext("invalid LDAP URL \"%s\": must have search scope
(base/one/sub)\n"),purl);
+ appendPQExpBuffer(errorMessage,
+ libpq_gettext("invalid LDAP URL \"%s\": must have search scope (base/one/sub)\n"),
+ purl);
free(url);
return 3;
}
@@ -4271,8 +4193,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
/* initialize LDAP structure */
if ((ld = ldap_init(hostname, port)) == NULL)
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("could not create LDAP structure\n"));
+ appendPQExpBufferStr(errorMessage,
+ libpq_gettext("could not create LDAP structure\n"));
free(url);
return 3;
}
@@ -4347,7 +4269,7 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
{
if (res != NULL)
ldap_msgfree(res);
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("lookup on LDAP server failed: %s\n"),
ldap_err2string(rc));
ldap_unbind(ld);
@@ -4358,9 +4280,9 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
/* complain if there was not exactly one result */
if ((rc = ldap_count_entries(ld, res)) != 1)
{
- printfPQExpBuffer(errorMessage,
- rc ? libpq_gettext("more than one entry found on LDAP lookup\n")
- : libpq_gettext("no entry found on LDAP lookup\n"));
+ appendPQExpBufferStr(errorMessage,
+ rc ? libpq_gettext("more than one entry found on LDAP lookup\n")
+ : libpq_gettext("no entry found on LDAP lookup\n"));
ldap_msgfree(res);
ldap_unbind(ld);
free(url);
@@ -4371,8 +4293,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
if ((entry = ldap_first_entry(ld, res)) == NULL)
{
/* should never happen */
- printfPQExpBuffer(errorMessage,
- libpq_gettext("no entry found on LDAP lookup\n"));
+ appendPQExpBufferStr(errorMessage,
+ libpq_gettext("no entry found on LDAP lookup\n"));
ldap_msgfree(res);
ldap_unbind(ld);
free(url);
@@ -4382,8 +4304,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
/* get values */
if ((values = ldap_get_values_len(ld, entry, attrs[0])) == NULL)
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("attribute has no values on LDAP lookup\n"));
+ appendPQExpBufferStr(errorMessage,
+ libpq_gettext("attribute has no values on LDAP lookup\n"));
ldap_msgfree(res);
ldap_unbind(ld);
free(url);
@@ -4395,8 +4317,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
if (values[0] == NULL)
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("attribute has no values on LDAP lookup\n"));
+ appendPQExpBufferStr(errorMessage,
+ libpq_gettext("attribute has no values on LDAP lookup\n"));
ldap_value_free_len(values);
ldap_unbind(ld);
return 1;
@@ -4447,8 +4369,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
}
else if (ld_is_nl_cr(*p))
{
- printfPQExpBuffer(errorMessage, libpq_gettext(
- "missing \"=\" after \"%s\" in connection info
string\n"),
+ appendPQExpBuffer(errorMessage,
+ libpq_gettext("missing \"=\" after \"%s\" in connection info string\n"),
optname);
free(result);
return 3;
@@ -4466,8 +4388,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
}
else if (!ld_is_sp_tab(*p))
{
- printfPQExpBuffer(errorMessage, libpq_gettext(
- "missing \"=\" after \"%s\" in connection info
string\n"),
+ appendPQExpBuffer(errorMessage,
+ libpq_gettext("missing \"=\" after \"%s\" in connection info string\n"),
optname);
free(result);
return 3;
@@ -4539,7 +4461,7 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
}
if (!found_keyword)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("invalid connection option \"%s\"\n"),
optname);
free(result);
@@ -4555,8 +4477,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
if (state == 5 || state == 6)
{
- printfPQExpBuffer(errorMessage, libpq_gettext(
- "unterminated quoted string in connection info string\n"));
+ appendPQExpBufferStr(errorMessage,
+ libpq_gettext("unterminated quoted string in connection info string\n"));
return 3;
}
@@ -4574,8 +4496,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
* Returns 0 on success, nonzero on failure. On failure, if errorMessage
* isn't null, also store an error message there. (Note: the only reason
* this function and related ones don't dump core on errorMessage == NULL
- * is the undocumented fact that printfPQExpBuffer does nothing when passed
- * a null PQExpBuffer pointer.)
+ * is the undocumented fact that printfPQExpBuffer and friends do nothing
+ * when passed a null PQExpBuffer pointer.)
*/
static int
parseServiceInfo(PQconninfoOption *options, PQExpBuffer errorMessage)
@@ -4638,7 +4560,7 @@ next_file:
last_file:
if (!group_found)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("definition of service \"%s\" not found\n"), service);
return 3;
}
@@ -4662,7 +4584,7 @@ parseServiceFile(const char *serviceFile,
f = fopen(serviceFile, "r");
if (f == NULL)
{
- printfPQExpBuffer(errorMessage, libpq_gettext("service file \"%s\" not found\n"),
+ appendPQExpBuffer(errorMessage, libpq_gettext("service file \"%s\" not found\n"),
serviceFile);
return 1;
}
@@ -4674,7 +4596,7 @@ parseServiceFile(const char *serviceFile,
if (strlen(line) >= sizeof(buf) - 1)
{
fclose(f);
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("line %d too long in service file \"%s\"\n"),
linenr,
serviceFile);
@@ -4745,7 +4667,7 @@ parseServiceFile(const char *serviceFile,
val = strchr(line, '=');
if (val == NULL)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("syntax error in service file \"%s\", line %d\n"),
serviceFile,
linenr);
@@ -4756,7 +4678,7 @@ parseServiceFile(const char *serviceFile,
if (strcmp(key, "service") == 0)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("nested service specifications not supported in service file
\"%s\",line %d\n"),
serviceFile,
linenr);
@@ -4789,7 +4711,7 @@ parseServiceFile(const char *serviceFile,
if (!found_keyword)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("syntax error in service file \"%s\", line %d\n"),
serviceFile,
linenr);
@@ -4995,7 +4917,7 @@ conninfo_parse(const char *conninfo, PQExpBuffer errorMessage,
/* Check that there is a following '=' */
if (*cp != '=')
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("missing \"=\" after \"%s\" in connection info string\n"),
pname);
PQconninfoFree(options);
@@ -5044,8 +4966,8 @@ conninfo_parse(const char *conninfo, PQExpBuffer errorMessage,
{
if (*cp == '\0')
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("unterminated quoted string in connection info string\n"));
+ appendPQExpBufferStr(errorMessage,
+ libpq_gettext("unterminated quoted string in connection info string\n"));
PQconninfoFree(options);
free(buf);
return NULL;
@@ -5180,7 +5102,7 @@ conninfo_array_parse(const char *const *keywords, const char *const *values,
/* Check for invalid connection option */
if (option->keyword == NULL)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("invalid connection option \"%s\"\n"),
pname);
PQconninfoFree(options);
@@ -5476,7 +5398,7 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri,
if (prefix_len == 0)
{
/* Should never happen */
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("invalid URI propagated to internal parser routine: \"%s\"\n"),
uri);
goto cleanup;
@@ -5553,14 +5475,14 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri,
++p;
if (!*p)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("end of string reached when looking for matching \"]\" in IPv6 host
addressin URI: \"%s\"\n"),
uri);
goto cleanup;
}
if (p == host)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("IPv6 host address may not be empty in URI: \"%s\"\n"),
uri);
goto cleanup;
@@ -5575,7 +5497,7 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri,
*/
if (*p && *p != ':' && *p != '/' && *p != '?' && *p != ',')
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("unexpected character \"%c\" at position %d in URI (expected \":\" or
\"/\"):\"%s\"\n"),
*p, (int) (p - buf + 1), uri);
goto cleanup;
@@ -5704,7 +5626,7 @@ conninfo_uri_parse_params(char *params,
/* Was there '=' already? */
if (value != NULL)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("extra key/value separator \"=\" in URI query parameter:
\"%s\"\n"),
keyword);
return false;
@@ -5724,7 +5646,7 @@ conninfo_uri_parse_params(char *params,
/* Was there '=' at all? */
if (value == NULL)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("missing key/value separator \"=\" in URI query parameter:
\"%s\"\n"),
keyword);
return false;
@@ -5775,7 +5697,7 @@ conninfo_uri_parse_params(char *params,
{
/* Insert generic message if conninfo_storeval didn't give one. */
if (errorMessage->len == 0)
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("invalid URI query parameter: \"%s\"\n"),
keyword);
/* And fail. */
@@ -5849,7 +5771,7 @@ conninfo_uri_decode(const char *str, PQExpBuffer errorMessage)
*/
if (!(get_hexdigit(*q++, &hi) && get_hexdigit(*q++, &lo)))
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("invalid percent-encoded token: \"%s\"\n"),
str);
free(buf);
@@ -5859,7 +5781,7 @@ conninfo_uri_decode(const char *str, PQExpBuffer errorMessage)
c = (hi << 4) | lo;
if (c == 0)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("forbidden value %%00 in percent-encoded value: \"%s\"\n"),
str);
free(buf);
@@ -5954,7 +5876,7 @@ conninfo_storeval(PQconninfoOption *connOptions,
if (option == NULL)
{
if (!ignoreMissing)
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("invalid connection option \"%s\"\n"),
keyword);
return NULL;
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 4c0114c..f51be2e 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -770,8 +770,7 @@ pqSaveErrorResult(PGconn *conn)
* This subroutine prepares an async result object for return to the caller.
* If there is not already an async result object, build an error object
* using whatever is in conn->errorMessage. In any case, clear the async
- * result storage and make sure PQerrorMessage will agree with the result's
- * error string.
+ * result storage.
*/
PGresult *
pqPrepareAsyncResult(PGconn *conn)
@@ -781,21 +780,11 @@ pqPrepareAsyncResult(PGconn *conn)
/*
* conn->result is the PGresult to return. If it is NULL (which probably
* shouldn't happen) we assume there is an appropriate error message in
- * conn->errorMessage.
+ * conn->errorMessage, and copy that into a PGresult.
*/
res = conn->result;
if (!res)
res = PQmakeEmptyPGresult(conn, PGRES_FATAL_ERROR);
- else
- {
- /*
- * Make sure PQerrorMessage agrees with result; it could be different
- * if we have concatenated messages.
- */
- resetPQExpBuffer(&conn->errorMessage);
- appendPQExpBufferStr(&conn->errorMessage,
- PQresultErrorMessage(res));
- }
/*
* Replace conn->result with next_result, if any. In the normal case
@@ -1188,8 +1177,8 @@ PQsendQuery(PGconn *conn, const char *query)
/* check the argument */
if (!query)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("command string is a null pointer\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("command string is a null pointer\n"));
return 0;
}
@@ -1246,14 +1235,14 @@ PQsendQueryParams(PGconn *conn,
/* check the arguments */
if (!command)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("command string is a null pointer\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("command string is a null pointer\n"));
return 0;
}
if (nParams < 0 || nParams > 65535)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("number of parameters must be between 0 and 65535\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("number of parameters must be between 0 and 65535\n"));
return 0;
}
@@ -1286,28 +1275,28 @@ PQsendPrepare(PGconn *conn,
/* check the arguments */
if (!stmtName)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("statement name is a null pointer\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("statement name is a null pointer\n"));
return 0;
}
if (!query)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("command string is a null pointer\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("command string is a null pointer\n"));
return 0;
}
if (nParams < 0 || nParams > 65535)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("number of parameters must be between 0 and 65535\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("number of parameters must be between 0 and 65535\n"));
return 0;
}
/* This isn't gonna work on a 2.0 server */
if (PG_PROTOCOL_MAJOR(conn->pversion) < 3)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("function requires at least protocol version 3.0\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("function requires at least protocol version 3.0\n"));
return 0;
}
@@ -1387,14 +1376,14 @@ PQsendQueryPrepared(PGconn *conn,
/* check the arguments */
if (!stmtName)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("statement name is a null pointer\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("statement name is a null pointer\n"));
return 0;
}
if (nParams < 0 || nParams > 65535)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("number of parameters must be between 0 and 65535\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("number of parameters must be between 0 and 65535\n"));
return 0;
}
@@ -1418,21 +1407,36 @@ PQsendQueryStart(PGconn *conn)
if (!conn)
return false;
- /* clear the error string */
- resetPQExpBuffer(&conn->errorMessage);
+ /*
+ * Clear the error string at start of a new query. All libpq code
+ * executed while we're running a query should append messages to
+ * errorMessage.
+ *
+ * (The exception to this rule is "out of memory" complaints; for those it
+ * seems better to reset errorMessage, so as to minimize the risk of a
+ * further OOM failure while trying to report the condition.)
+ *
+ * However, if we're in a connecting state (anything but CONNECTION_OK or
+ * CONNECTION_BAD), then we don't want to clear the errorMessage. We must
+ * be doing some libpq-generated query to check server state, and we want
+ * to continue building up the history of connection attempts across
+ * multiple servers --- see comments in connectDBStart.
+ */
+ if (conn->status <= CONNECTION_BAD)
+ resetPQExpBuffer(&conn->errorMessage);
/* Don't try to send if we know there's no live connection. */
- if (conn->status != CONNECTION_OK)
+ if (conn->status == CONNECTION_BAD)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("no connection to the server\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("no connection to the server\n"));
return false;
}
/* Can't send while already busy, either. */
if (conn->asyncStatus != PGASYNC_IDLE)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("another command is already in progress\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("another command is already in progress\n"));
return false;
}
@@ -1469,8 +1473,8 @@ PQsendQueryGuts(PGconn *conn,
/* This isn't gonna work on a 2.0 server */
if (PG_PROTOCOL_MAJOR(conn->pversion) < 3)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("function requires at least protocol version 3.0\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("function requires at least protocol version 3.0\n"));
return 0;
}
@@ -1545,8 +1549,8 @@ PQsendQueryGuts(PGconn *conn,
nbytes = paramLengths[i];
else
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("length must be given for binary parameter\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("length must be given for binary parameter\n"));
goto sendFailed;
}
}
@@ -1817,7 +1821,7 @@ PQgetResult(PGconn *conn)
res = getCopyResult(conn, PGRES_COPY_BOTH);
break;
default:
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("unexpected asyncStatus: %d\n"),
(int) conn->asyncStatus);
res = PQmakeEmptyPGresult(conn, PGRES_FATAL_ERROR);
@@ -1837,7 +1841,7 @@ PQgetResult(PGconn *conn)
if (!res->events[i].proc(PGEVT_RESULTCREATE, &evt,
res->events[i].passThrough))
{
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("PGEventProc \"%s\" failed during PGEVT_RESULTCREATE event\n"),
res->events[i].name);
pqSetResultError(res, conn->errorMessage.data);
@@ -2005,8 +2009,8 @@ PQexecStart(PGconn *conn)
else
{
/* In older protocols we have to punt */
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("COPY IN state must be terminated first\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("COPY IN state must be terminated first\n"));
return false;
}
}
@@ -2025,16 +2029,16 @@ PQexecStart(PGconn *conn)
else
{
/* In older protocols we have to punt */
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("COPY OUT state must be terminated first\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("COPY OUT state must be terminated first\n"));
return false;
}
}
else if (resultStatus == PGRES_COPY_BOTH)
{
/* We don't allow PQexec during COPY BOTH */
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("PQexec not allowed during COPY BOTH\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("PQexec not allowed during COPY BOTH\n"));
return false;
}
/* check for loss of connection, too */
@@ -2076,12 +2080,6 @@ PQexecFinish(PGconn *conn)
pqCatenateResultError(lastResult, result->errMsg);
PQclear(result);
result = lastResult;
-
- /*
- * Make sure PQerrorMessage agrees with concatenated result
- */
- resetPQExpBuffer(&conn->errorMessage);
- appendPQExpBufferStr(&conn->errorMessage, result->errMsg);
}
else
PQclear(lastResult);
@@ -2187,8 +2185,8 @@ PQsendDescribe(PGconn *conn, char desc_type, const char *desc_target)
/* This isn't gonna work on a 2.0 server */
if (PG_PROTOCOL_MAJOR(conn->pversion) < 3)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("function requires at least protocol version 3.0\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("function requires at least protocol version 3.0\n"));
return 0;
}
@@ -2276,8 +2274,8 @@ PQputCopyData(PGconn *conn, const char *buffer, int nbytes)
if (conn->asyncStatus != PGASYNC_COPY_IN &&
conn->asyncStatus != PGASYNC_COPY_BOTH)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("no COPY in progress\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("no COPY in progress\n"));
return -1;
}
@@ -2343,8 +2341,8 @@ PQputCopyEnd(PGconn *conn, const char *errormsg)
if (conn->asyncStatus != PGASYNC_COPY_IN &&
conn->asyncStatus != PGASYNC_COPY_BOTH)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("no COPY in progress\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("no COPY in progress\n"));
return -1;
}
@@ -2386,8 +2384,8 @@ PQputCopyEnd(PGconn *conn, const char *errormsg)
if (errormsg)
{
/* Oops, no way to do this in 2.0 */
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("function requires at least protocol version 3.0\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("function requires at least protocol version 3.0\n"));
return -1;
}
else
@@ -2405,7 +2403,6 @@ PQputCopyEnd(PGconn *conn, const char *errormsg)
conn->asyncStatus = PGASYNC_COPY_OUT;
else
conn->asyncStatus = PGASYNC_BUSY;
- resetPQExpBuffer(&conn->errorMessage);
/* Try to flush data */
if (pqFlush(conn) < 0)
@@ -2433,8 +2430,8 @@ PQgetCopyData(PGconn *conn, char **buffer, int async)
if (conn->asyncStatus != PGASYNC_COPY_OUT &&
conn->asyncStatus != PGASYNC_COPY_BOTH)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("no COPY in progress\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("no COPY in progress\n"));
return -2;
}
if (PG_PROTOCOL_MAJOR(conn->pversion) >= 3)
@@ -2617,14 +2614,14 @@ PQfn(PGconn *conn,
if (!conn)
return NULL;
- /* clear the error string */
+ /* Clear the error string, like PQsendQueryStart() */
resetPQExpBuffer(&conn->errorMessage);
if (conn->sock == PGINVALID_SOCKET || conn->asyncStatus != PGASYNC_IDLE ||
conn->result != NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("connection in wrong state\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("connection in wrong state\n"));
return NULL;
}
@@ -3343,8 +3340,8 @@ PQescapeStringInternal(PGconn *conn,
if (error)
*error = 1;
if (conn)
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("incomplete multibyte character\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("incomplete multibyte character\n"));
for (; i < len; i++)
{
if (((size_t) (target - to)) / 2 >= length)
@@ -3427,8 +3424,8 @@ PQescapeInternal(PGconn *conn, const char *str, size_t len, bool as_ident)
/* Multibyte character overruns allowable length. */
if ((s - str) + charlen > len || memchr(s, 0, charlen) != NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("incomplete multibyte character\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("incomplete multibyte character\n"));
return NULL;
}
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 2a6637f..627d4d8 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -394,7 +394,7 @@ pqCheckOutBufferSpace(size_t bytes_needed, PGconn *conn)
/* realloc failed. Probably out of memory */
printfPQExpBuffer(&conn->errorMessage,
- "cannot allocate memory for output buffer\n");
+ libpq_gettext("out of memory\n"));
return EOF;
}
@@ -488,7 +488,7 @@ pqCheckInBufferSpace(size_t bytes_needed, PGconn *conn)
/* realloc failed. Probably out of memory */
printfPQExpBuffer(&conn->errorMessage,
- "cannot allocate memory for input buffer\n");
+ libpq_gettext("out of memory\n"));
return EOF;
}
@@ -633,8 +633,8 @@ pqReadData(PGconn *conn)
if (conn->sock == PGINVALID_SOCKET)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("connection not open\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("connection not open\n"));
return -1;
}
@@ -802,11 +802,10 @@ retry4:
* means the connection has been closed. Cope.
*/
definitelyEOF:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext(
- "server closed the connection unexpectedly\n"
- "\tThis probably means the server terminated abnormally\n"
- "\tbefore or while processing the request.\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("server closed the connection unexpectedly\n"
+ "\tThis probably means the server terminated abnormally\n"
+ "\tbefore or while processing the request.\n"));
/* Come here if lower-level code already set a suitable errorMessage */
definitelyFailed:
@@ -834,8 +833,8 @@ pqSendSome(PGconn *conn, int len)
if (conn->sock == PGINVALID_SOCKET)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("connection not open\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("connection not open\n"));
/* Discard queued data; no chance it'll ever be sent */
conn->outCount = 0;
return -1;
@@ -1005,8 +1004,8 @@ pqWaitTimed(int forRead, int forWrite, PGconn *conn, time_t finish_time)
if (result == 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("timeout expired\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("timeout expired\n"));
return 1;
}
@@ -1050,8 +1049,8 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
return -1;
if (conn->sock == PGINVALID_SOCKET)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("invalid socket\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("invalid socket\n"));
return -1;
}
@@ -1073,7 +1072,7 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
{
char sebuf[256];
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("select() failed: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
}
diff --git a/src/interfaces/libpq/fe-protocol2.c b/src/interfaces/libpq/fe-protocol2.c
index 53e5083..521b03b 100644
--- a/src/interfaces/libpq/fe-protocol2.c
+++ b/src/interfaces/libpq/fe-protocol2.c
@@ -85,11 +85,9 @@ pqSetenvPoll(PGconn *conn)
return PGRES_POLLING_OK;
default:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext(
- "invalid setenv state %c, "
- "probably indicative of memory corruption\n"
- ),
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("invalid setenv state %c, "
+ "probably indicative of memory corruption\n"),
conn->setenv_state);
goto error_return;
}
@@ -385,7 +383,7 @@ pqSetenvPoll(PGconn *conn)
}
default:
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("invalid state %c, "
"probably indicative of memory corruption\n"),
conn->setenv_state);
@@ -499,7 +497,7 @@ pqParseInput2(PGconn *conn)
if (!conn->result)
{
printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory"));
+ libpq_gettext("out of memory\n"));
pqSaveErrorResult(conn);
}
}
@@ -534,7 +532,7 @@ pqParseInput2(PGconn *conn)
if (!conn->result)
{
printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory"));
+ libpq_gettext("out of memory\n"));
pqSaveErrorResult(conn);
}
}
@@ -627,9 +625,8 @@ pqParseInput2(PGconn *conn)
* never arrives from the server during protocol 2.0.
*/
default:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext(
- "unexpected response from server; first received character was
\"%c\"\n"),
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("unexpected response from server; first received character was
\"%c\"\n"),
id);
/* build an error result holding the error message */
pqSaveErrorResult(conn);
@@ -760,7 +757,7 @@ advance_and_error:
if (!errmsg)
errmsg = libpq_gettext("out of memory for query result");
- printfPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
+ appendPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
/*
* XXX: if PQmakeEmptyPGresult() fails, there's probably not much we can
@@ -935,7 +932,7 @@ set_error_result:
if (!errmsg)
errmsg = libpq_gettext("out of memory for query result");
- printfPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
+ appendPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
/*
* XXX: if PQmakeEmptyPGresult() fails, there's probably not much we can
@@ -1048,12 +1045,11 @@ pqGetErrorNotice2(PGconn *conn, bool isError)
{
pqClearAsyncResult(conn); /* redundant, but be safe */
conn->result = res;
- resetPQExpBuffer(&conn->errorMessage);
if (res && !PQExpBufferDataBroken(workBuf) && res->errMsg)
appendPQExpBufferStr(&conn->errorMessage, res->errMsg);
else
printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory"));
+ libpq_gettext("out of memory\n"));
if (conn->xactStatus == PQTRANS_INTRANS)
conn->xactStatus = PQTRANS_INERROR;
}
@@ -1355,8 +1351,8 @@ pqEndcopy2(PGconn *conn)
if (conn->asyncStatus != PGASYNC_COPY_IN &&
conn->asyncStatus != PGASYNC_COPY_OUT)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("no COPY in progress\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("no COPY in progress\n"));
return 1;
}
@@ -1373,7 +1369,6 @@ pqEndcopy2(PGconn *conn)
/* Return to active duty */
conn->asyncStatus = PGASYNC_BUSY;
- resetPQExpBuffer(&conn->errorMessage);
/* Wait for the completion response */
result = PQgetResult(conn);
@@ -1544,7 +1539,7 @@ pqFunctionCall2(PGconn *conn, Oid fnid,
else
{
/* The backend violates the protocol. */
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("protocol error: id=0x%x\n"),
id);
pqSaveErrorResult(conn);
@@ -1576,7 +1571,7 @@ pqFunctionCall2(PGconn *conn, Oid fnid,
return PQmakeEmptyPGresult(conn, status);
default:
/* The backend violates the protocol. */
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("protocol error: id=0x%x\n"),
id);
pqSaveErrorResult(conn);
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 8345faf..4a25d21 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -205,7 +205,7 @@ pqParseInput3(PGconn *conn)
if (!conn->result)
{
printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory"));
+ libpq_gettext("out of memory\n"));
pqSaveErrorResult(conn);
}
}
@@ -232,7 +232,7 @@ pqParseInput3(PGconn *conn)
if (!conn->result)
{
printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory"));
+ libpq_gettext("out of memory\n"));
pqSaveErrorResult(conn);
}
}
@@ -249,7 +249,7 @@ pqParseInput3(PGconn *conn)
if (!conn->result)
{
printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory"));
+ libpq_gettext("out of memory\n"));
pqSaveErrorResult(conn);
}
}
@@ -329,7 +329,7 @@ pqParseInput3(PGconn *conn)
if (!conn->result)
{
printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory"));
+ libpq_gettext("out of memory\n"));
pqSaveErrorResult(conn);
}
}
@@ -363,8 +363,8 @@ pqParseInput3(PGconn *conn)
else
{
/* Set up to report error at end of query */
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("server sent data (\"D\" message) without prior row
description(\"T\" message)\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("server sent data (\"D\" message) without prior row
description(\"T\" message)\n"));
pqSaveErrorResult(conn);
/* Discard the unexpected message */
conn->inCursor += msgLength;
@@ -406,9 +406,8 @@ pqParseInput3(PGconn *conn)
*/
break;
default:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext(
- "unexpected response from server; first received character was
\"%c\"\n"),
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("unexpected response from server; first received character was
\"%c\"\n"),
id);
/* build an error result holding the error message */
pqSaveErrorResult(conn);
@@ -428,7 +427,7 @@ pqParseInput3(PGconn *conn)
else
{
/* Trouble --- report it */
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("message contents do not agree with length in message type \"%c\"\n"),
id);
/* build an error result holding the error message */
@@ -448,9 +447,8 @@ pqParseInput3(PGconn *conn)
static void
handleSyncLoss(PGconn *conn, char id, int msgLength)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext(
- "lost synchronization with server: got message type \"%c\", length %d\n"),
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("lost synchronization with server: got message type \"%c\", length %d\n"),
id, msgLength);
/* build an error result holding the error message */
pqSaveErrorResult(conn);
@@ -625,7 +623,7 @@ advance_and_error:
if (!errmsg)
errmsg = libpq_gettext("out of memory for query result");
- printfPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
+ appendPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
pqSaveErrorResult(conn);
/*
@@ -725,7 +723,7 @@ advance_and_error:
*/
if (!errmsg)
errmsg = libpq_gettext("out of memory");
- printfPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
+ appendPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
pqSaveErrorResult(conn);
/*
@@ -852,7 +850,7 @@ set_error_result:
if (!errmsg)
errmsg = libpq_gettext("out of memory for query result");
- printfPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
+ appendPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
pqSaveErrorResult(conn);
/*
@@ -953,9 +951,11 @@ pqGetErrorNotice3(PGconn *conn, bool isError)
res->errMsg = pqResultStrdup(res, workBuf.data);
pqClearAsyncResult(conn); /* redundant, but be safe */
conn->result = res;
+
+ /* We also keep a running history of errors in conn->errorMessage. */
if (PQExpBufferDataBroken(workBuf))
printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory"));
+ libpq_gettext("out of memory\n"));
else
appendPQExpBufferStr(&conn->errorMessage, workBuf.data);
}
@@ -1714,8 +1714,8 @@ pqGetline3(PGconn *conn, char *s, int maxlen)
conn->asyncStatus != PGASYNC_COPY_BOTH) ||
conn->copy_is_binary)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("PQgetline: not doing text COPY OUT\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("PQgetline: not doing text COPY OUT\n"));
*s = '\0';
return EOF;
}
@@ -1820,8 +1820,8 @@ pqEndcopy3(PGconn *conn)
conn->asyncStatus != PGASYNC_COPY_OUT &&
conn->asyncStatus != PGASYNC_COPY_BOTH)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("no COPY in progress\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("no COPY in progress\n"));
return 1;
}
@@ -1854,7 +1854,6 @@ pqEndcopy3(PGconn *conn)
/* Return to active duty */
conn->asyncStatus = PGASYNC_BUSY;
- resetPQExpBuffer(&conn->errorMessage);
/*
* Non blocking connections may have to abort at this point. If everyone
@@ -2092,7 +2091,7 @@ pqFunctionCall3(PGconn *conn, Oid fnid,
break;
default:
/* The backend violates the protocol. */
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("protocol error: id=0x%x\n"),
id);
pqSaveErrorResult(conn);
diff --git a/src/interfaces/libpq/fe-secure-common.c b/src/interfaces/libpq/fe-secure-common.c
index b3f580f..772aca4 100644
--- a/src/interfaces/libpq/fe-secure-common.c
+++ b/src/interfaces/libpq/fe-secure-common.c
@@ -94,8 +94,8 @@ pq_verify_peer_name_matches_certificate_name(PGconn *conn,
if (!(host && host[0] != '\0'))
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("host name must be specified\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("host name must be specified\n"));
return -1;
}
@@ -120,8 +120,8 @@ pq_verify_peer_name_matches_certificate_name(PGconn *conn,
if (namelen != strlen(name))
{
free(name);
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SSL certificate's name contains embedded null\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("SSL certificate's name contains embedded null\n"));
return -1;
}
@@ -167,8 +167,8 @@ pq_verify_peer_name_matches_certificate(PGconn *conn)
/* Check that we have a hostname to compare with. */
if (!(host && host[0] != '\0'))
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("host name must be specified for a verified SSL connection\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("host name must be specified for a verified SSL connection\n"));
return false;
}
@@ -184,7 +184,7 @@ pq_verify_peer_name_matches_certificate(PGconn *conn)
*/
if (names_examined > 1)
{
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_ngettext("server certificate for \"%s\" (and %d other name) does not match host
name\"%s\"\n",
"server certificate for \"%s\" (and %d other names) does not match host
name\"%s\"\n",
names_examined - 1),
@@ -192,14 +192,14 @@ pq_verify_peer_name_matches_certificate(PGconn *conn)
}
else if (names_examined == 1)
{
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("server certificate for \"%s\" does not match host name \"%s\"\n"),
first_name, host);
}
else
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("could not get server's host name from server certificate\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("could not get server's host name from server certificate\n"));
}
}
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index bbae8ef..f995ed9 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -177,8 +177,8 @@ rloop:
if (n < 0)
{
/* Not supposed to happen, so we don't translate the msg */
- printfPQExpBuffer(&conn->errorMessage,
- "SSL_read failed but did not provide error information\n");
+ appendPQExpBufferStr(&conn->errorMessage,
+ "SSL_read failed but did not provide error information\n");
/* assume the connection is broken */
result_errno = ECONNRESET;
}
@@ -201,21 +201,20 @@ rloop:
result_errno = SOCK_ERRNO;
if (result_errno == EPIPE ||
result_errno == ECONNRESET)
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext(
- "server closed the connection unexpectedly\n"
- "\tThis probably means the server terminated abnormally\n"
- "\tbefore or while processing the request.\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("server closed the connection unexpectedly\n"
+ "\tThis probably means the server terminated abnormally\n"
+ "\tbefore or while processing the request.\n"));
else
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("SSL SYSCALL error: %s\n"),
SOCK_STRERROR(result_errno,
sebuf, sizeof(sebuf)));
}
else
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SSL SYSCALL error: EOF detected\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("SSL SYSCALL error: EOF detected\n"));
/* assume the connection is broken */
result_errno = ECONNRESET;
n = -1;
@@ -225,7 +224,7 @@ rloop:
{
char *errm = SSLerrmessage(ecode);
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("SSL error: %s\n"), errm);
SSLerrfree(errm);
/* assume the connection is broken */
@@ -240,13 +239,13 @@ rloop:
* a clean connection closure, so we should not report it as a
* server crash.
*/
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SSL connection has been closed unexpectedly\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("SSL connection has been closed unexpectedly\n"));
result_errno = ECONNRESET;
n = -1;
break;
default:
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("unrecognized SSL error code: %d\n"),
err);
/* assume the connection is broken */
@@ -287,8 +286,8 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
if (n < 0)
{
/* Not supposed to happen, so we don't translate the msg */
- printfPQExpBuffer(&conn->errorMessage,
- "SSL_write failed but did not provide error information\n");
+ appendPQExpBufferStr(&conn->errorMessage,
+ "SSL_write failed but did not provide error information\n");
/* assume the connection is broken */
result_errno = ECONNRESET;
}
@@ -309,21 +308,20 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
{
result_errno = SOCK_ERRNO;
if (result_errno == EPIPE || result_errno == ECONNRESET)
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext(
- "server closed the connection unexpectedly\n"
- "\tThis probably means the server terminated abnormally\n"
- "\tbefore or while processing the request.\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("server closed the connection unexpectedly\n"
+ "\tThis probably means the server terminated abnormally\n"
+ "\tbefore or while processing the request.\n"));
else
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("SSL SYSCALL error: %s\n"),
SOCK_STRERROR(result_errno,
sebuf, sizeof(sebuf)));
}
else
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SSL SYSCALL error: EOF detected\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("SSL SYSCALL error: EOF detected\n"));
/* assume the connection is broken */
result_errno = ECONNRESET;
n = -1;
@@ -333,7 +331,7 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
{
char *errm = SSLerrmessage(ecode);
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("SSL error: %s\n"), errm);
SSLerrfree(errm);
/* assume the connection is broken */
@@ -348,13 +346,13 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
* a clean connection closure, so we should not report it as a
* server crash.
*/
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SSL connection has been closed unexpectedly\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("SSL connection has been closed unexpectedly\n"));
result_errno = ECONNRESET;
n = -1;
break;
default:
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("unrecognized SSL error code: %d\n"),
err);
/* assume the connection is broken */
@@ -394,8 +392,8 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
if (!OBJ_find_sigid_algs(X509_get_signature_nid(peer_cert),
&algo_nid, NULL))
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("could not determine server certificate signature algorithm\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("could not determine server certificate signature algorithm\n"));
return NULL;
}
@@ -415,7 +413,7 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
algo_type = EVP_get_digestbynid(algo_nid);
if (algo_type == NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not find digest for NID %s\n"),
OBJ_nid2sn(algo_nid));
return NULL;
@@ -425,8 +423,8 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
if (!X509_digest(peer_cert, algo_type, hash, &hash_size))
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("could not generate peer certificate hash\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("could not generate peer certificate hash\n"));
return NULL;
}
@@ -443,7 +441,7 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
return cert_hash;
}
-#endif /* HAVE_X509_GET_SIGNATURE_NID */
+#endif /* HAVE_X509_GET_SIGNATURE_NID */
/* ------------------------------------------------------------ */
/* OpenSSL specific code */
@@ -482,8 +480,8 @@ openssl_verify_peer_name_matches_certificate_name(PGconn *conn, ASN1_STRING *nam
/* Should not happen... */
if (name_entry == NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SSL certificate's name entry is missing\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("SSL certificate's name entry is missing\n"));
return -1;
}
@@ -811,7 +809,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not create SSL context: %s\n"),
err);
SSLerrfree(err);
@@ -848,7 +846,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not read root certificate file \"%s\": %s\n"),
fnbuf, err);
SSLerrfree(err);
@@ -876,7 +874,7 @@ initialize_SSL(PGconn *conn)
#else
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("SSL library does not support CRL certificates (file \"%s\")\n"),
fnbuf);
SSLerrfree(err);
@@ -904,11 +902,11 @@ initialize_SSL(PGconn *conn)
* that it seems worth having a specialized error message for it.
*/
if (fnbuf[0] == '\0')
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("could not get home directory to locate root certificate file\n"
- "Either provide the file or change sslmode to disable server
certificateverification.\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("could not get home directory to locate root certificate file\n"
+ "Either provide the file or change sslmode to disable server
certificateverification.\n"));
else
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("root certificate file \"%s\" does not exist\n"
"Either provide the file or change sslmode to disable server
certificateverification.\n"), fnbuf);
SSL_CTX_free(SSL_context);
@@ -939,7 +937,7 @@ initialize_SSL(PGconn *conn)
*/
if (errno != ENOENT && errno != ENOTDIR)
{
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not open certificate file \"%s\": %s\n"),
fnbuf, pqStrerror(errno, sebuf, sizeof(sebuf)));
SSL_CTX_free(SSL_context);
@@ -958,7 +956,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not read certificate file \"%s\": %s\n"),
fnbuf, err);
SSLerrfree(err);
@@ -983,7 +981,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not establish SSL connection: %s\n"),
err);
SSLerrfree(err);
@@ -1037,7 +1035,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not load SSL engine \"%s\": %s\n"),
engine_str, err);
SSLerrfree(err);
@@ -1049,7 +1047,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not initialize SSL engine \"%s\": %s\n"),
engine_str, err);
SSLerrfree(err);
@@ -1065,7 +1063,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not read private SSL key \"%s\" from engine \"%s\": %s\n"),
engine_colon, engine_str, err);
SSLerrfree(err);
@@ -1079,7 +1077,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not load private SSL key \"%s\" from engine \"%s\": %s\n"),
engine_colon, engine_str, err);
SSLerrfree(err);
@@ -1116,7 +1114,7 @@ initialize_SSL(PGconn *conn)
if (stat(fnbuf, &buf) != 0)
{
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("certificate present, but not private key file \"%s\"\n"),
fnbuf);
return -1;
@@ -1124,7 +1122,7 @@ initialize_SSL(PGconn *conn)
#ifndef WIN32
if (!S_ISREG(buf.st_mode) || buf.st_mode & (S_IRWXG | S_IRWXO))
{
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("private key file \"%s\" has group or world access; permissions should be
u=rw(0600) or less\n"),
fnbuf);
return -1;
@@ -1135,7 +1133,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not load private key file \"%s\": %s\n"),
fnbuf, err);
SSLerrfree(err);
@@ -1149,7 +1147,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("certificate does not match private key file \"%s\": %s\n"),
fnbuf, err);
SSLerrfree(err);
@@ -1215,12 +1213,12 @@ open_client_SSL(PGconn *conn)
char sebuf[256];
if (r == -1)
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("SSL SYSCALL error: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
else
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SSL SYSCALL error: EOF detected\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("SSL SYSCALL error: EOF detected\n"));
pgtls_close(conn);
return PGRES_POLLING_FAILED;
}
@@ -1228,7 +1226,7 @@ open_client_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ecode);
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("SSL error: %s\n"),
err);
SSLerrfree(err);
@@ -1237,7 +1235,7 @@ open_client_SSL(PGconn *conn)
}
default:
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("unrecognized SSL error code: %d\n"),
err);
pgtls_close(conn);
@@ -1258,7 +1256,7 @@ open_client_SSL(PGconn *conn)
err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("certificate could not be obtained: %s\n"),
err);
SSLerrfree(err);
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index f7dc249..157a738 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -205,8 +205,8 @@ pqsecure_close(PGconn *conn)
/*
* Read data from a secure connection.
*
- * On failure, this function is responsible for putting a suitable message
- * into conn->errorMessage. The caller must still inspect errno, but only
+ * On failure, this function is responsible for appending a suitable message
+ * to conn->errorMessage. The caller must still inspect errno, but only
* to determine whether to continue/retry after error.
*/
ssize_t
@@ -256,16 +256,15 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len)
#ifdef ECONNRESET
case ECONNRESET:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext(
- "server closed the connection unexpectedly\n"
- "\tThis probably means the server terminated abnormally\n"
- "\tbefore or while processing the request.\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("server closed the connection unexpectedly\n"
+ "\tThis probably means the server terminated abnormally\n"
+ "\tbefore or while processing the request.\n"));
break;
#endif
default:
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not receive data from server: %s\n"),
SOCK_STRERROR(result_errno,
sebuf, sizeof(sebuf)));
@@ -282,8 +281,8 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len)
/*
* Write data to a secure connection.
*
- * On failure, this function is responsible for putting a suitable message
- * into conn->errorMessage. The caller must still inspect errno, but only
+ * On failure, this function is responsible for appending a suitable message
+ * to conn->errorMessage. The caller must still inspect errno, but only
* to determine whether to continue/retry after error.
*/
ssize_t
@@ -366,15 +365,14 @@ retry_masked:
case ECONNRESET:
#endif
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext(
- "server closed the connection unexpectedly\n"
- "\tThis probably means the server terminated abnormally\n"
- "\tbefore or while processing the request.\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("server closed the connection unexpectedly\n"
+ "\tThis probably means the server terminated abnormally\n"
+ "\tbefore or while processing the request.\n"));
break;
default:
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not send data to server: %s\n"),
SOCK_STRERROR(result_errno,
sebuf, sizeof(sebuf)));
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index bc60373..6df3af0 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -679,7 +679,7 @@ extern void pgtls_init_library(bool do_ssl, int do_crypto);
* The conn parameter is only used to be able to pass back an error
* message - no connection-local setup is made here.
*
- * Returns 0 if OK, -1 on failure (with a message in conn->errorMessage).
+ * Returns 0 if OK, -1 on failure (with a message added to conn->errorMessage).
*/
extern int pgtls_init(PGconn *conn);
@@ -696,8 +696,8 @@ extern void pgtls_close(PGconn *conn);
/*
* Read data from a secure connection.
*
- * On failure, this function is responsible for putting a suitable message
- * into conn->errorMessage. The caller must still inspect errno, but only
+ * On failure, this function is responsible for appending a suitable message
+ * to conn->errorMessage. The caller must still inspect errno, but only
* to determine whether to continue/retry after error.
*/
extern ssize_t pgtls_read(PGconn *conn, void *ptr, size_t len);
@@ -710,8 +710,8 @@ extern bool pgtls_read_pending(PGconn *conn);
/*
* Write data to a secure connection.
*
- * On failure, this function is responsible for putting a suitable message
- * into conn->errorMessage. The caller must still inspect errno, but only
+ * On failure, this function is responsible for appending a suitable message
+ * to conn->errorMessage. The caller must still inspect errno, but only
* to determine whether to continue/retry after error.
*/
extern ssize_t pgtls_write(PGconn *conn, const void *ptr, size_t len);
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 9b6a065..a16ad4b 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -2115,6 +2115,31 @@ keep_going: /* We will come back to here until there is
goto error_return;
}
conn->whichhost++;
+
+ /*
+ * If we're given more than one target host, append identification
+ * info to conn->errorMessage as we start to consider each one. This
+ * makes it easier to figure out which host error messages apply to,
+ * in case we fail to connect to all of them.
+ */
+ if (conn->nconnhost > 1)
+ {
+ pg_conn_host *ch = &conn->connhost[conn->whichhost];
+ const char *displayed_host;
+ const char *displayed_port;
+
+ if (ch->type == CHT_HOST_ADDRESS)
+ displayed_host = ch->hostaddr;
+ else
+ displayed_host = ch->host;
+ displayed_port = ch->port;
+ if (displayed_port == NULL || displayed_port[0] == '\0')
+ displayed_port = DEF_PGPORT_STR;
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("server \"%s\" port %s:\n"),
+ displayed_host, displayed_port);
+ }
+
conn->addr_cur = conn->connhost[conn->whichhost].addrlist;
/* If no addresses for this host, just try the next one */
if (conn->addr_cur == NULL)
@@ -3154,9 +3179,6 @@ keep_going: /* We will come back to here until there is
}
case CONNECTION_CHECK_WRITABLE:
{
- const char *displayed_host;
- const char *displayed_port;
-
if (!PQconsumeInput(conn))
goto error_return;
@@ -3173,25 +3195,10 @@ keep_going: /* We will come back to here until there is
if (strncmp(val, "on", 2) == 0)
{
/* Not writable; fail this connection. */
- const char *displayed_host;
- const char *displayed_port;
-
PQclear(res);
- /* Append error report to conn->errorMessage. */
- if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS)
- displayed_host = conn->connhost[conn->whichhost].hostaddr;
- else
- displayed_host = conn->connhost[conn->whichhost].host;
- displayed_port = conn->connhost[conn->whichhost].port;
- if (displayed_port == NULL || displayed_port[0] == '\0')
- displayed_port = DEF_PGPORT_STR;
-
- appendPQExpBuffer(&conn->errorMessage,
- libpq_gettext("could not make a writable "
- "connection to server "
- "\"%s:%s\"\n"),
- displayed_host, displayed_port);
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("could not open a read-write session\n"));
/* Close connection politely. */
conn->status = CONNECTION_OK;
@@ -3217,25 +3224,17 @@ keep_going: /* We will come back to here until there is
}
/*
- * Something went wrong with "SHOW transaction_read_only". We
- * should try next addresses.
+ * Something went wrong with "SHOW transaction_read_only". If
+ * there was a server error, it'll already have been appended
+ * to errorMessage, else say something generic.
*/
+ if (res == NULL || PQresultStatus(res) != PGRES_FATAL_ERROR)
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("\"SHOW transaction_read_only\" failed\n"));
+
if (res)
PQclear(res);
- /* Append error report to conn->errorMessage. */
- if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS)
- displayed_host = conn->connhost[conn->whichhost].hostaddr;
- else
- displayed_host = conn->connhost[conn->whichhost].host;
- displayed_port = conn->connhost[conn->whichhost].port;
- if (displayed_port == NULL || displayed_port[0] == '\0')
- displayed_port = DEF_PGPORT_STR;
- appendPQExpBuffer(&conn->errorMessage,
- libpq_gettext("test \"SHOW transaction_read_only\" failed "
- "on server \"%s:%s\"\n"),
- displayed_host, displayed_port);
-
/* Close connection politely. */
conn->status = CONNECTION_OK;
sendTerminateConn(conn);
Hello Tom, > Some points for discussion and review: > > 1. The bulk of patch 0001 is indeed just > s/printfPQExpBuffer/appendPQExpBuffer/g, though I also made an attempt > to use appendPQExpBufferStr wherever possible. There are two categories > of printfPQExpBuffer calls I didn't change: > 1a. Calls reporting an out-of-memory situation. There was already a > policy in some places that we'd intentionally printf not append such > messages, and I made that explicit. The idea is that we might not > have room to add more text to errorMessage, so resetting the buffer > provides more certainty of success. However, this makes for a pretty > weird inconsistency in the code; there are a lot of functions now in > which half of the messages are printf'd and half are append'd, so I'm > afraid that future patches will get it wrong as to which to use. Also, > in reality there often *would* be room to append "out of memory" without > enlarging errorMessage's buffer, so that this could just be pointless > destruction of useful error history. I didn't do anything about it here, > but I'm tempted to replace all the printf's for OOM messages with a > subroutine that will append if it can do so without enlarging the existing > buffer, else replace. > > 1b. There are a few places where it didn't seem worth changing anything > because it'd just result in needing to add a resetPQExpBuffer in the same > function or very nearby. Mostly in fe-lobj.c. > > 2. I had to drop some code (notably in pqPrepareAsyncResult) that > attempted to force conn->errorMessage to always have the same contents > as the current PGresult's PQresultErrorMessage; as it stood, server > errors such as "role does not exist" wiped out preceding contents of > errorMessage, breaking cases such as the second example above. This is > slightly annoying, but in the new dispensation it's possible that > conn->errorMessage contains a mix of libpq-generated errors and text > received from the server, so I'm not sure that preserving the old > equivalence is a useful goal anyway. We shouldn't cram pre-existing > errorMessage text back into a server-generated error; that would > invalidate the server's auxiliary error info such as the SQLSTATE. > I don't know if it's possible to do better here. One idea is to tweak > pqPrepareAsyncResult so that it does the message synchronization only > when in CONNECTION_OK state, allowing conn->errorMessage to be a > historical record only for connection processing. But that seems like > a hack. > > 3. In some places I failed to resist the temptation to copy-edit error > messages that didn't meet style guidelines, or to add libpq_gettext() > annotation where it seemed to have been missed. > > 4. connectDBComplete() cleared errorMessage after a successful > completion, but that's the wrong place to do it; we have to do that > at the success exits from PQconnectPoll, else applications that > use PQconnectPoll directly will see leftover garbage there. > > 5. There were places in PQconnectPoll that temporarily set > conn->status = CONNECTION_OK to prevent PQsendQueryStart from > complaining. A better idea is to leave the status alone and change > that test in PQsendQueryStart to be status != CONNECTION_BAD. This > not only simplifies PQconnectPoll, but allows PQsendQueryStart > to tell whether we're still in the connection sequence. I fixed > it to not clear errorMessage when we are, thus solving the problem > of PQsendQuery destroying prior connection error history. That allows > getting rid of saveErrorMessage/restoreErrorMessage, which were > (a) an incredibly ugly and inefficient hack, and (b) inadequate, > since there were other paths through PQconnectPoll that could reach > PQsendQueryStart but were not protected with calls to those. > > 6. In the 0002 patch, I have it printing the annotation only when > there's more than one connhost[], and only once per connhost not > once per IP address. The motivation for doing it like that was to > not change the behavior for simple cases with only one host name. > There is certainly room to argue that we should make an annotation > once per IP address, but that'd change the behavior for pretty common > cases like "localhost" resolving to both 127.0.0.1 and ::1, so I don't > know if it'd be a good idea or not. There's also an interaction to > consider with my patch at > https://www.postgresql.org/message-id/4913.1533827102%40sss.pgh.pa.us > namely that we have to be sure the labeling behaves sanely for > connhosts that fail to resolve any IP addresses. > > I'll put this into the September commitfest. * About the *first* patch It applies cleanly and compiles. Alas, global "make check" is ok although there is no change on regression tests, which suggest that the multiple error message reporting is not tested anywhere. Should there be at least one test? Getting rid of somehow ugly "let's save the current message and restore it later" logic in places looks like a definite improvement. There are still 86 instances of "printfPQExpBuffer", which seems like a lot. They are mostly OOM messages, but not only. This make checking the patch quite cumbersome. I'd rather there would be only one rule, and clear reset with a comment when needded (eg "fe-lobj.c"). I agree with your suggestion of adding a function trying to preserve messages on OOM errors if possible. Maybe "appendPQExpBufferOOM"? It is unclear to me why the "printf" variant is used in "PQencryptPasswordConn" and "connectOptions2", it looks that you skipped these functions. In passing, some OOM message are rather terse: "out of memory\n", other are more precise "out of memory allocating GSSAPI buffer (%d)\n". I do not think it is worth creating a l10n exception on a should not happen message, eg (there are a few): - /* shouldn't happen */ - printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("invalid SCRAM exchange state\n")); + /* shouldn't happen, so we don't translate the msg */ + appendPQExpBufferStr(&conn->errorMessage, + "invalid SCRAM exchange state\n"); I would have let "libpq_gettext" it as it was. I'm ok of libpq & server messages are mixed, because I do not see why one may take precedent on the others, and when some strange & combined errors, every clue is worthy. I have not seen anything which raises a "risky" alarm about undesired consequences, but who knows? Comments update seem ok. It is possible that some were missed. * About the second patch: It applies cleanly on top of previous and compiles. Yet again, global "make check" is ok although there were no test changes, which just show the abysmal coverage of psql regression tests. Should there be at least one test? The feature answers some of my issues with the "libpq should not look up all host addresses at once" patch. * "server \"%s\" port %s:\n" I do not like the resulting output server: problem found more details I'd rather have server: problem found more details which would require to append a '\n' at the beginning of the line from the second attempt. ISTM that both the hostname and ip should be shown to avoid confusion about hosts with multiple ips, esp. as ips are given in any order by the dns. The patch misses the one server with multiple ips case. ISTM that the introductory line should be shown in such case as well. sh> psql "host=local.coelho.net,localhost port=5434" psql: server "local.coelho.net" port 5434: ... server "localhost" port 5434: ... But: sh> psql "host=local2.coelho.net port=5434" psql: could not connect to server: Connection refused ... could not connect to server: Connection refused ... Even if the hint message in this case is explicit enough, ISTM that the server line is deserved. Also for homogeneity, I'd suggest to always add the server line. If the server introduction is inserted in all cases, including when only one host is used, hints become partially redundant: server "local.coelho.net" port 5434: could not connect to server: Connection refused Is the server running on host "local.coelho.net" (127.0.0.1) and accepting TCP/IP connections on port 5434? This would allow to simplify more hints, which you seem to have done on "open read-write session" and "SHOW transaction_read_only" but not others. -- Fabien.
Fabien COELHO <coelho@cri.ensmp.fr> writes:
> ISTM that both the hostname and ip should be shown to avoid confusion
> about hosts with multiple ips, esp. as ips are given in any order by the
> dns.
> ...
> Also for homogeneity, I'd suggest to always add the server line. If the
> server introduction is inserted in all cases, including when only one host
> is used, hints become partially redundant:
> server "local.coelho.net" port 5434:
> could not connect to server: Connection refused
> Is the server running on host "local.coelho.net" (127.0.0.1) and accepting
> TCP/IP connections on port 5434?
> This would allow to simplify more hints, which you seem to have done on
> "open read-write session" and "SHOW transaction_read_only" but not others.
As I explained in my comments, the reason I did not do these things
is that I didn't want to change the output for cases in which just a
single host name is given. I still don't. People will think it's
change for the sake of change, and they tend not to like that.
The multi-host feature is new enough that I think we can still get away
with changing how errors are reported in those cases ... but what you're
proposing here is to mess with error-reporting behavior that was settled
on decades ago. I'm not really interested in taking the flak that will
come with that.
regards, tom lane
Hello Tom, >> ISTM that both the hostname and ip should be shown to avoid confusion >> about hosts with multiple ips, esp. as ips are given in any order by the >> dns. >> ... >> Also for homogeneity, I'd suggest to always add the server line. If the >> server introduction is inserted in all cases, including when only one host >> is used, hints become partially redundant: >> server "local.coelho.net" port 5434: >> could not connect to server: Connection refused >> Is the server running on host "local.coelho.net" (127.0.0.1) and accepting >> TCP/IP connections on port 5434? >> This would allow to simplify more hints, which you seem to have done on >> "open read-write session" and "SHOW transaction_read_only" but not others. > > As I explained in my comments, the reason I did not do these things > is that I didn't want to change the output for cases in which just a > single host name is given. I still don't. Ok, I get your argument when there is just one target server (cluster), which is probably at least 99.9% of the use case in practice. However, ISTM multiple ip & multiple hostname look pretty close. I guess that the number of people that use these features is small, but for these when things go wrong better messages are useful... so I would see no harm to trigger the server introductory line when there are multiples servers, whatever the reason why there are multiples servers. -- Fabien.
Fabien COELHO <coelho@cri.ensmp.fr> writes:
>> As I explained in my comments, the reason I did not do these things
>> is that I didn't want to change the output for cases in which just a
>> single host name is given. I still don't.
> Ok, I get your argument when there is just one target server (cluster),
> which is probably at least 99.9% of the use case in practice.
> However, ISTM multiple ip & multiple hostname look pretty close.
> I guess that the number of people that use these features is small, but
> for these when things go wrong better messages are useful... so I would
> see no harm to trigger the server introductory line when there are
> multiples servers, whatever the reason why there are multiples servers.
The thing is that there are a *lot* of systems nowadays on which localhost
maps to both ipv4 and ipv6, so that "host=localhost" would be enough to
trigger the new behavior, and I think that would inevitably give rise to
more complaints than kudos. So I'm still of the opinion that we're better
off with the second patch's behavior as it stands.
Responding to your other points from upthread:
> There are still 86 instances of "printfPQExpBuffer", which seems like a
> lot. They are mostly OOM messages, but not only. This make checking the
> patch quite cumbersome.
> I'd rather there would be only one rule, and clear reset with a comment
> when needded (eg "fe-lobj.c").
Well, I did this for discussion's sake, but I don't really find the
resulting changes in fe-lobj.c to be any sort of improvement. In
particular, the messiness around suppressing extraneous complaints from
lo_close is still there, if anything worse than before (it's hard to
get rid of because lo_close will reset the buffer). I'd rather just
leave fe-lobj.c alone, possibly adding a header comment explaining why
it's different. Which is basically because none of those functions
expect to be appending multiple errors; they're all gonna quit after
the first one.
> I agree with your suggestion of adding a function trying to preserve
> messages on OOM errors if possible. Maybe "appendPQExpBufferOOM"?
Yeah, done.
> It is unclear to me why the "printf" variant is used in
> "PQencryptPasswordConn" and "connectOptions2", it looks that you skipped
> these functions.
I did because it seemed like unnecessary extra diff content, since
we're expecting the error buffer to be initially empty anyway (for
connectOptions2) or we'd just need an extra reset (for
PQencryptPasswordConn). In the attached I went ahead and made those
changes, but again I'm unsure that it's really a net improvement.
> In passing, some OOM message are rather terse: "out of memory\n", other
> are more precise "out of memory allocating GSSAPI buffer (%d)\n".
I got rid of the latter; I think they're unnecessary translation burdens,
and it'd be hard to fit them into a one-size-fits-all OOM reporting
subroutine, and it's pretty unclear what's the point of special-casing
them anyway.
> I do not like the resulting output
> server:
> problem found
> more details
> I'd rather have
> server: problem found
> more details
Done in the attached, although I'm concerned about the resulting effects
for error message line length --- in my tests, lines that used to reliably
fit in 80 columns don't anymore. Again, I'm not really convinced this is
an improvement. It would absolutely *not* be an improvement if we tried
to also shoehorn the server's IP address in there; that'd make the lines
way too long, with way too much stuff before the important part.
regards, tom lane
diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c
index 1d9c937..e7110d5 100644
--- a/src/interfaces/libpq/fe-auth-scram.c
+++ b/src/interfaces/libpq/fe-auth-scram.c
@@ -185,14 +185,14 @@ pg_fe_scram_exchange(void *opaq, char *input, int inputlen,
{
if (inputlen == 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("malformed SCRAM message (empty message)\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("malformed SCRAM message (empty message)\n"));
goto error;
}
if (inputlen != strlen(input))
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("malformed SCRAM message (length mismatch)\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("malformed SCRAM message (length mismatch)\n"));
goto error;
}
}
@@ -240,17 +240,17 @@ pg_fe_scram_exchange(void *opaq, char *input, int inputlen,
else
{
*success = false;
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("incorrect server signature\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("incorrect server signature\n"));
}
*done = true;
state->state = FE_SCRAM_FINISHED;
break;
default:
- /* shouldn't happen */
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("invalid SCRAM exchange state\n"));
+ /* shouldn't happen, so we don't translate the msg */
+ appendPQExpBufferStr(&conn->errorMessage,
+ "invalid SCRAM exchange state\n");
goto error;
}
return;
@@ -272,7 +272,7 @@ read_attr_value(char **input, char attr, PQExpBuffer errorMessage)
if (*begin != attr)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("malformed SCRAM message (attribute \"%c\" expected)\n"),
attr);
return NULL;
@@ -281,7 +281,7 @@ read_attr_value(char **input, char attr, PQExpBuffer errorMessage)
if (*begin != '=')
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("malformed SCRAM message (expected character \"=\" for attribute \"%c\")\n"),
attr);
return NULL;
@@ -322,16 +322,15 @@ build_client_first_message(fe_scram_state *state)
*/
if (!pg_frontend_random(raw_nonce, SCRAM_RAW_NONCE_LEN))
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("could not generate nonce\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("could not generate nonce\n"));
return NULL;
}
state->client_nonce = malloc(pg_b64_enc_len(SCRAM_RAW_NONCE_LEN) + 1);
if (state->client_nonce == NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return NULL;
}
encoded_len = pg_b64_encode(raw_nonce, SCRAM_RAW_NONCE_LEN, state->client_nonce);
@@ -397,8 +396,7 @@ build_client_first_message(fe_scram_state *state)
oom_error:
termPQExpBuffer(&buf);
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return NULL;
}
@@ -471,13 +469,13 @@ build_client_final_message(fe_scram_state *state)
#else
/*
* Chose channel binding, but the SSL library doesn't support it.
- * Shouldn't happen.
+ * Shouldn't happen, so we don't translate the msg.
*/
termPQExpBuffer(&buf);
- printfPQExpBuffer(&conn->errorMessage,
- "channel binding not supported by this build\n");
+ appendPQExpBufferStr(&conn->errorMessage,
+ "channel binding not supported by this build\n");
return NULL;
-#endif /* HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH */
+#endif /* HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH */
}
#ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
else if (conn->ssl_in_use)
@@ -519,8 +517,7 @@ build_client_final_message(fe_scram_state *state)
oom_error:
termPQExpBuffer(&buf);
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return NULL;
}
@@ -539,8 +536,7 @@ read_server_first_message(fe_scram_state *state, char *input)
state->server_first_message = strdup(input);
if (state->server_first_message == NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return false;
}
@@ -557,16 +553,15 @@ read_server_first_message(fe_scram_state *state, char *input)
if (strlen(nonce) < strlen(state->client_nonce) ||
memcmp(nonce, state->client_nonce, strlen(state->client_nonce)) != 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("invalid SCRAM response (nonce mismatch)\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("invalid SCRAM response (nonce mismatch)\n"));
return false;
}
state->nonce = strdup(nonce);
if (state->nonce == NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return false;
}
@@ -579,8 +574,7 @@ read_server_first_message(fe_scram_state *state, char *input)
state->salt = malloc(pg_b64_dec_len(strlen(encoded_salt)));
if (state->salt == NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return false;
}
state->saltlen = pg_b64_decode(encoded_salt,
@@ -596,14 +590,14 @@ read_server_first_message(fe_scram_state *state, char *input)
state->iterations = strtol(iterations_str, &endptr, 10);
if (*endptr != '\0' || state->iterations < 1)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("malformed SCRAM message (invalid iteration count)\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("malformed SCRAM message (invalid iteration count)\n"));
return false;
}
if (*input != '\0')
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("malformed SCRAM message (garbage at end of server-first-message)\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("malformed SCRAM message (garbage at end of server-first-message)\n"));
return true;
}
@@ -621,8 +615,7 @@ read_server_final_message(fe_scram_state *state, char *input)
state->server_final_message = strdup(input);
if (!state->server_final_message)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return false;
}
@@ -632,7 +625,7 @@ read_server_final_message(fe_scram_state *state, char *input)
char *errmsg = read_attr_value(&input, 'e',
&conn->errorMessage);
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("error received from server in SCRAM exchange: %s\n"),
errmsg);
return false;
@@ -648,16 +641,16 @@ read_server_final_message(fe_scram_state *state, char *input)
}
if (*input != '\0')
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("malformed SCRAM message (garbage at end of server-final-message)\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("malformed SCRAM message (garbage at end of server-final-message)\n"));
server_signature_len = pg_b64_decode(encoded_server_signature,
strlen(encoded_server_signature),
state->ServerSignature);
if (server_signature_len != SCRAM_KEY_LEN)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("malformed SCRAM message (invalid server signature)\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("malformed SCRAM message (invalid server signature)\n"));
return false;
}
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 540aba9..69f2efb 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -81,14 +81,12 @@ pg_GSS_error_int(PQExpBuffer str, const char *mprefix,
}
/*
- * GSSAPI errors contain two parts; put both into conn->errorMessage.
+ * GSSAPI errors contain two parts; append both to conn->errorMessage.
*/
static void
pg_GSS_error(const char *mprefix, PGconn *conn,
OM_uint32 maj_stat, OM_uint32 min_stat)
{
- resetPQExpBuffer(&conn->errorMessage);
-
/* Fetch major error codes */
pg_GSS_error_int(&conn->errorMessage, mprefix, maj_stat, GSS_C_GSS_CODE);
@@ -118,9 +116,7 @@ pg_GSS_continue(PGconn *conn, int payloadlen)
ginbuf.value = malloc(payloadlen);
if (!ginbuf.value)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory allocating GSSAPI buffer (%d)\n"),
- payloadlen);
+ pqReportOOM(conn);
return STATUS_ERROR;
}
if (pqGetnchar(ginbuf.value, payloadlen, conn))
@@ -203,15 +199,15 @@ pg_GSS_startup(PGconn *conn, int payloadlen)
if (!(host && host[0] != '\0'))
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("host name must be specified\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("host name must be specified\n"));
return STATUS_ERROR;
}
if (conn->gctx)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("duplicate GSS authentication request\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("duplicate GSS authentication request\n"));
return STATUS_ERROR;
}
@@ -223,8 +219,7 @@ pg_GSS_startup(PGconn *conn, int payloadlen)
temp_gbuf.value = (char *) malloc(maxlen);
if (!temp_gbuf.value)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return STATUS_ERROR;
}
snprintf(temp_gbuf.value, maxlen, "%s@%s",
@@ -268,10 +263,10 @@ pg_SSPI_error(PGconn *conn, const char *mprefix, SECURITY_STATUS r)
FORMAT_MESSAGE_FROM_SYSTEM,
NULL, r, 0,
sysmsg, sizeof(sysmsg), NULL) == 0)
- printfPQExpBuffer(&conn->errorMessage, "%s: SSPI error %x\n",
+ appendPQExpBuffer(&conn->errorMessage, "%s: SSPI error %x\n",
mprefix, (unsigned int) r);
else
- printfPQExpBuffer(&conn->errorMessage, "%s: %s (%x)\n",
+ appendPQExpBuffer(&conn->errorMessage, "%s: %s (%x)\n",
mprefix, sysmsg, (unsigned int) r);
}
@@ -299,9 +294,7 @@ pg_SSPI_continue(PGconn *conn, int payloadlen)
inputbuf = malloc(payloadlen);
if (!inputbuf)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory allocating SSPI buffer (%d)\n"),
- payloadlen);
+ pqReportOOM(conn);
return STATUS_ERROR;
}
if (pqGetnchar(inputbuf, payloadlen, conn))
@@ -359,7 +352,7 @@ pg_SSPI_continue(PGconn *conn, int payloadlen)
conn->sspictx = malloc(sizeof(CtxtHandle));
if (conn->sspictx == NULL)
{
- printfPQExpBuffer(&conn->errorMessage, libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return STATUS_ERROR;
}
memcpy(conn->sspictx, &newContext, sizeof(CtxtHandle));
@@ -376,9 +369,11 @@ pg_SSPI_continue(PGconn *conn, int payloadlen)
/*
* This should never happen, at least not for Kerberos
* authentication. Keep check in case it shows up with other
- * authentication methods later.
+ * authentication methods later. We intentionally don't translate
+ * the message, since we aren't expecting it to happen.
*/
- printfPQExpBuffer(&conn->errorMessage, "SSPI returned invalid number of output buffers\n");
+ appendPQExpBufferStr(&conn->errorMessage,
+ "SSPI returned invalid number of output buffers\n");
return STATUS_ERROR;
}
@@ -418,8 +413,8 @@ pg_SSPI_startup(PGconn *conn, int use_negotiate, int payloadlen)
if (conn->sspictx)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("duplicate SSPI authentication request\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("duplicate SSPI authentication request\n"));
return STATUS_ERROR;
}
@@ -429,7 +424,7 @@ pg_SSPI_startup(PGconn *conn, int use_negotiate, int payloadlen)
conn->sspicred = malloc(sizeof(CredHandle));
if (conn->sspicred == NULL)
{
- printfPQExpBuffer(&conn->errorMessage, libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return STATUS_ERROR;
}
@@ -457,14 +452,14 @@ pg_SSPI_startup(PGconn *conn, int use_negotiate, int payloadlen)
*/
if (!(host && host[0] != '\0'))
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("host name must be specified\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("host name must be specified\n"));
return STATUS_ERROR;
}
conn->sspitarget = malloc(strlen(conn->krbsrvname) + strlen(host) + 2);
if (!conn->sspitarget)
{
- printfPQExpBuffer(&conn->errorMessage, libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return STATUS_ERROR;
}
sprintf(conn->sspitarget, "%s/%s", conn->krbsrvname, host);
@@ -497,8 +492,8 @@ pg_SASL_init(PGconn *conn, int payloadlen)
if (conn->sasl_state)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("duplicate SASL authentication request\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("duplicate SASL authentication request\n"));
goto error;
}
@@ -513,8 +508,8 @@ pg_SASL_init(PGconn *conn, int payloadlen)
{
if (pqGets(&mechanism_buf, conn))
{
- printfPQExpBuffer(&conn->errorMessage,
- "fe_sendauth: invalid authentication request from server: invalid list of authentication
mechanisms\n");
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("invalid authentication request from server: invalid list of
authenticationmechanisms\n"));
goto error;
}
if (PQExpBufferDataBroken(mechanism_buf))
@@ -545,8 +540,8 @@ pg_SASL_init(PGconn *conn, int payloadlen)
* the client and server supported it. The SCRAM exchange
* checks for that, to prevent downgrade attacks.
*/
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("server offered SCRAM-SHA-256-PLUS authentication over a non-SSL
connection\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("server offered SCRAM-SHA-256-PLUS authentication over a non-SSL
connection\n"));
goto error;
}
}
@@ -557,8 +552,8 @@ pg_SASL_init(PGconn *conn, int payloadlen)
if (!selected_mechanism)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("none of the server's SASL authentication mechanisms are supported\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("none of the server's SASL authentication mechanisms are supported\n"));
goto error;
}
@@ -578,8 +573,9 @@ pg_SASL_init(PGconn *conn, int payloadlen)
password = conn->pgpass;
if (password == NULL || password[0] == '\0')
{
- printfPQExpBuffer(&conn->errorMessage,
- PQnoPasswordSupplied);
+ /* Intentionally not translated. */
+ appendPQExpBufferStr(&conn->errorMessage,
+ PQnoPasswordSupplied);
goto error;
}
@@ -639,8 +635,7 @@ oom_error:
termPQExpBuffer(&mechanism_buf);
if (initialresponse)
free(initialresponse);
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return STATUS_ERROR;
}
@@ -663,9 +658,7 @@ pg_SASL_continue(PGconn *conn, int payloadlen, bool final)
challenge = malloc(payloadlen + 1);
if (!challenge)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory allocating SASL buffer (%d)\n"),
- payloadlen);
+ pqReportOOM(conn);
return STATUS_ERROR;
}
@@ -688,8 +681,8 @@ pg_SASL_continue(PGconn *conn, int payloadlen, bool final)
if (outputlen != 0)
free(output);
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("AuthenticationSASLFinal received from server, but SASL authentication was not
completed\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("AuthenticationSASLFinal received from server, but SASL authentication was
notcompleted\n"));
return STATUS_ERROR;
}
if (outputlen != 0)
@@ -758,15 +751,15 @@ pg_local_sendauth(PGconn *conn)
{
char sebuf[256];
- printfPQExpBuffer(&conn->errorMessage,
- "pg_local_sendauth: sendmsg: %s\n",
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("could not send authentication message: %s\n"),
pqStrerror(errno, sebuf, sizeof(sebuf)));
return STATUS_ERROR;
}
return STATUS_OK;
#else
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SCM_CRED authentication method not supported\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("SCM_CRED authentication method not supported\n"));
return STATUS_ERROR;
#endif
}
@@ -798,8 +791,7 @@ pg_password_sendauth(PGconn *conn, const char *password, AuthRequest areq)
crypt_pwd = malloc(2 * (MD5_PASSWD_LEN + 1));
if (!crypt_pwd)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return STATUS_ERROR;
}
@@ -856,13 +848,13 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
break;
case AUTH_REQ_KRB4:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("Kerberos 4 authentication not supported\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("Kerberos 4 authentication not supported\n"));
return STATUS_ERROR;
case AUTH_REQ_KRB5:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("Kerberos 5 authentication not supported\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("Kerberos 5 authentication not supported\n"));
return STATUS_ERROR;
#if defined(ENABLE_GSS) || defined(ENABLE_SSPI)
@@ -932,8 +924,8 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
/* No GSSAPI *or* SSPI support */
case AUTH_REQ_GSS:
case AUTH_REQ_GSS_CONT:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("GSSAPI authentication not supported\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("GSSAPI authentication not supported\n"));
return STATUS_ERROR;
#endif /* defined(ENABLE_GSS) || defined(ENABLE_SSPI) */
@@ -964,16 +956,16 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
*/
#if !defined(ENABLE_GSS)
case AUTH_REQ_SSPI:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SSPI authentication not supported\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("SSPI authentication not supported\n"));
return STATUS_ERROR;
#endif /* !define(ENABLE_GSSAPI) */
#endif /* ENABLE_SSPI */
case AUTH_REQ_CRYPT:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("Crypt authentication not supported\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("Crypt authentication not supported\n"));
return STATUS_ERROR;
case AUTH_REQ_MD5:
@@ -987,14 +979,15 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
password = conn->pgpass;
if (password == NULL || password[0] == '\0')
{
- printfPQExpBuffer(&conn->errorMessage,
- PQnoPasswordSupplied);
+ /* Intentionally not translated. */
+ appendPQExpBufferStr(&conn->errorMessage,
+ PQnoPasswordSupplied);
return STATUS_ERROR;
}
if (pg_password_sendauth(conn, password, areq) != STATUS_OK)
{
- printfPQExpBuffer(&conn->errorMessage,
- "fe_sendauth: error sending password authentication\n");
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("error sending password authentication\n"));
return STATUS_ERROR;
}
break;
@@ -1017,17 +1010,15 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
case AUTH_REQ_SASL_FIN:
if (conn->sasl_state == NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- "fe_sendauth: invalid authentication request from server: AUTH_REQ_SASL_CONT without
AUTH_REQ_SASL\n");
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("invalid authentication request from server: AUTH_REQ_SASL_CONT
withoutAUTH_REQ_SASL\n"));
return STATUS_ERROR;
}
if (pg_SASL_continue(conn, payloadlen,
(areq == AUTH_REQ_SASL_FIN)) != STATUS_OK)
{
- /* Use error message, if set already */
- if (conn->errorMessage.len == 0)
- printfPQExpBuffer(&conn->errorMessage,
- "fe_sendauth: error in SASL authentication\n");
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("error in SASL authentication\n"));
return STATUS_ERROR;
}
break;
@@ -1038,7 +1029,7 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
break;
default:
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("authentication method %u not supported\n"), areq);
return STATUS_ERROR;
}
@@ -1052,7 +1043,7 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
*
* Returns a pointer to malloc'd space containing whatever name the user
* has authenticated to the system. If there is an error, return NULL,
- * and put a suitable error message in *errorMessage if that's not NULL.
+ * and append a suitable error message to *errorMessage if that's not NULL.
*/
char *
pg_fe_getauthname(PQExpBuffer errorMessage)
@@ -1085,7 +1076,7 @@ pg_fe_getauthname(PQExpBuffer errorMessage)
if (GetUserName(username, &namesize))
name = username;
else if (errorMessage)
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("user name lookup failure: error code %lu\n"),
GetLastError());
#else
@@ -1095,12 +1086,12 @@ pg_fe_getauthname(PQExpBuffer errorMessage)
else if (errorMessage)
{
if (pwerr != 0)
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("could not look up local user ID %d: %s\n"),
(int) user_id,
pqStrerror(pwerr, pwdbuf, sizeof(pwdbuf)));
else
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("local user with ID %d does not exist\n"),
(int) user_id);
}
@@ -1110,8 +1101,7 @@ pg_fe_getauthname(PQExpBuffer errorMessage)
{
result = strdup(name);
if (result == NULL && errorMessage)
- printfPQExpBuffer(errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOMBuffer(errorMessage);
}
pgunlock_thread();
@@ -1181,12 +1171,16 @@ PQencryptPasswordConn(PGconn *conn, const char *passwd, const char *user,
if (!conn)
return NULL;
+ /* This is a library entry point, so start with an empty error message. */
+ resetPQExpBuffer(&conn->errorMessage);
+
/* If no algorithm was given, ask the server. */
if (algorithm == NULL)
{
PGresult *res;
char *val;
+ /* PQexec will reset errorMessage again, but that's OK */
res = PQexec(conn, "show password_encryption");
if (res == NULL)
{
@@ -1202,8 +1196,8 @@ PQencryptPasswordConn(PGconn *conn, const char *passwd, const char *user,
if (PQntuples(res) != 1 || PQnfields(res) != 1)
{
PQclear(res);
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("unexpected shape of result set returned for SHOW\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("unexpected shape of result set returned for SHOW\n"));
return NULL;
}
val = PQgetvalue(res, 0, 0);
@@ -1211,8 +1205,8 @@ PQencryptPasswordConn(PGconn *conn, const char *passwd, const char *user,
if (strlen(val) > MAX_ALGORITHM_NAME_LEN)
{
PQclear(res);
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("password_encryption value too long\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("password_encryption value too long\n"));
return NULL;
}
strcpy(algobuf, val);
@@ -1251,15 +1245,14 @@ PQencryptPasswordConn(PGconn *conn, const char *passwd, const char *user,
}
else
{
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("unrecognized password encryption algorithm \"%s\"\n"),
algorithm);
return NULL;
}
if (!crypt_pwd)
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return crypt_pwd;
}
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 9315a27..647423c 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -831,8 +831,7 @@ fillPGconn(PGconn *conn, PQconninfoOption *connOptions)
*connmember = strdup(tmp);
if (*connmember == NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return false;
}
}
@@ -1014,7 +1013,7 @@ connectOptions2(PGconn *conn)
if (more || i != conn->nconnhost)
{
conn->status = CONNECTION_BAD;
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not match %d host names to %d hostaddr values\n"),
count_comma_separated_elems(conn->pghost), conn->nconnhost);
return false;
@@ -1090,7 +1089,7 @@ connectOptions2(PGconn *conn)
else if (more || i != conn->nconnhost)
{
conn->status = CONNECTION_BAD;
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not match %d port numbers to %d hosts\n"),
count_comma_separated_elems(conn->pgport), conn->nconnhost);
return false;
@@ -1186,7 +1185,7 @@ connectOptions2(PGconn *conn)
&& strcmp(conn->sslmode, "verify-full") != 0)
{
conn->status = CONNECTION_BAD;
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("invalid sslmode value: \"%s\"\n"),
conn->sslmode);
return false;
@@ -1207,7 +1206,7 @@ connectOptions2(PGconn *conn)
case 'r': /* "require" */
case 'v': /* "verify-ca" or "verify-full" */
conn->status = CONNECTION_BAD;
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("sslmode value \"%s\" invalid when SSL support is not compiled in\n"),
conn->sslmode);
return false;
@@ -1242,7 +1241,7 @@ connectOptions2(PGconn *conn)
&& strcmp(conn->target_session_attrs, "read-write") != 0)
{
conn->status = CONNECTION_BAD;
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("invalid target_session_attrs value: \"%s\"\n"),
conn->target_session_attrs);
return false;
@@ -1260,8 +1259,7 @@ connectOptions2(PGconn *conn)
oom_error:
conn->status = CONNECTION_BAD;
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return false;
}
@@ -1436,8 +1434,7 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
oom_error:
conn->status = CONNECTION_BAD;
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return conn;
}
@@ -1757,10 +1754,10 @@ connectDBStart(PGconn *conn)
conn->outCount = 0;
/*
- * Ensure errorMessage is empty, too. PQconnectPoll will append messages
- * to it in the process of scanning for a working server. Thus, if we
- * fail to connect to multiple hosts, the final error message will include
- * details about each failure.
+ * Ensure errorMessage is empty, too. PQconnectPoll and its subroutines
+ * will append messages to it in the process of scanning for a working
+ * server. Thus, if we fail to connect to multiple hosts, the final error
+ * message will include details about each failure.
*/
resetPQExpBuffer(&conn->errorMessage);
@@ -1955,12 +1952,6 @@ connectDBComplete(PGconn *conn)
switch (flag)
{
case PGRES_POLLING_OK:
-
- /*
- * Reset stored error messages since we now have a working
- * connection
- */
- resetPQExpBuffer(&conn->errorMessage);
return 1; /* success! */
case PGRES_POLLING_READING:
@@ -2005,46 +1996,6 @@ connectDBComplete(PGconn *conn)
}
}
-/*
- * This subroutine saves conn->errorMessage, which will be restored back by
- * restoreErrorMessage subroutine. Returns false on OOM failure.
- */
-static bool
-saveErrorMessage(PGconn *conn, PQExpBuffer savedMessage)
-{
- initPQExpBuffer(savedMessage);
- appendPQExpBufferStr(savedMessage,
- conn->errorMessage.data);
- if (PQExpBufferBroken(savedMessage))
- {
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
- return false;
- }
- /* Clear whatever is in errorMessage now */
- resetPQExpBuffer(&conn->errorMessage);
- return true;
-}
-
-/*
- * Restores saved error messages back to conn->errorMessage, prepending them
- * to whatever is in conn->errorMessage already. (This does the right thing
- * if anything's been added to conn->errorMessage since saveErrorMessage.)
- */
-static void
-restoreErrorMessage(PGconn *conn, PQExpBuffer savedMessage)
-{
- appendPQExpBufferStr(savedMessage, conn->errorMessage.data);
- resetPQExpBuffer(&conn->errorMessage);
- appendPQExpBufferStr(&conn->errorMessage, savedMessage->data);
- /* If any step above hit OOM, just report that */
- if (PQExpBufferBroken(savedMessage) ||
- PQExpBufferBroken(&conn->errorMessage))
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
- termPQExpBuffer(savedMessage);
-}
-
/* ----------------
* PQconnectPoll
*
@@ -2080,7 +2031,6 @@ PQconnectPoll(PGconn *conn)
PGresult *res;
char sebuf[256];
int optval;
- PQExpBufferData savedMessage;
if (conn == NULL)
return PGRES_POLLING_FAILED;
@@ -2601,12 +2551,7 @@ keep_going: /* We will come back to here until there is
EnvironmentOptions);
if (!startpacket)
{
- /*
- * will not appendbuffer here, since it's likely to also
- * run out of memory
- */
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
goto error_return;
}
@@ -2992,7 +2937,6 @@ keep_going: /* We will come back to here until there is
* avoid the Kerberos code doing a hostname look-up.
*/
res = pg_fe_sendauth(areq, msgLength, conn);
- conn->errorMessage.len = strlen(conn->errorMessage.data);
/* OK, we have processed the message; mark data consumed */
conn->inStart = conn->inCursor;
@@ -3112,31 +3056,19 @@ keep_going: /* We will come back to here until there is
conn->target_session_attrs != NULL &&
strcmp(conn->target_session_attrs, "read-write") == 0)
{
- /*
- * Save existing error messages across the PQsendQuery
- * attempt. This is necessary because PQsendQuery is
- * going to reset conn->errorMessage, so we would lose
- * error messages related to previous hosts we have tried
- * and failed to connect to.
- */
- if (!saveErrorMessage(conn, &savedMessage))
- goto error_return;
-
- conn->status = CONNECTION_OK;
if (!PQsendQuery(conn,
"SHOW transaction_read_only"))
- {
- restoreErrorMessage(conn, &savedMessage);
goto error_return;
- }
conn->status = CONNECTION_CHECK_WRITABLE;
- restoreErrorMessage(conn, &savedMessage);
return PGRES_POLLING_READING;
}
/* We can release the address lists now. */
release_all_addrinfo(conn);
+ /* Likewise drop any stored error messages. */
+ resetPQExpBuffer(&conn->errorMessage);
+
/* We are open for business! */
conn->status = CONNECTION_OK;
return PGRES_POLLING_OK;
@@ -3146,23 +3078,16 @@ keep_going: /* We will come back to here until there is
/*
* Do post-connection housekeeping (only needed in protocol 2.0).
- *
- * We pretend that the connection is OK for the duration of these
- * queries.
*/
- conn->status = CONNECTION_OK;
-
switch (pqSetenvPoll(conn))
{
case PGRES_POLLING_OK: /* Success */
break;
case PGRES_POLLING_READING: /* Still going */
- conn->status = CONNECTION_SETENV;
return PGRES_POLLING_READING;
case PGRES_POLLING_WRITING: /* Still going */
- conn->status = CONNECTION_SETENV;
return PGRES_POLLING_WRITING;
default:
@@ -3182,39 +3107,30 @@ keep_going: /* We will come back to here until there is
conn->target_session_attrs != NULL &&
strcmp(conn->target_session_attrs, "read-write") == 0)
{
- if (!saveErrorMessage(conn, &savedMessage))
- goto error_return;
-
- conn->status = CONNECTION_OK;
if (!PQsendQuery(conn,
"SHOW transaction_read_only"))
- {
- restoreErrorMessage(conn, &savedMessage);
goto error_return;
- }
conn->status = CONNECTION_CHECK_WRITABLE;
- restoreErrorMessage(conn, &savedMessage);
return PGRES_POLLING_READING;
}
/* We can release the address lists now. */
release_all_addrinfo(conn);
+ /* Likewise drop any stored error messages. */
+ resetPQExpBuffer(&conn->errorMessage);
+
/* We are open for business! */
conn->status = CONNECTION_OK;
return PGRES_POLLING_OK;
case CONNECTION_CONSUME:
{
- conn->status = CONNECTION_OK;
if (!PQconsumeInput(conn))
goto error_return;
if (PQisBusy(conn))
- {
- conn->status = CONNECTION_CONSUME;
return PGRES_POLLING_READING;
- }
/*
* Call PQgetResult() again to consume NULL result.
@@ -3223,10 +3139,15 @@ keep_going: /* We will come back to here until there is
if (res != NULL)
{
PQclear(res);
- conn->status = CONNECTION_CONSUME;
goto keep_going;
}
+ /* We can release the address lists now. */
+ release_all_addrinfo(conn);
+
+ /* Likewise drop any stored error messages. */
+ resetPQExpBuffer(&conn->errorMessage);
+
/* We are open for business! */
conn->status = CONNECTION_OK;
return PGRES_POLLING_OK;
@@ -3236,22 +3157,11 @@ keep_going: /* We will come back to here until there is
const char *displayed_host;
const char *displayed_port;
- if (!saveErrorMessage(conn, &savedMessage))
- goto error_return;
-
- conn->status = CONNECTION_OK;
if (!PQconsumeInput(conn))
- {
- restoreErrorMessage(conn, &savedMessage);
goto error_return;
- }
if (PQisBusy(conn))
- {
- conn->status = CONNECTION_CHECK_WRITABLE;
- restoreErrorMessage(conn, &savedMessage);
return PGRES_POLLING_READING;
- }
res = PQgetResult(conn);
if (res && (PQresultStatus(res) == PGRES_TUPLES_OK) &&
@@ -3267,7 +3177,6 @@ keep_going: /* We will come back to here until there is
const char *displayed_port;
PQclear(res);
- restoreErrorMessage(conn, &savedMessage);
/* Append error report to conn->errorMessage. */
if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS)
@@ -3298,10 +3207,6 @@ keep_going: /* We will come back to here until there is
/* Session is read-write, so we're good. */
PQclear(res);
- termPQExpBuffer(&savedMessage);
-
- /* We can release the address lists now. */
- release_all_addrinfo(conn);
/*
* Finish reading any remaining messages before being
@@ -3317,7 +3222,6 @@ keep_going: /* We will come back to here until there is
*/
if (res)
PQclear(res);
- restoreErrorMessage(conn, &savedMessage);
/* Append error report to conn->errorMessage. */
if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS)
@@ -3771,7 +3675,7 @@ PQreset(PGconn *conn)
conn->events[i].passThrough))
{
conn->status = CONNECTION_BAD;
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("PGEventProc \"%s\" failed during PGEVT_CONNRESET event\n"),
conn->events[i].name);
break;
@@ -3831,7 +3735,7 @@ PQresetPoll(PGconn *conn)
conn->events[i].passThrough))
{
conn->status = CONNECTION_BAD;
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("PGEventProc \"%s\" failed during PGEVT_CONNRESET event\n"),
conn->events[i].name);
return PGRES_POLLING_FAILED;
@@ -4166,7 +4070,7 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
if ((url = strdup(purl)) == NULL)
{
- printfPQExpBuffer(errorMessage, libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return 3;
}
@@ -4178,8 +4082,9 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
if (pg_strncasecmp(url, LDAP_URL, strlen(LDAP_URL)) != 0)
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("invalid LDAP URL \"%s\": scheme must be ldap://\n"), purl);
+ appendPQExpBuffer(errorMessage,
+ libpq_gettext("invalid LDAP URL \"%s\": scheme must be ldap://\n"),
+ purl);
free(url);
return 3;
}
@@ -4193,8 +4098,9 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
p = strchr(url + strlen(LDAP_URL), '/');
if (p == NULL || *(p + 1) == '\0' || *(p + 1) == '?')
{
- printfPQExpBuffer(errorMessage, libpq_gettext(
- "invalid LDAP URL \"%s\": missing distinguished name\n"), purl);
+ appendPQExpBuffer(errorMessage,
+ libpq_gettext("invalid LDAP URL \"%s\": missing distinguished name\n"),
+ purl);
free(url);
return 3;
}
@@ -4204,8 +4110,9 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
/* attribute */
if ((p = strchr(dn, '?')) == NULL || *(p + 1) == '\0' || *(p + 1) == '?')
{
- printfPQExpBuffer(errorMessage, libpq_gettext(
- "invalid LDAP URL \"%s\": must have exactly one attribute\n"),
purl);
+ appendPQExpBuffer(errorMessage,
+ libpq_gettext("invalid LDAP URL \"%s\": must have exactly one attribute\n"),
+ purl);
free(url);
return 3;
}
@@ -4215,7 +4122,9 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
/* scope */
if ((p = strchr(attrs[0], '?')) == NULL || *(p + 1) == '\0' || *(p + 1) == '?')
{
- printfPQExpBuffer(errorMessage, libpq_gettext("invalid LDAP URL \"%s\": must have search scope
(base/one/sub)\n"),purl);
+ appendPQExpBuffer(errorMessage,
+ libpq_gettext("invalid LDAP URL \"%s\": must have search scope (base/one/sub)\n"),
+ purl);
free(url);
return 3;
}
@@ -4225,7 +4134,7 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
/* filter */
if ((p = strchr(scopestr, '?')) == NULL || *(p + 1) == '\0' || *(p + 1) == '?')
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("invalid LDAP URL \"%s\": no filter\n"), purl);
free(url);
return 3;
@@ -4246,8 +4155,9 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
lport = strtol(portstr, &endptr, 10);
if (*portstr == '\0' || *endptr != '\0' || errno || lport < 0 || lport > 65535)
{
- printfPQExpBuffer(errorMessage, libpq_gettext(
- "invalid LDAP URL \"%s\": invalid port number\n"), purl);
+ appendPQExpBuffer(errorMessage,
+ libpq_gettext("invalid LDAP URL \"%s\": invalid port number\n"),
+ purl);
free(url);
return 3;
}
@@ -4257,8 +4167,9 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
/* Allow only one attribute */
if (strchr(attrs[0], ',') != NULL)
{
- printfPQExpBuffer(errorMessage, libpq_gettext(
- "invalid LDAP URL \"%s\": must have exactly one attribute\n"),
purl);
+ appendPQExpBuffer(errorMessage,
+ libpq_gettext("invalid LDAP URL \"%s\": must have exactly one attribute\n"),
+ purl);
free(url);
return 3;
}
@@ -4272,7 +4183,9 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
scope = LDAP_SCOPE_SUBTREE;
else
{
- printfPQExpBuffer(errorMessage, libpq_gettext("invalid LDAP URL \"%s\": must have search scope
(base/one/sub)\n"),purl);
+ appendPQExpBuffer(errorMessage,
+ libpq_gettext("invalid LDAP URL \"%s\": must have search scope (base/one/sub)\n"),
+ purl);
free(url);
return 3;
}
@@ -4280,8 +4193,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
/* initialize LDAP structure */
if ((ld = ldap_init(hostname, port)) == NULL)
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("could not create LDAP structure\n"));
+ appendPQExpBufferStr(errorMessage,
+ libpq_gettext("could not create LDAP structure\n"));
free(url);
return 3;
}
@@ -4356,7 +4269,7 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
{
if (res != NULL)
ldap_msgfree(res);
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("lookup on LDAP server failed: %s\n"),
ldap_err2string(rc));
ldap_unbind(ld);
@@ -4367,9 +4280,9 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
/* complain if there was not exactly one result */
if ((rc = ldap_count_entries(ld, res)) != 1)
{
- printfPQExpBuffer(errorMessage,
- rc ? libpq_gettext("more than one entry found on LDAP lookup\n")
- : libpq_gettext("no entry found on LDAP lookup\n"));
+ appendPQExpBufferStr(errorMessage,
+ rc ? libpq_gettext("more than one entry found on LDAP lookup\n")
+ : libpq_gettext("no entry found on LDAP lookup\n"));
ldap_msgfree(res);
ldap_unbind(ld);
free(url);
@@ -4380,8 +4293,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
if ((entry = ldap_first_entry(ld, res)) == NULL)
{
/* should never happen */
- printfPQExpBuffer(errorMessage,
- libpq_gettext("no entry found on LDAP lookup\n"));
+ appendPQExpBufferStr(errorMessage,
+ libpq_gettext("no entry found on LDAP lookup\n"));
ldap_msgfree(res);
ldap_unbind(ld);
free(url);
@@ -4391,8 +4304,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
/* get values */
if ((values = ldap_get_values_len(ld, entry, attrs[0])) == NULL)
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("attribute has no values on LDAP lookup\n"));
+ appendPQExpBufferStr(errorMessage,
+ libpq_gettext("attribute has no values on LDAP lookup\n"));
ldap_msgfree(res);
ldap_unbind(ld);
free(url);
@@ -4404,8 +4317,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
if (values[0] == NULL)
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("attribute has no values on LDAP lookup\n"));
+ appendPQExpBufferStr(errorMessage,
+ libpq_gettext("attribute has no values on LDAP lookup\n"));
ldap_value_free_len(values);
ldap_unbind(ld);
return 1;
@@ -4417,10 +4330,9 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
size += values[i]->bv_len + 1;
if ((result = malloc(size)) == NULL)
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("out of memory\n"));
ldap_value_free_len(values);
ldap_unbind(ld);
+ pqReportOOMBuffer(errorMessage);
return 3;
}
p = result;
@@ -4456,8 +4368,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
}
else if (ld_is_nl_cr(*p))
{
- printfPQExpBuffer(errorMessage, libpq_gettext(
- "missing \"=\" after \"%s\" in connection info
string\n"),
+ appendPQExpBuffer(errorMessage,
+ libpq_gettext("missing \"=\" after \"%s\" in connection info string\n"),
optname);
free(result);
return 3;
@@ -4475,8 +4387,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
}
else if (!ld_is_sp_tab(*p))
{
- printfPQExpBuffer(errorMessage, libpq_gettext(
- "missing \"=\" after \"%s\" in connection info
string\n"),
+ appendPQExpBuffer(errorMessage,
+ libpq_gettext("missing \"=\" after \"%s\" in connection info string\n"),
optname);
free(result);
return 3;
@@ -4536,9 +4448,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
options[i].val = strdup(optval);
if (!options[i].val)
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("out of memory\n"));
free(result);
+ pqReportOOMBuffer(errorMessage);
return 3;
}
}
@@ -4548,7 +4459,7 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
}
if (!found_keyword)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("invalid connection option \"%s\"\n"),
optname);
free(result);
@@ -4564,8 +4475,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
if (state == 5 || state == 6)
{
- printfPQExpBuffer(errorMessage, libpq_gettext(
- "unterminated quoted string in connection info string\n"));
+ appendPQExpBufferStr(errorMessage,
+ libpq_gettext("unterminated quoted string in connection info string\n"));
return 3;
}
@@ -4583,8 +4494,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
* Returns 0 on success, nonzero on failure. On failure, if errorMessage
* isn't null, also store an error message there. (Note: the only reason
* this function and related ones don't dump core on errorMessage == NULL
- * is the undocumented fact that printfPQExpBuffer does nothing when passed
- * a null PQExpBuffer pointer.)
+ * is the undocumented fact that printfPQExpBuffer and friends do nothing
+ * when passed a null PQExpBuffer pointer.)
*/
static int
parseServiceInfo(PQconninfoOption *options, PQExpBuffer errorMessage)
@@ -4647,7 +4558,7 @@ next_file:
last_file:
if (!group_found)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("definition of service \"%s\" not found\n"), service);
return 3;
}
@@ -4671,7 +4582,7 @@ parseServiceFile(const char *serviceFile,
f = fopen(serviceFile, "r");
if (f == NULL)
{
- printfPQExpBuffer(errorMessage, libpq_gettext("service file \"%s\" not found\n"),
+ appendPQExpBuffer(errorMessage, libpq_gettext("service file \"%s\" not found\n"),
serviceFile);
return 1;
}
@@ -4683,7 +4594,7 @@ parseServiceFile(const char *serviceFile,
if (strlen(line) >= sizeof(buf) - 1)
{
fclose(f);
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("line %d too long in service file \"%s\"\n"),
linenr,
serviceFile);
@@ -4754,7 +4665,7 @@ parseServiceFile(const char *serviceFile,
val = strchr(line, '=');
if (val == NULL)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("syntax error in service file \"%s\", line %d\n"),
serviceFile,
linenr);
@@ -4765,7 +4676,7 @@ parseServiceFile(const char *serviceFile,
if (strcmp(key, "service") == 0)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("nested service specifications not supported in service file
\"%s\",line %d\n"),
serviceFile,
linenr);
@@ -4786,9 +4697,8 @@ parseServiceFile(const char *serviceFile,
options[i].val = strdup(val);
if (!options[i].val)
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("out of memory\n"));
fclose(f);
+ pqReportOOMBuffer(errorMessage);
return 3;
}
found_keyword = true;
@@ -4798,7 +4708,7 @@ parseServiceFile(const char *serviceFile,
if (!found_keyword)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("syntax error in service file \"%s\", line %d\n"),
serviceFile,
linenr);
@@ -4842,7 +4752,7 @@ PQconninfoParse(const char *conninfo, char **errmsg)
if (PQExpBufferDataBroken(errorBuf))
return NULL; /* out of memory already :-( */
connOptions = parse_connection_string(conninfo, &errorBuf, false);
- if (connOptions == NULL && errmsg)
+ if (connOptions == NULL && errmsg && !PQExpBufferDataBroken(errorBuf))
*errmsg = errorBuf.data;
else
termPQExpBuffer(&errorBuf);
@@ -4866,8 +4776,7 @@ conninfo_init(PQExpBuffer errorMessage)
options = (PQconninfoOption *) malloc(sizeof(PQconninfoOption) * sizeof(PQconninfoOptions) /
sizeof(PQconninfoOptions[0]));
if (options == NULL)
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOMBuffer(errorMessage);
return NULL;
}
opt_dest = options;
@@ -4965,9 +4874,8 @@ conninfo_parse(const char *conninfo, PQExpBuffer errorMessage,
/* Need a modifiable copy of the input string */
if ((buf = strdup(conninfo)) == NULL)
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("out of memory\n"));
PQconninfoFree(options);
+ pqReportOOMBuffer(errorMessage);
return NULL;
}
cp = buf;
@@ -5004,7 +4912,7 @@ conninfo_parse(const char *conninfo, PQExpBuffer errorMessage,
/* Check that there is a following '=' */
if (*cp != '=')
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("missing \"=\" after \"%s\" in connection info string\n"),
pname);
PQconninfoFree(options);
@@ -5053,8 +4961,8 @@ conninfo_parse(const char *conninfo, PQExpBuffer errorMessage,
{
if (*cp == '\0')
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("unterminated quoted string in connection info string\n"));
+ appendPQExpBufferStr(errorMessage,
+ libpq_gettext("unterminated quoted string in connection info string\n"));
PQconninfoFree(options);
free(buf);
return NULL;
@@ -5189,7 +5097,7 @@ conninfo_array_parse(const char *const *keywords, const char *const *values,
/* Check for invalid connection option */
if (option->keyword == NULL)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("invalid connection option \"%s\"\n"),
pname);
PQconninfoFree(options);
@@ -5221,10 +5129,9 @@ conninfo_array_parse(const char *const *keywords, const char *const *values,
options[k].val = strdup(str_option->val);
if (!options[k].val)
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("out of memory\n"));
PQconninfoFree(options);
PQconninfoFree(dbname_options);
+ pqReportOOMBuffer(errorMessage);
return NULL;
}
break;
@@ -5250,10 +5157,9 @@ conninfo_array_parse(const char *const *keywords, const char *const *values,
option->val = strdup(pvalue);
if (!option->val)
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("out of memory\n"));
PQconninfoFree(options);
PQconninfoFree(dbname_options);
+ pqReportOOMBuffer(errorMessage);
return NULL;
}
}
@@ -5322,8 +5228,7 @@ conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
if (!option->val)
{
if (errorMessage)
- printfPQExpBuffer(errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOMBuffer(errorMessage);
return false;
}
continue;
@@ -5346,8 +5251,7 @@ conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
if (!option->val)
{
if (errorMessage)
- printfPQExpBuffer(errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOMBuffer(errorMessage);
return false;
}
continue;
@@ -5364,8 +5268,7 @@ conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
if (!option->val)
{
if (errorMessage)
- printfPQExpBuffer(errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOMBuffer(errorMessage);
return false;
}
continue;
@@ -5465,8 +5368,7 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri,
initPQExpBuffer(&portbuf);
if (PQExpBufferDataBroken(hostbuf) || PQExpBufferDataBroken(portbuf))
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOMBuffer(errorMessage);
goto cleanup;
}
@@ -5474,8 +5376,7 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri,
buf = strdup(uri);
if (buf == NULL)
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOMBuffer(errorMessage);
goto cleanup;
}
start = buf;
@@ -5485,7 +5386,7 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri,
if (prefix_len == 0)
{
/* Should never happen */
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("invalid URI propagated to internal parser routine: \"%s\"\n"),
uri);
goto cleanup;
@@ -5562,14 +5463,14 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri,
++p;
if (!*p)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("end of string reached when looking for matching \"]\" in IPv6 host
addressin URI: \"%s\"\n"),
uri);
goto cleanup;
}
if (p == host)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("IPv6 host address may not be empty in URI: \"%s\"\n"),
uri);
goto cleanup;
@@ -5584,7 +5485,7 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri,
*/
if (*p && *p != ':' && *p != '/' && *p != '?' && *p != ',')
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("unexpected character \"%c\" at position %d in URI (expected \":\" or
\"/\"):\"%s\"\n"),
*p, (int) (p - buf + 1), uri);
goto cleanup;
@@ -5713,7 +5614,7 @@ conninfo_uri_parse_params(char *params,
/* Was there '=' already? */
if (value != NULL)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("extra key/value separator \"=\" in URI query parameter:
\"%s\"\n"),
keyword);
return false;
@@ -5733,7 +5634,7 @@ conninfo_uri_parse_params(char *params,
/* Was there '=' at all? */
if (value == NULL)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("missing key/value separator \"=\" in URI query parameter:
\"%s\"\n"),
keyword);
return false;
@@ -5784,7 +5685,7 @@ conninfo_uri_parse_params(char *params,
{
/* Insert generic message if conninfo_storeval didn't give one. */
if (errorMessage->len == 0)
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("invalid URI query parameter: \"%s\"\n"),
keyword);
/* And fail. */
@@ -5831,7 +5732,7 @@ conninfo_uri_decode(const char *str, PQExpBuffer errorMessage)
buf = malloc(strlen(str) + 1);
if (buf == NULL)
{
- printfPQExpBuffer(errorMessage, libpq_gettext("out of memory\n"));
+ pqReportOOMBuffer(errorMessage);
return NULL;
}
p = buf;
@@ -5858,7 +5759,7 @@ conninfo_uri_decode(const char *str, PQExpBuffer errorMessage)
*/
if (!(get_hexdigit(*q++, &hi) && get_hexdigit(*q++, &lo)))
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("invalid percent-encoded token: \"%s\"\n"),
str);
free(buf);
@@ -5868,7 +5769,7 @@ conninfo_uri_decode(const char *str, PQExpBuffer errorMessage)
c = (hi << 4) | lo;
if (c == 0)
{
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("forbidden value %%00 in percent-encoded value: \"%s\"\n"),
str);
free(buf);
@@ -5963,7 +5864,7 @@ conninfo_storeval(PQconninfoOption *connOptions,
if (option == NULL)
{
if (!ignoreMissing)
- printfPQExpBuffer(errorMessage,
+ appendPQExpBuffer(errorMessage,
libpq_gettext("invalid connection option \"%s\"\n"),
keyword);
return NULL;
@@ -5981,7 +5882,7 @@ conninfo_storeval(PQconninfoOption *connOptions,
value_copy = strdup(value);
if (value_copy == NULL)
{
- printfPQExpBuffer(errorMessage, libpq_gettext("out of memory\n"));
+ pqReportOOMBuffer(errorMessage);
return NULL;
}
}
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 4c0114c..2c1baa7 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -770,8 +770,7 @@ pqSaveErrorResult(PGconn *conn)
* This subroutine prepares an async result object for return to the caller.
* If there is not already an async result object, build an error object
* using whatever is in conn->errorMessage. In any case, clear the async
- * result storage and make sure PQerrorMessage will agree with the result's
- * error string.
+ * result storage.
*/
PGresult *
pqPrepareAsyncResult(PGconn *conn)
@@ -781,21 +780,11 @@ pqPrepareAsyncResult(PGconn *conn)
/*
* conn->result is the PGresult to return. If it is NULL (which probably
* shouldn't happen) we assume there is an appropriate error message in
- * conn->errorMessage.
+ * conn->errorMessage, and copy that into a PGresult.
*/
res = conn->result;
if (!res)
res = PQmakeEmptyPGresult(conn, PGRES_FATAL_ERROR);
- else
- {
- /*
- * Make sure PQerrorMessage agrees with result; it could be different
- * if we have concatenated messages.
- */
- resetPQExpBuffer(&conn->errorMessage);
- appendPQExpBufferStr(&conn->errorMessage,
- PQresultErrorMessage(res));
- }
/*
* Replace conn->result with next_result, if any. In the normal case
@@ -1188,8 +1177,8 @@ PQsendQuery(PGconn *conn, const char *query)
/* check the argument */
if (!query)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("command string is a null pointer\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("command string is a null pointer\n"));
return 0;
}
@@ -1246,14 +1235,14 @@ PQsendQueryParams(PGconn *conn,
/* check the arguments */
if (!command)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("command string is a null pointer\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("command string is a null pointer\n"));
return 0;
}
if (nParams < 0 || nParams > 65535)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("number of parameters must be between 0 and 65535\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("number of parameters must be between 0 and 65535\n"));
return 0;
}
@@ -1286,28 +1275,28 @@ PQsendPrepare(PGconn *conn,
/* check the arguments */
if (!stmtName)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("statement name is a null pointer\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("statement name is a null pointer\n"));
return 0;
}
if (!query)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("command string is a null pointer\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("command string is a null pointer\n"));
return 0;
}
if (nParams < 0 || nParams > 65535)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("number of parameters must be between 0 and 65535\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("number of parameters must be between 0 and 65535\n"));
return 0;
}
/* This isn't gonna work on a 2.0 server */
if (PG_PROTOCOL_MAJOR(conn->pversion) < 3)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("function requires at least protocol version 3.0\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("function requires at least protocol version 3.0\n"));
return 0;
}
@@ -1387,14 +1376,14 @@ PQsendQueryPrepared(PGconn *conn,
/* check the arguments */
if (!stmtName)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("statement name is a null pointer\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("statement name is a null pointer\n"));
return 0;
}
if (nParams < 0 || nParams > 65535)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("number of parameters must be between 0 and 65535\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("number of parameters must be between 0 and 65535\n"));
return 0;
}
@@ -1418,21 +1407,32 @@ PQsendQueryStart(PGconn *conn)
if (!conn)
return false;
- /* clear the error string */
- resetPQExpBuffer(&conn->errorMessage);
+ /*
+ * Clear the error string at start of a new query. All libpq code
+ * executed while we're running a query should append messages to
+ * errorMessage.
+ *
+ * However, if we're in a connecting state (anything but CONNECTION_OK or
+ * CONNECTION_BAD), then we don't want to clear the errorMessage. We must
+ * be doing some libpq-generated query to check server state, and we want
+ * to continue building up the history of connection attempts across
+ * multiple servers --- see comments in connectDBStart.
+ */
+ if (conn->status <= CONNECTION_BAD)
+ resetPQExpBuffer(&conn->errorMessage);
/* Don't try to send if we know there's no live connection. */
- if (conn->status != CONNECTION_OK)
+ if (conn->status == CONNECTION_BAD)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("no connection to the server\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("no connection to the server\n"));
return false;
}
/* Can't send while already busy, either. */
if (conn->asyncStatus != PGASYNC_IDLE)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("another command is already in progress\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("another command is already in progress\n"));
return false;
}
@@ -1469,8 +1469,8 @@ PQsendQueryGuts(PGconn *conn,
/* This isn't gonna work on a 2.0 server */
if (PG_PROTOCOL_MAJOR(conn->pversion) < 3)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("function requires at least protocol version 3.0\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("function requires at least protocol version 3.0\n"));
return 0;
}
@@ -1545,8 +1545,8 @@ PQsendQueryGuts(PGconn *conn,
nbytes = paramLengths[i];
else
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("length must be given for binary parameter\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("length must be given for binary parameter\n"));
goto sendFailed;
}
}
@@ -1817,7 +1817,7 @@ PQgetResult(PGconn *conn)
res = getCopyResult(conn, PGRES_COPY_BOTH);
break;
default:
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("unexpected asyncStatus: %d\n"),
(int) conn->asyncStatus);
res = PQmakeEmptyPGresult(conn, PGRES_FATAL_ERROR);
@@ -1837,7 +1837,7 @@ PQgetResult(PGconn *conn)
if (!res->events[i].proc(PGEVT_RESULTCREATE, &evt,
res->events[i].passThrough))
{
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("PGEventProc \"%s\" failed during PGEVT_RESULTCREATE event\n"),
res->events[i].name);
pqSetResultError(res, conn->errorMessage.data);
@@ -2005,8 +2005,8 @@ PQexecStart(PGconn *conn)
else
{
/* In older protocols we have to punt */
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("COPY IN state must be terminated first\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("COPY IN state must be terminated first\n"));
return false;
}
}
@@ -2025,16 +2025,16 @@ PQexecStart(PGconn *conn)
else
{
/* In older protocols we have to punt */
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("COPY OUT state must be terminated first\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("COPY OUT state must be terminated first\n"));
return false;
}
}
else if (resultStatus == PGRES_COPY_BOTH)
{
/* We don't allow PQexec during COPY BOTH */
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("PQexec not allowed during COPY BOTH\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("PQexec not allowed during COPY BOTH\n"));
return false;
}
/* check for loss of connection, too */
@@ -2076,12 +2076,6 @@ PQexecFinish(PGconn *conn)
pqCatenateResultError(lastResult, result->errMsg);
PQclear(result);
result = lastResult;
-
- /*
- * Make sure PQerrorMessage agrees with concatenated result
- */
- resetPQExpBuffer(&conn->errorMessage);
- appendPQExpBufferStr(&conn->errorMessage, result->errMsg);
}
else
PQclear(lastResult);
@@ -2187,8 +2181,8 @@ PQsendDescribe(PGconn *conn, char desc_type, const char *desc_target)
/* This isn't gonna work on a 2.0 server */
if (PG_PROTOCOL_MAJOR(conn->pversion) < 3)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("function requires at least protocol version 3.0\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("function requires at least protocol version 3.0\n"));
return 0;
}
@@ -2276,8 +2270,8 @@ PQputCopyData(PGconn *conn, const char *buffer, int nbytes)
if (conn->asyncStatus != PGASYNC_COPY_IN &&
conn->asyncStatus != PGASYNC_COPY_BOTH)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("no COPY in progress\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("no COPY in progress\n"));
return -1;
}
@@ -2343,8 +2337,8 @@ PQputCopyEnd(PGconn *conn, const char *errormsg)
if (conn->asyncStatus != PGASYNC_COPY_IN &&
conn->asyncStatus != PGASYNC_COPY_BOTH)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("no COPY in progress\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("no COPY in progress\n"));
return -1;
}
@@ -2386,8 +2380,8 @@ PQputCopyEnd(PGconn *conn, const char *errormsg)
if (errormsg)
{
/* Oops, no way to do this in 2.0 */
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("function requires at least protocol version 3.0\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("function requires at least protocol version 3.0\n"));
return -1;
}
else
@@ -2405,7 +2399,6 @@ PQputCopyEnd(PGconn *conn, const char *errormsg)
conn->asyncStatus = PGASYNC_COPY_OUT;
else
conn->asyncStatus = PGASYNC_BUSY;
- resetPQExpBuffer(&conn->errorMessage);
/* Try to flush data */
if (pqFlush(conn) < 0)
@@ -2433,8 +2426,8 @@ PQgetCopyData(PGconn *conn, char **buffer, int async)
if (conn->asyncStatus != PGASYNC_COPY_OUT &&
conn->asyncStatus != PGASYNC_COPY_BOTH)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("no COPY in progress\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("no COPY in progress\n"));
return -2;
}
if (PG_PROTOCOL_MAJOR(conn->pversion) >= 3)
@@ -2617,14 +2610,14 @@ PQfn(PGconn *conn,
if (!conn)
return NULL;
- /* clear the error string */
+ /* Clear the error string, like PQsendQueryStart() */
resetPQExpBuffer(&conn->errorMessage);
if (conn->sock == PGINVALID_SOCKET || conn->asyncStatus != PGASYNC_IDLE ||
conn->result != NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("connection in wrong state\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("connection in wrong state\n"));
return NULL;
}
@@ -3343,8 +3336,8 @@ PQescapeStringInternal(PGconn *conn,
if (error)
*error = 1;
if (conn)
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("incomplete multibyte character\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("incomplete multibyte character\n"));
for (; i < len; i++)
{
if (((size_t) (target - to)) / 2 >= length)
@@ -3427,8 +3420,8 @@ PQescapeInternal(PGconn *conn, const char *str, size_t len, bool as_ident)
/* Multibyte character overruns allowable length. */
if ((s - str) + charlen > len || memchr(s, 0, charlen) != NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("incomplete multibyte character\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("incomplete multibyte character\n"));
return NULL;
}
@@ -3445,8 +3438,7 @@ PQescapeInternal(PGconn *conn, const char *str, size_t len, bool as_ident)
result = rp = (char *) malloc(result_size);
if (rp == NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return NULL;
}
@@ -3610,8 +3602,7 @@ PQescapeByteaInternal(PGconn *conn,
if (rp == NULL)
{
if (conn)
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return NULL;
}
diff --git a/src/interfaces/libpq/fe-lobj.c b/src/interfaces/libpq/fe-lobj.c
index a5ad3af..9259551 100644
--- a/src/interfaces/libpq/fe-lobj.c
+++ b/src/interfaces/libpq/fe-lobj.c
@@ -61,7 +61,13 @@ lo_open(PGconn *conn, Oid lobjId, int mode)
PQArgBlock argv[2];
PGresult *res;
- if (conn == NULL || conn->lobjfuncs == NULL)
+ if (conn == NULL)
+ return -1;
+
+ /* This is a library entry point, so start with an empty error message. */
+ resetPQExpBuffer(&conn->errorMessage);
+
+ if (conn->lobjfuncs == NULL)
{
if (lo_initialize(conn) < 0)
return -1;
@@ -103,7 +109,13 @@ lo_close(PGconn *conn, int fd)
int retval;
int result_len;
- if (conn == NULL || conn->lobjfuncs == NULL)
+ if (conn == NULL)
+ return -1;
+
+ /* This is a library entry point, so start with an empty error message. */
+ resetPQExpBuffer(&conn->errorMessage);
+
+ if (conn->lobjfuncs == NULL)
{
if (lo_initialize(conn) < 0)
return -1;
@@ -141,7 +153,13 @@ lo_truncate(PGconn *conn, int fd, size_t len)
int retval;
int result_len;
- if (conn == NULL || conn->lobjfuncs == NULL)
+ if (conn == NULL)
+ return -1;
+
+ /* This is a library entry point, so start with an empty error message. */
+ resetPQExpBuffer(&conn->errorMessage);
+
+ if (conn->lobjfuncs == NULL)
{
if (lo_initialize(conn) < 0)
return -1;
@@ -150,8 +168,8 @@ lo_truncate(PGconn *conn, int fd, size_t len)
/* Must check this on-the-fly because it's not there pre-8.3 */
if (conn->lobjfuncs->fn_lo_truncate == 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("cannot determine OID of function lo_truncate\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("cannot determine OID of function lo_truncate\n"));
return -1;
}
@@ -166,8 +184,8 @@ lo_truncate(PGconn *conn, int fd, size_t len)
*/
if (len > (size_t) INT_MAX)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("argument of lo_truncate exceeds integer range\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("argument of lo_truncate exceeds integer range\n"));
return -1;
}
@@ -209,7 +227,13 @@ lo_truncate64(PGconn *conn, int fd, pg_int64 len)
int retval;
int result_len;
- if (conn == NULL || conn->lobjfuncs == NULL)
+ if (conn == NULL)
+ return -1;
+
+ /* This is a library entry point, so start with an empty error message. */
+ resetPQExpBuffer(&conn->errorMessage);
+
+ if (conn->lobjfuncs == NULL)
{
if (lo_initialize(conn) < 0)
return -1;
@@ -217,8 +241,8 @@ lo_truncate64(PGconn *conn, int fd, pg_int64 len)
if (conn->lobjfuncs->fn_lo_truncate64 == 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("cannot determine OID of function lo_truncate64\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("cannot determine OID of function lo_truncate64\n"));
return -1;
}
@@ -261,7 +285,13 @@ lo_read(PGconn *conn, int fd, char *buf, size_t len)
PGresult *res;
int result_len;
- if (conn == NULL || conn->lobjfuncs == NULL)
+ if (conn == NULL)
+ return -1;
+
+ /* This is a library entry point, so start with an empty error message. */
+ resetPQExpBuffer(&conn->errorMessage);
+
+ if (conn->lobjfuncs == NULL)
{
if (lo_initialize(conn) < 0)
return -1;
@@ -275,8 +305,8 @@ lo_read(PGconn *conn, int fd, char *buf, size_t len)
*/
if (len > (size_t) INT_MAX)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("argument of lo_read exceeds integer range\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("argument of lo_read exceeds integer range\n"));
return -1;
}
@@ -316,7 +346,13 @@ lo_write(PGconn *conn, int fd, const char *buf, size_t len)
int result_len;
int retval;
- if (conn == NULL || conn->lobjfuncs == NULL)
+ if (conn == NULL)
+ return -1;
+
+ /* This is a library entry point, so start with an empty error message. */
+ resetPQExpBuffer(&conn->errorMessage);
+
+ if (conn->lobjfuncs == NULL)
{
if (lo_initialize(conn) < 0)
return -1;
@@ -330,8 +366,8 @@ lo_write(PGconn *conn, int fd, const char *buf, size_t len)
*/
if (len > (size_t) INT_MAX)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("argument of lo_write exceeds integer range\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("argument of lo_write exceeds integer range\n"));
return -1;
}
@@ -369,7 +405,13 @@ lo_lseek(PGconn *conn, int fd, int offset, int whence)
int retval;
int result_len;
- if (conn == NULL || conn->lobjfuncs == NULL)
+ if (conn == NULL)
+ return -1;
+
+ /* This is a library entry point, so start with an empty error message. */
+ resetPQExpBuffer(&conn->errorMessage);
+
+ if (conn->lobjfuncs == NULL)
{
if (lo_initialize(conn) < 0)
return -1;
@@ -413,7 +455,13 @@ lo_lseek64(PGconn *conn, int fd, pg_int64 offset, int whence)
pg_int64 retval;
int result_len;
- if (conn == NULL || conn->lobjfuncs == NULL)
+ if (conn == NULL)
+ return -1;
+
+ /* This is a library entry point, so start with an empty error message. */
+ resetPQExpBuffer(&conn->errorMessage);
+
+ if (conn->lobjfuncs == NULL)
{
if (lo_initialize(conn) < 0)
return -1;
@@ -421,8 +469,8 @@ lo_lseek64(PGconn *conn, int fd, pg_int64 offset, int whence)
if (conn->lobjfuncs->fn_lo_lseek64 == 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("cannot determine OID of function lo_lseek64\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("cannot determine OID of function lo_lseek64\n"));
return -1;
}
@@ -469,7 +517,13 @@ lo_creat(PGconn *conn, int mode)
int retval;
int result_len;
- if (conn == NULL || conn->lobjfuncs == NULL)
+ if (conn == NULL)
+ return InvalidOid;
+
+ /* This is a library entry point, so start with an empty error message. */
+ resetPQExpBuffer(&conn->errorMessage);
+
+ if (conn->lobjfuncs == NULL)
{
if (lo_initialize(conn) < 0)
return InvalidOid;
@@ -508,7 +562,13 @@ lo_create(PGconn *conn, Oid lobjId)
int retval;
int result_len;
- if (conn == NULL || conn->lobjfuncs == NULL)
+ if (conn == NULL)
+ return InvalidOid;
+
+ /* This is a library entry point, so start with an empty error message. */
+ resetPQExpBuffer(&conn->errorMessage);
+
+ if (conn->lobjfuncs == NULL)
{
if (lo_initialize(conn) < 0)
return InvalidOid;
@@ -517,8 +577,8 @@ lo_create(PGconn *conn, Oid lobjId)
/* Must check this on-the-fly because it's not there pre-8.1 */
if (conn->lobjfuncs->fn_lo_create == 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("cannot determine OID of function lo_create\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("cannot determine OID of function lo_create\n"));
return InvalidOid;
}
@@ -552,7 +612,13 @@ lo_tell(PGconn *conn, int fd)
PGresult *res;
int result_len;
- if (conn == NULL || conn->lobjfuncs == NULL)
+ if (conn == NULL)
+ return -1;
+
+ /* This is a library entry point, so start with an empty error message. */
+ resetPQExpBuffer(&conn->errorMessage);
+
+ if (conn->lobjfuncs == NULL)
{
if (lo_initialize(conn) < 0)
return -1;
@@ -588,7 +654,13 @@ lo_tell64(PGconn *conn, int fd)
PGresult *res;
int result_len;
- if (conn == NULL || conn->lobjfuncs == NULL)
+ if (conn == NULL)
+ return -1;
+
+ /* This is a library entry point, so start with an empty error message. */
+ resetPQExpBuffer(&conn->errorMessage);
+
+ if (conn->lobjfuncs == NULL)
{
if (lo_initialize(conn) < 0)
return -1;
@@ -596,8 +668,8 @@ lo_tell64(PGconn *conn, int fd)
if (conn->lobjfuncs->fn_lo_tell64 == 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("cannot determine OID of function lo_tell64\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("cannot determine OID of function lo_tell64\n"));
return -1;
}
@@ -632,7 +704,13 @@ lo_unlink(PGconn *conn, Oid lobjId)
int result_len;
int retval;
- if (conn == NULL || conn->lobjfuncs == NULL)
+ if (conn == NULL)
+ return -1;
+
+ /* This is a library entry point, so start with an empty error message. */
+ resetPQExpBuffer(&conn->errorMessage);
+
+ if (conn->lobjfuncs == NULL)
{
if (lo_initialize(conn) < 0)
return -1;
@@ -702,7 +780,7 @@ lo_import_internal(PGconn *conn, const char *filename, Oid oid)
fd = open(filename, O_RDONLY | PG_BINARY, 0666);
if (fd < 0)
{ /* error */
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not open file \"%s\": %s\n"),
filename, pqStrerror(errno, sebuf, sizeof(sebuf)));
return InvalidOid;
@@ -757,7 +835,9 @@ lo_import_internal(PGconn *conn, const char *filename, Oid oid)
(void) lo_close(conn, lobj);
(void) close(fd);
- printfPQExpBuffer(&conn->errorMessage,
+ /* Lose any error message reported by lo_close */
+ resetPQExpBuffer(&conn->errorMessage);
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not read from file \"%s\": %s\n"),
filename,
pqStrerror(save_errno, sebuf, sizeof(sebuf)));
@@ -811,7 +891,9 @@ lo_export(PGconn *conn, Oid lobjId, const char *filename)
int save_errno = errno;
(void) lo_close(conn, lobj);
- printfPQExpBuffer(&conn->errorMessage,
+ /* Lose any error message reported by lo_close */
+ resetPQExpBuffer(&conn->errorMessage);
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not open file \"%s\": %s\n"),
filename,
pqStrerror(save_errno, sebuf, sizeof(sebuf)));
@@ -831,7 +913,9 @@ lo_export(PGconn *conn, Oid lobjId, const char *filename)
(void) lo_close(conn, lobj);
(void) close(fd);
- printfPQExpBuffer(&conn->errorMessage,
+ /* Lose any error message reported by lo_close */
+ resetPQExpBuffer(&conn->errorMessage);
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not write to file \"%s\": %s\n"),
filename,
pqStrerror(save_errno, sebuf, sizeof(sebuf)));
@@ -852,10 +936,10 @@ lo_export(PGconn *conn, Oid lobjId, const char *filename)
result = -1;
}
- /* if we already failed, don't overwrite that msg with a close error */
+ /* if we already failed, we don't need an additional error from close() */
if (close(fd) && result >= 0)
{
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not write to file \"%s\": %s\n"),
filename, pqStrerror(errno, sebuf, sizeof(sebuf)));
result = -1;
@@ -882,17 +966,13 @@ lo_initialize(PGconn *conn)
const char *fname;
Oid foid;
- if (!conn)
- return -1;
-
/*
* Allocate the structure to hold the functions OID's
*/
lobjfuncs = (PGlobjfuncs *) malloc(sizeof(PGlobjfuncs));
if (lobjfuncs == NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return -1;
}
MemSet((char *) lobjfuncs, 0, sizeof(PGlobjfuncs));
@@ -931,6 +1011,7 @@ lo_initialize(PGconn *conn)
"or proname = 'loread' "
"or proname = 'lowrite'";
+ /* PQexec will reset errorMessage, but that's OK */
res = PQexec(conn, query);
if (res == NULL)
{
@@ -942,8 +1023,8 @@ lo_initialize(PGconn *conn)
{
free(lobjfuncs);
PQclear(res);
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("query to initialize large object functions did not return data\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("query to initialize large object functions did not return data\n"));
return -1;
}
@@ -991,57 +1072,57 @@ lo_initialize(PGconn *conn)
*/
if (lobjfuncs->fn_lo_open == 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("cannot determine OID of function lo_open\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("cannot determine OID of function lo_open\n"));
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_close == 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("cannot determine OID of function lo_close\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("cannot determine OID of function lo_close\n"));
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_creat == 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("cannot determine OID of function lo_creat\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("cannot determine OID of function lo_creat\n"));
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_unlink == 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("cannot determine OID of function lo_unlink\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("cannot determine OID of function lo_unlink\n"));
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_lseek == 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("cannot determine OID of function lo_lseek\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("cannot determine OID of function lo_lseek\n"));
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_tell == 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("cannot determine OID of function lo_tell\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("cannot determine OID of function lo_tell\n"));
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_read == 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("cannot determine OID of function loread\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("cannot determine OID of function loread\n"));
free(lobjfuncs);
return -1;
}
if (lobjfuncs->fn_lo_write == 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("cannot determine OID of function lowrite\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("cannot determine OID of function lowrite\n"));
free(lobjfuncs);
return -1;
}
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 2a6637f..81bbb8c 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -393,8 +393,7 @@ pqCheckOutBufferSpace(size_t bytes_needed, PGconn *conn)
}
/* realloc failed. Probably out of memory */
- printfPQExpBuffer(&conn->errorMessage,
- "cannot allocate memory for output buffer\n");
+ pqReportOOM(conn);
return EOF;
}
@@ -487,8 +486,7 @@ pqCheckInBufferSpace(size_t bytes_needed, PGconn *conn)
}
/* realloc failed. Probably out of memory */
- printfPQExpBuffer(&conn->errorMessage,
- "cannot allocate memory for input buffer\n");
+ pqReportOOM(conn);
return EOF;
}
@@ -633,8 +631,8 @@ pqReadData(PGconn *conn)
if (conn->sock == PGINVALID_SOCKET)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("connection not open\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("connection not open\n"));
return -1;
}
@@ -802,11 +800,10 @@ retry4:
* means the connection has been closed. Cope.
*/
definitelyEOF:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext(
- "server closed the connection unexpectedly\n"
- "\tThis probably means the server terminated abnormally\n"
- "\tbefore or while processing the request.\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("server closed the connection unexpectedly\n"
+ "\tThis probably means the server terminated abnormally\n"
+ "\tbefore or while processing the request.\n"));
/* Come here if lower-level code already set a suitable errorMessage */
definitelyFailed:
@@ -834,8 +831,8 @@ pqSendSome(PGconn *conn, int len)
if (conn->sock == PGINVALID_SOCKET)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("connection not open\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("connection not open\n"));
/* Discard queued data; no chance it'll ever be sent */
conn->outCount = 0;
return -1;
@@ -1005,8 +1002,8 @@ pqWaitTimed(int forRead, int forWrite, PGconn *conn, time_t finish_time)
if (result == 0)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("timeout expired\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("timeout expired\n"));
return 1;
}
@@ -1050,8 +1047,8 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
return -1;
if (conn->sock == PGINVALID_SOCKET)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("invalid socket\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("invalid socket\n"));
return -1;
}
@@ -1073,7 +1070,7 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
{
char sebuf[256];
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("select() failed: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
}
@@ -1211,11 +1208,49 @@ PQenv2encoding(void)
return encoding;
}
+/*
+ * Report an out-of-memory error by appending a message to conn->errorMessage.
+ *
+ * This task is just common enough, and nontrivial enough, to deserve its own
+ * subroutine.
+ */
+void
+pqReportOOM(PGconn *conn)
+{
+ pqReportOOMBuffer(&conn->errorMessage);
+}
+
+/*
+ * As above, but work with a bare error-message-buffer pointer.
+ */
+void
+pqReportOOMBuffer(PQExpBuffer errorMessage)
+{
+ const char *msg = libpq_gettext("out of memory\n");
+
+ /*
+ * First just try to append the message. Even though we're up against
+ * OOM, this is likely to succeed because of unused space within
+ * errorMessage's buffer.
+ */
+ appendPQExpBufferStr(errorMessage, msg);
+
+ /*
+ * If that didn't succeed, it'll have marked the buffer broken. Our
+ * fallback plan is to reset the buffer (hopefully regaining some space)
+ * and try again. If still no luck, not much we can do.
+ */
+ if (PQExpBufferBroken(errorMessage))
+ {
+ resetPQExpBuffer(errorMessage);
+ appendPQExpBufferStr(errorMessage, msg);
+ }
+}
#ifdef ENABLE_NLS
static void
-libpq_binddomain()
+libpq_binddomain(void)
{
static bool already_bound = false;
diff --git a/src/interfaces/libpq/fe-protocol2.c b/src/interfaces/libpq/fe-protocol2.c
index 53e5083..04a2e47 100644
--- a/src/interfaces/libpq/fe-protocol2.c
+++ b/src/interfaces/libpq/fe-protocol2.c
@@ -85,11 +85,9 @@ pqSetenvPoll(PGconn *conn)
return PGRES_POLLING_OK;
default:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext(
- "invalid setenv state %c, "
- "probably indicative of memory corruption\n"
- ),
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("invalid setenv state %c, "
+ "probably indicative of memory corruption\n"),
conn->setenv_state);
goto error_return;
}
@@ -385,7 +383,7 @@ pqSetenvPoll(PGconn *conn)
}
default:
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("invalid state %c, "
"probably indicative of memory corruption\n"),
conn->setenv_state);
@@ -498,8 +496,7 @@ pqParseInput2(PGconn *conn)
PGRES_COMMAND_OK);
if (!conn->result)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory"));
+ pqReportOOM(conn);
pqSaveErrorResult(conn);
}
}
@@ -533,8 +530,7 @@ pqParseInput2(PGconn *conn)
PGRES_EMPTY_QUERY);
if (!conn->result)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory"));
+ pqReportOOM(conn);
pqSaveErrorResult(conn);
}
}
@@ -627,9 +623,8 @@ pqParseInput2(PGconn *conn)
* never arrives from the server during protocol 2.0.
*/
default:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext(
- "unexpected response from server; first received character was
\"%c\"\n"),
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("unexpected response from server; first received character was
\"%c\"\n"),
id);
/* build an error result holding the error message */
pqSaveErrorResult(conn);
@@ -760,7 +755,7 @@ advance_and_error:
if (!errmsg)
errmsg = libpq_gettext("out of memory for query result");
- printfPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
+ appendPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
/*
* XXX: if PQmakeEmptyPGresult() fails, there's probably not much we can
@@ -935,7 +930,7 @@ set_error_result:
if (!errmsg)
errmsg = libpq_gettext("out of memory for query result");
- printfPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
+ appendPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
/*
* XXX: if PQmakeEmptyPGresult() fails, there's probably not much we can
@@ -1048,12 +1043,10 @@ pqGetErrorNotice2(PGconn *conn, bool isError)
{
pqClearAsyncResult(conn); /* redundant, but be safe */
conn->result = res;
- resetPQExpBuffer(&conn->errorMessage);
if (res && !PQExpBufferDataBroken(workBuf) && res->errMsg)
appendPQExpBufferStr(&conn->errorMessage, res->errMsg);
else
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory"));
+ pqReportOOM(conn);
if (conn->xactStatus == PQTRANS_INTRANS)
conn->xactStatus = PQTRANS_INERROR;
}
@@ -1209,8 +1202,7 @@ pqGetCopyData2(PGconn *conn, char **buffer, int async)
*buffer = (char *) malloc(msgLength + 1);
if (*buffer == NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return -2;
}
memcpy(*buffer, &conn->inBuffer[conn->inStart], msgLength);
@@ -1355,8 +1347,8 @@ pqEndcopy2(PGconn *conn)
if (conn->asyncStatus != PGASYNC_COPY_IN &&
conn->asyncStatus != PGASYNC_COPY_OUT)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("no COPY in progress\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("no COPY in progress\n"));
return 1;
}
@@ -1373,7 +1365,6 @@ pqEndcopy2(PGconn *conn)
/* Return to active duty */
conn->asyncStatus = PGASYNC_BUSY;
- resetPQExpBuffer(&conn->errorMessage);
/* Wait for the completion response */
result = PQgetResult(conn);
@@ -1544,7 +1535,7 @@ pqFunctionCall2(PGconn *conn, Oid fnid,
else
{
/* The backend violates the protocol. */
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("protocol error: id=0x%x\n"),
id);
pqSaveErrorResult(conn);
@@ -1576,7 +1567,7 @@ pqFunctionCall2(PGconn *conn, Oid fnid,
return PQmakeEmptyPGresult(conn, status);
default:
/* The backend violates the protocol. */
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("protocol error: id=0x%x\n"),
id);
pqSaveErrorResult(conn);
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 8345faf..98261d4 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -204,8 +204,7 @@ pqParseInput3(PGconn *conn)
PGRES_COMMAND_OK);
if (!conn->result)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory"));
+ pqReportOOM(conn);
pqSaveErrorResult(conn);
}
}
@@ -231,8 +230,7 @@ pqParseInput3(PGconn *conn)
PGRES_EMPTY_QUERY);
if (!conn->result)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory"));
+ pqReportOOM(conn);
pqSaveErrorResult(conn);
}
}
@@ -248,8 +246,7 @@ pqParseInput3(PGconn *conn)
PGRES_COMMAND_OK);
if (!conn->result)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory"));
+ pqReportOOM(conn);
pqSaveErrorResult(conn);
}
}
@@ -328,8 +325,7 @@ pqParseInput3(PGconn *conn)
PGRES_COMMAND_OK);
if (!conn->result)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory"));
+ pqReportOOM(conn);
pqSaveErrorResult(conn);
}
}
@@ -363,8 +359,8 @@ pqParseInput3(PGconn *conn)
else
{
/* Set up to report error at end of query */
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("server sent data (\"D\" message) without prior row
description(\"T\" message)\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("server sent data (\"D\" message) without prior row
description(\"T\" message)\n"));
pqSaveErrorResult(conn);
/* Discard the unexpected message */
conn->inCursor += msgLength;
@@ -406,9 +402,8 @@ pqParseInput3(PGconn *conn)
*/
break;
default:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext(
- "unexpected response from server; first received character was
\"%c\"\n"),
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("unexpected response from server; first received character was
\"%c\"\n"),
id);
/* build an error result holding the error message */
pqSaveErrorResult(conn);
@@ -428,7 +423,7 @@ pqParseInput3(PGconn *conn)
else
{
/* Trouble --- report it */
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("message contents do not agree with length in message type \"%c\"\n"),
id);
/* build an error result holding the error message */
@@ -448,9 +443,8 @@ pqParseInput3(PGconn *conn)
static void
handleSyncLoss(PGconn *conn, char id, int msgLength)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext(
- "lost synchronization with server: got message type \"%c\", length %d\n"),
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("lost synchronization with server: got message type \"%c\", length %d\n"),
id, msgLength);
/* build an error result holding the error message */
pqSaveErrorResult(conn);
@@ -625,7 +619,7 @@ advance_and_error:
if (!errmsg)
errmsg = libpq_gettext("out of memory for query result");
- printfPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
+ appendPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
pqSaveErrorResult(conn);
/*
@@ -725,7 +719,7 @@ advance_and_error:
*/
if (!errmsg)
errmsg = libpq_gettext("out of memory");
- printfPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
+ appendPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
pqSaveErrorResult(conn);
/*
@@ -852,7 +846,7 @@ set_error_result:
if (!errmsg)
errmsg = libpq_gettext("out of memory for query result");
- printfPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
+ appendPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
pqSaveErrorResult(conn);
/*
@@ -953,9 +947,10 @@ pqGetErrorNotice3(PGconn *conn, bool isError)
res->errMsg = pqResultStrdup(res, workBuf.data);
pqClearAsyncResult(conn); /* redundant, but be safe */
conn->result = res;
+
+ /* We also keep a running history of errors in conn->errorMessage. */
if (PQExpBufferDataBroken(workBuf))
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory"));
+ pqReportOOM(conn);
else
appendPQExpBufferStr(&conn->errorMessage, workBuf.data);
}
@@ -1681,8 +1676,7 @@ pqGetCopyData3(PGconn *conn, char **buffer, int async)
*buffer = (char *) malloc(msgLength + 1);
if (*buffer == NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return -2;
}
memcpy(*buffer, &conn->inBuffer[conn->inCursor], msgLength);
@@ -1714,8 +1708,8 @@ pqGetline3(PGconn *conn, char *s, int maxlen)
conn->asyncStatus != PGASYNC_COPY_BOTH) ||
conn->copy_is_binary)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("PQgetline: not doing text COPY OUT\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("PQgetline: not doing text COPY OUT\n"));
*s = '\0';
return EOF;
}
@@ -1820,8 +1814,8 @@ pqEndcopy3(PGconn *conn)
conn->asyncStatus != PGASYNC_COPY_OUT &&
conn->asyncStatus != PGASYNC_COPY_BOTH)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("no COPY in progress\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("no COPY in progress\n"));
return 1;
}
@@ -1854,7 +1848,6 @@ pqEndcopy3(PGconn *conn)
/* Return to active duty */
conn->asyncStatus = PGASYNC_BUSY;
- resetPQExpBuffer(&conn->errorMessage);
/*
* Non blocking connections may have to abort at this point. If everyone
@@ -2092,7 +2085,7 @@ pqFunctionCall3(PGconn *conn, Oid fnid,
break;
default:
/* The backend violates the protocol. */
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("protocol error: id=0x%x\n"),
id);
pqSaveErrorResult(conn);
diff --git a/src/interfaces/libpq/fe-secure-common.c b/src/interfaces/libpq/fe-secure-common.c
index b3f580f..2bea0d2 100644
--- a/src/interfaces/libpq/fe-secure-common.c
+++ b/src/interfaces/libpq/fe-secure-common.c
@@ -94,8 +94,8 @@ pq_verify_peer_name_matches_certificate_name(PGconn *conn,
if (!(host && host[0] != '\0'))
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("host name must be specified\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("host name must be specified\n"));
return -1;
}
@@ -106,8 +106,7 @@ pq_verify_peer_name_matches_certificate_name(PGconn *conn,
name = malloc(namelen + 1);
if (name == NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return -1;
}
memcpy(name, namedata, namelen);
@@ -120,8 +119,8 @@ pq_verify_peer_name_matches_certificate_name(PGconn *conn,
if (namelen != strlen(name))
{
free(name);
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SSL certificate's name contains embedded null\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("SSL certificate's name contains embedded null\n"));
return -1;
}
@@ -167,8 +166,8 @@ pq_verify_peer_name_matches_certificate(PGconn *conn)
/* Check that we have a hostname to compare with. */
if (!(host && host[0] != '\0'))
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("host name must be specified for a verified SSL connection\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("host name must be specified for a verified SSL connection\n"));
return false;
}
@@ -184,7 +183,7 @@ pq_verify_peer_name_matches_certificate(PGconn *conn)
*/
if (names_examined > 1)
{
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_ngettext("server certificate for \"%s\" (and %d other name) does not match host
name\"%s\"\n",
"server certificate for \"%s\" (and %d other names) does not match host
name\"%s\"\n",
names_examined - 1),
@@ -192,14 +191,14 @@ pq_verify_peer_name_matches_certificate(PGconn *conn)
}
else if (names_examined == 1)
{
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("server certificate for \"%s\" does not match host name \"%s\"\n"),
first_name, host);
}
else
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("could not get server's host name from server certificate\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("could not get server's host name from server certificate\n"));
}
}
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index bbae8ef..2cf3e22 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -177,8 +177,8 @@ rloop:
if (n < 0)
{
/* Not supposed to happen, so we don't translate the msg */
- printfPQExpBuffer(&conn->errorMessage,
- "SSL_read failed but did not provide error information\n");
+ appendPQExpBufferStr(&conn->errorMessage,
+ "SSL_read failed but did not provide error information\n");
/* assume the connection is broken */
result_errno = ECONNRESET;
}
@@ -201,21 +201,20 @@ rloop:
result_errno = SOCK_ERRNO;
if (result_errno == EPIPE ||
result_errno == ECONNRESET)
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext(
- "server closed the connection unexpectedly\n"
- "\tThis probably means the server terminated abnormally\n"
- "\tbefore or while processing the request.\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("server closed the connection unexpectedly\n"
+ "\tThis probably means the server terminated abnormally\n"
+ "\tbefore or while processing the request.\n"));
else
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("SSL SYSCALL error: %s\n"),
SOCK_STRERROR(result_errno,
sebuf, sizeof(sebuf)));
}
else
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SSL SYSCALL error: EOF detected\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("SSL SYSCALL error: EOF detected\n"));
/* assume the connection is broken */
result_errno = ECONNRESET;
n = -1;
@@ -225,7 +224,7 @@ rloop:
{
char *errm = SSLerrmessage(ecode);
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("SSL error: %s\n"), errm);
SSLerrfree(errm);
/* assume the connection is broken */
@@ -240,13 +239,13 @@ rloop:
* a clean connection closure, so we should not report it as a
* server crash.
*/
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SSL connection has been closed unexpectedly\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("SSL connection has been closed unexpectedly\n"));
result_errno = ECONNRESET;
n = -1;
break;
default:
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("unrecognized SSL error code: %d\n"),
err);
/* assume the connection is broken */
@@ -287,8 +286,8 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
if (n < 0)
{
/* Not supposed to happen, so we don't translate the msg */
- printfPQExpBuffer(&conn->errorMessage,
- "SSL_write failed but did not provide error information\n");
+ appendPQExpBufferStr(&conn->errorMessage,
+ "SSL_write failed but did not provide error information\n");
/* assume the connection is broken */
result_errno = ECONNRESET;
}
@@ -309,21 +308,20 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
{
result_errno = SOCK_ERRNO;
if (result_errno == EPIPE || result_errno == ECONNRESET)
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext(
- "server closed the connection unexpectedly\n"
- "\tThis probably means the server terminated abnormally\n"
- "\tbefore or while processing the request.\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("server closed the connection unexpectedly\n"
+ "\tThis probably means the server terminated abnormally\n"
+ "\tbefore or while processing the request.\n"));
else
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("SSL SYSCALL error: %s\n"),
SOCK_STRERROR(result_errno,
sebuf, sizeof(sebuf)));
}
else
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SSL SYSCALL error: EOF detected\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("SSL SYSCALL error: EOF detected\n"));
/* assume the connection is broken */
result_errno = ECONNRESET;
n = -1;
@@ -333,7 +331,7 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
{
char *errm = SSLerrmessage(ecode);
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("SSL error: %s\n"), errm);
SSLerrfree(errm);
/* assume the connection is broken */
@@ -348,13 +346,13 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
* a clean connection closure, so we should not report it as a
* server crash.
*/
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SSL connection has been closed unexpectedly\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("SSL connection has been closed unexpectedly\n"));
result_errno = ECONNRESET;
n = -1;
break;
default:
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("unrecognized SSL error code: %d\n"),
err);
/* assume the connection is broken */
@@ -394,8 +392,8 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
if (!OBJ_find_sigid_algs(X509_get_signature_nid(peer_cert),
&algo_nid, NULL))
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("could not determine server certificate signature algorithm\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("could not determine server certificate signature algorithm\n"));
return NULL;
}
@@ -415,7 +413,7 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
algo_type = EVP_get_digestbynid(algo_nid);
if (algo_type == NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not find digest for NID %s\n"),
OBJ_nid2sn(algo_nid));
return NULL;
@@ -425,8 +423,8 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
if (!X509_digest(peer_cert, algo_type, hash, &hash_size))
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("could not generate peer certificate hash\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("could not generate peer certificate hash\n"));
return NULL;
}
@@ -434,8 +432,7 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
cert_hash = malloc(hash_size);
if (cert_hash == NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return NULL;
}
memcpy(cert_hash, hash, hash_size);
@@ -443,7 +440,7 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
return cert_hash;
}
-#endif /* HAVE_X509_GET_SIGNATURE_NID */
+#endif /* HAVE_X509_GET_SIGNATURE_NID */
/* ------------------------------------------------------------ */
/* OpenSSL specific code */
@@ -482,8 +479,8 @@ openssl_verify_peer_name_matches_certificate_name(PGconn *conn, ASN1_STRING *nam
/* Should not happen... */
if (name_entry == NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SSL certificate's name entry is missing\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("SSL certificate's name entry is missing\n"));
return -1;
}
@@ -811,7 +808,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not create SSL context: %s\n"),
err);
SSLerrfree(err);
@@ -848,7 +845,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not read root certificate file \"%s\": %s\n"),
fnbuf, err);
SSLerrfree(err);
@@ -876,7 +873,7 @@ initialize_SSL(PGconn *conn)
#else
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("SSL library does not support CRL certificates (file \"%s\")\n"),
fnbuf);
SSLerrfree(err);
@@ -904,11 +901,11 @@ initialize_SSL(PGconn *conn)
* that it seems worth having a specialized error message for it.
*/
if (fnbuf[0] == '\0')
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("could not get home directory to locate root certificate file\n"
- "Either provide the file or change sslmode to disable server
certificateverification.\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("could not get home directory to locate root certificate file\n"
+ "Either provide the file or change sslmode to disable server
certificateverification.\n"));
else
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("root certificate file \"%s\" does not exist\n"
"Either provide the file or change sslmode to disable server
certificateverification.\n"), fnbuf);
SSL_CTX_free(SSL_context);
@@ -939,7 +936,7 @@ initialize_SSL(PGconn *conn)
*/
if (errno != ENOENT && errno != ENOTDIR)
{
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not open certificate file \"%s\": %s\n"),
fnbuf, pqStrerror(errno, sebuf, sizeof(sebuf)));
SSL_CTX_free(SSL_context);
@@ -958,7 +955,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not read certificate file \"%s\": %s\n"),
fnbuf, err);
SSLerrfree(err);
@@ -983,7 +980,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not establish SSL connection: %s\n"),
err);
SSLerrfree(err);
@@ -1021,8 +1018,7 @@ initialize_SSL(PGconn *conn)
if (engine_str == NULL)
{
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("out of memory\n"));
+ pqReportOOM(conn);
return -1;
}
@@ -1037,7 +1033,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not load SSL engine \"%s\": %s\n"),
engine_str, err);
SSLerrfree(err);
@@ -1049,7 +1045,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not initialize SSL engine \"%s\": %s\n"),
engine_str, err);
SSLerrfree(err);
@@ -1065,7 +1061,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not read private SSL key \"%s\" from engine \"%s\": %s\n"),
engine_colon, engine_str, err);
SSLerrfree(err);
@@ -1079,7 +1075,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not load private SSL key \"%s\" from engine \"%s\": %s\n"),
engine_colon, engine_str, err);
SSLerrfree(err);
@@ -1116,7 +1112,7 @@ initialize_SSL(PGconn *conn)
if (stat(fnbuf, &buf) != 0)
{
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("certificate present, but not private key file \"%s\"\n"),
fnbuf);
return -1;
@@ -1124,7 +1120,7 @@ initialize_SSL(PGconn *conn)
#ifndef WIN32
if (!S_ISREG(buf.st_mode) || buf.st_mode & (S_IRWXG | S_IRWXO))
{
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("private key file \"%s\" has group or world access; permissions should be
u=rw(0600) or less\n"),
fnbuf);
return -1;
@@ -1135,7 +1131,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not load private key file \"%s\": %s\n"),
fnbuf, err);
SSLerrfree(err);
@@ -1149,7 +1145,7 @@ initialize_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("certificate does not match private key file \"%s\": %s\n"),
fnbuf, err);
SSLerrfree(err);
@@ -1215,12 +1211,12 @@ open_client_SSL(PGconn *conn)
char sebuf[256];
if (r == -1)
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("SSL SYSCALL error: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
else
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext("SSL SYSCALL error: EOF detected\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("SSL SYSCALL error: EOF detected\n"));
pgtls_close(conn);
return PGRES_POLLING_FAILED;
}
@@ -1228,7 +1224,7 @@ open_client_SSL(PGconn *conn)
{
char *err = SSLerrmessage(ecode);
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("SSL error: %s\n"),
err);
SSLerrfree(err);
@@ -1237,7 +1233,7 @@ open_client_SSL(PGconn *conn)
}
default:
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("unrecognized SSL error code: %d\n"),
err);
pgtls_close(conn);
@@ -1258,7 +1254,7 @@ open_client_SSL(PGconn *conn)
err = SSLerrmessage(ERR_get_error());
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("certificate could not be obtained: %s\n"),
err);
SSLerrfree(err);
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index f7dc249..157a738 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -205,8 +205,8 @@ pqsecure_close(PGconn *conn)
/*
* Read data from a secure connection.
*
- * On failure, this function is responsible for putting a suitable message
- * into conn->errorMessage. The caller must still inspect errno, but only
+ * On failure, this function is responsible for appending a suitable message
+ * to conn->errorMessage. The caller must still inspect errno, but only
* to determine whether to continue/retry after error.
*/
ssize_t
@@ -256,16 +256,15 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len)
#ifdef ECONNRESET
case ECONNRESET:
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext(
- "server closed the connection unexpectedly\n"
- "\tThis probably means the server terminated abnormally\n"
- "\tbefore or while processing the request.\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("server closed the connection unexpectedly\n"
+ "\tThis probably means the server terminated abnormally\n"
+ "\tbefore or while processing the request.\n"));
break;
#endif
default:
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not receive data from server: %s\n"),
SOCK_STRERROR(result_errno,
sebuf, sizeof(sebuf)));
@@ -282,8 +281,8 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len)
/*
* Write data to a secure connection.
*
- * On failure, this function is responsible for putting a suitable message
- * into conn->errorMessage. The caller must still inspect errno, but only
+ * On failure, this function is responsible for appending a suitable message
+ * to conn->errorMessage. The caller must still inspect errno, but only
* to determine whether to continue/retry after error.
*/
ssize_t
@@ -366,15 +365,14 @@ retry_masked:
case ECONNRESET:
#endif
- printfPQExpBuffer(&conn->errorMessage,
- libpq_gettext(
- "server closed the connection unexpectedly\n"
- "\tThis probably means the server terminated abnormally\n"
- "\tbefore or while processing the request.\n"));
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("server closed the connection unexpectedly\n"
+ "\tThis probably means the server terminated abnormally\n"
+ "\tbefore or while processing the request.\n"));
break;
default:
- printfPQExpBuffer(&conn->errorMessage,
+ appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not send data to server: %s\n"),
SOCK_STRERROR(result_errno,
sebuf, sizeof(sebuf)));
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index bc60373..22cfa4f 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -644,6 +644,8 @@ extern int pqWaitTimed(int forRead, int forWrite, PGconn *conn,
time_t finish_time);
extern int pqReadReady(PGconn *conn);
extern int pqWriteReady(PGconn *conn);
+extern void pqReportOOM(PGconn *conn);
+extern void pqReportOOMBuffer(PQExpBuffer errorMessage);
/* === in fe-secure.c === */
@@ -679,7 +681,7 @@ extern void pgtls_init_library(bool do_ssl, int do_crypto);
* The conn parameter is only used to be able to pass back an error
* message - no connection-local setup is made here.
*
- * Returns 0 if OK, -1 on failure (with a message in conn->errorMessage).
+ * Returns 0 if OK, -1 on failure (with a message added to conn->errorMessage).
*/
extern int pgtls_init(PGconn *conn);
@@ -696,8 +698,8 @@ extern void pgtls_close(PGconn *conn);
/*
* Read data from a secure connection.
*
- * On failure, this function is responsible for putting a suitable message
- * into conn->errorMessage. The caller must still inspect errno, but only
+ * On failure, this function is responsible for appending a suitable message
+ * to conn->errorMessage. The caller must still inspect errno, but only
* to determine whether to continue/retry after error.
*/
extern ssize_t pgtls_read(PGconn *conn, void *ptr, size_t len);
@@ -710,8 +712,8 @@ extern bool pgtls_read_pending(PGconn *conn);
/*
* Write data to a secure connection.
*
- * On failure, this function is responsible for putting a suitable message
- * into conn->errorMessage. The caller must still inspect errno, but only
+ * On failure, this function is responsible for appending a suitable message
+ * to conn->errorMessage. The caller must still inspect errno, but only
* to determine whether to continue/retry after error.
*/
extern ssize_t pgtls_write(PGconn *conn, const void *ptr, size_t len);
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 647423c..3f278c3 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -2116,6 +2116,31 @@ keep_going: /* We will come back to here until there is
goto error_return;
}
conn->whichhost++;
+
+ /*
+ * If we're given more than one target host, append identification
+ * info to conn->errorMessage as we start to consider each one. This
+ * makes it easier to figure out which host error messages apply to,
+ * in case we fail to connect to all of them.
+ */
+ if (conn->nconnhost > 1)
+ {
+ pg_conn_host *ch = &conn->connhost[conn->whichhost];
+ const char *displayed_host;
+ const char *displayed_port;
+
+ if (ch->type == CHT_HOST_ADDRESS)
+ displayed_host = ch->hostaddr;
+ else
+ displayed_host = ch->host;
+ displayed_port = ch->port;
+ if (displayed_port == NULL || displayed_port[0] == '\0')
+ displayed_port = DEF_PGPORT_STR;
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("server \"%s\" port %s: "),
+ displayed_host, displayed_port);
+ }
+
conn->addr_cur = conn->connhost[conn->whichhost].addrlist;
/* If no addresses for this host, just try the next one */
if (conn->addr_cur == NULL)
@@ -3154,9 +3179,6 @@ keep_going: /* We will come back to here until there is
}
case CONNECTION_CHECK_WRITABLE:
{
- const char *displayed_host;
- const char *displayed_port;
-
if (!PQconsumeInput(conn))
goto error_return;
@@ -3173,25 +3195,10 @@ keep_going: /* We will come back to here until there is
if (strncmp(val, "on", 2) == 0)
{
/* Not writable; fail this connection. */
- const char *displayed_host;
- const char *displayed_port;
-
PQclear(res);
- /* Append error report to conn->errorMessage. */
- if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS)
- displayed_host = conn->connhost[conn->whichhost].hostaddr;
- else
- displayed_host = conn->connhost[conn->whichhost].host;
- displayed_port = conn->connhost[conn->whichhost].port;
- if (displayed_port == NULL || displayed_port[0] == '\0')
- displayed_port = DEF_PGPORT_STR;
-
- appendPQExpBuffer(&conn->errorMessage,
- libpq_gettext("could not make a writable "
- "connection to server "
- "\"%s:%s\"\n"),
- displayed_host, displayed_port);
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("could not open a read-write session\n"));
/* Close connection politely. */
conn->status = CONNECTION_OK;
@@ -3217,25 +3224,17 @@ keep_going: /* We will come back to here until there is
}
/*
- * Something went wrong with "SHOW transaction_read_only". We
- * should try next addresses.
+ * Something went wrong with "SHOW transaction_read_only". If
+ * there was a server error, it'll already have been appended
+ * to errorMessage, else say something generic.
*/
+ if (res == NULL || PQresultStatus(res) != PGRES_FATAL_ERROR)
+ appendPQExpBufferStr(&conn->errorMessage,
+ libpq_gettext("\"SHOW transaction_read_only\" failed\n"));
+
if (res)
PQclear(res);
- /* Append error report to conn->errorMessage. */
- if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS)
- displayed_host = conn->connhost[conn->whichhost].hostaddr;
- else
- displayed_host = conn->connhost[conn->whichhost].host;
- displayed_port = conn->connhost[conn->whichhost].port;
- if (displayed_port == NULL || displayed_port[0] == '\0')
- displayed_port = DEF_PGPORT_STR;
- appendPQExpBuffer(&conn->errorMessage,
- libpq_gettext("test \"SHOW transaction_read_only\" failed "
- "on server \"%s:%s\"\n"),
- displayed_host, displayed_port);
-
/* Close connection politely. */
conn->status = CONNECTION_OK;
sendTerminateConn(conn);
Hello Tom,
> The thing is that there are a *lot* of systems nowadays on which localhost
> maps to both ipv4 and ipv6, so that "host=localhost" would be enough to
> trigger the new behavior,
I think that people would survive having the ip spelled out on localhost
errors when there are several ips associated to the name.
You could also create an exception for "localhost" if you see it as a
large problem, but if someone redefined localhost somehow (I did that once
by inadvertence), it would be nice to help and be explicit.
> and I think that would inevitably give rise to more complaints than
> kudos.
Hmmm. I'm not sure about what the complaint could be in case of multiple
ips. "Hey, I have a 'host' with multiple ips, and on errors you tell me
which ip generated an issue, and I do not want to know, really".
> So I'm still of the opinion that we're better off with the
> second patch's behavior as it stands.
This was a mere suggestion.
As a user, and as a support person for others on occasions, I like precise
error messages which convey all relevant information. In this instance a
key information is hidden, hence the proposal.
> Responding to your other points from upthread:
>
>> There are still 86 instances of "printfPQExpBuffer", which seems like a
>> lot. They are mostly OOM messages, but not only. This make checking the
>> patch quite cumbersome.
>> I'd rather there would be only one rule, and clear reset with a comment
>> when needded (eg "fe-lobj.c").
>
> Well, I did this for discussion's sake, but I don't really find the
> resulting changes in fe-lobj.c to be any sort of improvement. [...]
The improvement I see is that if there would not be single remaining
printf, so it is easy to check that no case were forgotten in libpq, and
for future changes that everywhere in libpq there is only one rule which
is "append errors to expbuf", not "it depends on the file or function you
are in, please look at it in detail".
>> It is unclear to me why the "printf" variant is used in
>> "PQencryptPasswordConn" and "connectOptions2", it looks that you
>> skipped these functions.
>
> I did because it seemed like unnecessary extra diff content, since
> we're expecting the error buffer to be initially empty anyway (for
> connectOptions2) or we'd just need an extra reset (for
> PQencryptPasswordConn). In the attached I went ahead and made those
> changes, but again I'm unsure that it's really a net improvement.
Same as above: easier to check, one simple rule for all libpq errors.
>> I do not like the resulting output
>> server:
>> problem found
>> more details
>> I'd rather have
>> server: problem found
>> more details
>
> Done in the attached, although I'm concerned about the resulting effects
> for error message line length --- in my tests, lines that used to reliably
> fit in 80 columns don't anymore. Again, I'm not really convinced this is
> an improvement. It would absolutely *not* be an improvement if we tried
> to also shoehorn the server's IP address in there; that'd make the lines
> way too long, with way too much stuff before the important part.
I understand your concern about line length.
A possible compromise would be to newline *and* indent before "problem
found". To avoid messy code, there could be an internal function to manage
that, eg.
appendErrorContext(...), for "server:"
appendError(exbuf, fmt, ...) , for errors, which would indent the
initial line by two spaces.
server1:
problem found
details
server2:
problem found
details
This would also give room for the ip on a 80-column server line.
The alternative is that the committer does as they want, which is also
fine with me:-)
--
Fabien.
On Thu, Aug 9, 2018 at 11:44 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I noticed that, although most error reports during libpq's connection > setup code append to conn->errorMessage, the ones in fe-auth.c and > fe-auth-scram.c don't: they're all printfPQExpBuffer() not > appendPQExpBuffer(). This seems wrong to me. It makes no difference > in simple cases with a single target server, but as soon as you have > multiple servers listed in "host", this coding makes it impossible > to tell which server rejected your login. > > So I think we should basically s/printfPQExpBuffer/appendPQExpBuffer/g > anywhere those files touch conn->errorMessage, allowing any problems > with previous servers to be preserved in the eventually-reported message. > I won't bother posting an actual patch for that right now, but has anyone > got an objection? I guess the question in my mind is what behavior is most useful for users. There are, I'm sure, cases where it's useful to see all the details you are proposing to print. But it also seems like there could be a significant number of cases where it's really more than anybody wants to know. For instance, if I try to connect host=primary1,primary2,dr and primary1 is offline right now, because we're using primary2, and if it happens that I mistype my password, with your patch, I'll get an error saying that primary1 is down, followed by an error about my password. I only care about the second one. If I had typed my password correctly, I would have just gotten a connection, and everything would have been fine. Telling me that primary1 is down may make me (1) panic or (2) miss the real cause of my problem. So I'm not as convinced as you are that always displaying maximum detail is really the right thing to do. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Aug 9, 2018 at 11:44 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> So I think we should basically s/printfPQExpBuffer/appendPQExpBuffer/g
>> anywhere those files touch conn->errorMessage, allowing any problems
>> with previous servers to be preserved in the eventually-reported message.
> I guess the question in my mind is what behavior is most useful for
> users. There are, I'm sure, cases where it's useful to see all the
> details you are proposing to print. But it also seems like there
> could be a significant number of cases where it's really more than
> anybody wants to know. For instance, if I try to connect
> host=primary1,primary2,dr and primary1 is offline right now, because
> we're using primary2, and if it happens that I mistype my password,
> with your patch, I'll get an error saying that primary1 is down,
> followed by an error about my password. I only care about the second
> one. If I had typed my password correctly, I would have just gotten a
> connection, and everything would have been fine. Telling me that
> primary1 is down may make me (1) panic or (2) miss the real cause of
> my problem.
> So I'm not as convinced as you are that always displaying maximum
> detail is really the right thing to do.
Well, I'm actually not proposing to print "maximum detail", and Fabien
is complaining about that, which makes me think maybe I've hit a happy
medium ;-). In particular, the proposed patches won't change behavior
for cases where you just give one hostname or hostaddr, and I'd argue
that that is the only case where we really have enough track record to
be sure that people are happy with the amount of detail provided.
As soon as you have multiple target hosts, though, the current code's
behavior is inadequate IMO. Indisputably it's inconsistent, because
some code paths will concatenate error messages and some won't, without
any rhyme or reason that I can detect. I think the only reason we've
not had more complaints is that hardly anyone is using this feature yet.
regards, tom lane
On Wed, Aug 15, 2018 at 11:53 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Well, I'm actually not proposing to print "maximum detail", and Fabien > is complaining about that, which makes me think maybe I've hit a happy > medium ;-). In particular, the proposed patches won't change behavior > for cases where you just give one hostname or hostaddr, and I'd argue > that that is the only case where we really have enough track record to > be sure that people are happy with the amount of detail provided. > As soon as you have multiple target hosts, though, the current code's > behavior is inadequate IMO. I'm not entirely convinced; see the example I posted before. > Indisputably it's inconsistent, because > some code paths will concatenate error messages and some won't, without > any rhyme or reason that I can detect. I think the only reason we've > not had more complaints is that hardly anyone is using this feature yet. I agree that inconsistency is bad as a rule. :-) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Aug 15, 2018 at 11:53 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> As soon as you have multiple target hosts, though, the current code's
>> behavior is inadequate IMO.
> I'm not entirely convinced; see the example I posted before.
TBH I find your example to be the exact opposite of convincing.
You've cherry-picked a case where the current behavior tells you
what you need to know and not anything you don't, but very small
variations on the case make that not hold anymore. Remember the
complaint we started with, which was that if you get a bad-password
error it's not apparent which host gave that response. If, in
fact, the hosts you've listed don't all use the same password,
you could be tearing your hair out for quite awhile before you
figure out what's going wrong.
In any case, I'm a little bit confused as to why someone who'd
listed multiple hosts would be surprised to see that the connection
had in fact fallen back to not-the-first host.
regards, tom lane
On Wed, Aug 15, 2018 at 12:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > TBH I find your example to be the exact opposite of convincing. > You've cherry-picked a case where the current behavior tells you > what you need to know and not anything you don't, but very small > variations on the case make that not hold anymore. Remember the > complaint we started with, which was that if you get a bad-password > error it's not apparent which host gave that response. If, in > fact, the hosts you've listed don't all use the same password, > you could be tearing your hair out for quite awhile before you > figure out what's going wrong. That seems like a pretty unlikely use case, though. It seems to me that the virtue of the feature is in letting you connect to one of a number of hosts that are basically all equivalent. If they're not all equivalent, how come you're OK with connecting to any one of them? It's of course not impossible for somebody to have databases with equivalent contents but different authentication, but that sounds like some kind of confusing fringe thing to me. My guess -- and I think this is what underlay the original coding, though maybe I didn't get all the details right -- is that if your connection fails to reach any server, you want to know the details as to why that failed for each server in the list, but that if your connection fails for some reason other than not being able to reach the server at all, like a missing role name or password, you just want to know about that, and you don't really care about the servers to which you failed to connect altogether. Saying host=a,b,c basically means exactly that: I don't really care about failure to connect to a or even a and b; that's not an error condition. Of course if I can't connect to any of them -- that's still got to be an error. > In any case, I'm a little bit confused as to why someone who'd > listed multiple hosts would be surprised to see that the connection > had in fact fallen back to not-the-first host. In a perfect world where users never misinterpret error messages, they wouldn't, but that is not a world I have the privilege of inhabiting. Focusing the error on the thing that is probably what the user needs to fix is a very sensible decision, I think. We could print out a full stack backtrace, a memory context dump, and a node tree display of the plan every time a query errors out, but most of the time, that would be a truly excessive amount of information that would distract from, say, the fact that you divided by zero, or whatever. Brevity has value. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Aug 15, 2018 at 12:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> TBH I find your example to be the exact opposite of convincing.
> That seems like a pretty unlikely use case, though. It seems to me
> that the virtue of the feature is in letting you connect to one of a
> number of hosts that are basically all equivalent. If they're not all
> equivalent, how come you're OK with connecting to any one of them?
I'm still unpersuaded. The point you're ignoring is that the *intention*
might be that the hosts are equivalent, but the *reality* might be that
they're not, and the difference very likely is exactly why your connection
failed. Password not what you expected, user or database not there at
all, etc. In such situations, an error message that doesn't give you
enough info to realize where the problem really lies is very unfriendly.
Also, if I buy this line of argument, I fail to see why the existing
messages that explicitly mention which host we didn't connect to (because
of problems with the read-write session test) are like that. How is it
that those need to be labeled as to host, and concatenated, but other
sorts of failures don't? I think the author(s) of that patch understood
the problem perfectly well, but were too lazy or cowardly to fix it other
than in code they were adding. I'm here to clean that up.
> In a perfect world where users never misinterpret error messages, they
> wouldn't, but that is not a world I have the privilege of inhabiting.
Me neither, but a message that fails to provide critical context
is much more susceptible to misinterpretation than one that doesn't.
regards, tom lane
On Wed, Aug 15, 2018 at 12:46 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I think the author(s) of that patch understood > the problem perfectly well, but were too lazy or cowardly to fix it other > than in code they were adding. I think this is an ad hominum attack. I have explained some factors that are relevant from my point of view, and that explain some of the behavior you don't like. I have already said several times that the implementation might not be up to the mark, and I apologize for that. If you want to interpret the current state of affairs as being 0% attributable to a difference of opinion between reasonable people about what the behavior ought to be, and 100% due to laziness or cowardliness on my part and the part of the other people involved in these patches, I cannot prevent you from doing so. However, I think that judgement is inaccurate. More importantly, I think it's a nasty sentiment which is inappropriate on this mailing list or in any other civilized discourse. If you think that the code is bad, you can say that without commenting on my character. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, Aug 14, 2018 at 02:17:09PM +0200, Fabien COELHO wrote: > I think that people would survive having the ip spelled out on localhost > errors when there are several ips associated to the name. > > You could also create an exception for "localhost" if you see it as a large > problem, but if someone redefined localhost somehow (I did that once by > inadvertence), it would be nice to help and be explicit. The last status of this patch comes from August, which has been unanswered. So I am marking the patch as returned with feedback. The patch set does not apply cleanly by the way. -- Michael