From ea6cf43dfff9714bda7c2cb382cf93309d0f4651 Mon Sep 17 00:00:00 2001 From: Gihoon1123 Date: Mon, 27 Jul 2026 08:30:21 +0900 Subject: [PATCH 1/3] fix: don't mutate the caller's query config object Fixes #2651. normalizeQueryConfig() wrote values/callback directly onto the object it was given instead of copying it first. Reusing the same config object across calls (callback style, then promise style) left a stale callback on it, so a later promise-style call would see query.callback already set, skip creating the promise, and silently return undefined while the real result went to the old callback instead. Copy the object before writing to it so the caller's config is never touched, and add regression tests for both the direct case and the callback-then-promise reuse scenario. --- packages/pg/lib/utils.js | 5 ++++- .../pg/test/unit/client/simple-query-tests.js | 16 ++++++++++++++++ packages/pg/test/unit/utils-tests.js | 14 ++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/packages/pg/lib/utils.js b/packages/pg/lib/utils.js index 638b43970..f6b077fc1 100644 --- a/packages/pg/lib/utils.js +++ b/packages/pg/lib/utils.js @@ -143,7 +143,10 @@ function dateToStringUTC(date) { function normalizeQueryConfig(config, values, callback) { // can take in strings or config objects - config = typeof config === 'string' ? { text: config } : config + // the config object is caller-owned and must never be mutated -- copy it + // so callback/values assigned below don't leak into the caller's object + // and corrupt subsequent, unrelated invocations that reuse the same config + config = typeof config === 'string' ? { text: config } : { ...config } if (values) { if (typeof values === 'function') { config.callback = values diff --git a/packages/pg/test/unit/client/simple-query-tests.js b/packages/pg/test/unit/client/simple-query-tests.js index 8cc550830..ffeb816d3 100644 --- a/packages/pg/test/unit/client/simple-query-tests.js +++ b/packages/pg/test/unit/client/simple-query-tests.js @@ -150,4 +150,20 @@ test('executing query', function () { ) }) }) + + test('reusing a config object across calls', function () { + // Regression test for https://github.com/brianc/node-postgres/issues/2651 + // Using a config object with a callback once must not leave it unusable + // for a later, promise-style call with the same object. + test('does not leak callback state into a later promise-style call', function () { + const client = helper.client() + const config = { text: 'SELECT $1', values: [1] } + + client.query(config, function () {}) + const result = client.query(config) + + assert.ok(result instanceof Promise, 'expected client.query() to return a Promise') + result.catch(() => {}) + }) + }) }) diff --git a/packages/pg/test/unit/utils-tests.js b/packages/pg/test/unit/utils-tests.js index 5f75f6c2d..86a4c46f5 100644 --- a/packages/pg/test/unit/utils-tests.js +++ b/packages/pg/test/unit/utils-tests.js @@ -33,6 +33,20 @@ test('normalizing query configs', function () { assert.deepEqual(config, { text: 'TEXT', values: [10], callback: callback }) }) +test('normalizeQueryConfig does not mutate the passed-in config object', function () { + // Regression test for https://github.com/brianc/node-postgres/issues/2651 + // The config object is caller-owned; writing `callback`/`values` onto it + // leaks state into later, unrelated calls that reuse the same object. + const original = { text: 'TEXT' } + const callback = function () {} + + const normalized = utils.normalizeQueryConfig(original, [10], callback) + + assert.equal(original.callback, undefined) + assert.equal(original.values, undefined) + assert.deepEqual(normalized, { text: 'TEXT', values: [10], callback: callback }) +}) + test('prepareValues: buffer prepared properly', function () { const buf = Buffer.from('quack') const out = utils.prepareValue(buf) From f0bbfcfcea2284ec0ef60ba99987e9bf7aa38bb5 Mon Sep 17 00:00:00 2001 From: Gihoon1123 Date: Mon, 27 Jul 2026 08:41:43 +0900 Subject: [PATCH 2/3] test: simplify query config comments --- packages/pg/lib/utils.js | 4 +--- packages/pg/test/unit/client/simple-query-tests.js | 4 +--- packages/pg/test/unit/utils-tests.js | 4 +--- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/packages/pg/lib/utils.js b/packages/pg/lib/utils.js index f6b077fc1..56754ef72 100644 --- a/packages/pg/lib/utils.js +++ b/packages/pg/lib/utils.js @@ -143,9 +143,7 @@ function dateToStringUTC(date) { function normalizeQueryConfig(config, values, callback) { // can take in strings or config objects - // the config object is caller-owned and must never be mutated -- copy it - // so callback/values assigned below don't leak into the caller's object - // and corrupt subsequent, unrelated invocations that reuse the same config + // Copy config so normalization does not mutate the caller's object. config = typeof config === 'string' ? { text: config } : { ...config } if (values) { if (typeof values === 'function') { diff --git a/packages/pg/test/unit/client/simple-query-tests.js b/packages/pg/test/unit/client/simple-query-tests.js index ffeb816d3..bb1d54a89 100644 --- a/packages/pg/test/unit/client/simple-query-tests.js +++ b/packages/pg/test/unit/client/simple-query-tests.js @@ -152,9 +152,7 @@ test('executing query', function () { }) test('reusing a config object across calls', function () { - // Regression test for https://github.com/brianc/node-postgres/issues/2651 - // Using a config object with a callback once must not leave it unusable - // for a later, promise-style call with the same object. + // Regression test for https://github.com/brianc/node-postgres/issues/2651. test('does not leak callback state into a later promise-style call', function () { const client = helper.client() const config = { text: 'SELECT $1', values: [1] } diff --git a/packages/pg/test/unit/utils-tests.js b/packages/pg/test/unit/utils-tests.js index 86a4c46f5..3038d36eb 100644 --- a/packages/pg/test/unit/utils-tests.js +++ b/packages/pg/test/unit/utils-tests.js @@ -34,9 +34,7 @@ test('normalizing query configs', function () { }) test('normalizeQueryConfig does not mutate the passed-in config object', function () { - // Regression test for https://github.com/brianc/node-postgres/issues/2651 - // The config object is caller-owned; writing `callback`/`values` onto it - // leaks state into later, unrelated calls that reuse the same object. + // Regression test for https://github.com/brianc/node-postgres/issues/2651. const original = { text: 'TEXT' } const callback = function () {} From b4d11e47b3431c066df3009efb12a5b488c3d5f6 Mon Sep 17 00:00:00 2001 From: GiHoon1123 Date: Mon, 27 Jul 2026 15:06:43 +0900 Subject: [PATCH 3/3] fix: preserve query config property access --- packages/pg/lib/utils.js | 9 ++++++++- packages/pg/test/unit/utils-tests.js | 23 +++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/packages/pg/lib/utils.js b/packages/pg/lib/utils.js index 56754ef72..55dc6824d 100644 --- a/packages/pg/lib/utils.js +++ b/packages/pg/lib/utils.js @@ -144,7 +144,7 @@ function dateToStringUTC(date) { function normalizeQueryConfig(config, values, callback) { // can take in strings or config objects // Copy config so normalization does not mutate the caller's object. - config = typeof config === 'string' ? { text: config } : { ...config } + config = typeof config === 'string' ? { text: config } : cloneQueryConfig(config) if (values) { if (typeof values === 'function') { config.callback = values @@ -158,6 +158,13 @@ function normalizeQueryConfig(config, values, callback) { return config } +function cloneQueryConfig(config) { + if (config == null) { + return config + } + return Object.defineProperties(Object.create(Object.getPrototypeOf(config)), Object.getOwnPropertyDescriptors(config)) +} + // Ported from PostgreSQL 9.2.4 source code in src/interfaces/libpq/fe-exec.c const escapeIdentifier = function (str) { return '"' + str.replace(/"/g, '""') + '"' diff --git a/packages/pg/test/unit/utils-tests.js b/packages/pg/test/unit/utils-tests.js index 3038d36eb..5274a497e 100644 --- a/packages/pg/test/unit/utils-tests.js +++ b/packages/pg/test/unit/utils-tests.js @@ -45,6 +45,29 @@ test('normalizeQueryConfig does not mutate the passed-in config object', functio assert.deepEqual(normalized, { text: 'TEXT', values: [10], callback: callback }) }) +test('normalizeQueryConfig preserves inherited config properties', function () { + class QueryConfig { + constructor() { + this._text = 'TEXT' + } + + get text() { + return this._text + } + } + + const original = new QueryConfig() + const callback = function () {} + + const normalized = utils.normalizeQueryConfig(original, [10], callback) + + assert.equal(original.callback, undefined) + assert.equal(original.values, undefined) + assert.equal(normalized.text, 'TEXT') + assert.deepEqual(normalized.values, [10]) + assert.equal(normalized.callback, callback) +}) + test('prepareValues: buffer prepared properly', function () { const buf = Buffer.from('quack') const out = utils.prepareValue(buf)