Skip to content

Commit 7acbfd7

Browse files
committed
Use a third-party URL parser
The `url.parse` from the standard lib provides the username and password as one field, and decodes them. This means any percent-encoded characters are decoded, and it's impossible to tell whether a colon is the delimiter between the username and password, or was encoded and part of one or other. Newer versions of the Node standard library come with an updated URL parser. However, I would like to work with older versions of Node, so I've used a third party parser.
1 parent be11b7d commit 7acbfd7

File tree

3 files changed

+17
-24
lines changed

3 files changed

+17
-24
lines changed

lib/connect.js

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
'use strict';
88

9-
var URL = require('url');
9+
var URL = require('url-parse');
1010
var QS = require('querystring');
1111
var Connection = require('./connection').Connection;
1212
var fmt = require('util').format;
@@ -76,22 +76,12 @@ function openFrames(vhost, query, credentials, extraClientProperties) {
7676
};
7777
}
7878

79-
// Decide on credentials based on what we're supplied. Note that in a
80-
// parsed URL, the auth part is already URL-decoded, so e.g., '%3a' in
81-
// the URL is already decoded to ':'. This is a bit unhelpful, as it
82-
// means we can't tell whether a colon is a separator, or part of the
83-
// username. Assume no colons in usernames.
79+
// Decide on credentials based on what we're supplied.
8480
function credentialsFromUrl(parts) {
8581
var user = 'guest', passwd = 'guest';
86-
if (parts.auth) {
87-
var colon = parts.auth.indexOf(':')
88-
if (colon == -1) {
89-
user = parts.auth;
90-
passwd = '';
91-
} else {
92-
user = parts.auth.substring(0, colon);
93-
passwd = parts.auth.substring(colon+1);
94-
}
82+
if (parts.username != '' || parts.password != '') {
83+
user = (parts.username) ? unescape(parts.username) : '';
84+
passwd = (parts.password) ? unescape(parts.password) : '';
9585
}
9686
return credentials.plain(user, passwd);
9787
}
@@ -137,7 +127,7 @@ function connect(url, socketOptions, openCallback) {
137127

138128
fields = openFrames(url.vhost, config, sockopts.credentials || credentials.plain(user, pass), extraClientProperties);
139129
} else {
140-
var parts = URL.parse(url, true); // yes, parse the query string
130+
var parts = URL(url, true); // yes, parse the query string
141131
protocol = parts.protocol;
142132
sockopts.host = parts.hostname;
143133
sockopts.port = parseInt(parts.port) || ((protocol === 'amqp:') ? 5672 : 5671);

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
"bluebird": "^3.4.6",
1717
"buffer-more-ints": "0.0.2",
1818
"readable-stream": "1.x >=1.1.9",
19-
"safe-buffer": "^5.0.1"
19+
"safe-buffer": "^5.0.1",
20+
"url-parse": "^1.2.0"
2021
},
2122
"devDependencies": {
2223
"mocha": "~1",

test/connect.js

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ var format = require('util').format;
1313

1414
var URL = process.env.URL || 'amqp://localhost';
1515

16+
var urlparse = require('url-parse');
17+
1618
suite("Credentials", function() {
1719

1820
function checkCreds(creds, user, pass, done) {
@@ -27,29 +29,29 @@ suite("Credentials", function() {
2729
}
2830

2931
test("no creds", function(done) {
30-
var parts = {auth: ''};
32+
var parts = urlparse('amqp://localhost');
3133
var creds = credentialsFromUrl(parts);
3234
checkCreds(creds, 'guest', 'guest', done);
3335
});
3436
test("usual user:pass", function(done) {
35-
var parts = {auth: 'user:pass'};
37+
var parts = urlparse('amqp://user:pass@localhost')
3638
var creds = credentialsFromUrl(parts);
3739
checkCreds(creds, 'user', 'pass', done);
3840
});
3941
test("missing user", function(done) {
40-
var parts = {auth: ':password'};
42+
var parts = urlparse('amqps://:password@localhost');
4143
var creds = credentialsFromUrl(parts);
4244
checkCreds(creds, '', 'password', done);
4345
});
4446
test("missing password", function(done) {
45-
var parts = {auth: 'username'};
47+
var parts = urlparse('amqps://username:@localhost');
4648
var creds = credentialsFromUrl(parts);
4749
checkCreds(creds, 'username', '', done);
4850
});
49-
test("colon in password", function(done) {
50-
var parts = {auth: 'username:pass:word'};
51+
test("escaped colons", function(done) {
52+
var parts = urlparse('amqp://user%3Aname:pass%3Aword@localhost')
5153
var creds = credentialsFromUrl(parts);
52-
checkCreds(creds, 'username', 'pass:word', done);
54+
checkCreds(creds, 'user:name', 'pass:word', done);
5355
});
5456
});
5557

0 commit comments

Comments
 (0)