From aafd8ac64e588e689ed08e7957bc3c91f8fe01e3 Mon Sep 17 00:00:00 2001 From: Brian C Date: Mon, 30 Mar 2020 10:31:35 -0500 Subject: [PATCH] 8.0 Release (#2117) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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> * Remove password from stringified outputs (#2066) * 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> * 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 * 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 * Continue support for creating a pg.Pool from another instance’s options (#2076) * 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. * Use user name as default database when user is non-default (#1679) Not entirely backwards-compatible. * Make native client password property consistent with others i.e. configurable. * 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 * Revert "Revert "Support additional tls.connect() options (#1996)" (#2010)" (#2113) This reverts commit 510a273ce45fb73d0355cf384e97ea695c8a5bcc. * Fix ssl tests (#2116) * Convert Query to an ES6 class (#2126) The last missing `new` deprecation warning for pg 8. Co-authored-by: Charmander <~@charmander.me> Co-authored-by: Gabe Gorelick Co-authored-by: Natalie Wolfe --- packages/pg-pool/index.js | 12 + packages/pg-pool/package.json | 2 +- packages/pg/Makefile | 4 +- packages/pg/lib/client.js | 11 +- packages/pg/lib/compat/check-constructor.js | 22 -- packages/pg/lib/compat/warn-deprecation.js | 19 - packages/pg/lib/connection-fast.js | 20 +- packages/pg/lib/connection-parameters.js | 16 +- packages/pg/lib/connection.js | 28 +- packages/pg/lib/defaults.js | 2 +- packages/pg/lib/index.js | 50 +-- packages/pg/lib/native/client.js | 10 +- packages/pg/lib/query.js | 374 +++++++++--------- packages/pg/package.json | 2 +- .../integration/client/configuration-tests.js | 24 +- .../test/integration/client/notice-tests.js | 32 +- .../test/integration/gh-issues/1542-tests.js | 25 ++ .../test/integration/gh-issues/1992-tests.js | 11 + .../test/integration/gh-issues/2064-tests.js | 32 ++ .../test/integration/gh-issues/2085-tests.js | 16 +- .../connection-parameters/creation-tests.js | 7 +- .../connection-pool/configuration-tests.js | 14 + 22 files changed, 416 insertions(+), 317 deletions(-) delete mode 100644 packages/pg/lib/compat/check-constructor.js delete mode 100644 packages/pg/lib/compat/warn-deprecation.js create mode 100644 packages/pg/test/integration/gh-issues/1542-tests.js create mode 100644 packages/pg/test/integration/gh-issues/1992-tests.js create mode 100644 packages/pg/test/integration/gh-issues/2064-tests.js create mode 100644 packages/pg/test/unit/connection-pool/configuration-tests.js 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/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/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/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 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/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/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') +}) 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') +})