Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 78ca61e

Browse filesBrowse files
lundibundiTrott
lundibundi
authored and
Trott
committed
net: check args in net.connect() and socket.connect() calls
Previously Node.js would handle empty `net.connect()` and `socket.connect()` call as if the user passed empty options object which doesn't really make sense. This was due to the fact that it uses the same `normalizeArgs` function as `.listen()` call where such call is perfectly fine. This will make it clear what is the problem with such call and how it can be resolved. It now throws `ERR_MISSING_ARGS` if no arguments were passed or neither `path` nor `port` is specified. Fixes: #33930 PR-URL: #34022 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Zeyu Yang <himself65@outlook.com>
1 parent b546a2b commit 78ca61e
Copy full SHA for 78ca61e
Expand file treeCollapse file tree

7 files changed

+52
-15
lines changed
Open diff view settings
Collapse file

‎lib/net.js‎

Copy file name to clipboardExpand all lines: lib/net.js
+6-1Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,8 @@ const {
9595
ERR_INVALID_OPT_VALUE,
9696
ERR_SERVER_ALREADY_LISTEN,
9797
ERR_SERVER_NOT_RUNNING,
98-
ERR_SOCKET_CLOSED
98+
ERR_SOCKET_CLOSED,
99+
ERR_MISSING_ARGS,
99100
},
100101
errnoException,
101102
exceptionWithHostPort,
@@ -921,6 +922,10 @@ Socket.prototype.connect = function(...args) {
921922
const options = normalized[0];
922923
const cb = normalized[1];
923924

925+
// options.port === null will be checked later.
926+
if (options.port === undefined && options.path == null)
927+
throw new ERR_MISSING_ARGS(['options', 'port', 'path']);
928+
924929
if (this.write !== Socket.prototype.write)
925930
this.write = Socket.prototype.write;
926931

Collapse file

‎test/parallel/test-net-connect-no-arg.js‎

Copy file name to clipboard
+35Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
'use strict';
2+
3+
require('../common');
4+
const assert = require('assert');
5+
const net = require('net');
6+
7+
// Tests that net.connect() called without arguments throws ERR_MISSING_ARGS.
8+
9+
assert.throws(() => {
10+
net.connect();
11+
}, {
12+
code: 'ERR_MISSING_ARGS',
13+
message: 'The "options" or "port" or "path" argument must be specified',
14+
});
15+
16+
assert.throws(() => {
17+
new net.Socket().connect();
18+
}, {
19+
code: 'ERR_MISSING_ARGS',
20+
message: 'The "options" or "port" or "path" argument must be specified',
21+
});
22+
23+
assert.throws(() => {
24+
net.connect({});
25+
}, {
26+
code: 'ERR_MISSING_ARGS',
27+
message: 'The "options" or "port" or "path" argument must be specified',
28+
});
29+
30+
assert.throws(() => {
31+
new net.Socket().connect({});
32+
}, {
33+
code: 'ERR_MISSING_ARGS',
34+
message: 'The "options" or "port" or "path" argument must be specified',
35+
});
Collapse file

‎test/parallel/test-net-connect-options-port.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-net-connect-options-port.js
+1-2Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ const net = require('net');
6060
{
6161
// connect({hint}, cb) and connect({hint})
6262
const hints = (dns.ADDRCONFIG | dns.V4MAPPED | dns.ALL) + 42;
63-
const hintOptBlocks = doConnect([{ hints }],
63+
const hintOptBlocks = doConnect([{ port: 42, hints }],
6464
() => common.mustNotCall());
6565
for (const fn of hintOptBlocks) {
6666
assert.throws(fn, {
@@ -95,7 +95,6 @@ const net = require('net');
9595
// Try connecting to random ports, but do so once the server is closed
9696
server.on('close', () => {
9797
asyncFailToConnect(0);
98-
asyncFailToConnect(/* undefined */);
9998
});
10099
}
101100

Collapse file

‎test/parallel/test-net-normalize-args.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-net-normalize-args.js
+6-10Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ const common = require('../common');
44
const assert = require('assert');
55
const net = require('net');
66
const { normalizedArgsSymbol } = require('internal/net');
7-
const { getSystemErrorName } = require('util');
87

98
function validateNormalizedArgs(input, output) {
109
const args = net._normalizeArgs(input);
@@ -27,18 +26,15 @@ validateNormalizedArgs([{ port: 1234 }, assert.fail], res);
2726
const server = net.createServer(common.mustNotCall('should not connect'));
2827

2928
server.listen(common.mustCall(() => {
30-
const possibleErrors = ['ECONNREFUSED', 'EADDRNOTAVAIL'];
3129
const port = server.address().port;
3230
const socket = new net.Socket();
3331

34-
socket.on('error', common.mustCall((err) => {
35-
assert(possibleErrors.includes(err.code));
36-
assert(possibleErrors.includes(getSystemErrorName(err.errno)));
37-
assert.strictEqual(err.syscall, 'connect');
38-
server.close();
39-
}));
40-
41-
socket.connect([{ port }, assert.fail]);
32+
assert.throws(() => {
33+
socket.connect([{ port }, assert.fail]);
34+
}, {
35+
code: 'ERR_MISSING_ARGS'
36+
});
37+
server.close();
4238
}));
4339
}
4440

Collapse file

‎test/parallel/test-tls-connect-allow-half-open-option.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-tls-connect-allow-half-open-option.js
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,12 @@ const fixtures = require('../common/fixtures');
1212
const tls = require('tls');
1313

1414
{
15-
const socket = tls.connect({ lookup() {} });
15+
const socket = tls.connect({ port: 42, lookup() {} });
1616
assert.strictEqual(socket.allowHalfOpen, false);
1717
}
1818

1919
{
20-
const socket = tls.connect({ allowHalfOpen: false, lookup() {} });
20+
const socket = tls.connect({ port: 42, allowHalfOpen: false, lookup() {} });
2121
assert.strictEqual(socket.allowHalfOpen, false);
2222
}
2323

Collapse file

‎test/parallel/test-tls-connect-hints-option.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-tls-connect-hints-option.js
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ assert.notStrictEqual(hints, dns.V4MAPPED | dns.ALL);
2222
assert.notStrictEqual(hints, dns.ADDRCONFIG | dns.V4MAPPED | dns.ALL);
2323

2424
tls.connect({
25+
port: 42,
2526
lookup: common.mustCall((host, options) => {
2627
assert.strictEqual(host, 'localhost');
2728
assert.deepStrictEqual(options, { family: undefined, hints });
Collapse file

‎test/parallel/test-tls-connect-timeout-option.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-tls-connect-timeout-option.js
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const assert = require('assert');
1212
const tls = require('tls');
1313

1414
const socket = tls.connect({
15+
port: 42,
1516
lookup: () => {},
1617
timeout: 1000
1718
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.