From 6f1caadb85f70bb902c078d9b7a96e3c4b36b31f 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 2c5a05e2dcf7bb..4ed0dbda6cc0f0 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 7615de6bd71cb5..0ca5f2c9705de8 100644
--- a/lib/_tls_wrap.js
+++ b/lib/_tls_wrap.js
@@ -1051,7 +1051,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,
@@ -1066,19 +1066,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())));