Skip to content

Commit

Permalink
tls: prefer path over port in connect
Browse files Browse the repository at this point in the history
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: nodejs/node#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>
  • Loading branch information
bengl authored and addaleax committed Sep 23, 2017
1 parent ad61c91 commit 47d1691
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 15 deletions.
3 changes: 2 additions & 1 deletion doc/api/net.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

Expand Down
24 changes: 10 additions & 14 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();
});
}
Expand Down
64 changes: 64 additions & 0 deletions test/parallel/test-tls-net-connect-prefer-path.js
Original file line number Diff line number Diff line change
@@ -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())));

0 comments on commit 47d1691

Please sign in to comment.