https: allow url and options to be passed to https.request

PR-URL: nodejs#22003 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
Trott · Aug 1, 2018 · 19aa41c · 19aa41c
1 parent ede279c
commit 19aa41c
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 11 deletions.
diff --git a/lib/https.js b/lib/https.js
@@ -255,9 +255,11 @@ Agent.prototype._evictSession = function _evictSession(key) {
 const globalAgent = new Agent();
 
 let urlWarningEmitted = false;
-function request(options, cb) {
-  if (typeof options === 'string') {
-    const urlStr = options;
+function request(...args) {
+  let options = {};
+
+  if (typeof args[0] === 'string') {
+    const urlStr = args.shift();
     try {
       options = urlToOptions(new URL(urlStr));
     } catch (err) {
@@ -273,19 +275,24 @@ function request(options, cb) {
           'DeprecationWarning', 'DEP0109');
       }
     }
-  } else if (options && options[searchParamsSymbol] &&
-             options[searchParamsSymbol][searchParamsSymbol]) {
+  } else if (args[0] && args[0][searchParamsSymbol] &&
+             args[0][searchParamsSymbol][searchParamsSymbol]) {
     // url.URL instance
-    options = urlToOptions(options);
-  } else {
-    options = util._extend({}, options);
+    options = urlToOptions(args.shift());
+  }
+
+  if (args[0] && typeof args[0] !== 'function') {
+    options = util._extend(options, args.shift());
   }
+
   options._defaultAgent = globalAgent;
-  return new ClientRequest(options, cb);
+  args.unshift(options);
+
+  return new ClientRequest(...args);
 }
 
-function get(options, cb) {
-  const req = request(options, cb);
+function get(input, options, cb) {
+  const req = request(input, options, cb);
   req.end();
   return req;
 }

diff --git a/test/parallel/test-https-request-arguments.js b/test/parallel/test-https-request-arguments.js
@@ -0,0 +1,46 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const https = require('https');
+const fixtures = require('../common/fixtures');
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const options = {
+  key: fixtures.readKey('agent1-key.pem'),
+  cert: fixtures.readKey('agent1-cert.pem'),
+  ca: fixtures.readKey('ca1-cert.pem')
+};
+
+// Test providing both a url and options, with the options partially
+// replacing address and port portions of the URL provided.
+{
+  const server = https.createServer(
+    options,
+    common.mustCall((req, res) => {
+      assert.strictEqual(req.url, '/testpath');
+      res.end();
+      server.close();
+    })
+  );
+
+  server.listen(
+    0,
+    common.mustCall(() => {
+      https.get(
+        'https://example.com/testpath',
+
+        {
+          hostname: 'localhost',
+          port: server.address().port,
+          rejectUnauthorized: false
+        },
+
+        common.mustCall((res) => {
+          res.resume();
+        })
+      );
+    })
+  );
+}