From c0fbe98b7847d240771e8fa0a6270046574f97a0 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 v8 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 85da2a4e8dc..08b15c55526 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -3782,9 +3782,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. */
@@ -4411,6 +4427,20 @@ freePGconn(PGconn *conn)
 	}
 	free(conn->connhost);
 
+	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 f0143726bbc..7ad451c94f9 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

