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 2c36397

Browse filesBrowse files
ShogunPandaRafaelGSS
ShogunPanda
authored and
RafaelGSS
committed
net: improve network family autoselection handle handling
PR-URL: #48464 Fixes: npm/cli#6409 Fixes: KararTY/dank-twitch-irc#13 Fixes: #47644 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
1 parent 203c3cf commit 2c36397
Copy full SHA for 2c36397

File tree

Expand file treeCollapse file tree

4 files changed

+125
-36
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+125
-36
lines changed
Open diff view settings
Collapse file

‎lib/net.js‎

Copy file name to clipboardExpand all lines: lib/net.js
+75-35Lines changed: 75 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ const {
5656
UV_EINVAL,
5757
UV_ENOTCONN,
5858
UV_ECANCELED,
59+
UV_ETIMEDOUT,
5960
} = internalBinding('uv');
6061

6162
const { Buffer } = require('buffer');
@@ -482,6 +483,10 @@ function Socket(options) {
482483
}
483484
}
484485

486+
if (options.signal) {
487+
addClientAbortSignalOption(this, options);
488+
}
489+
485490
// Reserve properties
486491
this.server = null;
487492
this._server = null;
@@ -1091,6 +1096,11 @@ function internalConnectMultiple(context, canceled) {
10911096
clearTimeout(context[kTimeout]);
10921097
const self = context.socket;
10931098

1099+
// We were requested to abort. Stop all operations
1100+
if (self._aborted) {
1101+
return;
1102+
}
1103+
10941104
// All connections have been tried without success, destroy with error
10951105
if (canceled || context.current === context.addresses.length) {
10961106
if (context.errors.length === 0) {
@@ -1105,7 +1115,11 @@ function internalConnectMultiple(context, canceled) {
11051115
assert(self.connecting);
11061116

11071117
const current = context.current++;
1108-
const handle = current === 0 ? self._handle : new TCP(TCPConstants.SOCKET);
1118+
1119+
if (current > 0) {
1120+
self[kReinitializeHandle](new TCP(TCPConstants.SOCKET));
1121+
}
1122+
11091123
const { localPort, port, flags } = context;
11101124
const { address, family: addressType } = context.addresses[current];
11111125
let localAddress;
@@ -1114,16 +1128,16 @@ function internalConnectMultiple(context, canceled) {
11141128
if (localPort) {
11151129
if (addressType === 4) {
11161130
localAddress = DEFAULT_IPV4_ADDR;
1117-
err = handle.bind(localAddress, localPort);
1131+
err = self._handle.bind(localAddress, localPort);
11181132
} else { // addressType === 6
11191133
localAddress = DEFAULT_IPV6_ADDR;
1120-
err = handle.bind6(localAddress, localPort, flags);
1134+
err = self._handle.bind6(localAddress, localPort, flags);
11211135
}
11221136

11231137
debug('connect/multiple: binding to localAddress: %s and localPort: %d (addressType: %d)',
11241138
localAddress, localPort, addressType);
11251139

1126-
err = checkBindError(err, localPort, handle);
1140+
err = checkBindError(err, localPort, self._handle);
11271141
if (err) {
11281142
ArrayPrototypePush(context.errors, exceptionWithHostPort(err, 'bind', localAddress, localPort));
11291143
internalConnectMultiple(context);
@@ -1143,9 +1157,9 @@ function internalConnectMultiple(context, canceled) {
11431157
ArrayPrototypePush(self.autoSelectFamilyAttemptedAddresses, `${address}:${port}`);
11441158

11451159
if (addressType === 4) {
1146-
err = handle.connect(req, address, port);
1160+
err = self._handle.connect(req, address, port);
11471161
} else {
1148-
err = handle.connect6(req, address, port);
1162+
err = self._handle.connect6(req, address, port);
11491163
}
11501164

11511165
if (err) {
@@ -1165,7 +1179,7 @@ function internalConnectMultiple(context, canceled) {
11651179
debug('connect/multiple: setting the attempt timeout to %d ms', context.timeout);
11661180

11671181
// If the attempt has not returned an error, start the connection timer
1168-
context[kTimeout] = setTimeout(internalConnectMultipleTimeout, context.timeout, context, req, handle);
1182+
context[kTimeout] = setTimeout(internalConnectMultipleTimeout, context.timeout, context, req, self._handle);
11691183
}
11701184
}
11711185

@@ -1183,6 +1197,15 @@ Socket.prototype.connect = function(...args) {
11831197
const options = normalized[0];
11841198
const cb = normalized[1];
11851199

1200+
if (cb !== null) {
1201+
this.once('connect', cb);
1202+
}
1203+
1204+
// If the parent is already connecting, do not attempt to connect again
1205+
if (this._parent && this._parent.connecting) {
1206+
return this;
1207+
}
1208+
11861209
// options.port === null will be checked later.
11871210
if (options.port === undefined && options.path == null)
11881211
throw new ERR_MISSING_ARGS(['options', 'port', 'path']);
@@ -1207,10 +1230,6 @@ Socket.prototype.connect = function(...args) {
12071230
initSocketHandle(this);
12081231
}
12091232

1210-
if (cb !== null) {
1211-
this.once('connect', cb);
1212-
}
1213-
12141233
this._unrefTimer();
12151234

12161235
this.connecting = true;
@@ -1583,7 +1602,47 @@ function afterConnect(status, handle, req, readable, writable) {
15831602
}
15841603
}
15851604

1605+
function addClientAbortSignalOption(self, options) {
1606+
validateAbortSignal(options.signal, 'options.signal');
1607+
const { signal } = options;
1608+
1609+
function onAbort() {
1610+
signal.removeEventListener('abort', onAbort);
1611+
self._aborted = true;
1612+
}
1613+
1614+
if (signal.aborted) {
1615+
process.nextTick(onAbort);
1616+
} else {
1617+
process.nextTick(() => {
1618+
signal.addEventListener('abort', onAbort);
1619+
});
1620+
}
1621+
}
1622+
1623+
function createConnectionError(req, status) {
1624+
let details;
1625+
1626+
if (req.localAddress && req.localPort) {
1627+
details = req.localAddress + ':' + req.localPort;
1628+
}
1629+
1630+
const ex = exceptionWithHostPort(status,
1631+
'connect',
1632+
req.address,
1633+
req.port,
1634+
details);
1635+
if (details) {
1636+
ex.localAddress = req.localAddress;
1637+
ex.localPort = req.localPort;
1638+
}
1639+
1640+
return ex;
1641+
}
1642+
15861643
function afterConnectMultiple(context, current, status, handle, req, readable, writable) {
1644+
debug('connect/multiple: connection attempt to %s:%s completed with status %s', req.address, req.port, status);
1645+
15871646
// Make sure another connection is not spawned
15881647
clearTimeout(context[kTimeout]);
15891648

@@ -1596,35 +1655,15 @@ function afterConnectMultiple(context, current, status, handle, req, readable, w
15961655

15971656
const self = context.socket;
15981657

1599-
16001658
// Some error occurred, add to the list of exceptions
16011659
if (status !== 0) {
1602-
let details;
1603-
if (req.localAddress && req.localPort) {
1604-
details = req.localAddress + ':' + req.localPort;
1605-
}
1606-
const ex = exceptionWithHostPort(status,
1607-
'connect',
1608-
req.address,
1609-
req.port,
1610-
details);
1611-
if (details) {
1612-
ex.localAddress = req.localAddress;
1613-
ex.localPort = req.localPort;
1614-
}
1615-
1616-
ArrayPrototypePush(context.errors, ex);
1660+
ArrayPrototypePush(context.errors, createConnectionError(req, status));
16171661

16181662
// Try the next address
16191663
internalConnectMultiple(context, status === UV_ECANCELED);
16201664
return;
16211665
}
16221666

1623-
if (context.current > 1 && self[kReinitializeHandle]) {
1624-
self[kReinitializeHandle](handle);
1625-
handle = self._handle;
1626-
}
1627-
16281667
if (hasObserver('net')) {
16291668
startPerf(
16301669
self,
@@ -1633,17 +1672,18 @@ function afterConnectMultiple(context, current, status, handle, req, readable, w
16331672
);
16341673
}
16351674

1636-
afterConnect(status, handle, req, readable, writable);
1675+
afterConnect(status, self._handle, req, readable, writable);
16371676
}
16381677

16391678
function internalConnectMultipleTimeout(context, req, handle) {
16401679
debug('connect/multiple: connection to %s:%s timed out', req.address, req.port);
16411680
req.oncomplete = undefined;
1681+
ArrayPrototypePush(context.errors, createConnectionError(req, UV_ETIMEDOUT));
16421682
handle.close();
16431683
internalConnectMultiple(context);
16441684
}
16451685

1646-
function addAbortSignalOption(self, options) {
1686+
function addServerAbortSignalOption(self, options) {
16471687
if (options?.signal === undefined) {
16481688
return;
16491689
}
@@ -1932,7 +1972,7 @@ Server.prototype.listen = function(...args) {
19321972
listenInCluster(this, null, -1, -1, backlogFromArgs);
19331973
return this;
19341974
}
1935-
addAbortSignalOption(this, options);
1975+
addServerAbortSignalOption(this, options);
19361976
// (handle[, backlog][, cb]) where handle is an object with a fd
19371977
if (typeof options.fd === 'number' && options.fd >= 0) {
19381978
listenInCluster(this, null, null, null, backlogFromArgs, options.fd);
Collapse file

‎test/internet/test-https-autoselectfamily-slow-timeout.js‎

Copy file name to clipboardExpand all lines: test/internet/test-https-autoselectfamily-slow-timeout.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const { request } = require('https');
1111

1212
request(
1313
`https://${addresses.INET_HOST}/en`,
14-
// Purposely set this to false because we want all connection but the last to fail
14+
// Purposely set this to a low value because we want all connection but the last to fail
1515
{ autoSelectFamily: true, autoSelectFamilyAttemptTimeout: 10 },
1616
(res) => {
1717
assert.strictEqual(res.statusCode, 200);
Collapse file

‎test/internet/test-net-autoselectfamily-timeout-close.js‎

Copy file name to clipboard
+27Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const { addresses } = require('../common/internet');
5+
6+
const assert = require('assert');
7+
const { connect } = require('net');
8+
9+
// Test that when all errors are returned when no connections succeeded and that the close event is emitted
10+
{
11+
const connection = connect({
12+
host: addresses.INET_HOST,
13+
port: 10,
14+
autoSelectFamily: true,
15+
// Purposely set this to a low value because we want all non last connection to fail due to early timeout
16+
autoSelectFamilyAttemptTimeout: 10,
17+
});
18+
19+
connection.on('close', common.mustCall());
20+
connection.on('ready', common.mustNotCall());
21+
22+
connection.on('error', common.mustCall((error) => {
23+
assert.ok(connection.autoSelectFamilyAttemptedAddresses.length > 0);
24+
assert.strictEqual(error.constructor.name, 'AggregateError');
25+
assert.ok(error.errors.length > 0);
26+
}));
27+
}
Collapse file

‎test/internet/test-tls-autoselectfamily-backing-socket.js‎

Copy file name to clipboard
+22Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const { addresses: { INET_HOST } } = require('../common/internet');
5+
6+
if (!common.hasCrypto) {
7+
common.skip('missing crypto');
8+
}
9+
10+
const { Socket } = require('net');
11+
const { TLSSocket } = require('tls');
12+
13+
// Test that TLS connecting works with autoSelectFamily when using a backing socket
14+
{
15+
const socket = new Socket();
16+
const secureSocket = new TLSSocket(socket);
17+
18+
secureSocket.on('connect', common.mustCall(() => secureSocket.end()));
19+
20+
socket.connect({ host: INET_HOST, port: 443, servername: INET_HOST });
21+
secureSocket.connect({ host: INET_HOST, port: 443, servername: INET_HOST });
22+
}

0 commit comments

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