From: Tom Lane Date: Mon, 25 Jul 2011 03:29:27 +0000 (-0400) Subject: Fix previous patch so it also works if not USE_SSL (mea culpa). X-Git-Tag: REL8_3_16~24 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ee27058ac788585c8c5f91e1f05a927cd81e20b2;p=postgresql Fix previous patch so it also works if not USE_SSL (mea culpa). On balance, the need to cover this case changes my mind in favor of pushing all error-message generation duties into the two fe-secure.c routines. So do it that way. --- diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c index 9d67623e4a..45c4000d62 100644 --- a/src/interfaces/libpq/fe-misc.c +++ b/src/interfaces/libpq/fe-misc.c @@ -556,7 +556,6 @@ pqReadData(PGconn *conn) { int someread = 0; int nread; - char sebuf[256]; if (conn->sock < 0) { @@ -625,11 +624,7 @@ retry3: if (SOCK_ERRNO == ECONNRESET) goto definitelyFailed; #endif - /* in SSL mode, pqsecure_read set the error message */ - if (conn->ssl == NULL) - printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not receive data from server: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + /* pqsecure_read set the error message for us */ return -1; } if (nread > 0) @@ -689,6 +684,11 @@ retry3: /* ready for read */ break; default: + 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")); goto definitelyFailed; } @@ -717,11 +717,7 @@ retry4: if (SOCK_ERRNO == ECONNRESET) goto definitelyFailed; #endif - /* in SSL mode, pqsecure_read set the error message */ - if (conn->ssl == NULL) - printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not receive data from server: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + /* pqsecure_read set the error message for us */ return -1; } if (nread > 0) @@ -732,16 +728,10 @@ retry4: /* * OK, we are getting a zero read even though select() says ready. This - * means the connection has been closed. Cope. + * means the connection has been closed. Cope. Note that errorMessage + * has been set already. */ definitelyFailed: - /* in SSL mode, pqsecure_read set the error message */ - if (conn->ssl == NULL) - 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")); conn->status = CONNECTION_BAD; /* No more connection to backend */ pqsecure_close(conn); closesocket(conn->sock); @@ -777,7 +767,6 @@ pqSendSome(PGconn *conn, int len) while (len > 0) { int sent; - char sebuf[256]; #ifndef WIN32 sent = pqsecure_write(conn, ptr, len); @@ -792,11 +781,7 @@ pqSendSome(PGconn *conn, int len) if (sent < 0) { - /* - * Anything except EAGAIN/EWOULDBLOCK/EINTR is trouble. If it's - * EPIPE or ECONNRESET, assume we've lost the backend connection - * permanently. - */ + /* Anything except EAGAIN/EWOULDBLOCK/EINTR is trouble */ switch (SOCK_ERRNO) { #ifdef EAGAIN @@ -810,17 +795,8 @@ pqSendSome(PGconn *conn, int len) case EINTR: continue; - case EPIPE: -#ifdef ECONNRESET - case ECONNRESET: -#endif - /* in SSL mode, pqsecure_write set the error message */ - if (conn->ssl == NULL) - 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")); + default: + /* pqsecure_write set the error message for us */ /* * We used to close the socket here, but that's a bad idea @@ -832,16 +808,6 @@ pqSendSome(PGconn *conn, int len) */ conn->outCount = 0; return -1; - - default: - /* in SSL mode, pqsecure_write set the error message */ - if (conn->ssl == NULL) - printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not send data to server: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); - /* We don't assume it's a fatal error... */ - conn->outCount = 0; - return -1; } } else diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c index fdc36027e7..4a05da0221 100644 --- a/src/interfaces/libpq/fe-secure.c +++ b/src/interfaces/libpq/fe-secure.c @@ -302,15 +302,16 @@ pqsecure_close(PGconn *conn) /* * Read data from a secure connection. * - * If SSL is in use, this function is responsible for putting a suitable - * message into conn->errorMessage upon error; but the caller does that - * when not using SSL. In either case, caller uses the returned errno - * to decide whether to continue/retry after error. + * On failure, this function is responsible for putting a suitable message + * into conn->errorMessage. The caller must still inspect errno, but only + * to determine whether to continue/retry after error. */ ssize_t pqsecure_read(PGconn *conn, void *ptr, size_t len) { ssize_t n; + int result_errno = 0; + char sebuf[256]; #ifdef USE_SSL if (conn->ssl) @@ -329,10 +330,11 @@ rloop: case SSL_ERROR_NONE: if (n < 0) { + /* Not supposed to happen, so we don't translate the msg */ printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("SSL_read failed but did not provide error information\n")); + "SSL_read failed but did not provide error information\n"); /* assume the connection is broken */ - SOCK_ERRNO_SET(ECONNRESET); + result_errno = ECONNRESET; } break; case SSL_ERROR_WANT_READ: @@ -348,26 +350,32 @@ rloop: */ goto rloop; case SSL_ERROR_SYSCALL: + if (n < 0) { - char sebuf[256]; - - if (n < 0) - { - REMEMBER_EPIPE(SOCK_ERRNO == EPIPE); + result_errno = SOCK_ERRNO; + REMEMBER_EPIPE(result_errno == EPIPE); + if (result_errno == EPIPE || + result_errno == ECONNRESET) printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("SSL SYSCALL error: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); - } + 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, - libpq_gettext("SSL SYSCALL error: EOF detected\n")); - /* assume the connection is broken */ - SOCK_ERRNO_SET(ECONNRESET); - n = -1; - } - break; + 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")); + /* assume the connection is broken */ + result_errno = ECONNRESET; + n = -1; } + break; case SSL_ERROR_SSL: { char *errm = SSLerrmessage(); @@ -376,14 +384,19 @@ rloop: libpq_gettext("SSL error: %s\n"), errm); SSLerrfree(errm); /* assume the connection is broken */ - SOCK_ERRNO_SET(ECONNRESET); + result_errno = ECONNRESET; n = -1; break; } case SSL_ERROR_ZERO_RETURN: + /* + * Per OpenSSL documentation, this error code is only returned + * for 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")); - SOCK_ERRNO_SET(ECONNRESET); + result_errno = ECONNRESET; n = -1; break; default: @@ -391,7 +404,7 @@ rloop: libpq_gettext("unrecognized SSL error code: %d\n"), err); /* assume the connection is broken */ - SOCK_ERRNO_SET(ECONNRESET); + result_errno = ECONNRESET; n = -1; break; } @@ -399,24 +412,66 @@ rloop: RESTORE_SIGPIPE(); } else -#endif +#endif /* USE_SSL */ + { n = recv(conn->sock, ptr, len, 0); + if (n < 0) + { + result_errno = SOCK_ERRNO; + + /* Set error message if appropriate */ + switch (result_errno) + { +#ifdef EAGAIN + case EAGAIN: +#endif +#if defined(EWOULDBLOCK) && (!defined(EAGAIN) || (EWOULDBLOCK != EAGAIN)) + case EWOULDBLOCK: +#endif + case EINTR: + /* no error message, caller is expected to retry */ + break; + +#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")); + break; +#endif + + default: + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("could not receive data from server: %s\n"), + SOCK_STRERROR(result_errno, + sebuf, sizeof(sebuf))); + break; + } + } + } + + /* ensure we return the intended errno to caller */ + SOCK_ERRNO_SET(result_errno); + return n; } /* * Write data to a secure connection. * - * If SSL is in use, this function is responsible for putting a suitable - * message into conn->errorMessage upon error; but the caller does that - * when not using SSL. In either case, caller uses the returned errno - * to decide whether to continue/retry after error. + * On failure, this function is responsible for putting a suitable message + * into conn->errorMessage. The caller must still inspect errno, but only + * to determine whether to continue/retry after error. */ ssize_t pqsecure_write(PGconn *conn, const void *ptr, size_t len) { ssize_t n; + int result_errno = 0; + char sebuf[256]; DISABLE_SIGPIPE(return -1); @@ -433,10 +488,11 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len) case SSL_ERROR_NONE: if (n < 0) { + /* Not supposed to happen, so we don't translate the msg */ printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("SSL_write failed but did not provide error information\n")); + "SSL_write failed but did not provide error information\n"); /* assume the connection is broken */ - SOCK_ERRNO_SET(ECONNRESET); + result_errno = ECONNRESET; } break; case SSL_ERROR_WANT_READ: @@ -452,26 +508,32 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len) n = 0; break; case SSL_ERROR_SYSCALL: + if (n < 0) { - char sebuf[256]; - - if (n < 0) - { - REMEMBER_EPIPE(SOCK_ERRNO == EPIPE); + result_errno = SOCK_ERRNO; + REMEMBER_EPIPE(result_errno == EPIPE); + if (result_errno == EPIPE || + result_errno == ECONNRESET) printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("SSL SYSCALL error: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); - } + 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, - libpq_gettext("SSL SYSCALL error: EOF detected\n")); - /* assume the connection is broken */ - SOCK_ERRNO_SET(ECONNRESET); - n = -1; - } - break; + 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")); + /* assume the connection is broken */ + result_errno = ECONNRESET; + n = -1; } + break; case SSL_ERROR_SSL: { char *errm = SSLerrmessage(); @@ -480,14 +542,19 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len) libpq_gettext("SSL error: %s\n"), errm); SSLerrfree(errm); /* assume the connection is broken */ - SOCK_ERRNO_SET(ECONNRESET); + result_errno = ECONNRESET; n = -1; break; } case SSL_ERROR_ZERO_RETURN: + /* + * Per OpenSSL documentation, this error code is only returned + * for 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")); - SOCK_ERRNO_SET(ECONNRESET); + result_errno = ECONNRESET; n = -1; break; default: @@ -495,20 +562,63 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len) libpq_gettext("unrecognized SSL error code: %d\n"), err); /* assume the connection is broken */ - SOCK_ERRNO_SET(ECONNRESET); + result_errno = ECONNRESET; n = -1; break; } } else -#endif +#endif /* USE_SSL */ { n = send(conn->sock, ptr, len, 0); - REMEMBER_EPIPE(n < 0 && SOCK_ERRNO == EPIPE); + + if (n < 0) + { + result_errno = SOCK_ERRNO; + + /* Set error message if appropriate */ + switch (result_errno) + { +#ifdef EAGAIN + case EAGAIN: +#endif +#if defined(EWOULDBLOCK) && (!defined(EAGAIN) || (EWOULDBLOCK != EAGAIN)) + case EWOULDBLOCK: +#endif + case EINTR: + /* no error message, caller is expected to retry */ + break; + + case EPIPE: + /* Set flag for EPIPE */ + REMEMBER_EPIPE(true); + /* FALL THRU */ + +#ifdef ECONNRESET + 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")); + break; + + default: + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("could not send data to server: %s\n"), + SOCK_STRERROR(result_errno, + sebuf, sizeof(sebuf))); + break; + } + } } RESTORE_SIGPIPE(); + /* ensure we return the intended errno to caller */ + SOCK_ERRNO_SET(result_errno); + return n; }