From c3e3fc2ae1df236b6b9cbe29f6027cc85fe4211b Mon Sep 17 00:00:00 2001 From: raisinten Date: Sat, 9 Jan 2021 18:17:43 +0530 Subject: [PATCH 1/6] lib: remove usage of url.parse Since url.parse() is deprecated, it must not be used inside Node.js. --- lib/_http_client.js | 18 +----------------- lib/https.js | 18 +----------------- lib/tls.js | 18 +----------------- 3 files changed, 3 insertions(+), 51 deletions(-) diff --git a/lib/_http_client.js b/lib/_http_client.js index cdf728aa65846d..ae31f915724a4e 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -42,7 +42,6 @@ const { } = primordials; const net = require('net'); -const url = require('url'); const assert = require('internal/assert'); const { once } = require('internal/util'); const { @@ -98,27 +97,12 @@ class HTTPClientAsyncResource { } } -let urlWarningEmitted = false; function ClientRequest(input, options, cb) { FunctionPrototypeCall(OutgoingMessage, this); if (typeof input === 'string') { const urlStr = input; - try { - input = urlToHttpOptions(new URL(urlStr)); - } catch (err) { - input = url.parse(urlStr); - if (!input.hostname) { - throw err; - } - if (!urlWarningEmitted && !process.noDeprecation) { - urlWarningEmitted = true; - process.emitWarning( - `The provided URL ${urlStr} is not a valid URL, and is supported ` + - 'in the http module solely for compatibility.', - 'DeprecationWarning', 'DEP0109'); - } - } + input = urlToOptions(new URL(urlStr)); } else if (input && input[searchParamsSymbol] && input[searchParamsSymbol][searchParamsSymbol]) { // url.URL instance diff --git a/lib/https.js b/lib/https.js index 79ac6c6bd58707..9212107ef76385 100644 --- a/lib/https.js +++ b/lib/https.js @@ -37,7 +37,6 @@ const { require('internal/util').assertCrypto(); const tls = require('tls'); -const url = require('url'); const { Agent: HttpAgent } = require('_http_agent'); const { Server: HttpServer, @@ -296,27 +295,12 @@ Agent.prototype._evictSession = function _evictSession(key) { const globalAgent = new Agent(); -let urlWarningEmitted = false; function request(...args) { let options = {}; if (typeof args[0] === 'string') { const urlStr = ArrayPrototypeShift(args); - try { - options = urlToHttpOptions(new URL(urlStr)); - } catch (err) { - options = url.parse(urlStr); - if (!options.hostname) { - throw err; - } - if (!urlWarningEmitted && !process.noDeprecation) { - urlWarningEmitted = true; - process.emitWarning( - `The provided URL ${urlStr} is not a valid URL, and is supported ` + - 'in the https module solely for compatibility.', - 'DeprecationWarning', 'DEP0109'); - } - } + options = urlToOptions(new URL(urlStr)); } else if (args[0] && args[0][searchParamsSymbol] && args[0][searchParamsSymbol][searchParamsSymbol]) { // url.URL instance diff --git a/lib/tls.js b/lib/tls.js index 49c7d245171b10..eae34a4239145c 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -55,7 +55,6 @@ const { isArrayBufferView } = require('internal/util/types'); const net = require('net'); const { getOptionValue } = require('internal/options'); -const url = require('url'); const { getRootCertificates, getSSLCiphers } = internalBinding('crypto'); const { Buffer } = require('buffer'); const EventEmitter = require('events'); @@ -230,7 +229,6 @@ function check(hostParts, pattern, wildcards) { return true; } -let urlWarningEmitted = false; exports.checkServerIdentity = function checkServerIdentity(hostname, cert) { const subject = cert.subject; const altNames = cert.subjectaltname; @@ -246,21 +244,7 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) { if (StringPrototypeStartsWith(name, 'DNS:')) { ArrayPrototypePush(dnsNames, StringPrototypeSlice(name, 4)); } else if (StringPrototypeStartsWith(name, 'URI:')) { - let uri; - try { - uri = new URL(StringPrototypeSlice(name, 4)); - } catch { - const slicedName = StringPrototypeSlice(name, 4); - uri = url.parse(slicedName); - if (!urlWarningEmitted && !process.noDeprecation) { - urlWarningEmitted = true; - process.emitWarning( - `The URI ${slicedName} found in cert.subjectaltname ` + - 'is not a valid URI, and is supported in the tls module ' + - 'solely for compatibility.', - 'DeprecationWarning', 'DEP0109'); - } - } + const uri = new URL(StringPrototypeSlice(name, 4)); // TODO(bnoordhuis) Also use scheme. ArrayPrototypePush(uriNames, uri.hostname); From 9a1a62e25d4c8b4eb11741236396dfcb63ab10a4 Mon Sep 17 00:00:00 2001 From: raisinten Date: Sat, 9 Jan 2021 19:48:44 +0530 Subject: [PATCH 2/6] test: remove deprecated url.parse tests --- test/parallel/test-http-deprecated-urls.js | 33 ------------------- .../test-tls-check-server-identity.js | 14 -------- 2 files changed, 47 deletions(-) delete mode 100644 test/parallel/test-http-deprecated-urls.js diff --git a/test/parallel/test-http-deprecated-urls.js b/test/parallel/test-http-deprecated-urls.js deleted file mode 100644 index 5bb9f66ebe7126..00000000000000 --- a/test/parallel/test-http-deprecated-urls.js +++ /dev/null @@ -1,33 +0,0 @@ -/* eslint-disable node-core/crypto-check */ - -'use strict'; - -const common = require('../common'); - -const http = require('http'); -const modules = { http }; - -const deprecations = [ - ['The provided URL http://[www.nodejs.org] is not a valid URL, and is supported ' + - 'in the http module solely for compatibility.', - 'DEP0109'], -]; - -if (common.hasCrypto) { - const https = require('https'); - modules.https = https; - deprecations.push( - ['The provided URL https://[www.nodejs.org] is not a valid URL, and is supported ' + - 'in the https module solely for compatibility.', - 'DEP0109'], - ); -} - -common.expectWarning('DeprecationWarning', deprecations); - -Object.keys(modules).forEach((module) => { - const doNotCall = common.mustNotCall( - `${module}.request should not connect to ${module}://[www.nodejs.org]` - ); - modules[module].request(`${module}://[www.nodejs.org]`, doNotCall).abort(); -}); diff --git a/test/parallel/test-tls-check-server-identity.js b/test/parallel/test-tls-check-server-identity.js index deeeecdec9ec50..ad79d93a3df6fd 100644 --- a/test/parallel/test-tls-check-server-identity.js +++ b/test/parallel/test-tls-check-server-identity.js @@ -30,13 +30,6 @@ const util = require('util'); const tls = require('tls'); -common.expectWarning('DeprecationWarning', [ - ['The URI http://[a.b.a.com]/ found in cert.subjectaltname ' + - 'is not a valid URI, and is supported in the tls module ' + - 'solely for compatibility.', - 'DEP0109'], -]); - const tests = [ // False-y values. { @@ -282,13 +275,6 @@ const tests = [ error: 'Host: a.b.a.com. is not in the cert\'s altnames: ' + 'URI:http://*.b.a.com/' }, - // Invalid URI - { - host: 'a.b.a.com', cert: { - subjectaltname: 'URI:http://[a.b.a.com]/', - subject: {} - } - }, // IP addresses { host: 'a.b.a.com', cert: { From 7136d862bc94b86c70e7d8ef2864968b7423281e Mon Sep 17 00:00:00 2001 From: raisinten Date: Sat, 9 Jan 2021 20:03:30 +0530 Subject: [PATCH 3/6] doc: update DEP0109 to EOL --- doc/api/deprecations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index ca1a0c6ba4b4ca..488b347bfbe9be 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -2048,7 +2048,7 @@ because it also made sense to interpret the value as the number of bytes read by the engine, but is inconsistent with other streams in Node.js that expose values under these names. -### DEP0109: `http`, `https`, and `tls` support for invalid URLs +### EOL: `http`, `https`, and `tls` support for invalid URLs -Type: Runtime +Type: End-of-Life Some previously supported (but strictly invalid) URLs were accepted through the [`http.request()`][], [`http.get()`][], [`https.request()`][], From 6d5b91086c6c57605948c2f9cf149168660880e7 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Tue, 12 Jan 2021 19:06:55 +0530 Subject: [PATCH 5/6] fixup! doc: update DEP0109 to EOL Co-authored-by: Antoine du Hamel --- doc/api/deprecations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index c2d01bfa0b7603..638879cb53d9ee 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -2051,7 +2051,7 @@ expose values under these names. ### DEP0109: `http`, `https`, and `tls` support for invalid URLs