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 51da84a02a7fe04fdafeedeb793fa878b3bef2ce..f34d4a4e90695449922c4972f28f1573197bdc95 100644 (file)
--- 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 9a98e72fd7f7a6eef5b79332ad1d3060cf3a19e9..e4d5307141bca6429e5cb5994ab0f60c0b5c283c 100644 (file)
--- 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;
 }