Re: libpq support for NegotiateProtocolVersion

From: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
To: Jacob Champion <jchampion(at)timescale(dot)com>, Nathan Bossart <nathandbossart(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: libpq support for NegotiateProtocolVersion
Date: 2022-11-08 08:40:20
Message-ID: e13c7fca-7ac9-b14a-2a25-e34f8ddc7f9d@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 02.11.22 20:02, Jacob Champion wrote:
> A few notes:
>
>> + 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;
>> + }
>
> This new code path doesn't go through the message length checks that are
> done for the 'R' and 'E' cases, and pqGetNegotiateProtocolVersion3()
> doesn't take the message length to know where to stop anyway, so a
> misbehaving server can chew up client resources.

Fixed in new patch.

> It looks like the server is expecting to be able to continue the
> conversation with a newer client after sending a
> NegotiateProtocolVersion. Is an actual negotiation planned for the future?

The protocol documentation says:

| The client may then choose either
| to continue with the connection using the specified protocol version
| or to abort the connection.

In this case, we are choosing to abort the connection.

We could add negotiation in the future, but then we'd have to first have
a concrete case of something to negotiate about. For example, if we
added an optional performance feature into the protocol, then one could
negotiate by falling back to not using that. But for the kinds of
features I'm thinking about right now (column encryption), you can't
proceed if the feature is not supported. So I think this would need to
be considered case by case.

> I think the documentation on NegotiateProtocolVersion (not introduced in
> this patch) is misleading/wrong; it says that the version number sent
> back is the "newest minor protocol version supported by the server for
> the major protocol version requested by the client" which doesn't seem
> to match the actual usage seen here.

I don't follow. If libpq sends a protocol version of 3.1, then the
server responds by saying it supports only 3.0. What are you seeing?

Attachment Content-Type Size
v3-0001-libpq-Handle-NegotiateProtocolVersion-message.patch text/plain 5.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ronan Dunklau 2022-11-08 08:43:57 Re: Add proper planner support for ORDER BY / DISTINCT aggregates
Previous Message Bharath Rupireddy 2022-11-08 08:20:25 Re: Suppressing useless wakeups in walreceiver