0004-Bump-protocol-version-to-3.2.patch

From 019c03e1c218e85b42656e881d3fd904c327e413 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Tue, 1 Apr 2025 16:24:41 +0300
Subject: [PATCH 4/7] Bump protocol version to 3.2

In preparation of new additions to the protocol in the next commit,
this bumps the minor protocol version number.

Instead of bumping the version number to 3.1, which would be the next
minor version, we skip that one and bump straight to 3.2. The reason
for this is that many PgBouncer releases have, due to an off-by-one
bug, reported 3.1 as supported. These versions would interpret 3.1 as
equivalent to 3.0. So if we would now add extra messages to the 3.1
protocol, clients would succeed to connect to PgBouncer, but would
then cause connection failures when sending such new messages.

So instead of bumping to 3.1, we bump the protocol version to 3.2, for
which these buggy PgBouncer releases will correctly close the connection
at the startup packet. It's a bit strange to skip a version number due
to a bug in a third-party application, but PgBouncer is used widely
enough that it seems worth it to not cause user confusion when
connecting to recent versions of it. Especially since skipping a single
minor version number in the protocol versioning doesn't really cost us
anything. So, while this is not the most theoretically sound decission,
it is the most pragmatic one.

Author: Jelte Fennema-Nio <postgres@jeltef.nl>
Discussion: https://www.postgresql.org/message-id/CAGECzQTfc_O%2BHXqAo5_-xG4r3EFVsTefUeQzSvhEyyLDba-O9w%40mail.gmail.com
Discussion: https://www.postgresql.org/message-id/CAGECzQRbAGqJnnJJxTdKewTsNOovUt4bsx3NFfofz3m2j-t7tA@mail.gmail.com
Discussion: https://github.com/pgbouncer/pgbouncer/pull/1007
---
 doc/src/sgml/libpq.sgml                       |  12 +-
 doc/src/sgml/protocol.sgml                    |  31 ++++-
 src/include/libpq/pqcomm.h                    |   3 +-
 src/interfaces/libpq/fe-connect.c             |  10 ++
 src/interfaces/libpq/fe-protocol3.c           |   7 ++
 .../modules/libpq_pipeline/libpq_pipeline.c   | 110 +++++++++++++++---
 6 files changed, 145 insertions(+), 28 deletions(-)

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index a880e3acf0a..41e898e7a38 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2158,10 +2158,11 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
 
        <para>
         The current supported values are
-        <literal>3.0</literal>
+        <literal>3.0</literal>,
+        <literal>3.2</literal>,
         and <literal>latest</literal>. The <literal>latest</literal> value is
         equivalent to the latest protocol version that is supported by the used
-        libpq version, which currently is <literal>3.0</literal>.
+        libpq version, which currently is <literal>3.2</literal>.
        </para>
       </listitem>
      </varlistentry>
@@ -2184,10 +2185,11 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
 
        <para>
         The current supported values are
-        <literal>3.0</literal>
+        <literal>3.0</literal>,
+        <literal>3.2</literal>,
         and <literal>latest</literal>. The <literal>latest</literal> value is
-        equivalent to the latest protocol version that is supported by the
-        libpq version used, which is currently <literal>3.0</literal>.
+        equivalent to the latest protocol version that is supported by the used
+        libpq version, which currently is <literal>3.2</literal>.
        </para>
       </listitem>
      </varlistentry>
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 04d8e7d21af..0a60c2e7647 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -18,17 +18,23 @@
  </para>
 
  <para>
-  This document describes version 3.0 of the protocol, implemented in
+  This document describes version 3.2 of the protocol, introduced in
+  <productname>PostgreSQL</productname> version 17. The server is compatible with
+  protocol version 3.0, implemented in
   <productname>PostgreSQL</productname> 7.4 and later.  For descriptions
-  of the earlier protocol versions, see previous releases of the
-  <productname>PostgreSQL</productname> documentation.  A single server
+  of earlier protocol versions, see previous releases of the
+  <productname>PostgreSQL</productname> documentation.
+ </para>
+
+ <para>
+  A single server
   can support multiple protocol versions.  The initial startup-request
   message tells the server which protocol version the client is attempting to
   use.  If the major version requested by the client is not supported by
   the server, the connection will be rejected (for example, this would occur
   if the client requested protocol version 4.0, which does not exist as of
   this writing).  If the minor version requested by the client is not
-  supported by the server (e.g., the client requests version 3.1, but the
+  supported by the server (e.g., the client requests version 3.2, but the
   server supports only 3.0), the server may either reject the connection or
   may respond with a NegotiateProtocolVersion message containing the highest
   minor protocol version which it supports.  The client may then choose either
@@ -36,6 +42,13 @@
   to abort the connection.
  </para>
 
+ <para>
+  The protocol negotiation was introduced in
+  <productname>PostgreSQL</productname> version 9.3.21. Earlier versions would
+  reject the connection if the client requested a minor version that was not
+  supported by the server.
+ </para>
+
   <para>
    In order to serve multiple clients efficiently, the server launches
    a new <quote>backend</quote> process for each client.
@@ -413,8 +426,14 @@
         this message indicates the highest supported minor version.  This
         message will also be sent if the client requested unsupported protocol
         options (i.e., beginning with <literal>_pq_.</literal>) in the
-        startup packet.  This message will be followed by an ErrorResponse or
-        a message indicating the success or failure of authentication.
+        startup packet.
+       </para>
+       <para>
+        After this message, the authentication will continue using the version
+        indicated by the server.  If the client does not support the older
+        version, it should immediately close the connection.  If the server
+        does not send this message, it supports the client's requested
+        protocol version and all the protocol options.
        </para>
       </listitem>
      </varlistentry>
diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h
index 46b37e0e4eb..0aceb7147c7 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -91,11 +91,10 @@ is_unixsock_path(const char *path)
 
 /*
  * The earliest and latest frontend/backend protocol version supported.
- * (Only protocol version 3 is currently supported)
  */
 
 #define PG_PROTOCOL_EARLIEST	PG_PROTOCOL(3,0)
-#define PG_PROTOCOL_LATEST		PG_PROTOCOL(3,0)
+#define PG_PROTOCOL_LATEST		PG_PROTOCOL(3,2)
 
 typedef uint32 ProtocolVersion; /* FE/BE protocol version number */
 
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 8f95f47216c..455a29c6720 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -8225,6 +8225,16 @@ pqParseProtocolVersion(const char *value, ProtocolVersion *result, PGconn *conn,
 		return true;
 	}
 
+	/* 3.1 never existed, we went straight from 3.0 to 3.2 */
+	if (strcmp(value, "3.1") == 0)
+	{
+		conn->status = CONNECTION_BAD;
+		libpq_append_conn_error(conn, "invalid %s value: \"%s\"",
+								context,
+								value);
+		return false;
+	}
+
 	major = strtol(value, &end, 10);
 	if (*end != '.')
 	{
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 5015debca77..7ba49ea4592 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -1432,6 +1432,13 @@ pqGetNegotiateProtocolVersion3(PGconn *conn)
 		goto failure;
 	}
 
+	/* 3.1 never existed, we went straight from 3.0 to 3.2 */
+	if (their_version == PG_PROTOCOL(3, 1))
+	{
+		libpq_append_conn_error(conn, "received invalid protocol negotiation message: server requests downgrade to non-existent 3.1 protocol version");
+		goto failure;
+	}
+
 	if (num < 0)
 	{
 		libpq_append_conn_error(conn, "received invalid protocol negotiation message: server reported negative number of unsupported parameters");
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.c
index ac9ac95135f..92c54758196 100644
--- a/src/test/modules/libpq_pipeline/libpq_pipeline.c
+++ b/src/test/modules/libpq_pipeline/libpq_pipeline.c
@@ -197,24 +197,15 @@ send_cancellable_query_impl(int line, PGconn *conn, PGconn *monitorConn)
 }
 
 /*
- * Create a new connection with the same conninfo as the given one.
+ * Fills keywords and vals, with the same options as the ones in the opts
+ * linked-list. Returns the length of the filled in list.
  */
-static PGconn *
-copy_connection(PGconn *conn)
+static
+int
+copy_connection_options(PQconninfoOption *opts, const char **keywords, const char **vals)
 {
-	PGconn	   *copyConn;
-	PQconninfoOption *opts = PQconninfo(conn);
-	const char **keywords;
-	const char **vals;
-	int			nopts = 1;
 	int			i = 0;
 
-	for (PQconninfoOption *opt = opts; opt->keyword != NULL; ++opt)
-		nopts++;
-
-	keywords = pg_malloc(sizeof(char *) * nopts);
-	vals = pg_malloc(sizeof(char *) * nopts);
-
 	for (PQconninfoOption *opt = opts; opt->keyword != NULL; ++opt)
 	{
 		if (opt->val)
@@ -224,8 +215,28 @@ copy_connection(PGconn *conn)
 			i++;
 		}
 	}
-	keywords[i] = vals[i] = NULL;
+	return i;
+}
+
+/*
+ * Create a new connection with the same conninfo as the given one.
+ */
+static PGconn *
+copy_connection(PGconn *conn)
+{
+	const char **keywords;
+	const char **vals;
+	PGconn	   *copyConn;
+	PQconninfoOption *opts = PQconninfo(conn);
+	int			nopts = 1;		/* 1 for the NULL terminator */
 
+	for (PQconninfoOption *opt = opts; opt->keyword != NULL; ++opt)
+		nopts++;
+
+	keywords = pg_malloc0(sizeof(char *) * nopts);
+	vals = pg_malloc0(sizeof(char *) * nopts);
+
+	copy_connection_options(opts, keywords, vals);
 	copyConn = PQconnectdbParams(keywords, vals, false);
 
 	if (PQstatus(copyConn) != CONNECTION_OK)
@@ -1405,6 +1416,72 @@ test_prepared(PGconn *conn)
 	fprintf(stderr, "ok\n");
 }
 
+static void
+test_protocol_version(PGconn *conn)
+{
+	const char **keywords;
+	const char **vals;
+	int			nopts = 2;		/* NULL terminator + max_protocol_version */
+	PQconninfoOption *opts = PQconninfo(conn);
+	int			protocol_version;
+	int			max_protocol_version_index;
+
+	for (PQconninfoOption *opt = opts; opt->keyword != NULL; ++opt)
+		nopts++;
+
+	keywords = pg_malloc0(sizeof(char *) * nopts);
+	vals = pg_malloc0(sizeof(char *) * nopts);
+
+	max_protocol_version_index = copy_connection_options(opts, keywords, vals);
+
+	keywords[max_protocol_version_index] = "max_protocol_version";
+	vals[max_protocol_version_index] = "3.0";
+
+	conn = PQconnectdbParams(keywords, vals, false);
+
+	if (PQstatus(conn) != CONNECTION_OK)
+		pg_fatal("Connection to database failed: %s",
+				 PQerrorMessage(conn));
+
+	protocol_version = PQfullProtocolVersion(conn);
+	if (protocol_version != 30000)
+		pg_fatal("expected 30000, got %d", protocol_version);
+
+	PQfinish(conn);
+
+	vals[max_protocol_version_index] = "3.1";
+	conn = PQconnectdbParams(keywords, vals, false);
+
+	if (PQstatus(conn) != CONNECTION_BAD)
+		pg_fatal("Connecting with max_protocol_version 3.1 should have failed.");
+
+	PQfinish(conn);
+
+	vals[max_protocol_version_index] = "3.2";
+	conn = PQconnectdbParams(keywords, vals, false);
+
+	if (PQstatus(conn) != CONNECTION_OK)
+		pg_fatal("Connection to database failed: %s",
+				 PQerrorMessage(conn));
+
+	protocol_version = PQfullProtocolVersion(conn);
+	if (protocol_version != 30002)
+		pg_fatal("expected 30002, got %d", protocol_version);
+	PQfinish(conn);
+
+	vals[max_protocol_version_index] = "latest";
+	conn = PQconnectdbParams(keywords, vals, false);
+
+	if (PQstatus(conn) != CONNECTION_OK)
+		pg_fatal("Connection to database failed: %s",
+				 PQerrorMessage(conn));
+
+	protocol_version = PQfullProtocolVersion(conn);
+	if (protocol_version != 30002)
+		pg_fatal("expected 30002, got %d", protocol_version);
+	PQfinish(conn);
+}
+
 /* Notice processor: print notices, and count how many we got */
 static void
 notice_processor(void *arg, const char *message)
@@ -2153,6 +2230,7 @@ print_test_list(void)
 	printf("pipeline_idle\n");
 	printf("pipelined_insert\n");
 	printf("prepared\n");
+	printf("protocol_version\n");
 	printf("simple_pipeline\n");
 	printf("singlerow\n");
 	printf("transaction\n");
@@ -2263,6 +2341,8 @@ main(int argc, char **argv)
 		test_pipelined_insert(conn, numrows);
 	else if (strcmp(testname, "prepared") == 0)
 		test_prepared(conn);
+	else if (strcmp(testname, "protocol_version") == 0)
+		test_protocol_version(conn);
 	else if (strcmp(testname, "simple_pipeline") == 0)
 		test_simple_pipeline(conn);
 	else if (strcmp(testname, "singlerow") == 0)
-- 
2.39.5