Skip to content

Commit cfced9b

Browse files
JelteFCommitfest Bot
authored andcommitted
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.
1 parent 3a0e369 commit cfced9b

File tree

2 files changed

+43
-28
lines changed

2 files changed

+43
-28
lines changed

src/interfaces/libpq/fe-connect.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4067,9 +4067,13 @@ PQconnectPoll(PGconn *conn)
40674067
libpq_append_conn_error(conn, "received invalid protocol negotiation message");
40684068
goto error_return;
40694069
}
4070+
4071+
if (conn->error_result)
4072+
goto error_return;
4073+
40704074
/* OK, we read the message; mark data consumed */
40714075
pqParseDone(conn, conn->inCursor);
4072-
goto error_return;
4076+
goto keep_going;
40734077
}
40744078

40754079
/* It is an authentication request. */

src/interfaces/libpq/fe-protocol3.c

Lines changed: 38 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1407,49 +1407,60 @@ reportErrorPosition(PQExpBuffer msg, const char *query, int loc, int encoding)
14071407
int
14081408
pqGetNegotiateProtocolVersion3(PGconn *conn)
14091409
{
1410-
int tmp;
1411-
ProtocolVersion their_version;
1410+
int their_version;
14121411
int num;
1413-
PQExpBufferData buf;
14141412

1415-
if (pqGetInt(&tmp, 4, conn) != 0)
1413+
if (pqGetInt(&their_version, 4, conn) != 0)
14161414
return EOF;
1417-
their_version = tmp;
14181415

14191416
if (pqGetInt(&num, 4, conn) != 0)
14201417
return EOF;
14211418

1422-
initPQExpBuffer(&buf);
1419+
if (their_version > conn->pversion)
1420+
{
1421+
libpq_append_conn_error(conn, "received invalid protocol negotiation message: server requests downgrade to higher-numbered version");
1422+
goto failure;
1423+
}
1424+
1425+
if (their_version < PG_PROTOCOL(3, 0))
1426+
{
1427+
libpq_append_conn_error(conn, "received invalid protocol negotiation message: server requests downgrade to pre-3.0 protocol version");
1428+
goto failure;
1429+
}
1430+
1431+
if (num < 0)
1432+
{
1433+
libpq_append_conn_error(conn, "received invalid protocol negotiation message: server reported negative number of unsupported parameters");
1434+
goto failure;
1435+
}
1436+
1437+
if (their_version == conn->pversion && num == 0)
1438+
{
1439+
libpq_append_conn_error(conn, "received invalid protocol negotiation message: server negotiated but asks for no changes");
1440+
goto failure;
1441+
}
1442+
1443+
conn->pversion = their_version;
14231444
for (int i = 0; i < num; i++)
14241445
{
14251446
if (pqGets(&conn->workBuffer, conn))
14261447
{
1427-
termPQExpBuffer(&buf);
14281448
return EOF;
14291449
}
1430-
if (buf.len > 0)
1431-
appendPQExpBufferChar(&buf, ' ');
1432-
appendPQExpBufferStr(&buf, conn->workBuffer.data);
1433-
}
1434-
1435-
if (their_version < conn->pversion)
1436-
libpq_append_conn_error(conn, "protocol version not supported by server: client uses %u.%u, server supports up to %u.%u",
1437-
PG_PROTOCOL_MAJOR(conn->pversion), PG_PROTOCOL_MINOR(conn->pversion),
1438-
PG_PROTOCOL_MAJOR(their_version), PG_PROTOCOL_MINOR(their_version));
1439-
if (num > 0)
1440-
{
1441-
appendPQExpBuffer(&conn->errorMessage,
1442-
libpq_ngettext("protocol extension not supported by server: %s",
1443-
"protocol extensions not supported by server: %s", num),
1444-
buf.data);
1445-
appendPQExpBufferChar(&conn->errorMessage, '\n');
1450+
if (strncmp(conn->workBuffer.data, "_pq_.", 5) != 0)
1451+
{
1452+
libpq_append_conn_error(conn, "received invalid protocol negotiation message: server reported unsupported parameter name without a _pq_. prefix (\"%s\")", conn->workBuffer.data);
1453+
goto failure;
1454+
}
1455+
libpq_append_conn_error(conn, "received invalid protocol negotiation message: server reported an unsupported parameter that was not requested (\"%s\")", conn->workBuffer.data);
1456+
goto failure;
14461457
}
14471458

1448-
/* neither -- server shouldn't have sent it */
1449-
if (!(their_version < conn->pversion) && !(num > 0))
1450-
libpq_append_conn_error(conn, "invalid %s message", "NegotiateProtocolVersion");
1459+
return 0;
14511460

1452-
termPQExpBuffer(&buf);
1461+
failure:
1462+
conn->asyncStatus = PGASYNC_READY;
1463+
pqSaveErrorResult(conn);
14531464
return 0;
14541465
}
14551466

0 commit comments

Comments
 (0)