Skip to content

Commit 985fa6a

Browse files
committed
url: move bad port deprecation in legacy url to end-of-life
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.
1 parent f58613a commit 985fa6a

4 files changed

Lines changed: 14 additions & 32 deletions

File tree

doc/api/deprecations.md

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3545,6 +3545,9 @@ issued for `url.parse()` vulnerabilities.
35453545

35463546
<!-- YAML
35473547
changes:
3548+
- version: REPLACEME
3549+
pr-url: https://github.com/nodejs/node/pull/00000
3550+
description: End-of-Life.
35483551
- version:
35493552
- v20.0.0
35503553
pr-url: https://github.com/nodejs/node/pull/45526
@@ -3556,11 +3559,11 @@ changes:
35563559
description: Documentation-only deprecation.
35573560
-->
35583561

3559-
Type: Runtime
3562+
Type: End-of-Life
35603563

3561-
[`url.parse()`][] accepts URLs with ports that are not numbers. This behavior
3562-
might result in host name spoofing with unexpected input. These URLs will throw
3563-
an error in future versions of Node.js, as the [WHATWG URL API][] does already.
3564+
[`url.parse()`][] used to accept URLs with ports that are not numbers. This
3565+
behavior might result in host name spoofing with unexpected input. These URLs
3566+
will throw an error as the [WHATWG URL API][] does already.
35643567

35653568
### DEP0171: Setters for `http.IncomingMessage` headers and trailers
35663569

lib/url.js

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ const querystring = require('querystring');
4141
const {
4242
ERR_INVALID_ARG_TYPE,
4343
ERR_INVALID_URL,
44+
ERR_INVALID_ARG_VALUE,
4445
} = require('internal/errors').codes;
4546
const {
4647
validateString,
@@ -501,7 +502,6 @@ Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) {
501502
return this;
502503
};
503504

504-
let warnInvalidPort = true;
505505
function getHostname(self, rest, hostname, url) {
506506
for (let i = 0; i < hostname.length; ++i) {
507507
const code = hostname.charCodeAt(i);
@@ -513,12 +513,8 @@ function getHostname(self, rest, hostname, url) {
513513

514514
if (!isValid) {
515515
// If leftover starts with :, then it represents an invalid port.
516-
// But url.parse() is lenient about it for now.
517-
// Issue a warning and continue.
518-
if (warnInvalidPort && code === CHAR_COLON) {
519-
const detail = `The URL ${url} is invalid. Future versions of Node.js will throw an error.`;
520-
process.emitWarning(detail, 'DeprecationWarning', 'DEP0170');
521-
warnInvalidPort = false;
516+
if (code === CHAR_COLON) {
517+
throw new ERR_INVALID_ARG_VALUE('url', 'Invalid port in url', url);
522518
}
523519
self.hostname = hostname.slice(0, i);
524520
return `/${hostname.slice(i)}${rest}`;

test/parallel/test-url-parse-format.js

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -862,22 +862,6 @@ const parseTests = {
862862
href: 'http://a%22%20%3C\'b:b@cd/e?f'
863863
},
864864

865-
// Git urls used by npm
866-
'git+ssh://git@github.com:npm/npm': {
867-
protocol: 'git+ssh:',
868-
slashes: true,
869-
auth: 'git',
870-
host: 'github.com',
871-
port: null,
872-
hostname: 'github.com',
873-
hash: null,
874-
search: null,
875-
query: null,
876-
pathname: '/:npm/npm',
877-
path: '/:npm/npm',
878-
href: 'git+ssh://git@github.com/:npm/npm'
879-
},
880-
881865
'https://*': {
882866
protocol: 'https:',
883867
slashes: true,

test/parallel/test-url-parse-invalid-input.js

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,7 @@ if (common.hasIntl) {
8383
badURLs.forEach((badURL) => {
8484
common.spawnPromisified(process.execPath, ['-e', `url.parse(${JSON.stringify(badURL)})`])
8585
.then(common.mustCall(({ code, stdout, stderr }) => {
86-
assert.strictEqual(code, 0);
87-
assert.strictEqual(stdout, '');
88-
assert.match(stderr, /\[DEP0170\] DeprecationWarning:/);
86+
assert.strictEqual(code, 1);
8987
}));
9088
});
9189

@@ -94,10 +92,11 @@ if (common.hasIntl) {
9492
DeprecationWarning: {
9593
// eslint-disable-next-line @stylistic/js/max-len
9694
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.',
97-
DEP0170: `The URL ${badURLs[0]} is invalid. Future versions of Node.js will throw an error.`,
9895
},
9996
});
10097
badURLs.forEach((badURL) => {
101-
url.parse(badURL);
98+
assert.throws(() => url.parse(badURL), {
99+
code: 'ERR_INVALID_ARG_VALUE',
100+
});
102101
});
103102
}

0 commit comments

Comments
 (0)