test: Avoid usage of mixed IPV6 addresses

[gireeshpunathil]

Checklist

• make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
• commit message follows commit guidelines

Affected core subsystem(s)

test, cluster

Description of change

Please see #7563 for full details.

The test case test/parallel/test-cluster-disconnect-handles.js
fails in AIX due to the mixed-use of unspecified
and loopback addresses. This is not a problem in most platforms
but fails in AIX. (In Windows too, but does not manifest as the
test is omitted in Windows for a different reason).

There exists no documented evidence which supports the mixed use
of unspecified and loopback addresses.

While AIX strictly follows the IPV6 specification with respect to
unspecified address ('::') and loopback address ('::1'), the test
case latches on to the behavior exhibited by other platforms,
and hence it fails in AIX.

The proposed fix is to make it work in all platforms including
AIX by using the loopback address for the client to connect,
as that is the address at which the server listens.

[gibfahn]

@gireeshpunathil Do you want to make a similar change to gc/test-net-timeout.js to fix #7291? It seems to be the same problem, so I guess it would make sense to do it in the same PR.

[gireeshpunathil]

@gibfahn - sure, will check.

[gibfahn]

@gireeshpunathil I've checked @quaidn 's suggested fix at #7291 (comment) and it seems to work.

diff --git a/test/gc/test-net-timeout.js b/test/gc/test-net-timeout.js
index ff1d565..0e68f78 100644
--- a/test/gc/test-net-timeout.js+++ b/test/gc/test-net-timeout.js@@ -36,7 +36,11 @@ function getall() {
return;
(function() {
- var req = net.connect(server.address().port, server.address().address);+ if (server.address().family === 'IPv4') {+ var req = net.connect(server.address().port, '127.0.0.1');+ } else {+ var req = net.connect(server.address().port, '::1');+ }
req.resume();
req.setTimeout(10, function() {
//console.log('timeout (expected)')

[gireeshpunathil]

The problem is with the lack of cohesion between what is documented in the server.listen documentation and what is requested by the client in the proposed fix. For example, the API spec says 'If the hostname is omitted, the server will accept connections on any IPv6 address (::) when IPv6 is available'. When connecting to a server socket thus created, it becomes problematic if we use '::1' which is basically the loopback address, not 'any address'. While everything works in Linux magically, I see an issue in Mac.

For better traceability, I suggest we separate this (AIX) from #7291 (windows). I can work with you on that further.

[gibfahn]

@gireeshpunathil Okay, if they're different fixes I agree it makes sense to do different PRs.

[gireeshpunathil]

@gibfahn - FYI, I have opened #7728 to cover the inconsistency in general, which includes the windows issue too.

[mhdawson]

Seeing that this test is not intended to test the edge case it is failing on I'm good with the fix along with the plan that we'd add a test case back in for the edge case under #7728.

LGTM

@Trott FYI

[Trott]

Might it be more elegant to move the const address = common.hasIPv6 ? '[::1]' : common.localhostIPv4; on line 28 up a few lines so it is assigned whether isMaster is true or not, and then get rid of the checking hasIPv6 around this change, just always running server.listen(0, address, cb)?

[Trott]

Oh, wait never mind my previous comment. That sets an array, not a single address value. This change LGTM.

[Trott]

/cc @bnoordhuis

[bnoordhuis]

s/Avoid/avoid/ in the commit status line, otherwise LGTM.

[cjihrig]

LGTM. CI: https://ci.nodejs.org/job/node-test-pull-request/3316/

[gireeshpunathil]

Made the changes to commit message (s/Avoid/avoid/) as advised by @bnoordhuis

[mhdawson]

CI looks good landing

[mhdawson]

landed as 17883df