From 1bc84ce52cbd834f3ad82bfc7ed490c2fa59e720 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 24 Oct 2022 14:24:46 -0700 Subject: [PATCH] url: remove \t \n \r in url.parse() similar to WHATWG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/nodejs/node/pull/45116 Reviewed-By: James M Snell Reviewed-By: Yagiz Nizipli Reviewed-By: Tobias Nießen Reviewed-By: Antoine du Hamel --- lib/url.js | 4 ++++ test/parallel/test-url-parse-format.js | 14 +++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/url.js b/lib/url.js index fa7a3d21d20963..95b7f8afe29e77 100644 --- 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 index f046df74b1c0f7..03e7b7ece5ef5c 100644 --- 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,