From 2bb28d8c3664d5649ba3e5bd72988a9dff6fbcd8 Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio <postgres@jeltef.nl>
Date: Wed, 14 Aug 2024 18:25:16 +0200
Subject: [PATCH v15 6/9] libpq: Handle NegotiateProtocolVersion message
 differently

Previously libpq would always error when the server returns a
NegotiateProtocolVersion message. This was fine because libpq only
supported a single protocol version and did not support any protocol
parameters. But we now that we're discussing protocol changes we need to
change this behaviour, and introduce a fallback mechanism 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. Currently this "range" only contains the 3.0 version, but in
a future commit we'll change this. In addition it changes the errors for
protocol to say that they error because we did not request any
parameters, not because the server did not support some of them. In a
future commit more infrastructure for protocol parameters will be added,
so that these checks can also succeed when receiving unsupported
parameters back.

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 parameters. Which means that the client never receives a
NegotiateProtocolVersion message from the server.
---
 src/interfaces/libpq/fe-connect.c   |  6 ++-
 src/interfaces/libpq/fe-protocol3.c | 65 +++++++++++++++++------------
 2 files changed, 43 insertions(+), 28 deletions(-)

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 73891a125c..c30f1a5dfb 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -3918,9 +3918,13 @@ 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->error_result)
+						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. */
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 3170d484f0..daf544a791 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -1411,49 +1411,60 @@ 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);
+	if (their_version > conn->pversion)
+	{
+		libpq_append_conn_error(conn, "received invalid protocol negotiation message: server requests downgrade to higher-numbered version");
+		goto failure;
+	}
+
+	if (their_version < PG_PROTOCOL(3, 0))
+	{
+		libpq_append_conn_error(conn, "received invalid protocol negotiation message: server requests downgrade to pre-3.0 protocol version");
+		goto failure;
+	}
+	
+	if (num < 0)
+	{
+		libpq_append_conn_error(conn, "received invalid protocol negotiation message: server reported negative number of unsupported parameters");
+		goto failure;
+	}
+
+	if (their_version == conn->pversion && num == 0)
+	{
+		libpq_append_conn_error(conn, "received invalid protocol negotiation message: server negotiated but asks for no changes");
+		goto failure;
+	}
+
+	conn->pversion = their_version;
 	for (int i = 0; i < num; i++)
 	{
 		if (pqGets(&conn->workBuffer, conn))
 		{
-			termPQExpBuffer(&buf);
 			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');
+		if (strncmp(conn->workBuffer.data, "_pq_.", 5) != 0)
+		{
+			libpq_append_conn_error(conn, "received invalid protocol negotiation message: server reported unsupported parameter name without a _pq_. prefix (\"%s\")", conn->workBuffer.data);
+			goto failure;
+		}
+		libpq_append_conn_error(conn, "received invalid protocol negotiation message: server reported an unsupported parameter that was not requested (\"%s\")", conn->workBuffer.data);
+		goto failure;
 	}
 
-	/* neither -- server shouldn't have sent it */
-	if (!(their_version < conn->pversion) && !(num > 0))
-		libpq_append_conn_error(conn, "invalid %s message", "NegotiateProtocolVersion");
+	return 0;
 
-	termPQExpBuffer(&buf);
+failure:
+	conn->asyncStatus = PGASYNC_READY;
+	pqSaveErrorResult(conn);
 	return 0;
 }
 
-- 
2.34.1

