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, 173 insertions, 97 deletions

diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 51da84a02a7..f34d4a4e906 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 9a98e72fd7f..e4d5307141b 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;
 }