Skip to content

Commit

Permalink
Libpq connection string escaping (#1285)
Browse files Browse the repository at this point in the history
* Fix escaping of libpq connection string properties

Fix handlings of libpq connection properties to properly escape single
quotes and backslashes. Previously the values were surrounded in single
quotes which handled whitespace within the property value, but internal
single quotes and backslashes would cause invalid connection strings to
be generated.

* Update expected output in test to be quoted

Update the expect host output in the connection parameter test
to expect it to be surrounded by single quotes.

* Add test for configs with quotes and backslashes
  • Loading branch information
sehrope authored and brianc committed May 15, 2017
1 parent 4659d5d commit ee81936
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 9 deletions.
17 changes: 11 additions & 6 deletions lib/connection-parameters.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,15 @@ var ConnectionParameters = function(config) {
this.fallback_application_name = val('fallback_application_name', config, false);
};

// Convert arg to a string, surround in single quotes, and escape single quotes and backslashes
var quoteParamValue = function(value) {
return "'" + ('' + value).replace(/\\/g, "\\\\").replace(/'/g, "\\'") + "'";
};

var add = function(params, config, paramName) {
var value = config[paramName];
if(value) {
params.push(paramName+"='"+value+"'");
params.push(paramName + "=" + quoteParamValue(value));
}
};

Expand All @@ -87,23 +92,23 @@ ConnectionParameters.prototype.getLibpqConnectionString = function(cb) {
add(params, ssl, 'sslcert');

if(this.database) {
params.push("dbname='" + this.database + "'");
params.push("dbname=" + quoteParamValue(this.database));
}
if(this.replication) {
params.push("replication='" + this.replication + "'");
params.push("replication=" + quoteParamValue(this.replication));
}
if(this.host) {
params.push("host=" + this.host);
params.push("host=" + quoteParamValue(this.host));
}
if(this.isDomainSocket) {
return cb(null, params.join(' '));
}
if(this.client_encoding) {
params.push("client_encoding='" + this.client_encoding + "'");
params.push("client_encoding=" + quoteParamValue(this.client_encoding));
}
dns.lookup(this.host, function(err, address) {
if(err) return cb(err, null);
params.push("hostaddr=" + address);
params.push("hostaddr=" + quoteParamValue(address));
return cb(null, params.join(' '));
});
};
Expand Down
22 changes: 19 additions & 3 deletions test/unit/connection-parameters/creation-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ test('libpq connection string building', function() {
checkForPart(parts, "user='brian'");
checkForPart(parts, "password='xyz'");
checkForPart(parts, "port='888'");
checkForPart(parts, "hostaddr=127.0.0.1");
checkForPart(parts, "hostaddr='127.0.0.1'");
checkForPart(parts, "dbname='bam'");
}));
});
Expand All @@ -143,7 +143,7 @@ test('libpq connection string building', function() {
assert.isNull(err);
var parts = constring.split(" ");
checkForPart(parts, "user='brian'");
checkForPart(parts, "hostaddr=127.0.0.1");
checkForPart(parts, "hostaddr='127.0.0.1'");
}));
});

Expand Down Expand Up @@ -173,7 +173,23 @@ test('libpq connection string building', function() {
assert.isNull(err);
var parts = constring.split(" ");
checkForPart(parts, "user='brian'");
checkForPart(parts, "host=/tmp/");
checkForPart(parts, "host='/tmp/'");
}));
});

test('config contains quotes and backslashes', function() {
var config = {
user: 'not\\brian',
password: 'bad\'chars',
port: 5432,
host: '/tmp/'
};
var subject = new ConnectionParameters(config);
subject.getLibpqConnectionString(assert.calls(function(err, constring) {
assert.isNull(err);
var parts = constring.split(" ");
checkForPart(parts, "user='not\\\\brian'");
checkForPart(parts, "password='bad\\'chars'");
}));
});

Expand Down

0 comments on commit ee81936

Please sign in to comment.