net: fix family autoselection timeout handling

PR-URL: #47860 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Author: ShogunPanda

lib/net.js

@@ -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 = '::';
@@ -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;
 
@@ -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;
@@ -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) {
@@ -1478,7 +1482,6 @@ function lookupAndConnectMultiple(
  localPort,
  timeout,
  [kTimeout]: null,
- [kTimeoutTriggered]: false,
  errors: [],
  };
 
@@ -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) {
@@ -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);
 }
 

test/parallel/test-net-autoselectfamily.js

@@ -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');
 
@@ -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);
  }));
@@ -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 }) {