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: #16402
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: princjef

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:
 

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;
 };
 

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();
+      }));
+    })
+  ));
+}));

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'
 );