On 13.10.22 23:00, Nathan Bossart wrote:
On Thu, Oct 13, 2022 at 10:33:01AM +0200, Peter Eisentraut wrote:
+ if (their_version != conn->pversion)
Shouldn't this be 'their_version < conn->pversion'? If the server supports
a later protocol than what is requested but not all the requested protocol
extensions, I think libpq would still report "protocol version not
supported."
Ok, changed.
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("protocol version
not supported by server: client uses %d.%d, server supports %d.%d\n"),
+
PG_PROTOCOL_MAJOR(conn->pversion), PG_PROTOCOL_MINOR(conn->pversion),
+
PG_PROTOCOL_MAJOR(their_version), PG_PROTOCOL_MINOR(their_version));
Should this match the error in postmaster.c and provide the range of
versions the server supports? The FATAL in postmaster.c is for the major
version, but I believe the same information is relevant when a
NegotiateProtocolVersion message is sent.
ereport(FATAL,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("unsupported frontend protocol %u.%u: server
supports %u.0 to %u.%u",
If you increase the libpq minor protocol version and connect to an older
server, you would get an error like "server supports 3.0 to 3.0", which
is probably a bit confusing. I changed it to "up to 3.0" to convey that
it could be a range.
+ else
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("protocol extension
not supported by server: %s\n"), buf.data);
nitpick: s/extension/extensions
Ok, added proper plural support.
What if neither the protocol version nor the requested extensions are
supported? Right now, I think only the unsupported protocol version is
supported in that case, but presumably we could report both pretty easily.
Ok, I just appended both error messages in that case.
From 723027fe61a6f34acb7d7c0d518b4dbff03af9cd Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 20 Oct 2022 11:18:50 +0200
Subject: [PATCH v2] libpq: Handle NegotiateProtocolVersion message
Before, receiving a NegotiateProtocolVersion message would result in a
confusing error message like
expected authentication request from server, but received v
This adds proper handling of this protocol message and produces an
on-topic error message from it.
Discussion:
https://www.postgresql.org/message-id/flat/f9c7862f-b864-8ef7-a861-c4638c83e209%40enterprisedb.com
---
src/interfaces/libpq/fe-connect.c | 18 +++++++++--
src/interfaces/libpq/fe-protocol3.c | 50 +++++++++++++++++++++++++++++
src/interfaces/libpq/libpq-int.h | 1 +
3 files changed, 66 insertions(+), 3 deletions(-)
diff --git a/src/interfaces/libpq/fe-connect.c
b/src/interfaces/libpq/fe-connect.c
index 746e9b4f1efc..1c0d8243a6ca 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -3246,10 +3246,11 @@ PQconnectPoll(PGconn *conn)
/*
* Validate message type: we expect only an
authentication
- * request or an error here. Anything else
probably means
- * it's not Postgres on the other end at all.
+ * request, NegotiateProtocolVersion, or an
error here.
+ * Anything else probably means it's not
Postgres on the other
+ * end at all.
*/
- if (!(beresp == 'R' || beresp == 'E'))
+ if (!(beresp == 'R' || beresp == 'v' || beresp
== 'E'))
{
appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("expected authentication request from server, but received %c\n"),
@@ -3405,6 +3406,17 @@ PQconnectPoll(PGconn *conn)
goto error_return;
}
+ else if (beresp == 'v')
+ {
+ if
(pqGetNegotiateProtocolVersion3(conn))
+ {
+ /* We'll come back when there
is more data */
+ return PGRES_POLLING_READING;
+ }
+ /* OK, we read the message; mark data
consumed */
+ conn->inStart = conn->inCursor;
+ goto error_return;
+ }
/* It is an authentication request. */
conn->auth_req_received = true;
diff --git a/src/interfaces/libpq/fe-protocol3.c
b/src/interfaces/libpq/fe-protocol3.c
index f001137b7692..7bc81740ab35 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -1397,6 +1397,56 @@ reportErrorPosition(PQExpBuffer msg, const char *query,
int loc, int encoding)
}
+/*
+ * Attempt to read a NegotiateProtocolVersion message.
+ * Entry: 'v' message type and length have already been consumed.
+ * Exit: returns 0 if successfully consumed message.
+ * returns EOF if not enough data.
+ */
+int
+pqGetNegotiateProtocolVersion3(PGconn *conn)
+{
+ int tmp;
+ ProtocolVersion their_version;
+ int num;
+ PQExpBufferData buf;
+
+ initPQExpBuffer(&buf);
+ if (pqGetInt(&tmp, 4, conn) != 0)
+ return EOF;
+ their_version = tmp;
+ if (pqGetInt(&num, 4, conn) != 0)
+ return EOF;
+ for (int i = 0; i < num; i++)
+ {
+ if (pqGets(&conn->workBuffer, conn))
+ return EOF;
+ if (buf.len > 0)
+ appendPQExpBufferChar(&buf, ' ');
+ appendPQExpBufferStr(&buf, conn->workBuffer.data);
+ }
+
+ if (their_version < conn->pversion)
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("protocol
version not supported by server: client uses %u.%u, server supports up to
%u.%u\n"),
+
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\n",
+
"protocol extensions not supported by server: %s\n", num),
+ buf.data);
+
+ /* neither -- server shouldn't have sent it */
+ if (!(their_version < conn->pversion) && !(num > 0))
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("invalid %s
message"), "NegotiateProtocolVersion");
+
+ termPQExpBuffer(&buf);
+ return 0;
+}
+
+
/*
* Attempt to read a ParameterStatus message.
* This is possible in several places, so we break it out as a subroutine.
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index c75ed63a2c62..c59afac7a086 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -685,6 +685,7 @@ extern void pqParseInput3(PGconn *conn);
extern int pqGetErrorNotice3(PGconn *conn, bool isError);
extern void pqBuildErrorMessage3(PQExpBuffer msg, const PGresult *res,
PGVerbosity
verbosity, PGContextVisibility show_context);
+extern int pqGetNegotiateProtocolVersion3(PGconn *conn);
extern int pqGetCopyData3(PGconn *conn, char **buffer, int async);
extern int pqGetline3(PGconn *conn, char *s, int maxlen);
extern int pqGetlineAsync3(PGconn *conn, char *buffer, int bufsize);
base-commit: 39b8c293fcde1d845da4d7127a25d41df53faab5
--
2.37.3
