Add support for negotiating the protocol version with the client #1007
Conversation
JelteF
force-pushed
the
better-protocol-version-handling
branch
2 times, most recently
from
b3e885a
to
5308f66
Compare
| @@ -102,8 +102,10 @@ bool get_header(struct MBuf *data, PktHdr *pkt) | |||
| type = PKT_SSLREQ; | |||
| } else if (code == PKT_GSSENCREQ) { | |||
| type = PKT_GSSENCREQ; | |||
| } else if ((code >> 16) == 3 && (code & 0xFFFF) < 2) { | |||
There was a problem hiding this comment.
to be clear, this was accepting protocol 3.1. It should have been this to only support 3.0:
- } else if ((code >> 16) == 3 && (code & 0xFFFF) < 2) {
+ } else if ((code >> 16) == 3 && (code & 0xFFFF) < 1) {
JelteF
force-pushed
the
better-protocol-version-handling
branch
from
5308f66
to
f0004d7
Compare
|
How do you test this? I'm trying to send the protocol negotiation packet while connecting with psql and it always returns this error message: which is really weird because when I'm reading the implementation in libpq, it clearly should support |
|
Sounds like you're using a libpq from before bbf9c282ce92272ed7bf6771daf0f9efa209e61b But even after that libpq will complain. Just with a more helpful message. Better handling is in my patchset here: https://www.postgresql.org/message-id/flat/CAGECzQScQ3N-Ykv2j4NDyDtrPPc3FpRoa%3DLZ-2Uj2ocA4zr%3D4Q%40mail.gmail.com#cd9e8407820d492e8f677ee6a67c21ce |
JelteF
force-pushed
the
better-protocol-version-handling
branch
from
f0004d7
to
bfd7660
Compare
This is done so we can introduce PKT_STARTUP_V4 in a follow up commit.
Passing NULL to memcpy is undefined behaviour, even if len is 0. This avoids doing that that, so it starts to allow passing NULL as the data pointer if len is 0.
PgBouncer currently only supports protocol version 3.0. However, it was still accepting connections if the client asked for the 3.1 protocol version. This is obviously a bug. This starts sending the NegotiateProtocolVersion message when the client asks for a minor version that PgBouncer does not support. Also, instead of erroring when it receives unsupported protocol extensions parameters it will include these in this NegotiateProtocolVersion message.
JelteF
force-pushed
the
better-protocol-version-handling
branch
from
bfd7660
to
6a39179
Compare
PgBouncer currently only supports protocol version 3.0. However, it was
still accepting connections if the client asked for the 3.1 protocol
version. This is obviously a bug, because PgBouncer likely won't
support the changes introduced in 3.1 of the protocol without
any code changes.
This starts sending the NegotiateProtocolVersion message when the client
asks for a minor version that PgBouncer does not support. Also, instead
of erroring when it receives unsupported protocol extensions parameters
it will include these in this NegotiateProtocolVersion message.
For reference this is the Postgres commit introducing NegotiateProtocolVersion: postgres/postgres@ae65f60
I tested this manually using this patchset that bumps the protocol version. https://www.postgresql.org/message-id/flat/CAGECzQScQ3N-Ykv2j4NDyDtrPPc3FpRoa%3DLZ-2Uj2ocA4zr%3D4Q%40mail.gmail.com#cd9e8407820d492e8f677ee6a67c21ce