url: remove \t \n \r in url.parse() similar to WHATWG

WHATWG URL removes tab, new line, and carraige return characters before processing URL strings. To narrow the differences between WHATWG URL and url.parse(), and thus reduce opportunities for host spoofing etc. due to differences between the two APIs, let's do the same with url.parse(). PR-URL: #45116 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
nodejs · Nov 10, 2022 · 1bc84ce · 1bc84ce
1 parent 85cb4d7
commit 1bc84ce
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 7 deletions.
diff --git a/lib/url.js b/lib/url.js
@@ -319,6 +319,10 @@ Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) {
         case CHAR_TAB:
         case CHAR_LINE_FEED:
         case CHAR_CARRIAGE_RETURN:
+          // WHATWG URL removes tabs, newlines, and carriage returns. Let's do that too.
+          rest = rest.slice(0, i) + rest.slice(i + 1);
+          i -= 1;
+          break;
         case CHAR_SPACE:
         case CHAR_DOUBLE_QUOTE:
         case CHAR_PERCENT:

diff --git a/test/parallel/test-url-parse-format.js b/test/parallel/test-url-parse-format.js
@@ -853,16 +853,16 @@ const parseTests = {
   'http://a\r" \t\n<\'b:b@c\r\nd/e?f': {
     protocol: 'http:',
     slashes: true,
-    auth: 'a\r" \t\n<\'b:b',
-    host: 'c',
+    auth: 'a" <\'b:b',
+    host: 'cd',
     port: null,
-    hostname: 'c',
+    hostname: 'cd',
     hash: null,
     search: '?f',
     query: 'f',
-    pathname: '%0D%0Ad/e',
-    path: '%0D%0Ad/e?f',
-    href: 'http://a%0D%22%20%09%0A%3C\'b:b@c/%0D%0Ad/e?f'
+    pathname: '/e',
+    path: '/e?f',
+    href: 'http://a%22%20%3C\'b:b@cd/e?f'
   },
 
   'https://*': {
@@ -1007,7 +1007,7 @@ for (const u in parseTests) {
   assert.deepStrictEqual(
     actual,
     expected,
-    `expected ${inspect(expected)}, got ${inspect(actual)}`
+    `parsing ${u} and expected ${inspect(expected)} but got ${inspect(actual)}`
   );
   assert.deepStrictEqual(
     spaced,