diff --git a/packages/pg/lib/client.js b/packages/pg/lib/client.js index 2b13c1de7..1d4d54b68 100644 --- a/packages/pg/lib/client.js +++ b/packages/pg/lib/client.js @@ -79,6 +79,7 @@ class Client extends EventEmitter { this._ended = false this._connecting = false this._connected = false + this._authenticated = false this._connectionError = false this._queryable = true this._activeQuery = null @@ -247,6 +248,7 @@ class Client extends EventEmitter { } _attachListeners(con) { + con.on('authenticationOk', this._handleAuthenticationOk.bind(this)) // password request handling con.on('authenticationCleartextPassword', this._handleAuthCleartextPassword.bind(this)) // password request handling @@ -370,8 +372,21 @@ class Client extends EventEmitter { this.secretKey = msg.secretKey } + _handleAuthenticationOk() { + this._authenticated = true + } + _handleReadyForQuery(msg) { if (this._connecting) { + if (!this._authenticated) { + // The server must finish authentication with AuthenticationOk before + // its first ReadyForQuery. A ReadyForQuery here means the peer is not + // following the protocol, so refuse the connection instead of + // treating it as established - the same behavior as libpq. + this._handleErrorWhileConnecting(new Error('expected authentication request from server, but received Z')) + this.connection.end() + return + } this._connecting = false this._connected = true clearTimeout(this.connectionTimeoutHandle) diff --git a/packages/pg/test/unit/client/query-queue-tests.js b/packages/pg/test/unit/client/query-queue-tests.js index 1566afc43..1da201efd 100644 --- a/packages/pg/test/unit/client/query-queue-tests.js +++ b/packages/pg/test/unit/client/query-queue-tests.js @@ -14,6 +14,9 @@ test('drain', function () { } con.query = function () {} client.connect() + // a real server finishes authentication (AuthenticationOk) before its + // first ReadyForQuery + con.emit('authenticationOk') let raisedDrain = false client.on('drain', function () { diff --git a/packages/pg/test/unit/client/ready-for-query-before-auth-tests.js b/packages/pg/test/unit/client/ready-for-query-before-auth-tests.js new file mode 100644 index 000000000..0ac66676d --- /dev/null +++ b/packages/pg/test/unit/client/ready-for-query-before-auth-tests.js @@ -0,0 +1,71 @@ +'use strict' +require('./test-helper') +const net = require('net') +const pg = require('../../../lib/index.js') +const assert = require('assert') + +const message = (type, body) => { + const head = Buffer.alloc(5) + head.write(type, 0) + head.writeInt32BE(body.length + 4, 1) + return Buffer.concat([head, body]) +} +const authenticationOk = message('R', Buffer.alloc(4)) +const readyForQuery = message('Z', Buffer.from('I')) + +const withServer = function (onStartup, run) { + const sockets = [] + const server = net.createServer(function (socket) { + sockets.push(socket) + socket.once('data', function () { + onStartup(socket) + }) + }) + server.listen(0, '127.0.0.1', function () { + const client = new pg.Client({ + host: '127.0.0.1', + port: server.address().port, + user: 'app', + password: 'secret', + database: 'app', + }) + run(client, function () { + for (const socket of sockets) { + socket.destroy() + } + server.close() + }) + }) +} + +// a server answering the startup message with ReadyForQuery and no +// authentication message must be refused, not treated as connected +withServer( + function (socket) { + socket.write(readyForQuery) + }, + function (client, done) { + client.connect( + assert.calls(function (err) { + assert(err) + assert.strictEqual(err.message, 'expected authentication request from server, but received Z') + done() + }) + ) + } +) + +// sanity check: AuthenticationOk followed by ReadyForQuery still connects +withServer( + function (socket) { + socket.write(Buffer.concat([authenticationOk, readyForQuery])) + }, + function (client, done) { + client.connect( + assert.calls(function (err) { + assert.ifError(err) + client.end(done) + }) + ) + } +) diff --git a/packages/pg/test/unit/client/stream-and-query-error-interaction-tests.js b/packages/pg/test/unit/client/stream-and-query-error-interaction-tests.js index 166cd9f35..edec09704 100644 --- a/packages/pg/test/unit/client/stream-and-query-error-interaction-tests.js +++ b/packages/pg/test/unit/client/stream-and-query-error-interaction-tests.js @@ -30,6 +30,7 @@ suite.test('emits end when not in query', function () { assert.emits(client, 'end') client.connection.emit('connect') process.nextTick(function () { + client.connection.emit('authenticationOk') client.connection.emit('readyForQuery') process.nextTick(function () { stream.emit('close') diff --git a/packages/pg/test/unit/client/test-helper.js b/packages/pg/test/unit/client/test-helper.js index 8c3c3304e..e52cd8474 100644 --- a/packages/pg/test/unit/client/test-helper.js +++ b/packages/pg/test/unit/client/test-helper.js @@ -29,6 +29,9 @@ const makeClient = function (config) { const client = new Client({ connection: connection, ...config }) client.connect() client.connection.emit('connect') + // the fake connection skips the startup handshake, so report the + // AuthenticationOk a real server sends before its first ReadyForQuery + client.connection.emit('authenticationOk') return client }