diff --git a/README.md b/README.md index 308e973..6bb0fe0 100644 --- a/README.md +++ b/README.md @@ -72,7 +72,7 @@ var config = { //this initializes a connection pool -//it will keep idle connections open for a 30 seconds +//it will keep idle connections open for 30 seconds //and set a limit of maximum 10 idle clients var pool = new pg.Pool(config); @@ -83,8 +83,8 @@ pool.connect(function(err, client, done) { return console.error('error fetching client from pool', err); } client.query('SELECT $1::int AS number', ['1'], function(err, result) { - //call `done()` to release the client back to the pool - done(); + //call `done(err)` to release the client back to the pool (or destroy it if there is an error) + done(err); if(err) { return console.error('error running query', err); diff --git a/lib/client.js b/lib/client.js index ab3bd47..86206ab 100644 --- a/lib/client.js +++ b/lib/client.js @@ -173,7 +173,7 @@ Client.prototype.connect = function(callback) { self.readyForQuery = true; self._pulseQueryQueue(); if(activeQuery) { - activeQuery.handleReadyForQuery(); + activeQuery.handleReadyForQuery(con); } }); @@ -186,6 +186,7 @@ Client.prototype.connect = function(callback) { if(!callback) { return self.emit('error', error); } + con.end(); // make sure ECONNRESET errors don't cause error events callback(error); callback = null; }); diff --git a/lib/connection.js b/lib/connection.js index 26819ff..72545e6 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -15,6 +15,21 @@ var defaults = require('./defaults'); var Writer = require('buffer-writer'); var Reader = require('packet-reader'); +var indexOf = + 'indexOf' in Buffer.prototype ? + function indexOf(buffer, value, start) { + return buffer.indexOf(value, start); + } : + function indexOf(buffer, value, start) { + for (var i = start, len = buffer.length; i < len; i++) { + if (buffer[i] === value) { + return i; + } + } + + return -1; + }; + var TEXT_MODE = 0; var BINARY_MODE = 1; var Connection = function(config) { @@ -653,8 +668,9 @@ Connection.prototype.readBytes = function(buffer, length) { Connection.prototype.parseCString = function(buffer) { var start = this.offset; - while(buffer[this.offset++] !== 0) { } - return buffer.toString(this.encoding, start, this.offset - 1); + var end = indexOf(buffer, 0, start); + this.offset = end + 1; + return buffer.toString(this.encoding, start, end); }; //end parsing methods module.exports = Connection; diff --git a/lib/query.js b/lib/query.js index 36d52ba..f627860 100644 --- a/lib/query.js +++ b/lib/query.js @@ -80,7 +80,19 @@ Query.prototype.handleRowDescription = function(msg) { }; Query.prototype.handleDataRow = function(msg) { - var row = this._result.parseRow(msg.fields); + var row; + + if (this._canceledDueToError) { + 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); @@ -104,9 +116,9 @@ Query.prototype.handleEmptyQuery = function(con) { } }; -Query.prototype.handleReadyForQuery = function() { +Query.prototype.handleReadyForQuery = function(con) { if(this._canceledDueToError) { - return this.handleError(this._canceledDueToError); + return this.handleError(this._canceledDueToError, con); } if(this.callback) { this.callback(null, this._result); diff --git a/lib/result.js b/lib/result.js index 463fbdb..2955d92 100644 --- a/lib/result.js +++ b/lib/result.js @@ -7,6 +7,7 @@ */ var types = require('pg-types'); +var escape = require('js-string-escape'); //result object returned from query //in the 'end' event and also @@ -75,13 +76,13 @@ Result.prototype.addRow = function(row) { var inlineParser = function(fieldName, i) { return "\nthis['" + - //fields containing single quotes will break - //the evaluated javascript unless they are escaped - //see https://github.com/brianc/node-postgres/issues/507 - //Addendum: However, we need to make sure to replace all - //occurences of apostrophes, not just the first one. - //See https://github.com/brianc/node-postgres/issues/934 - fieldName.replace(/'/g, "\\'") + + // fields containing single quotes will break + // the evaluated javascript unless they are escaped + // see https://github.com/brianc/node-postgres/issues/507 + // Addendum: However, we need to make sure to replace all + // occurences of apostrophes, not just the first one. + // See https://github.com/brianc/node-postgres/issues/934 + escape(fieldName) + "'] = " + "rowData[" + i + "] == null ? null : parsers[" + i + "](rowData[" + i + "]);"; }; diff --git a/package.json b/package.json index 017e04c..5d9aef0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "pg", - "version": "6.1.2", + "version": "6.1.6", "description": "PostgreSQL client - pure javascript & libpq with the same API", "keywords": [ "postgres", @@ -19,6 +19,7 @@ "main": "./lib", "dependencies": { "buffer-writer": "1.0.1", + "js-string-escape": "1.0.1", "packet-reader": "0.2.0", "pg-connection-string": "0.1.3", "pg-pool": "1.*", diff --git a/test/integration/client/field-name-escape-tests.js b/test/integration/client/field-name-escape-tests.js new file mode 100644 index 0000000..146ad1b --- /dev/null +++ b/test/integration/client/field-name-escape-tests.js @@ -0,0 +1,10 @@ +var pg = require('./test-helper').pg + +var sql = 'SELECT 1 AS "\\\'/*", 2 AS "\\\'*/\n + process.exit(-1)] = null;\n//"' + +var client = new pg.Client() +client.connect() +client.query(sql, function (err, res) { + if (err) throw err + client.end() +}) diff --git a/test/integration/client/heroku-pgpass-tests.js b/test/integration/client/heroku-pgpass-tests.js deleted file mode 100644 index 578342f..0000000 --- a/test/integration/client/heroku-pgpass-tests.js +++ /dev/null @@ -1,39 +0,0 @@ -var helper = require(__dirname + '/../test-helper'); - -// Path to the password file -var passfile = __dirname + '/heroku.pgpass'; - -// Export the path to the password file -process.env.PGPASSFILE = passfile; - -// Do a chmod 660, because git doesn't track those permissions -require('fs').chmodSync(passfile, 384); - -var pg = helper.pg; - -var host = 'ec2-107-20-224-218.compute-1.amazonaws.com'; -var database = 'db6kfntl5qhp2'; -var user = 'kwdzdnqpdiilfs'; - -var config = { - host: host, - database: database, - user: user, - ssl: true -}; - -test('uses password file when PGPASSFILE env variable is set', function() { - // connect & disconnect from heroku - pg.connect(config, assert.calls(function(err, client, done) { - assert.isNull(err); - client.query('SELECT NOW() as time', assert.success(function(res) { - assert(res.rows[0].time.getTime()); - - // cleanup ... remove the env variable - delete process.env.PGPASSFILE; - - done(); - pg.end(); - })) - })); -}); diff --git a/test/integration/client/heroku-ssl-tests.js b/test/integration/client/heroku-ssl-tests.js deleted file mode 100644 index dce3701..0000000 --- a/test/integration/client/heroku-ssl-tests.js +++ /dev/null @@ -1,28 +0,0 @@ -var helper = require(__dirname + '/../test-helper'); -var pg = helper.pg; - -var host = 'ec2-107-20-224-218.compute-1.amazonaws.com'; -var database = 'db6kfntl5qhp2'; -var user = 'kwdzdnqpdiilfs'; -var port = 5432; - -var config = { - host: host, - port: port, - database: database, - user: user, - password: 'uaZoSSHgi7mVM7kYaROtusClKu', - ssl: true -}; - -test('connection with config ssl = true', function() { - //connect & disconnect from heroku - pg.connect(config, assert.calls(function(err, client, done) { - assert.isNull(err); - client.query('SELECT NOW() as time', assert.success(function(res) { - assert(res.rows[0].time.getTime()); - done(); - pg.end(); - })) - })); -}); diff --git a/test/integration/client/heroku.pgpass b/test/integration/client/heroku.pgpass deleted file mode 100644 index 39bba52..0000000 --- a/test/integration/client/heroku.pgpass +++ /dev/null @@ -1 +0,0 @@ -ec2-107-20-224-218.compute-1.amazonaws.com:5432:db6kfntl5qhp2:kwdzdnqpdiilfs:uaZoSSHgi7mVM7kYaROtusClKu diff --git a/test/unit/client/throw-in-type-parser-tests.js b/test/unit/client/throw-in-type-parser-tests.js new file mode 100644 index 0000000..ed37113 --- /dev/null +++ b/test/unit/client/throw-in-type-parser-tests.js @@ -0,0 +1,112 @@ +var helper = require(__dirname + "/test-helper"); +var types = require('pg-types') + +test('handles throws in type parsers', function() { + var typeParserError = new Error('TEST: Throw in type parsers'); + + types.setTypeParser('special oid that will throw', function () { + throw typeParserError; + }); + + test('emits error', function() { + var handled; + var client = helper.client(); + var con = client.connection; + var query = client.query('whatever'); + + handled = con.emit('readyForQuery'); + assert.ok(handled, "should have handled ready for query"); + + con.emit('rowDescription',{ + fields: [{ + name: 'boom', + dataTypeID: 'special oid that will throw' + }] + }); + assert.ok(handled, "should have handled row description"); + + assert.emits(query, 'error', function(err) { + assert.equal(err, typeParserError); + }); + + handled = con.emit('dataRow', { fields: ["hi"] }); + assert.ok(handled, "should have handled first data row message"); + + handled = con.emit('commandComplete', { text: 'INSERT 31 1' }); + assert.ok(handled, "should have handled command complete"); + + handled = con.emit('readyForQuery'); + assert.ok(handled, "should have handled ready for query"); + }); + + test('calls callback with error', function() { + var handled; + + var callbackCalled = 0; + + var client = helper.client(); + var con = client.connection; + var query = client.query('whatever', assert.calls(function (err) { + callbackCalled += 1; + + assert.equal(callbackCalled, 1); + assert.equal(err, typeParserError); + })); + + handled = con.emit('readyForQuery'); + assert.ok(handled, "should have handled ready for query"); + + handled = con.emit('rowDescription',{ + fields: [{ + name: 'boom', + dataTypeID: 'special oid that will throw' + }] + }); + assert.ok(handled, "should have handled row description"); + + handled = con.emit('dataRow', { fields: ["hi"] }); + assert.ok(handled, "should have handled first data row message"); + + handled = con.emit('dataRow', { fields: ["hi"] }); + assert.ok(handled, "should have handled second data row message"); + + con.emit('commandComplete', { text: 'INSERT 31 1' }); + assert.ok(handled, "should have handled command complete"); + + handled = con.emit('readyForQuery'); + assert.ok(handled, "should have handled ready for query"); + }); + + test('rejects promise with error', function() { + var handled; + var client = helper.client(); + var con = client.connection; + var query = client.query('whatever'); + var queryPromise = query.promise(); + + handled = con.emit('readyForQuery'); + assert.ok(handled, "should have handled ready for query"); + + handled = con.emit('rowDescription',{ + fields: [{ + name: 'boom', + dataTypeID: 'special oid that will throw' + }] + }); + assert.ok(handled, "should have handled row description"); + + handled = con.emit('dataRow', { fields: ["hi"] }); + assert.ok(handled, "should have handled first data row message"); + + handled = con.emit('commandComplete', { text: 'INSERT 31 1' }); + assert.ok(handled, "should have handled command complete"); + + handled = con.emit('readyForQuery'); + assert.ok(handled, "should have handled ready for query"); + + queryPromise.catch(assert.calls(function (err) { + assert.equal(err, typeParserError); + })); + }); + +});