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 02e011e1eb0ea46312402841f6a3034e36b50703..edf7682e192e99ea6a56d61e8d46af2bcdc0d95e 100644 (file)
--- 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 6e0fbd3a390a9bff9abc0c088567ff360d200f76..9c6ced6a8280130aaac2b680691430fd92490953 100644 (file)
--- 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;
 }