Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Commit

Permalink
Fix a url regression
Browse files Browse the repository at this point in the history 
The change for #954 introduced a regression that would cause
the url parser to fail on special chars found in the auth
segment.  Fix that, and also don't create invalid urls when
format() is called on an object containing an auth member
containing '@' characters or delimiters.
  • Loading branch information
@isaacs
isaacs committed May 10, 2011
1 parent 11beac7 commit 307f39c
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 7 deletions.
45 changes: 38 additions & 7 deletions lib/url.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ var protocolPattern = /^([a-z0-9]+:)/i,
// them.
nonHostChars = ['%', '/', '?', ';', '#']
.concat(unwise).concat(autoEscape),
nonAuthChars = ['/', '@', '?', '#'].concat(delims),

This comment has been minimized.

Sign in to view

Copy link
@dscape

dscape Jan 13, 2012

I'm kind of lost in RFC land here. I suppose according to the RFC cited in this file (Reference: RFC 3986, RFC 1808, RFC 2396) this is the correct behavior for chars that need to be encoded?

Personally I was reading RFC1738 which has a different take on this:

3.1. Common Internet Scheme Syntax

   While the syntax for the rest of the URL may vary depending on the
   particular scheme selected, URL schemes that involve the direct use
   of an IP-based protocol to a specified host on the Internet use a
   common syntax for the scheme-specific data:

        //<user>:<password>@<host>:<port>/<url-path>

   Some or all of the parts "<user>:<password>@", ":<password>",
   ":<port>", and "/<url-path>" may be excluded.  The scheme specific
   data start with a double slash "//" to indicate that it complies with
   the common Internet scheme syntax. The different components obey the
   following rules:

    user
        An optional user name. Some schemes (e.g., ftp) allow the
        specification of a user name.

    password
        An optional password. If present, it follows the user
        name separated from it by a colon.

   The user name (and password), if present, are followed by a
   commercial at-sign "@". Within the user and password field, any ":",
   "@", or "/" must be encoded.

Would like to clarify if I should / or shoudn't encode ? in auth?
hostnameMaxLen = 255,
hostnamePartPattern = /^[a-zA-Z0-9][a-z0-9A-Z-]{0,62}$/,
hostnamePartStart = /^([a-zA-Z0-9][a-z0-9A-Z-]{0,62})(.*)$/,
Expand Down Expand Up @@ -123,12 +124,37 @@ function urlParse(url, parseQueryString, slashesDenoteHost) {
// there's a hostname.
// the first instance of /, ?, ;, or # ends the host.
// don't enforce full RFC correctness, just be unstupid about it.

// If there is an @ in the hostname, then non-host chars *are* allowed
// to the left of the first @ sign, unless some non-auth character
// comes *before* the @-sign.
// URLs are obnoxious.
var atSign = rest.indexOf('@');
if (atSign !== -1) {
// there *may be* an auth
var hasAuth = true;
for (var i = 0, l = nonAuthChars.length; i < l; i++) {
var index = rest.indexOf(nonAuthChars[i]);
if (index !== -1 && index < atSign) {
// not a valid auth. Something like http://foo.com/bar@baz/
hasAuth = false;
break;
}
}
if (hasAuth) {
// pluck off the auth portion.
out.auth = rest.substr(0, atSign);
rest = rest.substr(atSign + 1);
}
}

var firstNonHost = -1;
for (var i = 0, l = nonHostChars.length; i < l; i++) {
var index = rest.indexOf(nonHostChars[i]);
if (index !== -1 &&
(firstNonHost < 0 || index < firstNonHost)) firstNonHost = index;
}

if (firstNonHost !== -1) {
out.host = rest.substr(0, firstNonHost);
rest = rest.substr(firstNonHost);
Expand All @@ -137,8 +163,9 @@ function urlParse(url, parseQueryString, slashesDenoteHost) {
rest = '';
}

// pull out the auth and port.
// pull out port.
var p = parseHost(out.host);
if (out.auth) out.host = out.auth + '@' + out.host;
var keys = Object.keys(p);
for (var i = 0, l = keys.length; i < l; i++) {
var key = keys[i];
Expand Down Expand Up @@ -250,10 +277,19 @@ function urlFormat(obj) {
// to clean up potentially wonky urls.
if (typeof(obj) === 'string') obj = urlParse(obj);

var auth = obj.auth;
if (auth) {
auth = auth.split('@').join('%40');
for (var i = 0, l = nonAuthChars.length; i < l; i++) {
var nAC = nonAuthChars[i];
auth = auth.split(nAC).join(encodeURIComponent(nAC));
}
}

var protocol = obj.protocol || '',
host = (obj.host !== undefined) ? obj.host :
obj.hostname !== undefined ? (
(obj.auth ? obj.auth + '@' : '') +
(auth ? auth + '@' : '') +
obj.hostname +
(obj.port ? ':' + obj.port : '')
) :
Expand Down Expand Up @@ -476,11 +512,6 @@ function urlResolveObject(source, relative) {

function parseHost(host) {
var out = {};
var at = host.indexOf('@');
if (at !== -1) {
out.auth = host.substr(0, at);
host = host.substr(at + 1); // drop the @
}
var port = portPattern.exec(host);
if (port) {
port = port[0];
Expand Down
26 changes: 26 additions & 0 deletions test/simple/test-url.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,18 @@ var parseTests = {
'host': '[email protected]',
'auth': 'isaacschlueter',
'hostname': 'jabber.org'
},
'http://atpass:foo%[email protected]:8080/path?search=foo#bar' : {
'href' : 'http://atpass:foo%[email protected]:8080/path?search=foo#bar',
'protocol' : 'http:',
'host' : 'atpass:foo%[email protected]:8080',
'auth' : 'atpass:foo%40bar',
'hostname' : '127.0.0.1',
'port' : '8080',
'pathname': '/path',
'search' : '?search=foo',
'query' : 'search=foo',
'hash' : '#bar'
}
};
for (var u in parseTests) {
Expand Down Expand Up @@ -367,6 +379,20 @@ var formatTests = {
'host': '[email protected]',
'auth': 'isaacschlueter',
'hostname': 'jabber.org'
},
'http://atpass:foo%[email protected]/' : {
'href': 'http://atpass:foo%[email protected]/',
'auth': 'atpass:foo@bar',
'hostname': '127.0.0.1',
'protocol': 'http:',
'pathname': '/'
},
'http://atslash%2F%40:%2F%40@foo/' : {
'href': 'http://atslash%2F%40:%2F%40@foo/',
'auth': 'atslash/@:/@',
'hostname': 'foo',
'protocol': 'http:',
'pathname': '/'
}
};
for (var u in formatTests) {
Expand Down

0 comments on commit 307f39c

Please sign in to comment.