The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation: not tested
Hi, thanks for the patch!
I’ve made a quick review and found one issue.
If the backend sends a CompressionAck message followed by some already compressed message (for example,
AuthenticationOk),then there is a chance that pqReadData() will read both messages into the read buffer at once. In
thiscase, the CompressionAck message will be read normally, but the client will fail to recognize the next message (for
example,AuthenticationOk) since it came in a compressed form but was incorrectly read as a regular message. So the
clientwould not be able to recognize the second message and will crash.
Example of a successful launch (added some debug output):
usernamedt-osx: ~ usernamedt $ psql -d "host = x.x.x.x port = 6432 dbname = testdb user = testuser compression = 1"
NUM_READ: 6 (pqReadData read CompressionAck (6 bytes) and nothing more)
pqReadData RC: 1
NUM_READ: 346
pqReadData RC: 1
psql (14devel)
Type "help" for help.
testdb => // OK
Example of a failed launch:
usernamedt-osx: ~ usernamedt $ psql -d "host = x.x.x.x port = 6432 dbname = testdb user = testuser compression = 1"
NUM_READ: 24 (pqReadData read CompressionAck (6 bytes) and compressed AuthenticationOk (18 bytes) came after it)
pqReadData RC: 1
psql: error: could not connect to server: expected authentication request from server, but received x // FAIL
--
Daniil Zakhlystov
