Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: fix family autoselection timeout handling. #47860

Merged
merged 2 commits into from
May 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 19 additions & 13 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ let autoSelectFamilyAttemptTimeoutDefault = 250;

const { clearTimeout, setTimeout } = require('timers');
const { kTimeout } = require('internal/timers');
const kTimeoutTriggered = Symbol('kTimeoutTriggered');

const DEFAULT_IPV4_ADDR = '0.0.0.0';
const DEFAULT_IPV6_ADDR = '::';
Expand Down Expand Up @@ -1106,9 +1105,10 @@ function internalConnectMultiple(context, canceled) {

assert(self.connecting);

const handle = context.current === 0 ? self._handle : new TCP(TCPConstants.SOCKET);
const current = context.current++;
const handle = current === 0 ? self._handle : new TCP(TCPConstants.SOCKET);
const { localPort, port, flags } = context;
const { address, family: addressType } = context.addresses[context.current++];
const { address, family: addressType } = context.addresses[current];
let localAddress;
let err;

Expand All @@ -1135,7 +1135,7 @@ function internalConnectMultiple(context, canceled) {
debug('connect/multiple: attempting to connect to %s:%d (addressType: %d)', address, port, addressType);

const req = new TCPConnectWrap();
req.oncomplete = FunctionPrototypeBind(afterConnectMultiple, undefined, context);
req.oncomplete = FunctionPrototypeBind(afterConnectMultiple, undefined, context, current);
req.address = address;
req.port = port;
req.localAddress = localAddress;
Expand All @@ -1162,8 +1162,12 @@ function internalConnectMultiple(context, canceled) {
return;
}

// If the attempt has not returned an error, start the connection timer
context[kTimeout] = setTimeout(internalConnectMultipleTimeout, context.timeout, context, req);
if (current < context.addresses.length - 1) {
debug('connect/multiple: setting the attempt timeout to %d ms', context.timeout);

// If the attempt has not returned an error, start the connection timer
context[kTimeout] = setTimeout(internalConnectMultipleTimeout, context.timeout, context, req, handle);
}
}

Socket.prototype.connect = function(...args) {
Expand Down Expand Up @@ -1478,7 +1482,6 @@ function lookupAndConnectMultiple(
localPort,
timeout,
[kTimeout]: null,
[kTimeoutTriggered]: false,
errors: [],
};

Expand Down Expand Up @@ -1581,18 +1584,19 @@ function afterConnect(status, handle, req, readable, writable) {
}
}

function afterConnectMultiple(context, status, handle, req, readable, writable) {
function afterConnectMultiple(context, current, status, handle, req, readable, writable) {
// Make sure another connection is not spawned
clearTimeout(context[kTimeout]);

// One of the connection has completed and correctly dispatched but after timeout, ignore this one
if (context[kTimeoutTriggered]) {
if (status === 0 && current !== context.current - 1) {
debug('connect/multiple: ignoring successful but timedout connection to %s:%s', req.address, req.port);
handle.close();
return;
}

const self = context.socket;

// Make sure another connection is not spawned
clearTimeout(context[kTimeout]);

// Some error occurred, add to the list of exceptions
if (status !== 0) {
Expand Down Expand Up @@ -1633,8 +1637,10 @@ function afterConnectMultiple(context, status, handle, req, readable, writable)
afterConnect(status, handle, req, readable, writable);
}

function internalConnectMultipleTimeout(context, req) {
context[kTimeoutTriggered] = true;
function internalConnectMultipleTimeout(context, req, handle) {
debug('connect/multiple: connection to %s:%s timed out', req.address, req.port);
req.oncomplete = undefined;
handle.close();
internalConnectMultiple(context);
}

Expand Down
64 changes: 61 additions & 3 deletions test/parallel/test-net-autoselectfamily.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,15 @@ function _lookup(resolver, hostname, options, cb) {
});
}

function createDnsServer(ipv6Addr, ipv4Addr, cb) {
function createDnsServer(ipv6Addrs, ipv4Addrs, cb) {
if (!Array.isArray(ipv6Addrs)) {
ipv6Addrs = [ipv6Addrs];
}

if (!Array.isArray(ipv4Addrs)) {
ipv4Addrs = [ipv4Addrs];
}

// Create a DNS server which replies with a AAAA and a A record for the same host
const socket = dgram.createSocket('udp4');

Expand All @@ -49,8 +57,8 @@ function createDnsServer(ipv6Addr, ipv4Addr, cb) {
id: parsed.id,
questions: parsed.questions,
answers: [
{ type: 'AAAA', address: ipv6Addr, ttl: 123, domain: 'example.org' },
{ type: 'A', address: ipv4Addr, ttl: 123, domain: 'example.org' },
...ipv6Addrs.map((address) => ({ type: 'AAAA', address, ttl: 123, domain: 'example.org' })),
...ipv4Addrs.map((address) => ({ type: 'A', address, ttl: 123, domain: 'example.org' })),
]
}), port, address);
}));
Expand Down Expand Up @@ -106,6 +114,56 @@ function createDnsServer(ipv6Addr, ipv4Addr, cb) {
}));
}

// Test that only the last successful connection is established.
{
createDnsServer(
'::1',
['104.20.22.46', '104.20.23.46', '127.0.0.1'],
common.mustCall(function({ dnsServer, lookup }) {
const ipv4Server = createServer((socket) => {
socket.on('data', common.mustCall(() => {
socket.write('response-ipv4');
socket.end();
}));
});

ipv4Server.listen(0, '127.0.0.1', common.mustCall(() => {
const port = ipv4Server.address().port;

const connection = createConnection({
host: 'example.org',
port: port,
lookup,
autoSelectFamily: true,
autoSelectFamilyAttemptTimeout,
});

let response = '';
connection.setEncoding('utf-8');

connection.on('ready', common.mustCall(() => {
assert.deepStrictEqual(
connection.autoSelectFamilyAttemptedAddresses,
[`::1:${port}`, `104.20.22.46:${port}`, `104.20.23.46:${port}`, `127.0.0.1:${port}`]
);
}));

connection.on('data', (chunk) => {
response += chunk;
});

connection.on('end', common.mustCall(() => {
assert.strictEqual(response, 'response-ipv4');
ipv4Server.close();
dnsServer.close();
}));

connection.write('request');
}));
})
);
}

// Test that IPV4 is NOT reached if IPV6 is reachable
if (common.hasIPv6) {
createDnsServer('::1', '127.0.0.1', common.mustCall(function({ dnsServer, lookup }) {
Expand Down