From ee157e5a7ffc81ba6be588460a8fba6a89528413 Mon Sep 17 00:00:00 2001
From: Bryan English <bryan@bryanenglish.com>
Date: Mon, 31 Jul 2017 23:58:39 -0700
Subject: [PATCH] tls: prefer path over port in connect

Makes tls.connect() behave as documented, preferring options.path over
options.port. This makes it consistent with net.connect(), so the
included test demonstrates that both behave in this way.

Also, for consistency, noting the precedence of options.path in net
doc.

PR-URL: https://github.com/nodejs/node/pull/14564
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
---
 doc/api/net.md                                |  3 +-
 lib/_tls_wrap.js                              | 24 +++----
 .../test-tls-net-connect-prefer-path.js       | 64 +++++++++++++++++++
 3 files changed, 76 insertions(+), 15 deletions(-)
 create mode 100644 test/parallel/test-tls-net-connect-prefer-path.js

diff --git a/doc/api/net.md b/doc/api/net.md
index 36280c06493350..2eeb88f28e6dea 100644
--- a/doc/api/net.md
+++ b/doc/api/net.md
@@ -591,7 +591,8 @@ For TCP connections, available `options` are:
 For [IPC][] connections, available `options` are:
 
 * `path` {string} Required. Path the client should connect to.
-  See [Identifying paths for IPC connections][].
+  See [Identifying paths for IPC connections][]. If provided, the TCP-specific
+  options above are ignored.
 
 Returns `socket`.
 
diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
index fde93a79fcc596..c51720016075ef 100644
--- a/lib/_tls_wrap.js
+++ b/lib/_tls_wrap.js
@@ -1050,7 +1050,7 @@ exports.connect = function(...args /* [port,] [host,] [options,] [cb] */) {
   tls.convertALPNProtocols(options.ALPNProtocols, ALPN);
 
   var socket = new TLSSocket(options.socket, {
-    pipe: options.path && !options.port,
+    pipe: !!options.path,
     secureContext: context,
     isServer: false,
     requestCert: true,
@@ -1065,19 +1065,15 @@ exports.connect = function(...args /* [port,] [host,] [options,] [cb] */) {
     socket.once('secureConnect', cb);
 
   if (!options.socket) {
-    var connect_opt;
-    if (options.path && !options.port) {
-      connect_opt = { path: options.path };
-    } else {
-      connect_opt = {
-        port: options.port,
-        host: options.host,
-        family: options.family,
-        localAddress: options.localAddress,
-        lookup: options.lookup
-      };
-    }
-    socket.connect(connect_opt, function() {
+    const connectOpt = {
+      path: options.path,
+      port: options.port,
+      host: options.host,
+      family: options.family,
+      localAddress: options.localAddress,
+      lookup: options.lookup
+    };
+    socket.connect(connectOpt, function() {
       socket._start();
     });
   }
diff --git a/test/parallel/test-tls-net-connect-prefer-path.js b/test/parallel/test-tls-net-connect-prefer-path.js
new file mode 100644
index 00000000000000..10f00b4eb2b253
--- /dev/null
+++ b/test/parallel/test-tls-net-connect-prefer-path.js
@@ -0,0 +1,64 @@
+'use strict';
+const common = require('../common');
+
+// This tests that both tls and net will ignore host and port if path is
+// provided.
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+common.refreshTmpDir();
+
+const tls = require('tls');
+const net = require('net');
+const fs = require('fs');
+const assert = require('assert');
+
+function libName(lib) {
+  return lib === net ? 'net' : 'tls';
+}
+
+function mkServer(lib, tcp, cb) {
+  const handler = (socket) => {
+    socket.write(`${libName(lib)}:${
+      server.address().port || server.address()
+    }`);
+    socket.end();
+  };
+  const args = [handler];
+  if (lib === tls) {
+    args.unshift({
+      cert: fs.readFileSync(`${common.fixturesDir}/test_cert.pem`),
+      key: fs.readFileSync(`${common.fixturesDir}/test_key.pem`)
+    });
+  }
+  const server = lib.createServer(...args);
+  server.listen(tcp ? 0 : common.PIPE, common.mustCall(() => cb(server)));
+}
+
+function testLib(lib, cb) {
+  mkServer(lib, true, (tcpServer) => {
+    mkServer(lib, false, (unixServer) => {
+      const client = lib.connect({
+        path: unixServer.address(),
+        port: tcpServer.address().port,
+        host: 'localhost',
+        rejectUnauthorized: false
+      }, () => {
+        const bufs = [];
+        client.on('data', common.mustCall((d) => {
+          bufs.push(d);
+        }));
+        client.on('end', common.mustCall(() => {
+          const resp = Buffer.concat(bufs).toString();
+          assert.strictEqual(`${libName(lib)}:${unixServer.address()}`, resp);
+          tcpServer.close();
+          unixServer.close();
+          cb();
+        }));
+      });
+    });
+  });
+}
+
+testLib(net, common.mustCall(() => testLib(tls, common.mustCall())));