From bcf23ba4bf8323f875168c5dbc93265a140753e8 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 24 Jul 2011 23:29:03 -0400 Subject: [PATCH] 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. --- src/interfaces/libpq/fe-misc.c | 58 ++------- src/interfaces/libpq/fe-secure.c | 210 +++++++++++++++++++++++-------- 2 files changed, 170 insertions(+), 98 deletions(-) diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c index 02e011e1eb..edf7682e19 100644 --- a/src/interfaces/libpq/fe-misc.c +++ b/src/interfaces/libpq/fe-misc.c @@ -578,7 +578,6 @@ pqReadData(PGconn *conn) { int someread = 0; int nread; - char sebuf[256]; if (conn->sock < 0) { @@ -647,11 +646,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) @@ -711,6 +706,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; } @@ -739,11 +739,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) @@ -754,16 +750,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); @@ -799,7 +789,6 @@ pqSendSome(PGconn *conn, int len) while (len > 0) { int sent; - char sebuf[256]; #ifndef WIN32 sent = pqsecure_write(conn, ptr, len); @@ -815,11 +804,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 @@ -833,17 +818,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 @@ -855,16 +831,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 6e0fbd3a39..9c6ced6a82 100644 --- a/src/interfaces/libpq/fe-secure.c +++ b/src/interfaces/libpq/fe-secure.c @@ -303,15 +303,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) @@ -332,10 +333,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: @@ -351,26 +353,32 @@ rloop: */ goto rloop; case SSL_ERROR_SYSCALL: + if (n < 0) { - char sebuf[256]; - - if (n < 0) - { - REMEMBER_EPIPE(spinfo, SOCK_ERRNO == EPIPE); + result_errno = SOCK_ERRNO; + REMEMBER_EPIPE(spinfo, 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(); @@ -379,14 +387,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: @@ -394,7 +407,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; } @@ -402,24 +415,66 @@ rloop: RESTORE_SIGPIPE(conn, spinfo); } 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]; DECLARE_SIGPIPE_INFO(spinfo); @@ -438,10 +493,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: @@ -457,26 +513,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(spinfo, SOCK_ERRNO == EPIPE); + result_errno = SOCK_ERRNO; + REMEMBER_EPIPE(spinfo, 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(); @@ -485,14 +547,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: @@ -500,13 +567,13 @@ 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 */ { int flags = 0; @@ -523,13 +590,15 @@ retry_masked: if (n < 0) { + result_errno = SOCK_ERRNO; + /* * If we see an EINVAL, it may be because MSG_NOSIGNAL isn't * available on this machine. So, clear sigpipe_flag so we don't * try the flag again, and retry the send(). */ #ifdef MSG_NOSIGNAL - if (flags != 0 && SOCK_ERRNO == EINVAL) + if (flags != 0 && result_errno == EINVAL) { conn->sigpipe_flag = false; flags = 0; @@ -537,12 +606,49 @@ retry_masked: } #endif /* MSG_NOSIGNAL */ - REMEMBER_EPIPE(spinfo, SOCK_ERRNO == EPIPE); + /* 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(spinfo, 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(conn, spinfo); + /* ensure we return the intended errno to caller */ + SOCK_ERRNO_SET(result_errno); + return n; }