WIP: http: replace url.parse with WHATWG URL parser

doc/api/deprecations.md

@@ -994,6 +994,16 @@ 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.
 
+<a id="DEP00XX"></a>
+### DEP00XX: http.request() and http.get() URL parsing
+
+Type: Runtime
+
+Some previously supported (but strictly invalid) URLs were accepted through the
+[`http.request()`][] and related APIs. This behavior is now deprecated. It is
+recommended that one passes a [`URL`][] object to the API as the first argument
+which ensures URL validity.
+
 [`--pending-deprecation`]: cli.html#cli_pending_deprecation
 [`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size
 [`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array
@@ -1009,6 +1019,7 @@ expose values under these names.
 [`Server.getConnections()`]: net.html#net_server_getconnections_callback
 [`Server.listen({fd: <number>})`]: net.html#net_server_listen_handle_backlog_callback
 [`SlowBuffer`]: buffer.html#buffer_class_slowbuffer
+[`URL`]: url.html#url_class_url
 [`asyncResource.runInAsyncScope()`]: async_hooks.html#async_hooks_asyncresource_runinasyncscope_fn_thisarg_args
 [`child_process`]: child_process.html
 [`console.error()`]: console.html#console_console_error_data_args
@@ -1035,6 +1046,7 @@ expose values under these names.
 [`fs.read()`]: fs.html#fs_fs_read_fd_buffer_offset_length_position_callback
 [`fs.readSync()`]: fs.html#fs_fs_readsync_fd_buffer_offset_length_position
 [`fs.stat()`]: fs.html#fs_fs_stat_path_callback
+[`http.request()`]: http.html#http_http_request_options_callback
 [`os.networkInterfaces`]: os.html#os_os_networkinterfaces
 [`os.tmpdir()`]: os.html#os_os_tmpdir
 [`process.env`]: process.html#process_process_env

lib/_http_client.js

@@ -37,7 +37,7 @@ const { OutgoingMessage } = require('_http_outgoing');
 const Agent = require('_http_agent');
 const { Buffer } = require('buffer');
 const { defaultTriggerAsyncIdScope } = require('internal/async_hooks');
-const { urlToOptions, searchParamsSymbol } = require('internal/url');
+const { urlToOptions, searchParamsSymbol, URL } = require('internal/url');
 const { outHeadersKey, ondrain } = require('internal/http');
 const {
   ERR_HTTP_HEADERS_SENT,
@@ -51,6 +51,8 @@ const { validateTimerDuration } = require('internal/timers');
 
 const INVALID_PATH_REGEX = /[^\u0021-\u00ff]/;
 
+let urlWarningEmitted = false;
+
 function validateHost(host, name) {
   if (host !== null && host !== undefined && typeof host !== 'string') {
     throw new ERR_INVALID_ARG_TYPE(`options.${name}`,
@@ -64,9 +66,25 @@ function ClientRequest(options, cb) {
   OutgoingMessage.call(this);
 
   if (typeof options === 'string') {
-    options = url.parse(options);
-    if (!options.hostname) {
-      throw new ERR_INVALID_DOMAIN_NAME();
+    const urlStr = options;
+    try {
+      options = urlToOptions(new URL(urlStr));
+    } catch (err) {
+      try {
+        options = url.parse(urlStr);
+        if (!options.hostname) {
+          throw new ERR_INVALID_DOMAIN_NAME();
+        }
+        if (!urlWarningEmitted) {
+          urlWarningEmitted = true;
+          process.emitWarning(
+            `The provided URL ${urlStr} is not a valid URL, and is supported ` +
+            'in the http module solely for compatibility.',
+            'DeprecationWarning', 'DEP00XX');
+        }
+      } catch (urlParseErr) {
+        throw err;
+      }
     }
   } else if (options && options[searchParamsSymbol] &&
              options[searchParamsSymbol][searchParamsSymbol]) {

lib/internal/safe_globals.js

@@ -21,6 +21,7 @@ const makeSafe = (unsafe, safe) => {
   return safe;
 };
 
+exports.makeSafe = makeSafe;
 exports.SafeMap = makeSafe(Map, class SafeMap extends Map {});
 exports.SafeSet = makeSafe(Set, class SafeSet extends Set {});
 exports.SafePromise = makeSafe(Promise, class SafePromise extends Promise {});

lib/internal/url.js

@@ -6,6 +6,8 @@ const {
   isHexTable
 } = require('internal/querystring');
 
+const { makeSafe } = require('internal/safe_globals');
+
 const { getConstructorOf, removeColors } = require('internal/util');
 const {
   ERR_ARG_NOT_ITERABLE,
@@ -1308,11 +1310,26 @@ function domainToUnicode(domain) {
   return _domainToUnicode(`${domain}`);
 }
 
+const SafeURL = makeSafe(URL, class SafeURL extends URL {
+  // Wraps an existing URL in SafeURL's prototype object so we can be sure that
+  // the getters/setters have not been tampered with. No such guarantees for
+  // the url.searchParams object however. All modifications reflect on the
+  // original URL object.
+  static fromURL(url) {
+    return {
+      __proto__: SafeURL.prototype,
+      [context]: url[context],
+      [searchParams]: url[searchParams]
+    };
+  }
+});
+
 // Utility function that converts a URL object into an ordinary
 // options object as expected by the http.request and https.request
 // APIs.
 function urlToOptions(url) {
-  var options = {
+  url = SafeURL.fromURL(url);
+  const options = {
     protocol: url.protocol,
     hostname: url.hostname.startsWith('[') ?
       url.hostname.slice(1, -1) :

test/parallel/test-whatwg-url-properties.js

@@ -148,6 +148,25 @@ assert.strictEqual(url.searchParams, oldParams);
   assert.strictEqual(hostname, '::1');
 }
 
+// Ensure urlToOptions works on URL with prototype that has been tampered with.
+{
+  const url = new URL('http://user:[email protected]:21/aaa/zzz?l=24#test');
+  Object.setPrototypeOf(url, Object.create(null));
+  const opts = urlToOptions(url);
+  assert.strictEqual(opts instanceof URL, false);
+  assert.strictEqual(opts.protocol, 'http:');
+  assert.strictEqual(opts.auth, 'user:pass');
+  assert.strictEqual(opts.hostname, 'foo.bar.com');
+  assert.strictEqual(opts.port, 21);
+  assert.strictEqual(opts.path, '/aaa/zzz?l=24');
+  assert.strictEqual(opts.pathname, '/aaa/zzz');
+  assert.strictEqual(opts.search, '?l=24');
+  assert.strictEqual(opts.hash, '#test');
+
+  const { hostname } = urlToOptions(new URL('http://[::1]:21'));
+  assert.strictEqual(hostname, '::1');
+}
+
 // Test special origins
 [
   { expected: 'https://whatwg.org',