Skip to content

Commit 6120472

Browse files
jondavidjohntrevnorris
jondavidjohn
authored and
trevnorris
committed
url: change hostname regex to negate invalid chars
Regarding nodejs/node-v0.x-archive#8520 This changes hostname validation from a whitelist regex approach to a blacklist regex 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.
1 parent c4f6c22 commit 6120472

2 files changed

Lines changed: 27 additions & 14 deletions

File tree

‎lib/url.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,9 @@ var protocolPattern = /^([a-z0-9.+-]+:)/i,
7070
nonHostChars = ['%', '/', '?', ';', '#'].concat(autoEscape),
7171
hostEndingChars = ['/', '?', '#'],
7272
hostnameMaxLen = 255,
73-
hostnamePartPattern = /^[a-z0-9A-Z_-]{0,63}$/,
74-
hostnamePartStart = /^([a-z0-9A-Z_-]{0,63})(.*)$/,
73+
hostnamePatternString = '[^' + nonHostChars.join('') + ']{0,63}',
74+
hostnamePartPattern = new RegExp('^' + hostnamePatternString + '$'),
75+
hostnamePartStart = new RegExp('^(' + hostnamePatternString + ')(.*)$'),
7576
// protocols that can allow "unsafe" and "unwise" chars.
7677
unsafeProtocol = {
7778
'javascript': true,

‎test/simple/test-url.js‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -177,32 +177,44 @@ var parseTests = {
177177
'path': '/Y'
178178
},
179179

180+
// + not an invalid host character
181+
// per https://url.spec.whatwg.org/#host-parsing
182+
'http://x.y.com+a/b/c' : {
183+
'href': 'http://x.y.com+a/b/c',
184+
'protocol': 'http:',
185+
'slashes': true,
186+
'host': 'x.y.com+a',
187+
'hostname': 'x.y.com+a',
188+
'pathname': '/b/c',
189+
'path': '/b/c'
190+
},
191+
180192
// an unexpected invalid char in the hostname.
181-
'HtTp://x.y.cOm*a/b/c?d=e#f g<h>i' : {
182-
'href': 'http://x.y.com/*a/b/c?d=e#f%20g%3Ch%3Ei',
193+
'HtTp://x.y.cOm;a/b/c?d=e#f g<h>i' : {
194+
'href': 'http://x.y.com/;a/b/c?d=e#f%20g%3Ch%3Ei',
183195
'protocol': 'http:',
184196
'slashes': true,
185197
'host': 'x.y.com',
186198
'hostname': 'x.y.com',
187-
'pathname': '/*a/b/c',
199+
'pathname': ';a/b/c',
188200
'search': '?d=e',
189201
'query': 'd=e',
190202
'hash': '#f%20g%3Ch%3Ei',
191-
'path': '/*a/b/c?d=e'
203+
'path': ';a/b/c?d=e'
192204
},
193205

194206
// make sure that we don't accidentally lcast the path parts.
195-
'HtTp://x.y.cOm*A/b/c?d=e#f g<h>i' : {
196-
'href': 'http://x.y.com/*A/b/c?d=e#f%20g%3Ch%3Ei',
207+
'HtTp://x.y.cOm;A/b/c?d=e#f g<h>i' : {
208+
'href': 'http://x.y.com/;A/b/c?d=e#f%20g%3Ch%3Ei',
197209
'protocol': 'http:',
198210
'slashes': true,
199211
'host': 'x.y.com',
200212
'hostname': 'x.y.com',
201-
'pathname': '/*A/b/c',
213+
'pathname': ';A/b/c',
202214
'search': '?d=e',
203215
'query': 'd=e',
204216
'hash': '#f%20g%3Ch%3Ei',
205-
'path': '/*A/b/c?d=e'
217+
'path': ';A/b/c?d=e'
206218
},
207219

208220
'http://x...y...#p': {
@@ -517,17 +529,17 @@ var parseTests = {
517529
'path': '/'
518530
},
519531

520-
'http://www.Äffchen.cOm*A/b/c?d=e#f g<h>i' : {
521-
'href': 'http://www.xn--ffchen-9ta.com/*A/b/c?d=e#f%20g%3Ch%3Ei',
532+
'http://www.Äffchen.cOm;A/b/c?d=e#f g<h>i' : {
533+
'href': 'http://www.xn--ffchen-9ta.com/;A/b/c?d=e#f%20g%3Ch%3Ei',
522534
'protocol': 'http:',
523535
'slashes': true,
524536
'host': 'www.xn--ffchen-9ta.com',
525537
'hostname': 'www.xn--ffchen-9ta.com',
526-
'pathname': '/*A/b/c',
538+
'pathname': ';A/b/c',
527539
'search': '?d=e',
528540
'query': 'd=e',
529541
'hash': '#f%20g%3Ch%3Ei',
530-
'path': '/*A/b/c?d=e'
542+
'path': ';A/b/c?d=e'
531543
},
532544

533545
'http://SÉLIER.COM/' : {

0 commit comments

Comments
 (0)