From c909aa64cf540da9e4f0a36babce0e4c7e3ec281 Mon Sep 17 00:00:00 2001 From: Brian C Date: Mon, 13 Jan 2020 13:00:18 -0600 Subject: [PATCH 01/11] Drop support for EOL versions of node (#2062) * Drop support for EOL versions of node * Re-add testing for node@8.x * Revert changes to .travis.yml * Update packages/pg-pool/package.json Co-Authored-By: Charmander <~@charmander.me> Co-authored-by: Charmander <~@charmander.me> --- packages/pg-pool/package.json | 2 +- packages/pg/Makefile | 4 +--- packages/pg/package.json | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/pg-pool/package.json b/packages/pg-pool/package.json index dc0275699..a1fa1465e 100644 --- a/packages/pg-pool/package.json +++ b/packages/pg-pool/package.json @@ -34,6 +34,6 @@ "pg-cursor": "^1.3.0" }, "peerDependencies": { - "pg": ">5.0" + "pg": ">=8.0" } } diff --git a/packages/pg/Makefile b/packages/pg/Makefile index 52d0545d3..a5b0bc1da 100644 --- a/packages/pg/Makefile +++ b/packages/pg/Makefile @@ -62,6 +62,4 @@ test-pool: lint: @echo "***Starting lint***" - node -e "process.exit(Number(process.versions.node.split('.')[0]) < 8 ? 0 : 1)" \ - && echo "***Skipping lint (node version too old)***" \ - || node_modules/.bin/eslint lib + node_modules/.bin/eslint lib diff --git a/packages/pg/package.json b/packages/pg/package.json index 87e7a1eb1..5d7e1eb20 100644 --- a/packages/pg/package.json +++ b/packages/pg/package.json @@ -51,6 +51,6 @@ ], "license": "MIT", "engines": { - "node": ">= 4.5.0" + "node": ">= 8.0.0" } } From 31eaa05017d173dfd57d80c1bc165d997bde4a42 Mon Sep 17 00:00:00 2001 From: Brian C Date: Mon, 13 Jan 2020 14:31:48 -0600 Subject: [PATCH 02/11] Remove password from stringified outputs (#2066) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove password from stringified outputs Theres a security concern where if you're not careful and you include your client or pool instance in console.log or stack traces it might include the database password. To widen the pit of success I'm making that field non-enumerable. You can still get at it...it just wont show up "by accident" when you're logging things now. The backwards compatiblity impact of this is very small, but it is still technically somewhat an API change so...8.0. * Implement feedback * Fix more whitespace the autoformatter changed * Simplify code a bit * Remove password from stringified outputs (#2070) * Keep ConnectionParameters’s password property writable `Client` writes to it when `password` is a function. * Avoid creating password property on pool options when it didn’t exist previously. * Allow password option to be non-enumerable to avoid breaking uses like `new Pool(existingPool.options)`. * Make password property definitions consistent in formatting and configurability. Co-authored-by: Charmander <~@charmander.me> --- .gitignore | 1 + packages/pg-pool/index.js | 12 +++++++ packages/pg/lib/client.js | 11 ++++++- packages/pg/lib/connection-parameters.js | 11 ++++++- packages/pg/lib/native/client.js | 10 +++++- .../test/integration/gh-issues/2064-tests.js | 32 +++++++++++++++++++ 6 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 packages/pg/test/integration/gh-issues/2064-tests.js diff --git a/.gitignore b/.gitignore index df95fda07..bae2a20a1 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ package-lock.json *.swp dist .DS_Store +.vscode/ diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js index 1c7faf210..dd0d478d2 100644 --- a/packages/pg-pool/index.js +++ b/packages/pg-pool/index.js @@ -60,6 +60,18 @@ class Pool extends EventEmitter { constructor (options, Client) { super() this.options = Object.assign({}, options) + + if (options != null && 'password' in options) { + // "hiding" the password so it doesn't show up in stack traces + // or if the client is console.logged + Object.defineProperty(this.options, 'password', { + configurable: true, + enumerable: false, + writable: true, + value: options.password + }) + } + this.options.max = this.options.max || this.options.poolSize || 10 this.log = this.options.log || function () { } this.Client = this.options.Client || Client || require('pg').Client diff --git a/packages/pg/lib/client.js b/packages/pg/lib/client.js index 93807e48c..c929d26f3 100644 --- a/packages/pg/lib/client.js +++ b/packages/pg/lib/client.js @@ -30,7 +30,16 @@ var Client = function (config) { this.database = this.connectionParameters.database this.port = this.connectionParameters.port this.host = this.connectionParameters.host - this.password = this.connectionParameters.password + + // "hiding" the password so it doesn't show up in stack traces + // or if the client is console.logged + Object.defineProperty(this, 'password', { + configurable: true, + enumerable: false, + writable: true, + value: this.connectionParameters.password + }) + this.replication = this.connectionParameters.replication var c = config || {} diff --git a/packages/pg/lib/connection-parameters.js b/packages/pg/lib/connection-parameters.js index 0d5e0376d..c0f8498eb 100644 --- a/packages/pg/lib/connection-parameters.js +++ b/packages/pg/lib/connection-parameters.js @@ -54,7 +54,16 @@ var ConnectionParameters = function (config) { this.database = val('database', config) this.port = parseInt(val('port', config), 10) this.host = val('host', config) - this.password = val('password', config) + + // "hiding" the password so it doesn't show up in stack traces + // or if the client is console.logged + Object.defineProperty(this, 'password', { + configurable: true, + enumerable: false, + writable: true, + value: val('password', config) + }) + this.binary = val('binary', config) this.ssl = typeof config.ssl === 'undefined' ? useSsl() : config.ssl this.client_encoding = val('client_encoding', config) diff --git a/packages/pg/lib/native/client.js b/packages/pg/lib/native/client.js index 6859bc2cc..d06166573 100644 --- a/packages/pg/lib/native/client.js +++ b/packages/pg/lib/native/client.js @@ -43,7 +43,15 @@ var Client = module.exports = function (config) { // for the time being. TODO: deprecate all this jazz var cp = this.connectionParameters = new ConnectionParameters(config) this.user = cp.user - this.password = cp.password + + // "hiding" the password so it doesn't show up in stack traces + // or if the client is console.logged + const hiddenPassword = cp.password + Object.defineProperty(this, 'password', { + enumerable: false, + writable: true, + value: hiddenPassword + }) this.database = cp.database this.host = cp.host this.port = cp.port diff --git a/packages/pg/test/integration/gh-issues/2064-tests.js b/packages/pg/test/integration/gh-issues/2064-tests.js new file mode 100644 index 000000000..64c150bd0 --- /dev/null +++ b/packages/pg/test/integration/gh-issues/2064-tests.js @@ -0,0 +1,32 @@ + +"use strict" +const helper = require('./../test-helper') +const assert = require('assert') +const util = require('util') + +const suite = new helper.Suite() + +const password = 'FAIL THIS TEST' + +suite.test('Password should not exist in toString() output', () => { + const pool = new helper.pg.Pool({ password }) + const client = new helper.pg.Client({ password }) + assert(pool.toString().indexOf(password) === -1); + assert(client.toString().indexOf(password) === -1); +}) + +suite.test('Password should not exist in util.inspect output', () => { + const pool = new helper.pg.Pool({ password }) + const client = new helper.pg.Client({ password }) + const depth = 20; + assert(util.inspect(pool, { depth }).indexOf(password) === -1); + assert(util.inspect(client, { depth }).indexOf(password) === -1); +}) + +suite.test('Password should not exist in json.stringfy output', () => { + const pool = new helper.pg.Pool({ password }) + const client = new helper.pg.Client({ password }) + const depth = 20; + assert(JSON.stringify(pool).indexOf(password) === -1); + assert(JSON.stringify(client).indexOf(password) === -1); +}) From e85afe157fb30feb006ac38958d6f5be44bfd308 Mon Sep 17 00:00:00 2001 From: Brian C Date: Wed, 15 Jan 2020 14:12:48 -0600 Subject: [PATCH 03/11] Make `native` non-enumerable (#2065) * Make `native` non-enumerable Making it non-enumerable means less spurious "Cannot find module" errors in your logs when iterating over `pg` objects. `Object.defineProperty` has been available since Node 0.12. See https://github.com/brianc/node-postgres/issues/1894#issuecomment-543300178 * Add test for `native` enumeration Co-authored-by: Gabe Gorelick --- packages/pg/lib/index.js | 34 ++++++++++++------- .../test/integration/gh-issues/1992-tests.js | 11 ++++++ 2 files changed, 32 insertions(+), 13 deletions(-) create mode 100644 packages/pg/test/integration/gh-issues/1992-tests.js diff --git a/packages/pg/lib/index.js b/packages/pg/lib/index.js index de33c086d..3f8508734 100644 --- a/packages/pg/lib/index.js +++ b/packages/pg/lib/index.js @@ -44,20 +44,28 @@ if (typeof process.env.NODE_PG_FORCE_NATIVE !== 'undefined') { module.exports = new PG(Client) // lazy require native module...the native module may not have installed - module.exports.__defineGetter__('native', function () { - delete module.exports.native - var native = null - try { - native = new PG(require('./native')) - } catch (err) { - if (err.code !== 'MODULE_NOT_FOUND') { - throw err + Object.defineProperty(module.exports, 'native', { + configurable: true, + enumerable: false, + get() { + var native = null + try { + native = new PG(require('./native')) + } catch (err) { + if (err.code !== 'MODULE_NOT_FOUND') { + throw err + } + /* eslint-disable no-console */ + console.error(err.message) + /* eslint-enable no-console */ } - /* eslint-disable no-console */ - console.error(err.message) - /* eslint-enable no-console */ + + // overwrite module.exports.native so that getter is never called again + Object.defineProperty(module.exports, 'native', { + value: native + }) + + return native } - module.exports.native = native - return native }) } diff --git a/packages/pg/test/integration/gh-issues/1992-tests.js b/packages/pg/test/integration/gh-issues/1992-tests.js new file mode 100644 index 000000000..1832f5f8a --- /dev/null +++ b/packages/pg/test/integration/gh-issues/1992-tests.js @@ -0,0 +1,11 @@ + +"use strict" +const helper = require('./../test-helper') +const assert = require('assert') + +const suite = new helper.Suite() + +suite.test('Native should not be enumerable', () => { + const keys = Object.keys(helper.pg) + assert.strictEqual(keys.indexOf('native'), -1) +}) From 224703fc63109cadc2627a83fb2b0752bde9f917 Mon Sep 17 00:00:00 2001 From: Natalie Wolfe Date: Wed, 15 Jan 2020 13:10:59 -0800 Subject: [PATCH 04/11] Use class-extends to wrap Pool (#1541) * Use class-extends to wrap Pool * Minimize diff * Test `BoundPool` inheritance Co-authored-by: Charmander <~@charmander.me> Co-authored-by: Brian C --- packages/pg/lib/index.js | 17 ++++--------- .../test/integration/gh-issues/1542-tests.js | 25 +++++++++++++++++++ 2 files changed, 30 insertions(+), 12 deletions(-) create mode 100644 packages/pg/test/integration/gh-issues/1542-tests.js diff --git a/packages/pg/lib/index.js b/packages/pg/lib/index.js index 3f8508734..c3062947d 100644 --- a/packages/pg/lib/index.js +++ b/packages/pg/lib/index.js @@ -7,25 +7,18 @@ * README.md file in the root directory of this source tree. */ -var util = require('util') var Client = require('./client') var defaults = require('./defaults') var Connection = require('./connection') var Pool = require('pg-pool') -const checkConstructor = require('./compat/check-constructor') const poolFactory = (Client) => { - var BoundPool = function (options) { - // eslint-disable-next-line no-eval - checkConstructor('pg.Pool', 'PG-POOL-NEW', () => eval('new.target')) - - var config = Object.assign({ Client: Client }, options) - return new Pool(config) + return class BoundPool extends Pool { + constructor (options) { + var config = Object.assign({ Client: Client }, options) + super(config) + } } - - util.inherits(BoundPool, Pool) - - return BoundPool } var PG = function (clientConstructor) { diff --git a/packages/pg/test/integration/gh-issues/1542-tests.js b/packages/pg/test/integration/gh-issues/1542-tests.js new file mode 100644 index 000000000..4d30d6020 --- /dev/null +++ b/packages/pg/test/integration/gh-issues/1542-tests.js @@ -0,0 +1,25 @@ + +"use strict" +const helper = require('./../test-helper') +const assert = require('assert') + +const suite = new helper.Suite() + +suite.testAsync('BoundPool can be subclassed', async () => { + const Pool = helper.pg.Pool; + class SubPool extends Pool { + + } + const subPool = new SubPool() + const client = await subPool.connect() + client.release() + await subPool.end() + assert(subPool instanceof helper.pg.Pool) +}) + +suite.test('calling pg.Pool without new throws', () => { + const Pool = helper.pg.Pool; + assert.throws(() => { + const pool = Pool() + }) +}) From 05c76651382df9dc9d894de2be11c7e17bfb3b43 Mon Sep 17 00:00:00 2001 From: Charmander <~@charmander.me> Date: Fri, 17 Jan 2020 08:49:06 -0800 Subject: [PATCH 05/11] =?UTF-8?q?Continue=20support=20for=20creating=20a?= =?UTF-8?q?=20pg.Pool=20from=20another=20instance=E2=80=99s=20options=20(#?= =?UTF-8?q?2076)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add failing test for creating a `BoundPool` from another instance’s settings * Continue support for creating a pg.Pool from another instance’s options by dropping the requirement for the `password` property to be enumerable. --- packages/pg/lib/index.js | 3 +-- .../unit/connection-pool/configuration-tests.js | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 packages/pg/test/unit/connection-pool/configuration-tests.js diff --git a/packages/pg/lib/index.js b/packages/pg/lib/index.js index c3062947d..c73064cf2 100644 --- a/packages/pg/lib/index.js +++ b/packages/pg/lib/index.js @@ -15,8 +15,7 @@ var Pool = require('pg-pool') const poolFactory = (Client) => { return class BoundPool extends Pool { constructor (options) { - var config = Object.assign({ Client: Client }, options) - super(config) + super(options, Client) } } } diff --git a/packages/pg/test/unit/connection-pool/configuration-tests.js b/packages/pg/test/unit/connection-pool/configuration-tests.js new file mode 100644 index 000000000..10c991839 --- /dev/null +++ b/packages/pg/test/unit/connection-pool/configuration-tests.js @@ -0,0 +1,14 @@ +'use strict' + +const assert = require('assert') +const helper = require('../test-helper') + +test('pool with copied settings includes password', () => { + const original = new helper.pg.Pool({ + password: 'original', + }) + + const copy = new helper.pg.Pool(original.options) + + assert.equal(copy.options.password, 'original') +}) From c26caa80d2289a50ee81bafb36f1768e95e9f4a4 Mon Sep 17 00:00:00 2001 From: Charmander <~@charmander.me> Date: Thu, 23 Jan 2020 09:03:50 -0800 Subject: [PATCH 06/11] Use user name as default database when user is non-default (#1679) Not entirely backwards-compatible. --- packages/pg/lib/connection-parameters.js | 5 ++++ packages/pg/lib/defaults.js | 2 +- .../integration/client/configuration-tests.js | 24 ++++++++++++++++++- .../connection-parameters/creation-tests.js | 7 +++++- 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/packages/pg/lib/connection-parameters.js b/packages/pg/lib/connection-parameters.js index c0f8498eb..cd6d3b8a9 100644 --- a/packages/pg/lib/connection-parameters.js +++ b/packages/pg/lib/connection-parameters.js @@ -52,6 +52,11 @@ var ConnectionParameters = function (config) { this.user = val('user', config) this.database = val('database', config) + + if (this.database === undefined) { + this.database = this.user + } + this.port = parseInt(val('port', config), 10) this.host = val('host', config) diff --git a/packages/pg/lib/defaults.js b/packages/pg/lib/defaults.js index 120b8c7b5..eb58550d6 100644 --- a/packages/pg/lib/defaults.js +++ b/packages/pg/lib/defaults.js @@ -15,7 +15,7 @@ module.exports = { user: process.platform === 'win32' ? process.env.USERNAME : process.env.USER, // name of database to connect - database: process.platform === 'win32' ? process.env.USERNAME : process.env.USER, + database: undefined, // database user's password password: null, diff --git a/packages/pg/test/integration/client/configuration-tests.js b/packages/pg/test/integration/client/configuration-tests.js index 87bb52d47..a6756ddee 100644 --- a/packages/pg/test/integration/client/configuration-tests.js +++ b/packages/pg/test/integration/client/configuration-tests.js @@ -14,7 +14,7 @@ for (var key in process.env) { suite.test('default values are used in new clients', function () { assert.same(pg.defaults, { user: process.env.USER, - database: process.env.USER, + database: undefined, password: null, port: 5432, rows: 0, @@ -54,6 +54,28 @@ suite.test('modified values are passed to created clients', function () { }) }) +suite.test('database defaults to user when user is non-default', () => { + { + pg.defaults.database = undefined + + const client = new Client({ + user: 'foo', + }) + + assert.strictEqual(client.database, 'foo') + } + + { + pg.defaults.database = 'bar' + + const client = new Client({ + user: 'foo', + }) + + assert.strictEqual(client.database, 'bar') + } +}) + suite.test('cleanup', () => { // restore process.env for (var key in realEnv) { diff --git a/packages/pg/test/unit/connection-parameters/creation-tests.js b/packages/pg/test/unit/connection-parameters/creation-tests.js index 5d200be0a..fdb4e6627 100644 --- a/packages/pg/test/unit/connection-parameters/creation-tests.js +++ b/packages/pg/test/unit/connection-parameters/creation-tests.js @@ -16,8 +16,13 @@ test('ConnectionParameters construction', function () { }) var compare = function (actual, expected, type) { + const expectedDatabase = + expected.database === undefined + ? expected.user + : expected.database + assert.equal(actual.user, expected.user, type + ' user') - assert.equal(actual.database, expected.database, type + ' database') + assert.equal(actual.database, expectedDatabase, type + ' database') assert.equal(actual.port, expected.port, type + ' port') assert.equal(actual.host, expected.host, type + ' host') assert.equal(actual.password, expected.password, type + ' password') From 94fbb24f94910b905e2aea793fbf4740d9cfca28 Mon Sep 17 00:00:00 2001 From: Charmander <~@charmander.me> Date: Thu, 30 Jan 2020 04:58:29 -0800 Subject: [PATCH 07/11] Make native client password property consistent with others i.e. configurable. --- packages/pg/lib/native/client.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/pg/lib/native/client.js b/packages/pg/lib/native/client.js index d06166573..ba9d141eb 100644 --- a/packages/pg/lib/native/client.js +++ b/packages/pg/lib/native/client.js @@ -46,11 +46,11 @@ var Client = module.exports = function (config) { // "hiding" the password so it doesn't show up in stack traces // or if the client is console.logged - const hiddenPassword = cp.password Object.defineProperty(this, 'password', { + configurable: true, enumerable: false, writable: true, - value: hiddenPassword + value: cp.password }) this.database = cp.database this.host = cp.host From 1d480517fe69e5bb15fa04618c6d8e377ee86e4c Mon Sep 17 00:00:00 2001 From: Brian C Date: Wed, 19 Feb 2020 12:16:17 -0600 Subject: [PATCH 08/11] Make notice messages not an instance of Error (#2090) * Make notice messages not an instance of Error Slight API cleanup to make a notice instance the same shape as it was, but not be an instance of error. This is a backwards incompatible change though I expect the impact to be minimal. Closes #1982 * skip notice test in travis * Pin node@13.6 for regression in async iterators * Check and see if node 13.8 is still borked on async iterator * Yeah, node still has changed edge case behavior on stream * Emit notice messages on travis --- .travis.yml | 2 +- packages/pg/lib/connection.js | 8 ++--- .../test/integration/client/notice-tests.js | 32 ++++++++++++------- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/.travis.yml b/.travis.yml index 61a7a79af..7ea347e9b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,7 +10,7 @@ env: node_js: - lts/dubnium - lts/erbium - - 13 + - 13.6 addons: postgresql: "10" diff --git a/packages/pg/lib/connection.js b/packages/pg/lib/connection.js index 435c1a965..ca904432c 100644 --- a/packages/pg/lib/connection.js +++ b/packages/pg/lib/connection.js @@ -597,7 +597,7 @@ Connection.prototype._readValue = function (buffer) { } // parses error -Connection.prototype.parseE = function (buffer, length) { +Connection.prototype.parseE = function (buffer, length, isNotice) { var fields = {} var fieldType = this.readString(buffer, 1) while (fieldType !== '\0') { @@ -606,10 +606,10 @@ Connection.prototype.parseE = function (buffer, length) { } // the msg is an Error instance - var msg = new Error(fields.M) + var msg = isNotice ? { message: fields.M } : new Error(fields.M) // for compatibility with Message - msg.name = 'error' + msg.name = isNotice ? 'notice' : 'error' msg.length = length msg.severity = fields.S @@ -633,7 +633,7 @@ Connection.prototype.parseE = function (buffer, length) { // same thing, different name Connection.prototype.parseN = function (buffer, length) { - var msg = this.parseE(buffer, length) + var msg = this.parseE(buffer, length, true) msg.name = 'notice' return msg } diff --git a/packages/pg/test/integration/client/notice-tests.js b/packages/pg/test/integration/client/notice-tests.js index f3dc5090e..a6fc8a56f 100644 --- a/packages/pg/test/integration/client/notice-tests.js +++ b/packages/pg/test/integration/client/notice-tests.js @@ -1,12 +1,13 @@ 'use strict' -var helper = require('./test-helper') +const helper = require('./test-helper') +const assert = require('assert') const suite = new helper.Suite() suite.test('emits notify message', function (done) { - var client = helper.client() + const client = helper.client() client.query('LISTEN boom', assert.calls(function () { - var otherClient = helper.client() - var bothEmitted = -1 + const otherClient = helper.client() + let bothEmitted = -1 otherClient.query('LISTEN boom', assert.calls(function () { assert.emits(client, 'notification', function (msg) { // make sure PQfreemem doesn't invalidate string pointers @@ -32,25 +33,34 @@ suite.test('emits notify message', function (done) { }) // this test fails on travis due to their config -suite.test('emits notice message', false, function (done) { +suite.test('emits notice message', function (done) { if (helper.args.native) { - console.error('need to get notice message working on native') + console.error('notice messages do not work curreintly with node-libpq') return done() } - // TODO this doesn't work on all versions of postgres - var client = helper.client() + + const client = helper.client() const text = ` DO language plpgsql $$ BEGIN - RAISE NOTICE 'hello, world!'; + RAISE NOTICE 'hello, world!' USING ERRCODE = '23505', DETAIL = 'this is a test'; END $$; ` - client.query(text, () => { - client.end() + client.query('SET SESSION client_min_messages=notice', (err) => { + assert.ifError(err) + client.query(text, () => { + client.end() + }) }) assert.emits(client, 'notice', function (notice) { assert.ok(notice != null) + // notice messages should not be error instances + assert(notice instanceof Error === false) + assert.strictEqual(notice.name, 'notice') + assert.strictEqual(notice.message, 'hello, world!') + assert.strictEqual(notice.detail, 'this is a test') + assert.strictEqual(notice.code, '23505') done() }) }) From 5341a2a157ff2d4c754d6ed545c5237358535c28 Mon Sep 17 00:00:00 2001 From: Charmander <~@charmander.me> Date: Tue, 25 Feb 2020 08:42:45 -0800 Subject: [PATCH 09/11] Revert "Revert "Support additional tls.connect() options (#1996)" (#2010)" (#2113) This reverts commit 510a273ce45fb73d0355cf384e97ea695c8a5bcc. --- packages/pg/lib/connection-fast.js | 20 +++----------------- packages/pg/lib/connection.js | 20 +++----------------- 2 files changed, 6 insertions(+), 34 deletions(-) diff --git a/packages/pg/lib/connection-fast.js b/packages/pg/lib/connection-fast.js index a31d92a20..631ea3b0e 100644 --- a/packages/pg/lib/connection-fast.js +++ b/packages/pg/lib/connection-fast.js @@ -15,8 +15,6 @@ var Writer = require('buffer-writer') // eslint-disable-next-line var PacketStream = require('pg-packet-stream') -var warnDeprecation = require('./compat/warn-deprecation') - var TEXT_MODE = 0 // TODO(bmc) support binary mode here @@ -95,21 +93,9 @@ Connection.prototype.connect = function (port, host) { return self.emit('error', new Error('There was an error establishing an SSL connection')) } var tls = require('tls') - const options = { - socket: self.stream, - checkServerIdentity: self.ssl.checkServerIdentity || tls.checkServerIdentity, - rejectUnauthorized: self.ssl.rejectUnauthorized, - ca: self.ssl.ca, - pfx: self.ssl.pfx, - key: self.ssl.key, - passphrase: self.ssl.passphrase, - cert: self.ssl.cert, - secureOptions: self.ssl.secureOptions, - NPNProtocols: self.ssl.NPNProtocols - } - if (typeof self.ssl.rejectUnauthorized !== 'boolean') { - warnDeprecation('Implicit disabling of certificate verification is deprecated and will be removed in pg 8. Specify `rejectUnauthorized: true` to require a valid CA or `rejectUnauthorized: false` to explicitly opt out of MITM protection.', 'PG-SSL-VERIFY') - } + const options = Object.assign({ + socket: self.stream + }, self.ssl) if (net.isIP(host) === 0) { options.servername = host } diff --git a/packages/pg/lib/connection.js b/packages/pg/lib/connection.js index ca904432c..03b85bf50 100644 --- a/packages/pg/lib/connection.js +++ b/packages/pg/lib/connection.js @@ -14,8 +14,6 @@ var util = require('util') var Writer = require('buffer-writer') var Reader = require('packet-reader') -var warnDeprecation = require('./compat/warn-deprecation') - var TEXT_MODE = 0 var BINARY_MODE = 1 var Connection = function (config) { @@ -93,21 +91,9 @@ Connection.prototype.connect = function (port, host) { return self.emit('error', new Error('There was an error establishing an SSL connection')) } var tls = require('tls') - const options = { - socket: self.stream, - checkServerIdentity: self.ssl.checkServerIdentity || tls.checkServerIdentity, - rejectUnauthorized: self.ssl.rejectUnauthorized, - ca: self.ssl.ca, - pfx: self.ssl.pfx, - key: self.ssl.key, - passphrase: self.ssl.passphrase, - cert: self.ssl.cert, - secureOptions: self.ssl.secureOptions, - NPNProtocols: self.ssl.NPNProtocols - } - if (typeof self.ssl.rejectUnauthorized !== 'boolean') { - warnDeprecation('Implicit disabling of certificate verification is deprecated and will be removed in pg 8. Specify `rejectUnauthorized: true` to require a valid CA or `rejectUnauthorized: false` to explicitly opt out of MITM protection.', 'PG-SSL-VERIFY') - } + const options = Object.assign({ + socket: self.stream + }, self.ssl) if (net.isIP(host) === 0) { options.servername = host } From c5ea02eae1cb80e52509f289f4d8c166fc25f76a Mon Sep 17 00:00:00 2001 From: Brian C Date: Tue, 25 Feb 2020 11:25:12 -0600 Subject: [PATCH 10/11] Fix ssl tests (#2116) --- .../pg/test/integration/gh-issues/2085-tests.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/pg/test/integration/gh-issues/2085-tests.js b/packages/pg/test/integration/gh-issues/2085-tests.js index 36f30c747..8ccdca150 100644 --- a/packages/pg/test/integration/gh-issues/2085-tests.js +++ b/packages/pg/test/integration/gh-issues/2085-tests.js @@ -7,9 +7,23 @@ var assert = require('assert') const suite = new helper.Suite() suite.testAsync('it should connect over ssl', async () => { - const client = new helper.pg.Client({ ssl: 'require'}) + const ssl = helper.args.native ? 'require' : { + rejectUnauthorized: false + } + const client = new helper.pg.Client({ ssl }) await client.connect() const { rows } = await client.query('SELECT NOW()') assert.strictEqual(rows.length, 1) await client.end() }) + +suite.testAsync('it should fail with self-signed cert error w/o rejectUnauthorized being passed', async () => { + const ssl = helper.args.native ? 'verify-ca' : { } + const client = new helper.pg.Client({ ssl }) + try { + await client.connect() + } catch (e) { + return; + } + throw new Error('this test should have thrown an error due to self-signed cert') +}) From 4b0c21d82fcf10be6cf0bd0bec5967764c23606b Mon Sep 17 00:00:00 2001 From: Charmander <~@charmander.me> Date: Thu, 12 Mar 2020 08:30:16 -0700 Subject: [PATCH 11/11] Convert Query to an ES6 class (#2126) The last missing `new` deprecation warning for pg 8. --- packages/pg/lib/compat/check-constructor.js | 22 -- packages/pg/lib/compat/warn-deprecation.js | 19 - packages/pg/lib/query.js | 374 ++++++++++---------- 3 files changed, 184 insertions(+), 231 deletions(-) delete mode 100644 packages/pg/lib/compat/check-constructor.js delete mode 100644 packages/pg/lib/compat/warn-deprecation.js diff --git a/packages/pg/lib/compat/check-constructor.js b/packages/pg/lib/compat/check-constructor.js deleted file mode 100644 index 5920633a0..000000000 --- a/packages/pg/lib/compat/check-constructor.js +++ /dev/null @@ -1,22 +0,0 @@ -'use strict' - -const warnDeprecation = require('./warn-deprecation') - -// Node 4 doesn’t support new.target. -let hasNewTarget - -try { - // eslint-disable-next-line no-eval - eval('(function () { new.target })') - hasNewTarget = true -} catch (error) { - hasNewTarget = false -} - -const checkConstructor = (name, code, getNewTarget) => { - if (hasNewTarget && getNewTarget() === undefined) { - warnDeprecation(`Constructing a ${name} without new is deprecated and will stop working in pg 8.`, code) - } -} - -module.exports = checkConstructor diff --git a/packages/pg/lib/compat/warn-deprecation.js b/packages/pg/lib/compat/warn-deprecation.js deleted file mode 100644 index 558275900..000000000 --- a/packages/pg/lib/compat/warn-deprecation.js +++ /dev/null @@ -1,19 +0,0 @@ -'use strict' - -const util = require('util') - -const dummyFunctions = new Map() - -// Node 4 doesn’t support process.emitWarning(message, 'DeprecationWarning', code). -const warnDeprecation = (message, code) => { - let dummy = dummyFunctions.get(code) - - if (dummy === undefined) { - dummy = util.deprecate(() => {}, message) - dummyFunctions.set(code, dummy) - } - - dummy() -} - -module.exports = warnDeprecation diff --git a/packages/pg/lib/query.js b/packages/pg/lib/query.js index 548380fe1..4fcfe391e 100644 --- a/packages/pg/lib/query.js +++ b/packages/pg/lib/query.js @@ -7,226 +7,220 @@ * README.md file in the root directory of this source tree. */ -var EventEmitter = require('events').EventEmitter -var util = require('util') -const checkConstructor = require('./compat/check-constructor') - -var Result = require('./result') -var utils = require('./utils') - -var Query = function (config, values, callback) { - // use of "new" optional in pg 7 - // eslint-disable-next-line no-eval - checkConstructor('Query', 'PG-QUERY-NEW', () => eval('new.target')) - if (!(this instanceof Query)) { return new Query(config, values, callback) } - - config = utils.normalizeQueryConfig(config, values, callback) - - this.text = config.text - this.values = config.values - this.rows = config.rows - this.types = config.types - this.name = config.name - this.binary = config.binary - // use unique portal name each time - this.portal = config.portal || '' - this.callback = config.callback - this._rowMode = config.rowMode - if (process.domain && config.callback) { - this.callback = process.domain.bind(config.callback) - } - this._result = new Result(this._rowMode, this.types) - - // potential for multiple results - this._results = this._result - this.isPreparedStatement = false - this._canceledDueToError = false - this._promise = null - EventEmitter.call(this) -} - -util.inherits(Query, EventEmitter) - -Query.prototype.requiresPreparation = function () { - // named queries must always be prepared - if (this.name) { return true } - // always prepare if there are max number of rows expected per - // portal execution - if (this.rows) { return true } - // don't prepare empty text queries - if (!this.text) { return false } - // prepare if there are values - if (!this.values) { return false } - return this.values.length > 0 -} - -Query.prototype._checkForMultirow = function () { - // if we already have a result with a command property - // then we've already executed one query in a multi-statement simple query - // turn our results into an array of results - if (this._result.command) { - if (!Array.isArray(this._results)) { - this._results = [this._result] +const { EventEmitter } = require('events') + +const Result = require('./result') +const utils = require('./utils') + +class Query extends EventEmitter { + constructor(config, values, callback) { + super() + + config = utils.normalizeQueryConfig(config, values, callback) + + this.text = config.text + this.values = config.values + this.rows = config.rows + this.types = config.types + this.name = config.name + this.binary = config.binary + // use unique portal name each time + this.portal = config.portal || '' + this.callback = config.callback + this._rowMode = config.rowMode + if (process.domain && config.callback) { + this.callback = process.domain.bind(config.callback) } this._result = new Result(this._rowMode, this.types) - this._results.push(this._result) + + // potential for multiple results + this._results = this._result + this.isPreparedStatement = false + this._canceledDueToError = false + this._promise = null + } + + requiresPreparation() { + // named queries must always be prepared + if (this.name) { return true } + // always prepare if there are max number of rows expected per + // portal execution + if (this.rows) { return true } + // don't prepare empty text queries + if (!this.text) { return false } + // prepare if there are values + if (!this.values) { return false } + return this.values.length > 0 + } + + _checkForMultirow() { + // if we already have a result with a command property + // then we've already executed one query in a multi-statement simple query + // turn our results into an array of results + if (this._result.command) { + if (!Array.isArray(this._results)) { + this._results = [this._result] + } + this._result = new Result(this._rowMode, this.types) + this._results.push(this._result) + } } -} -// associates row metadata from the supplied -// message with this query object -// metadata used when parsing row results -Query.prototype.handleRowDescription = function (msg) { - this._checkForMultirow() - this._result.addFields(msg.fields) - this._accumulateRows = this.callback || !this.listeners('row').length -} + // associates row metadata from the supplied + // message with this query object + // metadata used when parsing row results + handleRowDescription(msg) { + this._checkForMultirow() + this._result.addFields(msg.fields) + this._accumulateRows = this.callback || !this.listeners('row').length + } -Query.prototype.handleDataRow = function (msg) { - var row + handleDataRow(msg) { + let row - if (this._canceledDueToError) { - return - } + if (this._canceledDueToError) { + return + } - try { - row = this._result.parseRow(msg.fields) - } catch (err) { - this._canceledDueToError = err - return - } + try { + row = this._result.parseRow(msg.fields) + } catch (err) { + this._canceledDueToError = err + return + } - this.emit('row', row, this._result) - if (this._accumulateRows) { - this._result.addRow(row) + this.emit('row', row, this._result) + if (this._accumulateRows) { + this._result.addRow(row) + } } -} -Query.prototype.handleCommandComplete = function (msg, con) { - this._checkForMultirow() - this._result.addCommandComplete(msg) - // need to sync after each command complete of a prepared statement - if (this.isPreparedStatement) { - con.sync() + handleCommandComplete(msg, con) { + this._checkForMultirow() + this._result.addCommandComplete(msg) + // need to sync after each command complete of a prepared statement + if (this.isPreparedStatement) { + con.sync() + } } -} -// if a named prepared statement is created with empty query text -// the backend will send an emptyQuery message but *not* a command complete message -// execution on the connection will hang until the backend receives a sync message -Query.prototype.handleEmptyQuery = function (con) { - if (this.isPreparedStatement) { - con.sync() + // if a named prepared statement is created with empty query text + // the backend will send an emptyQuery message but *not* a command complete message + // execution on the connection will hang until the backend receives a sync message + handleEmptyQuery(con) { + if (this.isPreparedStatement) { + con.sync() + } } -} -Query.prototype.handleReadyForQuery = function (con) { - if (this._canceledDueToError) { - return this.handleError(this._canceledDueToError, con) - } - if (this.callback) { - this.callback(null, this._results) + handleReadyForQuery(con) { + if (this._canceledDueToError) { + return this.handleError(this._canceledDueToError, con) + } + if (this.callback) { + this.callback(null, this._results) + } + this.emit('end', this._results) } - this.emit('end', this._results) -} -Query.prototype.handleError = function (err, connection) { - // need to sync after error during a prepared statement - if (this.isPreparedStatement) { - connection.sync() - } - if (this._canceledDueToError) { - err = this._canceledDueToError - this._canceledDueToError = false - } - // if callback supplied do not emit error event as uncaught error - // events will bubble up to node process - if (this.callback) { - return this.callback(err) + handleError(err, connection) { + // need to sync after error during a prepared statement + if (this.isPreparedStatement) { + connection.sync() + } + if (this._canceledDueToError) { + err = this._canceledDueToError + this._canceledDueToError = false + } + // if callback supplied do not emit error event as uncaught error + // events will bubble up to node process + if (this.callback) { + return this.callback(err) + } + this.emit('error', err) } - this.emit('error', err) -} -Query.prototype.submit = function (connection) { - if (typeof this.text !== 'string' && typeof this.name !== 'string') { - return new Error('A query must have either text or a name. Supplying neither is unsupported.') - } - const previous = connection.parsedStatements[this.name] - if (this.text && previous && this.text !== previous) { - return new Error(`Prepared statements must be unique - '${this.name}' was used for a different statement`) + submit(connection) { + if (typeof this.text !== 'string' && typeof this.name !== 'string') { + return new Error('A query must have either text or a name. Supplying neither is unsupported.') + } + const previous = connection.parsedStatements[this.name] + if (this.text && previous && this.text !== previous) { + return new Error(`Prepared statements must be unique - '${this.name}' was used for a different statement`) + } + if (this.values && !Array.isArray(this.values)) { + return new Error('Query values must be an array') + } + if (this.requiresPreparation()) { + this.prepare(connection) + } else { + connection.query(this.text) + } + return null } - if (this.values && !Array.isArray(this.values)) { - return new Error('Query values must be an array') + + hasBeenParsed(connection) { + return this.name && connection.parsedStatements[this.name] } - if (this.requiresPreparation()) { - this.prepare(connection) - } else { - connection.query(this.text) + + handlePortalSuspended(connection) { + this._getRows(connection, this.rows) } - return null -} -Query.prototype.hasBeenParsed = function (connection) { - return this.name && connection.parsedStatements[this.name] -} + _getRows(connection, rows) { + connection.execute({ + portal: this.portal, + rows: rows + }, true) + connection.flush() + } + + prepare(connection) { + // prepared statements need sync to be called after each command + // complete or when an error is encountered + this.isPreparedStatement = true + // TODO refactor this poor encapsulation + if (!this.hasBeenParsed(connection)) { + connection.parse({ + text: this.text, + name: this.name, + types: this.types + }, true) + } -Query.prototype.handlePortalSuspended = function (connection) { - this._getRows(connection, this.rows) -} + if (this.values) { + try { + this.values = this.values.map(utils.prepareValue) + } catch (err) { + this.handleError(err, connection) + return + } + } -Query.prototype._getRows = function (connection, rows) { - connection.execute({ - portal: this.portal, - rows: rows - }, true) - connection.flush() -} + // http://developer.postgresql.org/pgdocs/postgres/protocol-flow.html#PROTOCOL-FLOW-EXT-QUERY + connection.bind({ + portal: this.portal, + statement: this.name, + values: this.values, + binary: this.binary + }, true) -Query.prototype.prepare = function (connection) { - var self = this - // prepared statements need sync to be called after each command - // complete or when an error is encountered - this.isPreparedStatement = true - // TODO refactor this poor encapsulation - if (!this.hasBeenParsed(connection)) { - connection.parse({ - text: self.text, - name: self.name, - types: self.types + connection.describe({ + type: 'P', + name: this.portal || '' }, true) - } - if (self.values) { - try { - self.values = self.values.map(utils.prepareValue) - } catch (err) { - this.handleError(err, connection) - return - } + this._getRows(connection, this.rows) } - // http://developer.postgresql.org/pgdocs/postgres/protocol-flow.html#PROTOCOL-FLOW-EXT-QUERY - connection.bind({ - portal: self.portal, - statement: self.name, - values: self.values, - binary: self.binary - }, true) - - connection.describe({ - type: 'P', - name: self.portal || '' - }, true) - - this._getRows(connection, this.rows) -} + handleCopyInResponse(connection) { + connection.sendCopyFail('No source stream defined') + } -Query.prototype.handleCopyInResponse = function (connection) { - connection.sendCopyFail('No source stream defined') + // eslint-disable-next-line no-unused-vars + handleCopyData(msg, connection) { + // noop + } } -// eslint-disable-next-line no-unused-vars -Query.prototype.handleCopyData = function (msg, connection) { - // noop -} module.exports = Query