net: net.Server.address() behaves inconsistently

[raymondfeng]

net.Server.address() returns IPV4/IPV6 inconsistently. To reproduce the problem, run the following test with v0.10.28 and v0.11.13.

var net = require('net');

function createServer(name) {
  var server = new net.Server();
  server.on('listening', function() {
    console.log("%s %j", name, server.address());
    server.close();
  });
  return server;
}

createServer('1').listen(0);
createServer('2').listen(0, null);
createServer('3').listen(0, undefined);
createServer('4').listen(3000);
createServer('5').listen(3000, null);

Result from v0.10.28:

1 {"address":"0.0.0.0","family":"IPv4","port":64208}
3 {"address":"0.0.0.0","family":"IPv4","port":64209}
4 {"address":"0.0.0.0","family":"IPv4","port":3000}
6 {"address":"0.0.0.0","family":"IPv4","port":64210}
2 {"address":"0.0.0.0","family":"IPv4","port":64211}
5 {"address":"0.0.0.0","family":"IPv4","port":3000}

Result from v0.11.13:

1 {"address":"0.0.0.0","family":"IPv4","port":64121}
3 {"address":"0.0.0.0","family":"IPv4","port":64122}
4 {"address":"::","family":"IPv6","port":3000}
6 {"address":"0.0.0.0","family":"IPv4","port":64123}
2 {"address":"0.0.0.0","family":"IPv4","port":64124}
5 {"address":"::","family":"IPv6","port":3000}

Is this a bug or by design?

[indutny]

This is by design: node v0.11 listens on both protocols by default now.

[raymondfeng]

Two more questions:

1. Do we document the new behavior?
2. Why does case 1-3 show 'IPv4' while 4-5 show 'IPV6' if the server listens on both protocols?

[bnoordhuis]

Do we document the new behavior?

It should be.

Why does case 1-3 show 'IPv4' while 4-5 show 'IPV6' if the server listens on both protocols?

I observe that:

1. With port === 0 && host == '' (i.e. false-y), node creates an AF_INET socket but does not call bind().
2. With port > 0 && host == '', node creates an AF_INET6 socket and calls bind().

Which feels rather incongruent. I suggest making 1 consistent with 2.

[raymondfeng]

Can we reopen the issue?

[misterdjules]

@raymondfeng @indutny The new behavior doesn't seem to be documented. server.listen's doc still states:

If the host is omitted, the server will accept connections directed to any IPv4 address (INADDR_ANY)

Did I miss something? If not, I suggest updating the doc. Should I create a new PR for this?

[STRML]

Just wanted to mention that this is biting in an attempt to upgrade to Node v0.12 - supertest's default behavior is to listen using app.listen(0) which causes it to bind via IPv6, breaking all sorts of IP checks in my app that expect IPv4 addresses.

Documentation isn't much help here - it appears to be out of date and doesn't tell me what the expected behavior is, or how to bind to IPv4 only.

[misterdjules]

@STRML It seems that the documentation is lacking in this regard, thank you for pointing that out. I created #9239 to track this.

I came across similar issues in other projects in the past, for instance in express. Instead of having IP addresses tests rely on a specific address family/protocol, the tests were fixed to "discover" what protocol/address family is used at runtime. Does that help you solve your problem?

[STRML]

That seems like a decent way to do it. Perhaps it would be best for express to deprecate req.ip and instead provide req.ipv4 and req.ipv6, with req.ipv4 returning an ipv6 mapped address if that is indeed the case.

For test suites, it would be very helpful if the old behavior could be forced via an environment variable, rather than using these IPv6 mapped addresses. There are plenty of legacy libraries out there that will expect the old behavior and I expect this to be a source of grief for a long time to come.