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.

2 files changed, 170 insertions, 98 deletions

diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 02e011e1eb0..edf7682e192 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 6e0fbd3a390..9c6ced6a828 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;
 }