diff --git a/contrib/test_decoding/expected/slot_creation_error.out b/contrib/test_decoding/expected/slot_creation_error.out index 321648c339..043bdae0a2 100644 --- a/contrib/test_decoding/expected/slot_creation_error.out +++ b/contrib/test_decoding/expected/slot_creation_error.out @@ -98,7 +98,6 @@ t step s2_init: <... completed> FATAL: terminating connection due to administrator command -FATAL: terminating connection due to administrator command server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c index f8f4111fef..6fceff561b 100644 --- a/src/interfaces/libpq/fe-auth.c +++ b/src/interfaces/libpq/fe-auth.c @@ -1237,7 +1237,7 @@ PQencryptPasswordConn(PGconn *conn, const char *passwd, const char *user, if (!conn) return NULL; - resetPQExpBuffer(&conn->errorMessage); + pqClearConnErrorState(conn); /* If no algorithm was given, ask the server. */ if (algorithm == NULL) diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 9c9416e8ff..2a3d68b4d1 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -3685,7 +3685,7 @@ keep_going: /* We will come back to here until there is * (and it seems some clients expect it to be empty after a * successful connection). */ - resetPQExpBuffer(&conn->errorMessage); + pqClearConnErrorState(conn); /* We are open for business! */ conn->status = CONNECTION_OK; @@ -4231,7 +4231,7 @@ closePGconn(PGconn *conn) /* * Close the connection, reset all transient state, flush I/O buffers. - * Note that this includes clearing conn->errorMessage; we're no longer + * Note that this includes clearing conn's error state; we're no longer * interested in any failures associated with the old connection, and we * want a clean slate for any new connection attempt. */ @@ -4241,7 +4241,7 @@ closePGconn(PGconn *conn) conn->xactStatus = PQTRANS_IDLE; conn->pipelineStatus = PQ_PIPELINE_OFF; pqClearAsyncResult(conn); /* deallocate result */ - resetPQExpBuffer(&conn->errorMessage); + pqClearConnErrorState(conn); release_conn_addrinfo(conn); /* Reset all state obtained from server, too */ @@ -5236,7 +5236,7 @@ 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 + * is the undocumented fact that appendPQExpBuffer does nothing when passed * a null PQExpBuffer pointer.) */ static int diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index c7c48d07dc..ceeb0a4fd9 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -44,6 +44,13 @@ char *const pgresStatus[] = { "PGRES_PIPELINE_ABORTED" }; +/* We return this if we're unable to make a PGresult at all */ +static const PGresult OOM_result = { + .resultStatus = PGRES_FATAL_ERROR, + .client_encoding = PG_SQL_ASCII, + .errMsg = "out of memory\n", +}; + /* * static state needed by PQescapeString and PQescapeBytea; initialize to * values that result in backward-compatible behavior @@ -141,6 +148,10 @@ static int pqPipelineFlush(PGconn *conn); * returns a newly allocated, initialized PGresult with given status. * If conn is not NULL and status indicates an error, the conn's * errorMessage is copied. Also, any PGEvents are copied from the conn. + * + * Note: the logic to copy the conn's errorMessage is now vestigial; + * no internal caller uses it. However, that behavior is documented for + * outside callers, so we'd better keep it. */ PGresult * PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status) @@ -191,7 +202,8 @@ PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status) /* non-error cases */ break; default: - pqSetResultError(result, &conn->errorMessage); + /* we intentionally do not use or modify errorReported here */ + pqSetResultError(result, &conn->errorMessage, 0); break; } @@ -657,9 +669,12 @@ pqResultStrdup(PGresult *res, const char *str) /* * pqSetResultError - * assign a new error message to a PGresult + * + * Copy text from errorMessage buffer beginning at given offset + * (it's caller's responsibility that offset is valid) */ void -pqSetResultError(PGresult *res, PQExpBuffer errorMessage) +pqSetResultError(PGresult *res, PQExpBuffer errorMessage, int offset) { char *msg; @@ -674,7 +689,7 @@ pqSetResultError(PGresult *res, PQExpBuffer errorMessage) * at a constant "out of memory" string. */ if (!PQExpBufferBroken(errorMessage)) - msg = pqResultStrdup(res, errorMessage->data); + msg = pqResultStrdup(res, errorMessage->data + offset); else msg = NULL; if (msg) @@ -693,9 +708,14 @@ PQclear(PGresult *res) PGresult_data *block; int i; + /* As a convenience, do nothing for a NULL pointer */ if (!res) return; + /* Also, do nothing if the argument is OOM_result */ + if (res == unconstify(PGresult *, &OOM_result)) + return; + /* Close down any events we may have */ for (i = 0; i < res->nEvents; i++) { /* only send DESTROY to successfully-initialized event procs */ @@ -748,24 +768,39 @@ pqClearAsyncResult(PGconn *conn) if (conn->result) PQclear(conn->result); conn->result = NULL; + conn->error_result = false; if (conn->next_result) PQclear(conn->next_result); conn->next_result = NULL; } /* - * This subroutine deletes any existing async result, sets conn->result - * to a PGresult with status PGRES_FATAL_ERROR, and stores the current - * contents of conn->errorMessage into that result. + * pqSaveErrorResult - + * remember that we have an error condition + * + * In much of libpq, reporting an error just requires appending text to + * conn->errorMessage and returning a failure code to one's caller. + * Where returning a failure code is impractical, instead call this + * function to remember that an error needs to be reported. + * + * (It might seem that appending text to conn->errorMessage should be + * sufficient, but we can't rely on that working under out-of-memory + * conditions. The OOM hazard is also why we don't try to make a new + * PGresult right here.) */ void pqSaveErrorResult(PGconn *conn) { + /* Drop any pending result ... */ pqClearAsyncResult(conn); - conn->result = PQmakeEmptyPGresult(conn, PGRES_FATAL_ERROR); + /* ... and set flag to remember to make an error result later */ + conn->error_result = true; } /* + * pqSaveWriteError - + * report a write failure + * * As above, after appending conn->write_err_msg to whatever other error we * have. This is used when we've detected a write failure and have exhausted * our chances of reporting something else instead. @@ -792,24 +827,79 @@ pqSaveWriteError(PGconn *conn) } /* - * This subroutine prepares an async result object for return to the caller. + * pqPrepareAsyncResult - + * prepare the current 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. + * result storage, and update our notion of how much error text has been + * returned to the application. */ PGresult * pqPrepareAsyncResult(PGconn *conn) { PGresult *res; - /* - * 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. - */ res = conn->result; - if (!res) - res = PQmakeEmptyPGresult(conn, PGRES_FATAL_ERROR); + if (res) + { + /* + * If the pre-existing result is an ERROR (presumably something + * received from the server), assume that it represents whatever is in + * conn->errorMessage, and advance errorReported. + */ + if (res->resultStatus == PGRES_FATAL_ERROR) + conn->errorReported = conn->errorMessage.len; + } + else + { + /* + * We get here after internal-to-libpq errors. We should probably + * always have error_result = true, but if we don't, gin up some error + * text. + */ + if (!conn->error_result) + appendPQExpBufferStr(&conn->errorMessage, + libpq_gettext("no error text available\n")); + + /* Paranoia: be sure errorReported offset is sane */ + if (conn->errorReported < 0 || + conn->errorReported >= conn->errorMessage.len) + conn->errorReported = 0; + + /* + * Make a PGresult struct for the error. We temporarily lie about the + * result status, so that PQmakeEmptyPGresult doesn't uselessly copy + * all of conn->errorMessage. + */ + res = PQmakeEmptyPGresult(conn, PGRES_EMPTY_QUERY); + if (res) + { + /* + * Report whatever new error text we have, and advance + * errorReported. + */ + res->resultStatus = PGRES_FATAL_ERROR; + pqSetResultError(res, &conn->errorMessage, conn->errorReported); + conn->errorReported = conn->errorMessage.len; + } + else + { + /* + * Ouch, not enough memory for a PGresult. Fortunately, we have a + * card up our sleeve: we can use the static OOM_result. Casting + * away const here is a bit ugly, but it seems best to declare + * OOM_result as const, in hopes it will be allocated in read-only + * storage. + */ + res = unconstify(PGresult *, &OOM_result); + + /* + * Don't advance errorReported. Perhaps we'll be able to report + * the text later. + */ + } + } /* * Replace conn->result with next_result, if any. In the normal case @@ -819,6 +909,7 @@ pqPrepareAsyncResult(PGconn *conn) */ conn->result = conn->next_result; conn->next_result = NULL; + conn->error_result = false; /* next_result is never an error */ return res; } @@ -1278,7 +1369,7 @@ pqAppendCmdQueueEntry(PGconn *conn, PGcmdQueueEntry *entry) */ if (conn->asyncStatus == PGASYNC_IDLE) { - resetPQExpBuffer(&conn->errorMessage); + pqClearConnErrorState(conn); pqPipelineProcessQueue(conn); } break; @@ -1626,10 +1717,10 @@ PQsendQueryStart(PGconn *conn, bool newQuery) return false; /* - * If this is the beginning of a query cycle, reset the error buffer. + * If this is the beginning of a query cycle, reset the error state. */ if (newQuery) - resetPQExpBuffer(&conn->errorMessage); + pqClearConnErrorState(conn); /* Don't try to send if we know there's no live connection. */ if (conn->status != CONNECTION_OK) @@ -1687,8 +1778,8 @@ PQsendQueryStart(PGconn *conn, bool newQuery) /* reset single-row processing mode */ conn->singleRowMode = false; - } + /* ready to send command message */ return true; } @@ -1884,7 +1975,7 @@ PQsetSingleRowMode(PGconn *conn) (conn->cmd_queue_head->queryclass != PGQUERY_SIMPLE && conn->cmd_queue_head->queryclass != PGQUERY_EXTENDED)) return 0; - if (conn->result) + if (conn->result || conn->error_result) return 0; /* OK, set flag */ @@ -2015,10 +2106,7 @@ PQgetResult(PGconn *conn) pqWait(true, false, conn) || pqReadData(conn) < 0) { - /* - * conn->errorMessage has been set by pqWait or pqReadData. We - * want to append it to any already-received error message. - */ + /* Report the error saved by pqWait or pqReadData */ pqSaveErrorResult(conn); conn->asyncStatus = PGASYNC_IDLE; return pqPrepareAsyncResult(conn); @@ -2053,7 +2141,7 @@ PQgetResult(PGconn *conn) * is the start of the results of the next query, clear any * prior error message. */ - resetPQExpBuffer(&conn->errorMessage); + pqClearConnErrorState(conn); pqPipelineProcessQueue(conn); } break; @@ -2117,7 +2205,9 @@ PQgetResult(PGconn *conn) appendPQExpBuffer(&conn->errorMessage, libpq_gettext("unexpected asyncStatus: %d\n"), (int) conn->asyncStatus); - res = PQmakeEmptyPGresult(conn, PGRES_FATAL_ERROR); + pqSaveErrorResult(conn); + conn->asyncStatus = PGASYNC_IDLE; /* try to restore valid state */ + res = pqPrepareAsyncResult(conn); break; } @@ -2268,9 +2358,9 @@ PQexecStart(PGconn *conn) } /* - * Since this is the beginning of a query cycle, reset the error buffer. + * Since this is the beginning of a query cycle, reset the error state. */ - resetPQExpBuffer(&conn->errorMessage); + pqClearConnErrorState(conn); /* * Silently discard any prior query result that application didn't eat. @@ -2825,9 +2915,9 @@ PQfn(PGconn *conn, return NULL; /* - * Since this is the beginning of a query cycle, reset the error buffer. + * Since this is the beginning of a query cycle, reset the error state. */ - resetPQExpBuffer(&conn->errorMessage); + pqClearConnErrorState(conn); if (conn->pipelineStatus != PQ_PIPELINE_OFF) { @@ -2837,7 +2927,7 @@ PQfn(PGconn *conn, } if (conn->sock == PGINVALID_SOCKET || conn->asyncStatus != PGASYNC_IDLE || - conn->result != NULL) + conn->result || conn->error_result) { appendPQExpBufferStr(&conn->errorMessage, libpq_gettext("connection in wrong state\n")); @@ -3707,9 +3797,9 @@ PQsetnonblocking(PGconn *conn, int arg) * behavior. this is ok because either they are making a transition _from_ * or _to_ blocking mode, either way we can block them. * - * Clear errorMessage in case pqFlush adds to it. + * Clear error state in case pqFlush adds to it. */ - resetPQExpBuffer(&conn->errorMessage); + pqClearConnErrorState(conn); /* if we are going from blocking to non-blocking flush here */ if (pqFlush(conn)) @@ -3901,7 +3991,7 @@ PQescapeStringConn(PGconn *conn, return 0; } - resetPQExpBuffer(&conn->errorMessage); + pqClearConnErrorState(conn); return PQescapeStringInternal(conn, to, from, length, error, conn->client_encoding, @@ -3939,7 +4029,7 @@ PQescapeInternal(PGconn *conn, const char *str, size_t len, bool as_ident) if (!conn) return NULL; - resetPQExpBuffer(&conn->errorMessage); + pqClearConnErrorState(conn); /* Scan the string for characters that must be escaped. */ for (s = str; (s - str) < len && *s != '\0'; ++s) @@ -4204,7 +4294,7 @@ PQescapeByteaConn(PGconn *conn, if (!conn) return NULL; - resetPQExpBuffer(&conn->errorMessage); + pqClearConnErrorState(conn); return PQescapeByteaInternal(conn, from, from_length, to_length, conn->std_strings, diff --git a/src/interfaces/libpq/fe-lobj.c b/src/interfaces/libpq/fe-lobj.c index 48399a90cb..075a5ed85b 100644 --- a/src/interfaces/libpq/fe-lobj.c +++ b/src/interfaces/libpq/fe-lobj.c @@ -665,8 +665,8 @@ lo_import_internal(PGconn *conn, const char *filename, Oid oid) if (conn == NULL) return InvalidOid; - /* Since this is the beginning of a query cycle, reset the error buffer */ - resetPQExpBuffer(&conn->errorMessage); + /* Since this is the beginning of a query cycle, reset the error state */ + pqClearConnErrorState(conn); /* * open the file to be read in @@ -730,7 +730,8 @@ lo_import_internal(PGconn *conn, const char *filename, Oid oid) (void) lo_close(conn, lobj); (void) close(fd); /* deliberately overwrite any error from lo_close */ - printfPQExpBuffer(&conn->errorMessage, + pqClearConnErrorState(conn); + appendPQExpBuffer(&conn->errorMessage, libpq_gettext("could not read from file \"%s\": %s\n"), filename, strerror_r(save_errno, sebuf, sizeof(sebuf))); @@ -785,7 +786,8 @@ lo_export(PGconn *conn, Oid lobjId, const char *filename) (void) lo_close(conn, lobj); /* deliberately overwrite any error from lo_close */ - printfPQExpBuffer(&conn->errorMessage, + pqClearConnErrorState(conn); + appendPQExpBuffer(&conn->errorMessage, libpq_gettext("could not open file \"%s\": %s\n"), filename, strerror_r(save_errno, sebuf, sizeof(sebuf))); @@ -806,7 +808,8 @@ lo_export(PGconn *conn, Oid lobjId, const char *filename) (void) lo_close(conn, lobj); (void) close(fd); /* deliberately overwrite any error from lo_close */ - printfPQExpBuffer(&conn->errorMessage, + pqClearConnErrorState(conn); + appendPQExpBuffer(&conn->errorMessage, libpq_gettext("could not write to file \"%s\": %s\n"), filename, strerror_r(save_errno, sebuf, sizeof(sebuf))); @@ -863,8 +866,8 @@ lo_initialize(PGconn *conn) if (conn == NULL) return -1; - /* Since this is the beginning of a query cycle, reset the error buffer */ - resetPQExpBuffer(&conn->errorMessage); + /* Since this is the beginning of a query cycle, reset the error state */ + pqClearConnErrorState(conn); /* Nothing else to do if we already collected info */ if (conn->lobjfuncs != NULL) diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c index 26dbeaed97..dc12a8b1f5 100644 --- a/src/interfaces/libpq/fe-protocol3.c +++ b/src/interfaces/libpq/fe-protocol3.c @@ -316,8 +316,9 @@ pqParseInput3(PGconn *conn) return; break; case 'T': /* Row Description */ - if (conn->result != NULL && - conn->result->resultStatus == PGRES_FATAL_ERROR) + if (conn->error_result || + (conn->result != NULL && + conn->result->resultStatus == PGRES_FATAL_ERROR)) { /* * We've already choked for some reason. Just discard @@ -387,8 +388,9 @@ pqParseInput3(PGconn *conn) if (getAnotherTuple(conn, msgLength)) return; } - else if (conn->result != NULL && - conn->result->resultStatus == PGRES_FATAL_ERROR) + else if (conn->error_result || + (conn->result != NULL && + conn->result->resultStatus == PGRES_FATAL_ERROR)) { /* * We've already choked for some reason. Just discard @@ -966,10 +968,18 @@ pqGetErrorNotice3(PGconn *conn, bool isError) */ if (isError) { - if (res) - pqSetResultError(res, &workBuf); pqClearAsyncResult(conn); /* redundant, but be safe */ - conn->result = res; + if (res) + { + pqSetResultError(res, &workBuf, 0); + conn->result = res; + } + else + { + /* Fall back to using the internal-error processing paths */ + conn->error_result = true; + } + if (PQExpBufferDataBroken(workBuf)) appendPQExpBufferStr(&conn->errorMessage, libpq_gettext("out of memory\n")); @@ -2116,10 +2126,33 @@ pqFunctionCall3(PGconn *conn, Oid fnid, continue; /* consume the message and exit */ conn->inStart += 5 + msgLength; - /* if we saved a result object (probably an error), use it */ - if (conn->result) - return pqPrepareAsyncResult(conn); - return PQmakeEmptyPGresult(conn, status); + + /* + * If we already have a result object (probably an error), use + * that. Otherwise, if we saw a function result message, + * report COMMAND_OK. Otherwise, the backend violated the + * protocol, so complain. + */ + if (!(conn->result || conn->error_result)) + { + if (status == PGRES_COMMAND_OK) + { + conn->result = PQmakeEmptyPGresult(conn, status); + if (!conn->result) + { + appendPQExpBufferStr(&conn->errorMessage, + libpq_gettext("out of memory\n")); + pqSaveErrorResult(conn); + } + } + else + { + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("protocol error: no function result\n")); + pqSaveErrorResult(conn); + } + } + return pqPrepareAsyncResult(conn); case 'S': /* parameter status */ if (getParameterStatus(conn)) continue; diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 4290553482..e0cee4b142 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -496,8 +496,17 @@ struct pg_conn PGdataValue *rowBuf; /* array for passing values to rowProcessor */ int rowBufLen; /* number of entries allocated in rowBuf */ - /* Status for asynchronous result construction */ + /* + * Status for asynchronous result construction. If result isn't NULL, it + * is a result being constructed or ready to return. If result is NULL + * and error_result is true, then we need to return a PGRES_FATAL_ERROR + * result, but haven't yet constructed it; text for the error has been + * appended to conn->errorMessage. (Delaying construction simplifies + * dealing with out-of-memory cases.) If next_result isn't NULL, it is a + * PGresult that will replace "result" after we return that one. + */ PGresult *result; /* result being constructed */ + bool error_result; /* do we need to make an ERROR result? */ PGresult *next_result; /* next result (used in single-row mode) */ /* Assorted state for SASL, SSL, GSS, etc */ @@ -567,8 +576,14 @@ struct pg_conn * Buffer for current error message. This is cleared at the start of any * connection attempt or query cycle; after that, all code should append * messages to it, never overwrite. + * + * In some situations we might report an error more than once in a query + * cycle. If so, errorMessage accumulates text from all the errors, and + * errorReported tracks how much we've already reported, so that the + * individual error PGresult objects don't contain duplicative text. */ PQExpBufferData errorMessage; /* expansible string */ + int errorReported; /* # bytes of string already reported */ /* Buffer for receiving various parts of messages */ PQExpBufferData workBuffer; /* expansible string */ @@ -644,7 +659,7 @@ extern pgthreadlock_t pg_g_threadlock; /* === in fe-exec.c === */ -extern void pqSetResultError(PGresult *res, PQExpBuffer errorMessage); +extern void pqSetResultError(PGresult *res, PQExpBuffer errorMessage, int offset); extern void *pqResultAlloc(PGresult *res, size_t nBytes, bool isBinary); extern char *pqResultStrdup(PGresult *res, const char *str); extern void pqClearAsyncResult(PGconn *conn); @@ -830,6 +845,13 @@ extern void pqTraceOutputNoTypeByteMessage(PGconn *conn, const char *message); /* === miscellaneous macros === */ +/* + * Reset the conn's error-reporting state. + */ +#define pqClearConnErrorState(conn) \ + (resetPQExpBuffer(&(conn)->errorMessage), \ + (conn)->errorReported = 0) + /* * this is so that we can check if a connection is non-blocking internally * without the overhead of a function call