From 16976b026a80567695e68a52592a73a75dc391ed Mon Sep 17 00:00:00 2001 From: Tom Lane <tgl@sss.pgh.pa.us> Date: Sun, 24 Jul 2011 23:29:21 -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 | 212 +++++++++++++++++++++++-------- 2 files changed, 173 insertions(+), 97 deletions(-) diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c index 51da84a02a..f34d4a4e90 100644 --- a/src/interfaces/libpq/fe-misc.c +++ b/src/interfaces/libpq/fe-misc.c @@ -570,7 +570,6 @@ pqReadData(PGconn *conn) { int someread = 0; int nread; - char sebuf[256]; if (conn->sock < 0) { @@ -639,11 +638,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) @@ -703,6 +698,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; } @@ -731,11 +731,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) @@ -746,16 +742,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); @@ -791,7 +781,6 @@ pqSendSome(PGconn *conn, int len) while (len > 0) { int sent; - char sebuf[256]; #ifndef WIN32 sent = pqsecure_write(conn, ptr, len); @@ -807,11 +796,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 @@ -825,17 +810,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 @@ -847,16 +823,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 9a98e72fd7..e4d5307141 100644 --- a/src/interfaces/libpq/fe-secure.c +++ b/src/interfaces/libpq/fe-secure.c @@ -274,15 +274,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) @@ -301,10 +302,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: @@ -320,26 +322,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(); @@ -348,14 +356,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: @@ -363,7 +376,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; } @@ -371,24 +384,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); @@ -405,10 +460,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: @@ -424,26 +480,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(); @@ -452,14 +514,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: @@ -467,20 +534,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; }