Skip to content

Commit 6f1caad

Browse files
committed
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]>
1 parent ba5a668 commit 6f1caad

File tree

3 files changed

+76
-15
lines changed

3 files changed

+76
-15
lines changed

doc/api/net.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,8 @@ For TCP connections, available `options` are:
591591
For [IPC][] connections, available `options` are:
592592

593593
* `path` {string} Required. Path the client should connect to.
594-
See [Identifying paths for IPC connections][].
594+
See [Identifying paths for IPC connections][]. If provided, the TCP-specific
595+
options above are ignored.
595596

596597
Returns `socket`.
597598

lib/_tls_wrap.js

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1051,7 +1051,7 @@ exports.connect = function(...args /* [port,] [host,] [options,] [cb] */) {
10511051
tls.convertALPNProtocols(options.ALPNProtocols, ALPN);
10521052

10531053
var socket = new TLSSocket(options.socket, {
1054-
pipe: options.path && !options.port,
1054+
pipe: !!options.path,
10551055
secureContext: context,
10561056
isServer: false,
10571057
requestCert: true,
@@ -1066,19 +1066,15 @@ exports.connect = function(...args /* [port,] [host,] [options,] [cb] */) {
10661066
socket.once('secureConnect', cb);
10671067

10681068
if (!options.socket) {
1069-
var connect_opt;
1070-
if (options.path && !options.port) {
1071-
connect_opt = { path: options.path };
1072-
} else {
1073-
connect_opt = {
1074-
port: options.port,
1075-
host: options.host,
1076-
family: options.family,
1077-
localAddress: options.localAddress,
1078-
lookup: options.lookup
1079-
};
1080-
}
1081-
socket.connect(connect_opt, function() {
1069+
const connectOpt = {
1070+
path: options.path,
1071+
port: options.port,
1072+
host: options.host,
1073+
family: options.family,
1074+
localAddress: options.localAddress,
1075+
lookup: options.lookup
1076+
};
1077+
socket.connect(connectOpt, function() {
10821078
socket._start();
10831079
});
10841080
}
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
'use strict';
2+
const common = require('../common');
3+
4+
// This tests that both tls and net will ignore host and port if path is
5+
// provided.
6+
7+
if (!common.hasCrypto)
8+
common.skip('missing crypto');
9+
10+
common.refreshTmpDir();
11+
12+
const tls = require('tls');
13+
const net = require('net');
14+
const fs = require('fs');
15+
const assert = require('assert');
16+
17+
function libName(lib) {
18+
return lib === net ? 'net' : 'tls';
19+
}
20+
21+
function mkServer(lib, tcp, cb) {
22+
const handler = (socket) => {
23+
socket.write(`${libName(lib)}:${
24+
server.address().port || server.address()
25+
}`);
26+
socket.end();
27+
};
28+
const args = [handler];
29+
if (lib === tls) {
30+
args.unshift({
31+
cert: fs.readFileSync(`${common.fixturesDir}/test_cert.pem`),
32+
key: fs.readFileSync(`${common.fixturesDir}/test_key.pem`)
33+
});
34+
}
35+
const server = lib.createServer(...args);
36+
server.listen(tcp ? 0 : common.PIPE, common.mustCall(() => cb(server)));
37+
}
38+
39+
function testLib(lib, cb) {
40+
mkServer(lib, true, (tcpServer) => {
41+
mkServer(lib, false, (unixServer) => {
42+
const client = lib.connect({
43+
path: unixServer.address(),
44+
port: tcpServer.address().port,
45+
host: 'localhost',
46+
rejectUnauthorized: false
47+
}, () => {
48+
const bufs = [];
49+
client.on('data', common.mustCall((d) => {
50+
bufs.push(d);
51+
}));
52+
client.on('end', common.mustCall(() => {
53+
const resp = Buffer.concat(bufs).toString();
54+
assert.strictEqual(`${libName(lib)}:${unixServer.address()}`, resp);
55+
tcpServer.close();
56+
unixServer.close();
57+
cb();
58+
}));
59+
});
60+
});
61+
});
62+
}
63+
64+
testLib(net, common.mustCall(() => testLib(tls, common.mustCall())));

0 commit comments

Comments
 (0)