diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index a0ddcf40acc445..63bdb60c179400 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -3302,13 +3302,17 @@ issued for `url.parse()` vulnerabilities. -Type: Documentation-only +Type: Runtime [`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 diff --git a/lib/url.js b/lib/url.js index 4d7374a8e3f358..b9ce18f3ba4ca3 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); + rest = getHostname(this, rest, hostname, url); } if (this.hostname.length > hostnameMaxLen) { @@ -506,7 +506,8 @@ Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) { return this; }; -function getHostname(self, rest, hostname) { +let warnInvalidPort = true; +function getHostname(self, rest, hostname, url) { for (let i = 0; i < hostname.length; ++i) { const code = hostname.charCodeAt(i); const isValid = (code !== CHAR_FORWARD_SLASH && @@ -516,6 +517,14 @@ function getHostname(self, rest, hostname) { code !== CHAR_COLON); 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; + } self.hostname = hostname.slice(0, i); return `/${hostname.slice(i)}${rest}`; } diff --git a/test/parallel/test-url-parse-invalid-input.js b/test/parallel/test-url-parse-invalid-input.js index 794a756524925b..023f74ee86a70e 100644 --- a/test/parallel/test-url-parse-invalid-input.js +++ b/test/parallel/test-url-parse-invalid-input.js @@ -1,6 +1,7 @@ 'use strict'; const common = require('../common'); const assert = require('assert'); +const childProcess = require('child_process'); const url = require('url'); // https://github.com/joyent/node/issues/568 @@ -74,3 +75,31 @@ 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) => { + childProcess.exec(`${process.execPath} -e "url.parse('${badURL}')"`, + common.mustCall((err, stdout, stderr) => { + assert.strictEqual(err, null); + assert.strictEqual(stdout, ''); + assert.match(stderr, /\[DEP0170\] DeprecationWarning:/); + }) + ); + }); + + // Warning should only happen once per process. + const expectedWarning = [ + `The URL ${badURLs[0]} is invalid. Future versions of Node.js will throw an error.`, + 'DEP0170', + ]; + common.expectWarning({ + DeprecationWarning: expectedWarning, + }); + badURLs.forEach((badURL) => { + url.parse(badURL); + }); +}