tls: prefer path over port in connect

bengl · bengl · commit 27cf4d23d83f · 2017-09-20T00:32:18.000-07:00
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.
diff --git 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
@@ -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
@@ -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())));
