From 6bdd2c388461b0fe198fa484b49c286d21d0bd59 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 19 Nov 2022 10:34:59 -0800 Subject: [PATCH] Revert "url: improve port validation" This reverts commit 5f7730e2f2689a6af0530d7de16370f9fcce1227. This change broke too many edge cases in the ecosystem. Reverting it re-introduces some host-spoofing possibilities, so we won't want to revert forever, but the issue is long-lived enough and not sufficiently critical that we can't wait for a major release to introduce it as a breaking change. After this lands, I plan to re-introduce this as a change that throws a warning rather than an error, after which we can land a semver-major that re-introduces the error and try to get the word out to maintainers of likely-affected packages. Closes: https://github.com/nodejs/node/issues/45514 Refs: https://github.com/nodejs/node/pull/45012 PR-URL: https://github.com/nodejs/node/pull/45517 Fixes: https://github.com/nodejs/node/issues/45514 Reviewed-By: James M Snell Reviewed-By: Richard Lau Reviewed-By: Yagiz Nizipli Reviewed-By: Antoine du Hamel --- lib/url.js | 8 ++------ test/parallel/test-url-parse-format.js | 16 ++++++++++++++++ test/parallel/test-url-parse-invalid-input.js | 12 ------------ 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/lib/url.js b/lib/url.js index 95b7f8afe29e77..4d7374a8e3f358 100644 --- a/lib/url.js +++ b/lib/url.js @@ -387,7 +387,7 @@ Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) { // validate a little. if (!ipv6Hostname) { - rest = getHostname(this, rest, hostname, url); + rest = getHostname(this, rest, hostname); } if (this.hostname.length > hostnameMaxLen) { @@ -506,7 +506,7 @@ Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) { return this; }; -function getHostname(self, rest, hostname, url) { +function getHostname(self, rest, hostname) { for (let i = 0; i < hostname.length; ++i) { const code = hostname.charCodeAt(i); const isValid = (code !== CHAR_FORWARD_SLASH && @@ -516,10 +516,6 @@ function getHostname(self, rest, hostname, url) { code !== CHAR_COLON); if (!isValid) { - // If leftover starts with :, then it represents an invalid port. - if (hostname.charCodeAt(i) === 58) { - throw new ERR_INVALID_URL(url); - } self.hostname = hostname.slice(0, i); return `/${hostname.slice(i)}${rest}`; } diff --git a/test/parallel/test-url-parse-format.js b/test/parallel/test-url-parse-format.js index 03e7b7ece5ef5c..ef4aad51d0e3ac 100644 --- a/test/parallel/test-url-parse-format.js +++ b/test/parallel/test-url-parse-format.js @@ -865,6 +865,22 @@ const parseTests = { href: 'http://a%22%20%3C\'b:b@cd/e?f' }, + // Git urls used by npm + 'git+ssh://git@github.com:npm/npm': { + protocol: 'git+ssh:', + slashes: true, + auth: 'git', + host: 'github.com', + port: null, + hostname: 'github.com', + hash: null, + search: null, + query: null, + pathname: '/:npm/npm', + path: '/:npm/npm', + href: 'git+ssh://git@github.com/:npm/npm' + }, + 'https://*': { protocol: 'https:', slashes: true, diff --git a/test/parallel/test-url-parse-invalid-input.js b/test/parallel/test-url-parse-invalid-input.js index 7ab86380ad7384..794a756524925b 100644 --- a/test/parallel/test-url-parse-invalid-input.js +++ b/test/parallel/test-url-parse-invalid-input.js @@ -74,15 +74,3 @@ if (common.hasIntl) { (e) => e.code === 'ERR_INVALID_URL', 'parsing http://\u00AD/bad.com/'); } - -{ - const badURLs = [ - 'https://evil.com:.example.com', - 'git+ssh://git@github.com:npm/npm', - ]; - badURLs.forEach((badURL) => { - assert.throws(() => { url.parse(badURL); }, - (e) => e.code === 'ERR_INVALID_URL', - `parsing ${badURL}`); - }); -}