diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js index 83ec51e09..e144bb83b 100644 --- a/packages/pg-pool/index.js +++ b/packages/pg-pool/index.js @@ -64,6 +64,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-pool/package.json b/packages/pg-pool/package.json index 3813df242..8d5cf2a9d 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/lib/client.js b/packages/pg/lib/client.js index cdae3b7c2..ac7ab4c27 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-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-parameters.js b/packages/pg/lib/connection-parameters.js index 0d5e0376d..cd6d3b8a9 100644 --- a/packages/pg/lib/connection-parameters.js +++ b/packages/pg/lib/connection-parameters.js @@ -52,9 +52,23 @@ 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) - 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/connection.js b/packages/pg/lib/connection.js index a63d9cde7..b7fde90a2 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) { @@ -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 } @@ -602,7 +588,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') { @@ -611,10 +597,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 @@ -638,7 +624,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/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/lib/index.js b/packages/pg/lib/index.js index de33c086d..c73064cf2 100644 --- a/packages/pg/lib/index.js +++ b/packages/pg/lib/index.js @@ -7,25 +7,17 @@ * 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) { + super(options, Client) + } } - - util.inherits(BoundPool, Pool) - - return BoundPool } var PG = function (clientConstructor) { @@ -44,20 +36,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/lib/native/client.js b/packages/pg/lib/native/client.js index 581ef72d1..165147f9b 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 + Object.defineProperty(this, 'password', { + configurable: true, + enumerable: false, + writable: true, + value: cp.password + }) this.database = cp.database this.host = cp.host this.port = cp.port diff --git a/packages/pg/package.json b/packages/pg/package.json index 9e06af528..6aec51616 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" } } 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/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() }) }) diff --git a/packages/pg/test/integration/client/ssl-tests.js b/packages/pg/test/integration/client/ssl-tests.js index bd864d1e1..d582d0b2e 100644 --- a/packages/pg/test/integration/client/ssl-tests.js +++ b/packages/pg/test/integration/client/ssl-tests.js @@ -1,15 +1,28 @@ 'use strict' -var pg = require(__dirname + '/../../../lib') -var config = require(__dirname + '/test-helper').config -test('can connect with ssl', function () { - return false - config.ssl = { - rejectUnauthorized: false +const { pg, Suite } = require('./test-helper') +const assert = require('assert') + +const suite = new Suite() + +suite.testAsync('it can connect w/ ssl', async () => { + const ssl = { + rejectUnauthorized: false, } - pg.connect(config, assert.success(function (client) { - return false - client.query('SELECT NOW()', assert.success(function () { - pg.end() - })) - })) + + const client = new pg.Client({ ssl }) + await client.connect() + assert.strictEqual(client.connection.stream.encrypted, true) + await client.end() +}) + +suite.testAsync('it rejects on self-signed cert', async () => { + const ssl = true + const client = new pg.Client({ ssl }) + try { + await client.connect() + } catch (e) { + assert(e.message.includes('self signed')) + return + } + throw new Error('connection should have failed with self-signed cert error') }) 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() + }) +}) 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) +}) 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); +}) 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') 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') +})