test-net-remote-address-port.js sometimes fails with IPv4-mapped addresses

[misterdjules]

Currently, test-net-remote-address-port.js fails on our SmartOS jenkins node because the first client TCP connection created with the following code:

var client = net.createConnection(common.PORT, 'localhost');

returns an IPv6 IPv4-mapped address when accessing its remoteAddress property.

Specifying a host to connect to makes Socket.prototype.connect sets the AI_V4MAPPED flag when calling getaddrinfo:

} else if (!options.host) {
    debug('connect: missing host');
    self._host = '127.0.0.1';
    connect(self, self._host, options.port, 4);

  } else {
    var dns = require('dns');
    var host = options.host;
    var dnsopts = {
      family: options.family,
      hints: 0
    };

    if (dnsopts.family !== 4 && dnsopts.family !== 6)
      dnsopts.hints = dns.ADDRCONFIG | dns.V4MAPPED;

cares_wrap::GetAddrinfo defaults to AF_UNSPEC when the family is not set:

  switch (args[2]->Int32Value()) {
  case 0:
    family = AF_UNSPEC;
    break;

getaddrinfo's man page on SmartOS seems indicates that:
The AI_V4MAPPED flag is ignored unless ai_family equals AF_INET6.
but clearly, it doesn't seem to be the case. Later in the same man page, we can read:

When ai_family is not specified (AF_UNSPEC), AI_V4MAPPED and AI_ALL flags are used only if
AF_INET6 is supported.

which is slightly confusing.

We can fix this test by passing an options object to createConnection with family set to '4'. However, I'm wondering if we actually need to set dns.V4MAPPED by default when no host and no family is supplied, and if so why?

Removing dns.V4MAPPED from the hints doesn't break any of the current tests (simple and internet).

/cc @cjihrig

[cjihrig]

@misterdjules the original issue (#7637) was that Node was using IPv4 unless it was explicitly told to use IPv6. AI_V4MAPPED probably doesn't need to be enabled by default since it only applies if the user has already specified that they want IPv6. I will remove it from the default settings and set it if family === 6. Sound good?

[misterdjules]

@cjihrig Now that I had more time to think about it, I'm actually concerned about removing the AI_V4MAPPED flag when setting the ADDR_CONFIG flag for setups that only support IPv6. That would prevent them from communicating to IPv4 only nodes. See section 4.2 of RFC 4308.

The real issue to me seems to be in SmartOS' implementation of getaddrinfo in the case that the hostname parameter is NULL. It seems that in this case getaddrinfo returns an AF_INET6 family address first. I'll have a closer look at this code, and if I can confirm that, then I suggest changing the test code to handle IPv4-mapped addresses.

Sorry for the confusion.

[cjihrig]

@misterdjules OK. I'll close #8097 for the time being.

[misterdjules]

@cjihrig When using ADDR_CONFIG, we can't really know what type of IP address we'll get back, so I updated the test such that it doesn't make any assumption about addresses family.