From 3aaa2ebe19712b0e77ea75793f0de08094997974 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 7 Jun 2025 08:01:15 -0700 Subject: [PATCH] url: move bad port deprecation in legacy url to end-of-life MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Calling `url.parse()` with a URL that has a bad port will now throw an error instead of emitting a deprecation warning. It's been deprecated for ~ 3 years now. PR-URL: https://github.com/nodejs/node/pull/58617 Reviewed-By: Yagiz Nizipli Reviewed-By: Michaƫl Zasso Reviewed-By: Rich Trott --- doc/api/deprecations.md | 11 +++++++---- lib/url.js | 10 +++------- test/parallel/test-url-parse-format.js | 16 ---------------- test/parallel/test-url-parse-invalid-input.js | 9 ++++----- 4 files changed, 14 insertions(+), 32 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 83a3ca25ea4..02592d8013b 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -3548,6 +3548,9 @@ issued for `url.parse()` vulnerabilities. -Type: Runtime +Type: End-of-Life -[`url.parse()`][] accepts URLs with ports that are not numbers. This behavior -might result in host name spoofing with unexpected input. These URLs will throw -an error in future versions of Node.js, as the [WHATWG URL API][] does already. +[`url.parse()`][] used to accept URLs with ports that are not numbers. This +behavior might result in host name spoofing with unexpected input. These URLs +will throw an error (which the [WHATWG URL API][] also does). ### DEP0171: Setters for `http.IncomingMessage` headers and trailers diff --git a/lib/url.js b/lib/url.js index d7c274a9692..0cccdac2c8c 100644 --- a/lib/url.js +++ b/lib/url.js @@ -41,6 +41,7 @@ const querystring = require('querystring'); const { ERR_INVALID_ARG_TYPE, ERR_INVALID_URL, + ERR_INVALID_ARG_VALUE, } = require('internal/errors').codes; const { validateString, @@ -501,7 +502,6 @@ Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) { return this; }; -let warnInvalidPort = true; function getHostname(self, rest, hostname, url) { for (let i = 0; i < hostname.length; ++i) { const code = hostname.charCodeAt(i); @@ -513,12 +513,8 @@ function getHostname(self, rest, hostname, url) { if (!isValid) { // If leftover starts with :, then it represents an invalid port. - // But url.parse() is lenient about it for now. - // Issue a warning and continue. - if (warnInvalidPort && code === CHAR_COLON) { - const detail = `The URL ${url} is invalid. Future versions of Node.js will throw an error.`; - process.emitWarning(detail, 'DeprecationWarning', 'DEP0170'); - warnInvalidPort = false; + if (code === CHAR_COLON) { + throw new ERR_INVALID_ARG_VALUE('url', 'Invalid port in 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 4e36e1306d3..405625410fd 100644 --- a/test/parallel/test-url-parse-format.js +++ b/test/parallel/test-url-parse-format.js @@ -862,22 +862,6 @@ 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 b1124128af9..036b8ff5e68 100644 --- a/test/parallel/test-url-parse-invalid-input.js +++ b/test/parallel/test-url-parse-invalid-input.js @@ -83,9 +83,7 @@ if (common.hasIntl) { badURLs.forEach((badURL) => { common.spawnPromisified(process.execPath, ['-e', `url.parse(${JSON.stringify(badURL)})`]) .then(common.mustCall(({ code, stdout, stderr }) => { - assert.strictEqual(code, 0); - assert.strictEqual(stdout, ''); - assert.match(stderr, /\[DEP0170\] DeprecationWarning:/); + assert.strictEqual(code, 1); })); }); @@ -94,10 +92,11 @@ if (common.hasIntl) { DeprecationWarning: { // eslint-disable-next-line @stylistic/js/max-len DEP0169: '`url.parse()` behavior is not standardized and prone to errors that have security implications. Use the WHATWG URL API instead. CVEs are not issued for `url.parse()` vulnerabilities.', - DEP0170: `The URL ${badURLs[0]} is invalid. Future versions of Node.js will throw an error.`, }, }); badURLs.forEach((badURL) => { - url.parse(badURL); + assert.throws(() => url.parse(badURL), { + code: 'ERR_INVALID_ARG_VALUE', + }); }); }