Re: libpq support for NegotiateProtocolVersion - Mailing list pgsql-hackers

From Peter Eisentraut
Subject Re: libpq support for NegotiateProtocolVersion
Date
Msg-id e13c7fca-7ac9-b14a-2a25-e34f8ddc7f9d@enterprisedb.com
Whole thread Raw
In response to Re: libpq support for NegotiateProtocolVersion  (Jacob Champion <jchampion@timescale.com>)
Responses Re: libpq support for NegotiateProtocolVersion
List 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

pgsql-hackers by date:

Previous
From: Bharath Rupireddy
Date:
Subject: Re: Suppressing useless wakeups in walreceiver
Next
From: Ronan Dunklau
Date:
Subject: Re: Add proper planner support for ORDER BY / DISTINCT aggregates