Skip to content

Remove password from stringified outputs #2066

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jan 13, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 34 additions & 19 deletions packages/pg-pool/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,20 @@ const removeWhere = (list, predicate) => {
}

class IdleItem {
constructor (client, idleListener, timeoutId) {
constructor(client, idleListener, timeoutId) {
this.client = client
this.idleListener = idleListener
this.timeoutId = timeoutId
}
}

class PendingItem {
constructor (callback) {
constructor(callback) {
this.callback = callback
}
}

function promisify (Promise, callback) {
function promisify(Promise, callback) {
if (callback) {
return { callback: callback, result: undefined }
}
Expand All @@ -41,8 +41,8 @@ function promisify (Promise, callback) {
return { callback: cb, result: result }
}

function makeIdleListener (pool, client) {
return function idleListener (err) {
function makeIdleListener(pool, client) {
return function idleListener(err) {
err.client = client

client.removeListener('error', idleListener)
Expand All @@ -57,9 +57,24 @@ function makeIdleListener (pool, client) {
}

class Pool extends EventEmitter {
constructor (options, Client) {
constructor(options, Client) {
super()
this.options = Object.assign({}, options)

// "hiding" the password so it doesn't show up in stack traces
// or if the client is console.logged
const optionsWithPasswordHidden = Object.assign({}, options)
let password = optionsWithPasswordHidden.password
Object.defineProperty(optionsWithPasswordHidden, 'password', {
enumerable: false,
get() {
return password
},
set(value) {
password = value
}
})

this.options = optionsWithPasswordHidden
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
Expand All @@ -77,11 +92,11 @@ class Pool extends EventEmitter {
this.ended = false
}

_isFull () {
_isFull() {
return this._clients.length >= this.options.max
}

_pulseQueue () {
_pulseQueue() {
this.log('pulse queue')
if (this.ended) {
this.log('pulse queue ended')
Expand Down Expand Up @@ -124,7 +139,7 @@ class Pool extends EventEmitter {
throw new Error('unexpected condition')
}

_remove (client) {
_remove(client) {
const removed = removeWhere(
this._idle,
item => item.client === client
Expand All @@ -139,7 +154,7 @@ class Pool extends EventEmitter {
this.emit('remove', client)
}

connect (cb) {
connect(cb) {
if (this.ending) {
const err = new Error('Cannot use a pool after calling end on the pool')
return cb ? cb(err) : this.Promise.reject(err)
Expand Down Expand Up @@ -185,7 +200,7 @@ class Pool extends EventEmitter {
return result
}

newClient (pendingItem) {
newClient(pendingItem) {
const client = new this.Client(this.options)
this._clients.push(client)
const idleListener = makeIdleListener(this, client)
Expand Down Expand Up @@ -233,7 +248,7 @@ class Pool extends EventEmitter {
}

// acquire a client for a pending work item
_acquireClient (client, pendingItem, idleListener, isNew) {
_acquireClient(client, pendingItem, idleListener, isNew) {
if (isNew) {
this.emit('connect', client)
}
Expand Down Expand Up @@ -277,7 +292,7 @@ class Pool extends EventEmitter {

// release a client back to the poll, include an error
// to remove it from the pool
_release (client, idleListener, err) {
_release(client, idleListener, err) {
client.on('error', idleListener)

if (err || this.ending) {
Expand All @@ -299,7 +314,7 @@ class Pool extends EventEmitter {
this._pulseQueue()
}

query (text, values, cb) {
query(text, values, cb) {
// guard clause against passing a function as the first parameter
if (typeof text === 'function') {
const response = promisify(this.Promise, text)
Expand Down Expand Up @@ -352,7 +367,7 @@ class Pool extends EventEmitter {
return response.result
}

end (cb) {
end(cb) {
this.log('ending')
if (this.ending) {
const err = new Error('Called end on pool more than once')
Expand All @@ -365,15 +380,15 @@ class Pool extends EventEmitter {
return promised.result
}

get waitingCount () {
get waitingCount() {
return this._pendingQueue.length
}

get idleCount () {
get idleCount() {
return this._idle.length
}

get totalCount () {
get totalCount() {
return this._clients.length
}
}
Expand Down
16 changes: 15 additions & 1 deletion packages/pg/lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,21 @@ 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
let password = this.connectionParameters.password
Object.defineProperty(this, 'password', {
enumerable: false,
configurable: false,
get() {
return password
},
set(value) {
password = value
}
})

this.replication = this.connectionParameters.replication

var c = config || {}
Expand Down
13 changes: 12 additions & 1 deletion packages/pg/lib/connection-parameters.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,18 @@ 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
const password = val('password', config)
Object.defineProperty(this, 'password', {
enumerable: false,
configurable: false,
get() {
return password
}
})

this.binary = val('binary', config)
this.ssl = typeof config.ssl === 'undefined' ? useSsl() : config.ssl
this.client_encoding = val('client_encoding', config)
Expand Down
18 changes: 15 additions & 3 deletions packages/pg/lib/native/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,19 @@ 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
let hiddenPassword = cp.password
Object.defineProperty(this, 'password', {
enumerable: false,
get() {
return hiddenPassword
},
set(value) {
hiddenPassword = value
}
})
this.database = cp.database
this.host = cp.host
this.port = cp.port
Expand Down Expand Up @@ -187,7 +199,7 @@ Client.prototype.query = function (config, values, callback) {

// we already returned an error,
// just do nothing if query completes
query.callback = () => {}
query.callback = () => { }

// Remove from queue
var index = this._queryQueue.indexOf(query)
Expand Down Expand Up @@ -280,7 +292,7 @@ Client.prototype._pulseQueryQueue = function (initialConnection) {
// attempt to cancel an in-progress query
Client.prototype.cancel = function (query) {
if (this._activeQuery === query) {
this.native.cancel(function () {})
this.native.cancel(function () { })
} else if (this._queryQueue.indexOf(query) !== -1) {
this._queryQueue.splice(this._queryQueue.indexOf(query), 1)
}
Expand Down
32 changes: 32 additions & 0 deletions packages/pg/test/integration/gh-issues/2064-tests.js
Original file line number Diff line number Diff line change
@@ -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);
})