From 87b4e3ed49d9e54f3960a1850875b7475f0a8671 Mon Sep 17 00:00:00 2001
From: "Mark S. Everitt" <mark.s.everitt@gmail.com>
Date: Tue, 31 Oct 2017 20:03:28 +0000
Subject: [PATCH] tls: accept array of protocols in TLSSocket

Brings the ALPNProtocols & NPNProtocols options of TLSSocket in line
with the documentation. i.e. an array of strings for protocols may be
used, not only a buffer.

PR-URL: https://github.com/nodejs/node/pull/16655
Fixes: https://github.com/node/issues/16643
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
---
 lib/_tls_wrap.js                              | 22 +++---
 ...et-constructor-alpn-npn-options-parsing.js | 78 +++++++++++++++++++
 2 files changed, 89 insertions(+), 11 deletions(-)
 create mode 100644 test/parallel/test-tls-socket-constructor-alpn-npn-options-parsing.js

diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
index d7e349b239cb05..1bd76a7f97c870 100644
--- a/lib/_tls_wrap.js
+++ b/lib/_tls_wrap.js
@@ -280,11 +280,15 @@ function initRead(tls, wrapped) {
  * Provides a wrap of socket stream to do encrypted communication.
  */
 
-function TLSSocket(socket, options) {
-  if (options === undefined)
-    this._tlsOptions = {};
-  else
-    this._tlsOptions = options;
+function TLSSocket(socket, opts) {
+  const tlsOptions = Object.assign({}, opts);
+
+  if (tlsOptions.NPNProtocols)
+    tls.convertNPNProtocols(tlsOptions.NPNProtocols, tlsOptions);
+  if (tlsOptions.ALPNProtocols)
+    tls.convertALPNProtocols(tlsOptions.ALPNProtocols, tlsOptions);
+
+  this._tlsOptions = tlsOptions;
   this._secureEstablished = false;
   this._securePending = false;
   this._newSessionPending = false;
@@ -1099,11 +1103,7 @@ exports.connect = function(...args /* [port,] [host,] [options,] [cb] */) {
          'options.minDHSize is not a positive number: ' +
          options.minDHSize);
 
-  const NPN = {};
-  const ALPN = {};
   const context = options.secureContext || tls.createSecureContext(options);
-  tls.convertNPNProtocols(options.NPNProtocols, NPN);
-  tls.convertALPNProtocols(options.ALPNProtocols, ALPN);
 
   var socket = new TLSSocket(options.socket, {
     pipe: !!options.path,
@@ -1112,8 +1112,8 @@ exports.connect = function(...args /* [port,] [host,] [options,] [cb] */) {
     requestCert: true,
     rejectUnauthorized: options.rejectUnauthorized !== false,
     session: options.session,
-    NPNProtocols: NPN.NPNProtocols,
-    ALPNProtocols: ALPN.ALPNProtocols,
+    NPNProtocols: options.NPNProtocols,
+    ALPNProtocols: options.ALPNProtocols,
     requestOCSP: options.requestOCSP
   });
 
diff --git a/test/parallel/test-tls-socket-constructor-alpn-npn-options-parsing.js b/test/parallel/test-tls-socket-constructor-alpn-npn-options-parsing.js
new file mode 100644
index 00000000000000..fec06c94eec11f
--- /dev/null
+++ b/test/parallel/test-tls-socket-constructor-alpn-npn-options-parsing.js
@@ -0,0 +1,78 @@
+'use strict';
+
+// Test that TLSSocket can take arrays of strings for ALPNProtocols and
+// NPNProtocols.
+
+const common = require('../common');
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const tls = require('tls');
+
+new tls.TLSSocket(null, {
+  ALPNProtocols: ['http/1.1'],
+  NPNProtocols: ['http/1.1']
+});
+
+if (!process.features.tls_npn)
+  common.skip('node compiled without NPN feature of OpenSSL');
+
+if (!process.features.tls_alpn)
+  common.skip('node compiled without ALPN feature of OpenSSL');
+
+const assert = require('assert');
+const net = require('net');
+const fixtures = require('../common/fixtures');
+
+const key = fixtures.readKey('agent1-key.pem');
+const cert = fixtures.readKey('agent1-cert.pem');
+
+const protocols = [];
+
+const server = net.createServer(common.mustCall((s) => {
+  const tlsSocket = new tls.TLSSocket(s, {
+    isServer: true,
+    server,
+    key,
+    cert,
+    ALPNProtocols: ['http/1.1'],
+    NPNProtocols: ['http/1.1']
+  });
+
+  tlsSocket.on('secure', common.mustCall(() => {
+    protocols.push({
+      alpnProtocol: tlsSocket.alpnProtocol,
+      npnProtocol: tlsSocket.npnProtocol
+    });
+    tlsSocket.end();
+  }));
+}, 2));
+
+server.listen(0, common.mustCall(() => {
+  const alpnOpts = {
+    port: server.address().port,
+    rejectUnauthorized: false,
+    ALPNProtocols: ['h2', 'http/1.1']
+  };
+  const npnOpts = {
+    port: server.address().port,
+    rejectUnauthorized: false,
+    NPNProtocols: ['h2', 'http/1.1']
+  };
+
+  tls.connect(alpnOpts, function() {
+    this.end();
+
+    tls.connect(npnOpts, function() {
+      this.end();
+
+      server.close();
+
+      assert.deepStrictEqual(protocols, [
+        { alpnProtocol: 'http/1.1', npnProtocol: false },
+        { alpnProtocol: false, npnProtocol: 'http/1.1' }
+      ]);
+    });
+  });
+}));