v23-0002-Return-2-from-pqReadData-on-EOF.patch

From d54af2f66144752eac72026d5b3420b3ac48b4fe Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio <jelte.fennema@microsoft.com>
Date: Thu, 14 Dec 2023 13:38:55 +0100
Subject: [PATCH v23 2/4] Return -2 from pqReadData on EOF

This patch changes pqReadData to return -2 when a connection is cleanly
closed by the other side. For most of the Postgres protocol this is
considered an error, because the client is expected to close the
connection, not the server. But for Postgres its cancellation protocol
the distinction between errors and clean connection closure is
important, because clean connection closure is the way for the server to
signal that the cancellation was handled.

This patch is in preparation for a follow-up patch where pqReadData is
used for the cancellation protocol implementation.

No existing callsites of pqReadData or any of its internal functions
need to be updated as all of them check if the result is less than 0
instead a strict comparison against -1.
---
 src/interfaces/libpq/fe-misc.c           | 15 +++++++++++----
 src/interfaces/libpq/fe-secure-openssl.c |  2 +-
 src/interfaces/libpq/fe-secure.c         |  6 ++++++
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 660cdec93c9..2d49188d910 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -556,8 +556,11 @@ pqPutMsgEnd(PGconn *conn)
  * Possible return values:
  *	 1: successfully loaded at least one more byte
  *	 0: no data is presently available, but no error detected
- *	-1: error detected (including EOF = connection closure);
+ *	-1: error detected (excluding EOF = clean connection closure);
  *		conn->errorMessage set
+ *	-2: EOF detected, connection is closed cleanly by other side;
+ *		conn->errorMessage set
+ *
  * NOTE: callers must not assume that pointers or indexes into conn->inBuffer
  * remain valid across this call!
  * ----------
@@ -639,7 +642,7 @@ retry3:
 
 			default:
 				/* pqsecure_read set the error message for us */
-				return -1;
+				return nread;
 		}
 	}
 	if (nread > 0)
@@ -734,7 +737,7 @@ retry4:
 
 			default:
 				/* pqsecure_read set the error message for us */
-				return -1;
+				return nread;
 		}
 	}
 	if (nread > 0)
@@ -751,13 +754,17 @@ definitelyEOF:
 	libpq_append_conn_error(conn, "server closed the connection unexpectedly\n"
 							"\tThis probably means the server terminated abnormally\n"
 							"\tbefore or while processing the request.");
+	/* Do *not* drop any already-read data; caller still wants it */
+	pqDropConnection(conn, false);
+	conn->status = CONNECTION_BAD;	/* No more connection to backend */
+	return -2;
 
 	/* Come here if lower-level code already set a suitable errorMessage */
 definitelyFailed:
 	/* Do *not* drop any already-read data; caller still wants it */
 	pqDropConnection(conn, false);
 	conn->status = CONNECTION_BAD;	/* No more connection to backend */
-	return -1;
+	return nread < 0 ? nread : -1;
 }
 
 /*
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 2b221e7d151..9c1f6646a31 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -241,7 +241,7 @@ rloop:
 			 */
 			libpq_append_conn_error(conn, "SSL connection has been closed unexpectedly");
 			result_errno = ECONNRESET;
-			n = -1;
+			n = -2;
 			break;
 		default:
 			libpq_append_conn_error(conn, "unrecognized SSL error code: %d", err);
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index b2430362a90..ab04a3ea34f 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -177,6 +177,12 @@ pqsecure_close(PGconn *conn)
  * On failure, this function is responsible for appending a suitable message
  * to conn->errorMessage.  The caller must still inspect errno, but only
  * to determine whether to continue/retry after error.
+ *
+ * Returns -1 in case of failures, except in the case of where a failure means
+ * that there was a clean connection closure, in those cases -2 is returned.
+ * Currently only the TLS implementation of pqsecure_read ever returns -2. For
+ * the other implementations a clean connection closure is detected in
+ * pqReadData instead.
  */
 ssize_t
 pqsecure_read(PGconn *conn, void *ptr, size_t len)
-- 
2.34.1