https: add extra options to Agent#getName()

Adds the remaining options from tls.createSecureContext() to the
string generated by Agent#getName(). This allows https.request() to
accept the options and generate unique sockets appropriately.

PR-URL: https://github.com/nodejs/node/pull/16402
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

5 files changed, 127 insertions, 64 deletions

diff --git a/doc/api/https.md b/doc/api/https.md
index 2cc7502012..58e62ccced 100644
--- a/doc/api/https.md
+++ b/doc/api/https.md
@@ -12,7 +12,7 @@ separate module.
 added: v0.4.5
 -->
 
-An Agent object for HTTPS similar to [`http.Agent`][].  See [`https.request()`][]
+An [`Agent`][] object for HTTPS similar to [`http.Agent`][].  See [`https.request()`][]
 for more information.
 
 ## Class: https.Server
@@ -168,9 +168,10 @@ changes:
 
 Makes a request to a secure web server.
 
-The following additional `options` from [`tls.connect()`][] are also accepted
-when using a custom [`Agent`][]: `ca`, `cert`, `ciphers`, `clientCertEngine`,
-`key`, `passphrase`, `pfx`, `rejectUnauthorized`, `secureProtocol`, `servername`
+The following additional `options` from [`tls.connect()`][] are also accepted:
+`ca`, `cert`, `ciphers`, `clientCertEngine`, `crl`, `dhparam`, `ecdhCurve`,
+`honorCipherOrder`, `key`, `passphrase`, `pfx`, `rejectUnauthorized`,
+`secureOptions`, `secureProtocol`, `servername`, `sessionIdContext`
 
 `options` can be an object, a string, or a [`URL`][] object. If `options` is a
 string, it is automatically parsed with [`url.parse()`][]. If it is a [`URL`][]
@@ -220,7 +221,7 @@ const req = https.request(options, (res) => {
 });
 ```
 
-Alternatively, opt out of connection pooling by not using an `Agent`.
+Alternatively, opt out of connection pooling by not using an [`Agent`][].
 
 Example:
 
diff --git a/lib/https.js b/lib/https.js
index 741ce84d2f..84ddeb5036 100644
--- a/lib/https.js
+++ b/lib/https.js
@@ -194,6 +194,30 @@ Agent.prototype.getName = function getName(options) {
   if (options.secureProtocol)
     name += options.secureProtocol;
 
+  name += ':';
+  if (options.crl)
+    name += options.crl;
+
+  name += ':';
+  if (options.honorCipherOrder !== undefined)
+    name += options.honorCipherOrder;
+
+  name += ':';
+  if (options.ecdhCurve)
+    name += options.ecdhCurve;
+
+  name += ':';
+  if (options.dhparam)
+    name += options.dhparam;
+
+  name += ':';
+  if (options.secureOptions !== undefined)
+    name += options.secureOptions;
+
+  name += ':';
+  if (options.sessionIdContext)
+    name += options.sessionIdContext;
+
   return name;
 };
 
diff --git a/test/parallel/test-https-agent-additional-options.js b/test/parallel/test-https-agent-additional-options.js
new file mode 100644
index 0000000000..8d10524d90
--- /dev/null
+++ b/test/parallel/test-https-agent-additional-options.js
@@ -0,0 +1,87 @@
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const assert = require('assert');
+const crypto = require('crypto');
+const https = require('https');
+const fixtures = require('../common/fixtures');
+
+const options = {
+  key: fixtures.readKey('agent1-key.pem'),
+  cert: fixtures.readKey('agent1-cert.pem'),
+  ca: fixtures.readKey('ca1-cert.pem')
+};
+
+const server = https.Server(options, function(req, res) {
+  res.writeHead(200);
+  res.end('hello world\n');
+});
+
+function getBaseOptions(port) {
+  return {
+    path: '/',
+    port: port,
+    ca: options.ca,
+    rejectUnautorized: true,
+    servername: 'agent1',
+  };
+}
+
+const updatedValues = new Map([
+  ['dhparam', fixtures.readKey('dh2048.pem')],
+  ['ecdhCurve', 'secp384r1'],
+  ['honorCipherOrder', true],
+  ['secureOptions', crypto.constants.SSL_OP_CIPHER_SERVER_PREFERENCE],
+  ['secureProtocol', 'TLSv1_method'],
+  ['sessionIdContext', 'sessionIdContext'],
+]);
+
+function variations(iter, port, cb) {
+  const { done, value } = iter.next();
+  if (done) {
+    return common.mustCall(cb);
+  } else {
+    const [key, val] = value;
+    return common.mustCall(function(res) {
+      res.resume();
+      https.globalAgent.once('free', common.mustCall(function() {
+        https.get(
+          Object.assign({}, getBaseOptions(port), { [key]: val }),
+          variations(iter, port, cb)
+        );
+      }));
+    });
+  }
+}
+
+server.listen(0, common.mustCall(function() {
+  const port = this.address().port;
+  const globalAgent = https.globalAgent;
+  globalAgent.keepAlive = true;
+  https.get(getBaseOptions(port), variations(
+    updatedValues.entries(),
+    port,
+    common.mustCall(function(res) {
+      res.resume();
+      globalAgent.once('free', common.mustCall(function() {
+        // Verify that different keep-alived connections are created
+        // for the base call and each variation
+        const keys = Object.keys(globalAgent.freeSockets);
+        assert.strictEqual(keys.length, 1 + updatedValues.size);
+        let i = 1;
+        for (const [, value] of updatedValues) {
+          assert.ok(
+            keys[i].startsWith(value.toString() + ':') ||
+            keys[i].endsWith(':' + value.toString()) ||
+            keys[i].includes(':' + value.toString() + ':')
+          );
+          i++;
+        }
+        globalAgent.destroy();
+        server.close();
+      }));
+    })
+  ));
+}));
diff --git a/test/parallel/test-https-agent-getname.js b/test/parallel/test-https-agent-getname.js
index 0cdc9568d8..c29e09731d 100644
--- a/test/parallel/test-https-agent-getname.js
+++ b/test/parallel/test-https-agent-getname.js
@@ -12,7 +12,7 @@ const agent = new https.Agent();
 // empty options
 assert.strictEqual(
   agent.getName({}),
-  'localhost:::::::::::'
+  'localhost:::::::::::::::::'
 );
 
 // pass all options arguments
@@ -23,13 +23,21 @@ const options = {
   ca: 'ca',
   cert: 'cert',
   ciphers: 'ciphers',
+  crl: [Buffer.from('c'), Buffer.from('r'), Buffer.from('l')],
+  dhparam: 'dhparam',
+  ecdhCurve: 'ecdhCurve',
+  honorCipherOrder: false,
   key: 'key',
   pfx: 'pfx',
   rejectUnauthorized: false,
+  secureOptions: 0,
+  secureProtocol: 'secureProtocol',
   servername: 'localhost',
+  sessionIdContext: 'sessionIdContext'
 };
 
 assert.strictEqual(
   agent.getName(options),
-  '0.0.0.0:443:192.168.1.1:ca:cert::ciphers:key:pfx:false:localhost:'
+  '0.0.0.0:443:192.168.1.1:ca:cert::ciphers:key:pfx:false:localhost:' +
+    'secureProtocol:c,r,l:false:ecdhCurve:dhparam:0:sessionIdContext'
 );
diff --git a/test/parallel/test-https-agent-secure-protocol.js b/test/parallel/test-https-agent-secure-protocol.js
deleted file mode 100644
index 82554952e8..0000000000
--- a/test/parallel/test-https-agent-secure-protocol.js
+++ /dev/null
@@ -1,57 +0,0 @@
-'use strict';
-const common = require('../common');
-if (!common.hasCrypto)
-  common.skip('missing crypto');
-
-const assert = require('assert');
-const https = require('https');
-const fixtures = require('../common/fixtures');
-
-const options = {
-  key: fixtures.readKey('agent1-key.pem'),
-  cert: fixtures.readKey('agent1-cert.pem'),
-  ca: fixtures.readKey('ca1-cert.pem')
-};
-
-const server = https.Server(options, function(req, res) {
-  res.writeHead(200);
-  res.end('hello world\n');
-});
-
-server.listen(0, common.mustCall(function() {
-  const port = this.address().port;
-  const globalAgent = https.globalAgent;
-  globalAgent.keepAlive = true;
-  https.get({
-    path: '/',
-    port: port,
-    ca: options.ca,
-    rejectUnauthorized: true,
-    servername: 'agent1',
-    secureProtocol: 'SSLv23_method'
-  }, common.mustCall(function(res) {
-    res.resume();
-    globalAgent.once('free', common.mustCall(function() {
-      https.get({
-        path: '/',
-        port: port,
-        ca: options.ca,
-        rejectUnauthorized: true,
-        servername: 'agent1',
-        secureProtocol: 'TLSv1_method'
-      }, common.mustCall(function(res) {
-        res.resume();
-        globalAgent.once('free', common.mustCall(function() {
-          // Verify that two keep-alive connections are created
-          // due to the different secureProtocol settings:
-          const keys = Object.keys(globalAgent.freeSockets);
-          assert.strictEqual(keys.length, 2);
-          assert.ok(keys[0].includes(':SSLv23_method'));
-          assert.ok(keys[1].includes(':TLSv1_method'));
-          globalAgent.destroy();
-          server.close();
-        }));
-      }));
-    }));
-  }));
-}));