Move libpq's write_failed mechanism down to pqsecure_raw_write().

Commit 1f39a1c06 implemented write-failure postponement in pqSendSome,
which is above SSL/GSS processing.  However, we've now seen failures
indicating that (some versions of?) OpenSSL have a tendency to report
write failures prematurely too.  Hence, move the primary responsibility
for postponing write failures down to pqsecure_raw_write(), below
SSL/GSS processing.  pqSendSome now sets write_failed only in corner
cases where we'd lost the connection already.

A side-effect of this change is that errors detected in the SSL/GSS
layer itself will be reported immediately (as if they were read
errors) rather than being postponed like write errors.  That's
reverting an effect of 1f39a1c06, and I think it's fine: if there's
not a socket-level error, it's hard to be sure whether an OpenSSL
error ought to be considered a read or write failure anyway.

Another important point is that write-failure postponement is now
effective during connection setup.  OpenSSL's misbehavior of this
sort occurs during SSL_connect(), so that's a change we want.

Per bug #17391 from Nazir Bilal Yavuz.  Possibly this should be
back-patched, but I think it prudent to let it age awhile in HEAD
first.

Discussion: https://postgr.es/m/17391-304f81bcf724b58b@postgresql.org

1 files changed, 20 insertions, 27 deletions

diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 90a312bf2c2..d76bb3957ae 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -777,19 +777,19 @@ definitelyFailed:
  * (putting it in conn->inBuffer) in any situation where we can't send
  * all the specified data immediately.
  *
- * Upon write failure, conn->write_failed is set and the error message is
- * saved in conn->write_err_msg, but we clear the output buffer and return
- * zero anyway; this is because callers should soldier on until it's possible
- * to read from the server and check for an error message.  write_err_msg
- * should be reported only when we are unable to obtain a server error first.
- * (Thus, a -1 result is returned only for an internal *read* failure.)
+ * If a socket-level write failure occurs, conn->write_failed is set and the
+ * error message is saved in conn->write_err_msg, but we clear the output
+ * buffer and return zero anyway; this is because callers should soldier on
+ * until we have read what we can from the server and checked for an error
+ * message.  write_err_msg should be reported only when we are unable to
+ * obtain a server error first.  Much of that behavior is implemented at
+ * lower levels, but this function deals with some edge cases.
  */
 static int
 pqSendSome(PGconn *conn, int len)
 {
 	char	   *ptr = conn->outBuffer;
 	int			remaining = conn->outCount;
-	int			oldmsglen = conn->errorMessage.len;
 	int			result = 0;
 
 	/*
@@ -817,7 +817,7 @@ pqSendSome(PGconn *conn, int len)
 	if (conn->sock == PGINVALID_SOCKET)
 	{
 		conn->write_failed = true;
-		/* Insert error message into conn->write_err_msg, if possible */
+		/* Store error message in conn->write_err_msg, if possible */
 		/* (strdup failure is OK, we'll cope later) */
 		conn->write_err_msg = strdup(libpq_gettext("connection not open\n"));
 		/* Discard queued data; no chance it'll ever be sent */
@@ -859,24 +859,6 @@ pqSendSome(PGconn *conn, int len)
 					continue;
 
 				default:
-					/* pqsecure_write set the error message for us */
-					conn->write_failed = true;
-
-					/*
-					 * Transfer error message to conn->write_err_msg, if
-					 * possible (strdup failure is OK, we'll cope later).
-					 *
-					 * We only want to transfer whatever has been appended to
-					 * conn->errorMessage since we entered this routine.
-					 */
-					if (!PQExpBufferBroken(&conn->errorMessage))
-					{
-						conn->write_err_msg = strdup(conn->errorMessage.data +
-													 oldmsglen);
-						conn->errorMessage.len = oldmsglen;
-						conn->errorMessage.data[oldmsglen] = '\0';
-					}
-
 					/* Discard queued data; no chance it'll ever be sent */
 					conn->outCount = 0;
 
@@ -886,7 +868,18 @@ pqSendSome(PGconn *conn, int len)
 						if (pqReadData(conn) < 0)
 							return -1;
 					}
-					return 0;
+
+					/*
+					 * Lower-level code should already have filled
+					 * conn->write_err_msg (and set conn->write_failed) or
+					 * conn->errorMessage.  In the former case, we pretend
+					 * there's no problem; the write_failed condition will be
+					 * dealt with later.  Otherwise, report the error now.
+					 */
+					if (conn->write_failed)
+						return 0;
+					else
+						return -1;
 			}
 		}
 		else