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: #14564 Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
nodejs · Sep 22, 2017 · 6f1caad · 6f1caad
1 parent ba5a668
commit 6f1caad
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 15 deletions.
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())));