diff --git a/packages/pg/lib/connection.js b/packages/pg/lib/connection.js index 62d38fa69..ecf60534b 100644 --- a/packages/pg/lib/connection.js +++ b/packages/pg/lib/connection.js @@ -23,8 +23,11 @@ class Connection extends EventEmitter { this._keepAlive = config.keepAlive this._keepAliveInitialDelayMillis = config.keepAliveInitialDelayMillis - this.parsedStatements = {} - this.submittedNamedStatements = {} + // Prepared-statement caches are keyed by user-supplied statement names, so they must not + // inherit from Object.prototype: a statement named e.g. `constructor` would otherwise read + // back as already-prepared and skip its Parse message. See issue #3625. + this.parsedStatements = Object.create(null) + this.submittedNamedStatements = Object.create(null) this.ssl = config.ssl || false this.sslNegotiation = config.sslNegotiation || 'postgres' this._ending = false diff --git a/packages/pg/lib/native/client.js b/packages/pg/lib/native/client.js index 9ec3c8c03..6cbc624a8 100644 --- a/packages/pg/lib/native/client.js +++ b/packages/pg/lib/native/client.js @@ -57,8 +57,10 @@ const Client = (module.exports = function (config) { this.host = cp.host this.port = cp.port - // a hash to hold named queries - this.namedQueries = {} + // a hash to hold named queries; prototypeless so a query named after an + // Object.prototype key (e.g. `constructor`) isn't mistaken for one already + // prepared. See issue #3625. + this.namedQueries = Object.create(null) }) Client.Query = NativeQuery diff --git a/packages/pg/test/unit/connection/prepared-statement-name-tests.js b/packages/pg/test/unit/connection/prepared-statement-name-tests.js new file mode 100644 index 000000000..2fdd5e529 --- /dev/null +++ b/packages/pg/test/unit/connection/prepared-statement-name-tests.js @@ -0,0 +1,69 @@ +'use strict' +// Regression tests for https://github.com/brianc/node-postgres/issues/3625 +// +// The connection tracks which prepared statements have already been sent to the +// backend in `parsedStatements` / `submittedNamedStatements`. When those are +// plain `{}` objects, a statement named after an `Object.prototype` key such as +// `constructor` reads back as truthy even though it was never prepared, so the +// client skips the `Parse` message and sends a `Bind` for a statement the +// backend has never seen. +const helper = require('./test-helper') +const assert = require('assert') +const Connection = require('../../../lib/connection') +const Query = require('../../../lib/query') + +const suite = new helper.Suite() +const { MemoryStream } = helper + +const PROTOTYPE_STATEMENT_NAMES = ['constructor', 'hasOwnProperty', 'toString', '__proto__'] + +const makeConnection = function () { + const con = new Connection({ stream: new MemoryStream() }) + con.connect() + return con +} + +suite.test('a fresh connection has no prepared statements for Object.prototype names', function () { + const con = makeConnection() + for (const name of PROTOTYPE_STATEMENT_NAMES) { + assert.ok( + !con.parsedStatements[name], + `parsedStatements should not report '${name}' as already parsed on a fresh connection` + ) + assert.ok( + !con.submittedNamedStatements[name], + `submittedNamedStatements should not report '${name}' as already submitted on a fresh connection` + ) + } +}) + +suite.test('a statement named "constructor" is parsed before it is bound', function () { + const con = makeConnection() + + const parsed = [] + const bound = [] + con.parse = function (query) { + parsed.push(query) + } + con.bind = function (config) { + bound.push(config) + } + + const query = new Query({ text: 'SELECT $1::text', name: 'constructor', values: ['ok'] }) + const err = query.submit(con) + + assert.ifError(err) + assert.equal(parsed.length, 1, 'a Parse message must be sent for a never-before-seen "constructor" statement') + assert.equal(parsed[0].name, 'constructor') + assert.equal(parsed[0].text, 'SELECT $1::text') + assert.equal(bound.length, 1, 'the statement should also be bound after being parsed') + assert.equal(bound[0].statement, 'constructor') +}) + +suite.test('hasBeenParsed is false for an unprepared Object.prototype-named statement', function () { + const con = makeConnection() + for (const name of PROTOTYPE_STATEMENT_NAMES) { + const query = new Query({ text: 'SELECT 1', name }) + assert.ok(!query.hasBeenParsed(con), `hasBeenParsed must be false for the unprepared statement named '${name}'`) + } +})