From 307f39ce9ed8f4d3de06f63cd1855157be2db82f Mon Sep 17 00:00:00 2001 From: isaacs Date: Tue, 10 May 2011 13:42:49 -0700 Subject: [PATCH] Fix a url regression 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. --- lib/url.js | 45 ++++++++++++++++++++++++++++++++++++++------- test/simple/test-url.js | 26 ++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/lib/url.js b/lib/url.js index 99a0e67..21d3acb 100644 --- a/lib/url.js +++ b/lib/url.js @@ -42,6 +42,7 @@ var protocolPattern = /^([a-z0-9]+:)/i, // them. nonHostChars = ['%', '/', '?', ';', '#'] .concat(unwise).concat(autoEscape), + nonAuthChars = ['/', '@', '?', '#'].concat(delims), hostnameMaxLen = 255, hostnamePartPattern = /^[a-zA-Z0-9][a-z0-9A-Z-]{0,62}$/, hostnamePartStart = /^([a-zA-Z0-9][a-z0-9A-Z-]{0,62})(.*)$/, @@ -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); @@ -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]; @@ -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 : '') ) : @@ -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]; diff --git a/test/simple/test-url.js b/test/simple/test-url.js index e52dacd..954454f 100644 --- a/test/simple/test-url.js +++ b/test/simple/test-url.js @@ -236,6 +236,18 @@ var parseTests = { 'host': 'isaacschlueter@jabber.org', 'auth': 'isaacschlueter', 'hostname': 'jabber.org' + }, + 'http://atpass:foo%40bar@127.0.0.1:8080/path?search=foo#bar' : { + 'href' : 'http://atpass:foo%40bar@127.0.0.1:8080/path?search=foo#bar', + 'protocol' : 'http:', + 'host' : 'atpass:foo%40bar@127.0.0.1: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) { @@ -367,6 +379,20 @@ var formatTests = { 'host': 'isaacschlueter@jabber.org', 'auth': 'isaacschlueter', 'hostname': 'jabber.org' + }, + 'http://atpass:foo%40bar@127.0.0.1/' : { + 'href': 'http://atpass:foo%40bar@127.0.0.1/', + '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) { -- 2.7.4