Skip to content

Commit ee157e5

Browse files
bengljasnell
authored andcommitted
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 <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>
1 parent 1fbb86c commit ee157e5

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
@@ -1050,7 +1050,7 @@ exports.connect = function(...args /* [port,] [host,] [options,] [cb] */) {
10501050
tls.convertALPNProtocols(options.ALPNProtocols, ALPN);
10511051

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

10671067
if (!options.socket) {
1068-
var connect_opt;
1069-
if (options.path && !options.port) {
1070-
connect_opt = { path: options.path };
1071-
} else {
1072-
connect_opt = {
1073-
port: options.port,
1074-
host: options.host,
1075-
family: options.family,
1076-
localAddress: options.localAddress,
1077-
lookup: options.lookup
1078-
};
1079-
}
1080-
socket.connect(connect_opt, function() {
1068+
const connectOpt = {
1069+
path: options.path,
1070+
port: options.port,
1071+
host: options.host,
1072+
family: options.family,
1073+
localAddress: options.localAddress,
1074+
lookup: options.lookup
1075+
};
1076+
socket.connect(connectOpt, function() {
10811077
socket._start();
10821078
});
10831079
}
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)