v10-0002-libpq-Handle-NegotiateProtocolVersion-message-mo.patch

From aa40b8dc8cc9b45b62069d6735ab9763ea8ce0f3 Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio <jelte.fennema@microsoft.com>
Date: Tue, 2 Jan 2024 11:16:19 +0100
Subject: [PATCH v10 02/13] libpq: Handle NegotiateProtocolVersion message more
 leniently

Currently libpq would always error when the server returned a
NegotiateProtocolVersion message. This was fine because libpq only
supports a single protocol version and did not support any protocol
extensions. But we now need to change that to be able to add support for
future protocol changes, with a working fallback when connecting to an
older server.

This patch modifies the client side checks to allow a range of supported
protocol versions, instead of only allowing the exact version that was
requested. In addition it now allows connecting when the server does not
support some of the requested protocol extensions.

Note that at the moment this change does not have any behavioural
effect, because libpq will only request version 3.0 and will never send
protocol extension parameters. Which means that the client never
receives a NegotiateProtocolVersion message from the server.
---
 src/interfaces/libpq/fe-connect.c   | 32 +++++++++++++++++++-
 src/interfaces/libpq/fe-protocol3.c | 46 ++++++++++-------------------
 src/interfaces/libpq/libpq-int.h    |  2 ++
 3 files changed, 48 insertions(+), 32 deletions(-)

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 887fd0bf705..3ff4403858b 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -3729,9 +3729,25 @@ keep_going:						/* We will come back to here until there is
 						libpq_append_conn_error(conn, "received invalid protocol negotiation message");
 						goto error_return;
 					}
+
+					if (conn->pversion < PG_PROTOCOL_EARLIEST)
+					{
+						libpq_append_conn_error(conn, "protocol version not supported by server: client supports down to %u.%u, server supports up to %u.%u",
+												PG_PROTOCOL_MAJOR(PG_PROTOCOL_EARLIEST), PG_PROTOCOL_MINOR(PG_PROTOCOL_EARLIEST),
+												PG_PROTOCOL_MAJOR(conn->pversion), PG_PROTOCOL_MINOR(conn->pversion));
+						goto error_return;
+					}
+
+					/* neither -- server shouldn't have sent it */
+					if (!(conn->pversion < PG_PROTOCOL_LATEST) && !conn->unsupported_pextension_params)
+					{
+						libpq_append_conn_error(conn, "invalid %s message", "NegotiateProtocolVersion");
+						goto error_return;
+					}
+
 					/* OK, we read the message; mark data consumed */
 					conn->inStart = conn->inCursor;
-					goto error_return;
+					goto keep_going;
 				}
 
 				/* It is an authentication request. */
@@ -4346,6 +4362,20 @@ freePGconn(PGconn *conn)
 
 	pqReleaseConnHosts(conn);
 
+	if (conn->unsupported_pextension_params)
+	{
+		/* clean up unsupported_pextension_params entries */
+		int			i = 0;
+
+		while (conn->unsupported_pextension_params[i])
+		{
+			free(conn->unsupported_pextension_params[i]);
+			i++;
+		}
+		free(conn->unsupported_pextension_params);
+	}
+
+
 	free(conn->client_encoding_initial);
 	free(conn->events);
 	free(conn->pghost);
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 701d58e1087..249fa2984f3 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -1410,49 +1410,33 @@ reportErrorPosition(PQExpBuffer msg, const char *query, int loc, int encoding)
 int
 pqGetNegotiateProtocolVersion3(PGconn *conn)
 {
-	int			tmp;
-	ProtocolVersion their_version;
+	int			their_version;
 	int			num;
-	PQExpBufferData buf;
 
-	if (pqGetInt(&tmp, 4, conn) != 0)
+	if (pqGetInt(&their_version, 4, conn) != 0)
 		return EOF;
-	their_version = tmp;
 
 	if (pqGetInt(&num, 4, conn) != 0)
 		return EOF;
 
-	initPQExpBuffer(&buf);
-	for (int i = 0; i < num; i++)
+	conn->pversion = their_version;
+	if (num)
 	{
-		if (pqGets(&conn->workBuffer, conn))
+		conn->unsupported_pextension_params = calloc(num + 1, sizeof(char *));
+		for (int i = 0; i < num; i++)
 		{
-			termPQExpBuffer(&buf);
-			return EOF;
+			if (pqGets(&conn->workBuffer, conn))
+			{
+				return EOF;
+			}
+			conn->unsupported_pextension_params[i] = strdup(conn->workBuffer.data);
+			if (!conn->unsupported_pextension_params[i])
+			{
+				return EOF;
+			}
 		}
-		if (buf.len > 0)
-			appendPQExpBufferChar(&buf, ' ');
-		appendPQExpBufferStr(&buf, conn->workBuffer.data);
 	}
 
-	if (their_version < conn->pversion)
-		libpq_append_conn_error(conn, "protocol version not supported by server: client uses %u.%u, server supports up to %u.%u",
-								PG_PROTOCOL_MAJOR(conn->pversion), PG_PROTOCOL_MINOR(conn->pversion),
-								PG_PROTOCOL_MAJOR(their_version), PG_PROTOCOL_MINOR(their_version));
-	if (num > 0)
-	{
-		appendPQExpBuffer(&conn->errorMessage,
-						  libpq_ngettext("protocol extension not supported by server: %s",
-										 "protocol extensions not supported by server: %s", num),
-						  buf.data);
-		appendPQExpBufferChar(&conn->errorMessage, '\n');
-	}
-
-	/* neither -- server shouldn't have sent it */
-	if (!(their_version < conn->pversion) && !(num > 0))
-		libpq_append_conn_error(conn, "invalid %s message", "NegotiateProtocolVersion");
-
-	termPQExpBuffer(&buf);
 	return 0;
 }
 
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 82c18f870d2..16f65c66c9c 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -461,6 +461,8 @@ struct pg_conn
 	SockAddr	laddr;			/* Local address */
 	SockAddr	raddr;			/* Remote address */
 	ProtocolVersion pversion;	/* FE/BE protocol version in use */
+	char	  **unsupported_pextension_params;	/* Unsupported protocol
+											 * extensions, null-terminated */
 	int			sversion;		/* server version, e.g. 70401 for 7.4.1 */
 	bool		auth_req_received;	/* true if any type of auth req received */
 	bool		password_needed;	/* true if server demanded a password */
-- 
2.34.1