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 this case, 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 client would 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
