Skip to content

Commit

Permalink
url: change hostname regex to negate invalid chars
Browse files Browse the repository at this point in the history
Regarding nodejs#8520

This approach changes hostname validation from a whitelist approach
to a blacklist approach as described in https://url.spec.whatwg.org/#host-parsing.

url.parse misinterpreted `https://good.com+.evil.org/`
as `https://good.com/+.evil.org/`.  If we use url.parse to check the
validity of the hostname, the test passes, but in the browser the
user is redirected to the evil.org website.
  • Loading branch information
jondavidjohn committed Nov 26, 2014
1 parent ea69dd7 commit 2637a5d
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 14 deletions.
5 changes: 3 additions & 2 deletions lib/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,9 @@ var protocolPattern = /^([a-z0-9.+-]+:)/i,
.concat(unwise).concat(autoEscape),
hostEndingChars = ['/', '?', '#'],
hostnameMaxLen = 255,
hostnamePartPattern = /^[a-z0-9A-Z_-]{0,63}$/,
hostnamePartStart = /^([a-z0-9A-Z_-]{0,63})(.*)$/,
hostnamePatternString = '[^' + nonHostChars.join('') + ']{0,63}',
hostnamePartPattern = new RegExp('^' + hostnamePatternString + '$'),
hostnamePartStart = new RegExp('^(' + hostnamePatternString + ')(.*)$'),
// protocols that can allow "unsafe" and "unwise" chars.
unsafeProtocol = {
'javascript': true,
Expand Down
36 changes: 24 additions & 12 deletions test/simple/test-url.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,32 +153,44 @@ var parseTests = {
'path': '/Y'
},

// + not an invalid host character
// per https://url.spec.whatwg.org/#host-parsing
'http://x.y.com+a/b/c' : {
'href': 'http://x.y.com+a/b/c',
'protocol': 'http:',
'slashes': true,
'host': 'x.y.com+a',
'hostname': 'x.y.com+a',
'pathname': '/b/c',
'path': '/b/c'
},

// an unexpected invalid char in the hostname.
'HtTp://x.y.cOm*a/b/c?d=e#f g<h>i' : {
'href': 'http://x.y.com/*a/b/c?d=e#f%20g%3Ch%3Ei',
'HtTp://x.y.cOm;a/b/c?d=e#f g<h>i' : {
'href': 'http://x.y.com/;a/b/c?d=e#f%20g%3Ch%3Ei',
'protocol': 'http:',
'slashes': true,
'host': 'x.y.com',
'hostname': 'x.y.com',
'pathname': '/*a/b/c',
'pathname': ';a/b/c',
'search': '?d=e',
'query': 'd=e',
'hash': '#f%20g%3Ch%3Ei',
'path': '/*a/b/c?d=e'
'path': ';a/b/c?d=e'
},

// make sure that we don't accidentally lcast the path parts.
'HtTp://x.y.cOm*A/b/c?d=e#f g<h>i' : {
'href': 'http://x.y.com/*A/b/c?d=e#f%20g%3Ch%3Ei',
'HtTp://x.y.cOm;A/b/c?d=e#f g<h>i' : {
'href': 'http://x.y.com/;A/b/c?d=e#f%20g%3Ch%3Ei',
'protocol': 'http:',
'slashes': true,
'host': 'x.y.com',
'hostname': 'x.y.com',
'pathname': '/*A/b/c',
'pathname': ';A/b/c',
'search': '?d=e',
'query': 'd=e',
'hash': '#f%20g%3Ch%3Ei',
'path': '/*A/b/c?d=e'
'path': ';A/b/c?d=e'
},

'http://x...y...#p': {
Expand Down Expand Up @@ -493,17 +505,17 @@ var parseTests = {
'path': '/'
},

'http://www.Äffchen.cOm*A/b/c?d=e#f g<h>i' : {
'href': 'http://www.xn--ffchen-9ta.com/*A/b/c?d=e#f%20g%3Ch%3Ei',
'http://www.Äffchen.cOm;A/b/c?d=e#f g<h>i' : {
'href': 'http://www.xn--ffchen-9ta.com/;A/b/c?d=e#f%20g%3Ch%3Ei',
'protocol': 'http:',
'slashes': true,
'host': 'www.xn--ffchen-9ta.com',
'hostname': 'www.xn--ffchen-9ta.com',
'pathname': '/*A/b/c',
'pathname': ';A/b/c',
'search': '?d=e',
'query': 'd=e',
'hash': '#f%20g%3Ch%3Ei',
'path': '/*A/b/c?d=e'
'path': ';A/b/c?d=e'
},

'http://SÉLIER.COM/' : {
Expand Down

0 comments on commit 2637a5d

Please sign in to comment.