url: allow use of URL with http.request and https.request

jasnell · Italo A. Casas · commit 2f9fdc454fc8 · 2017-01-30T12:08:37.000-05:00
PR-URL: #10638 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Michal Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
diff --git a/lib/_http_client.js b/lib/_http_client.js
@@ -13,7 +13,7 @@ const debug = common.debug;
 const OutgoingMessage = require('_http_outgoing').OutgoingMessage;
 const Agent = require('_http_agent');
 const Buffer = require('buffer').Buffer;
-
+const urlToOptions = require('internal/url').urlToOptions;
 
 // The actual list of disallowed characters in regexp form is more like:
 //    /[^A-Za-z0-9\-._~!$&'()*+,;=/:@]/
@@ -52,6 +52,8 @@ function ClientRequest(options, cb) {
     if (!options.hostname) {
       throw new Error('Unable to determine the domain name');
     }
+  } else if (options instanceof url.URL) {
+    options = urlToOptions(options);
   } else {
     options = util._extend({}, options);
   }
diff --git a/lib/https.js b/lib/https.js
@@ -8,6 +8,7 @@ const http = require('http');
 const util = require('util');
 const inherits = util.inherits;
 const debug = util.debuglog('https');
+const urlToOptions = require('internal/url').urlToOptions;
 
 function Server(opts, requestListener) {
   if (!(this instanceof Server)) return new Server(opts, requestListener);
@@ -196,6 +197,8 @@ exports.request = function request(options, cb) {
     if (!options.hostname) {
       throw new Error('Unable to determine the domain name');
     }
+  } else if (options instanceof url.URL) {
+    options = urlToOptions(options);
   } else {
     options = util._extend({}, options);
   }
diff --git a/lib/internal/url.js b/lib/internal/url.js
@@ -1019,9 +1019,33 @@ function domainToUnicode(domain) {
   return binding.domainToUnicode(String(domain));
 }
 
+// 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 = {
+    protocol: url.protocol,
+    host: url.host,
+    hostname: url.hostname,
+    hash: url.hash,
+    search: url.search,
+    pathname: url.pathname,
+    path: `${url.pathname}${url.search}`,
+    href: url.href
+  };
+  if (url.port !== '') {
+    options.port = Number(url.port);
+  }
+  if (url.username || url.password) {
+    options.auth = `${url.username}:${url.password}`;
+  }
+  return options;
+}
+
 exports.URL = URL;
 exports.URLSearchParams = URLSearchParams;
 exports.originFor = originFor;
 exports.domainToASCII = domainToASCII;
 exports.domainToUnicode = domainToUnicode;
 exports.encodeAuth = encodeAuth;
+exports.urlToOptions = urlToOptions;
diff --git a/test/parallel/test-http-client-get-url.js b/test/parallel/test-http-client-get-url.js
@@ -2,6 +2,8 @@
 const common = require('../common');
 const assert = require('assert');
 const http = require('http');
+const url = require('url');
+const URL = url.URL;
 
 var server = http.createServer(common.mustCall(function(req, res) {
   assert.equal('GET', req.method);
@@ -10,8 +12,11 @@ var server = http.createServer(common.mustCall(function(req, res) {
   res.write('hello\n');
   res.end();
   server.close();
-}));
+}, 3));
 
 server.listen(0, function() {
-  http.get(`http://127.0.0.1:${this.address().port}/foo?bar`);
+  const u = `http://127.0.0.1:${this.address().port}/foo?bar`;
+  http.get(u);
+  http.get(url.parse(u));
+  http.get(new URL(u));
 });
diff --git a/test/parallel/test-https-client-get-url.js b/test/parallel/test-https-client-get-url.js
@@ -12,6 +12,8 @@ if (!common.hasCrypto) {
 const https = require('https');
 
 const fs = require('fs');
+const url = require('url');
+const URL = url.URL;
 
 var options = {
   key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'),
@@ -25,8 +27,11 @@ var server = https.createServer(options, common.mustCall(function(req, res) {
   res.write('hello\n');
   res.end();
   server.close();
-}));
+}, 3));
 
 server.listen(0, function() {
-  https.get(`https://127.0.0.1:${this.address().port}/foo?bar`);
+  const u = `https://127.0.0.1:${this.address().port}/foo?bar`;
+  https.get(u);
+  https.get(url.parse(u));
+  https.get(new URL(u));
 });
diff --git a/test/parallel/test-whatwg-url-properties.js b/test/parallel/test-whatwg-url-properties.js
@@ -1,9 +1,11 @@
+// Flags: --expose-internals
 'use strict';
 
 require('../common');
 
 const URL = require('url').URL;
 const assert = require('assert');
+const urlToOptions = require('internal/url').urlToOptions;
 
 const url = new URL('http://user:pass@foo.bar.com:21/aaa/zzz?l=24#test');
 const oldParams = url.searchParams;  // for test of [SameObject]
@@ -125,3 +127,18 @@ assert.strictEqual(url.toString(),
                    'https://user2:pass2@foo.bar.org:23/aaa/bbb?k=99#abcd');
 assert.strictEqual((delete url.searchParams), true);
 assert.strictEqual(url.searchParams, oldParams);
+
+// Test urlToOptions
+{
+  const opts =
+    urlToOptions(new URL('http://user:pass@foo.bar.com:21/aaa/zzz?l=24#test'));
+  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');
+}
