test: remove common.PORT from test-tls-connect

[tanuck]

Replace common.PORT with 0 to prevent the occurence of
any EADDRINUSE errors.

Checklist

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

Affected core subsystem(s)

test tls https

[thefourtheye]

Now that you are doing this, it would be better if you could assert the actual error messages. On my machine I got

{ Error: connect ECONNREFUSED 127.0.0.1:12346
    at Object.exports._errnoException (util.js:1042:11)
    at exports._exceptionWithHostPort (util.js:1065:20)
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1115:14)
  code: 'ECONNREFUSED',
  errno: 'ECONNREFUSED',
  syscall: 'connect',
  address: '127.0.0.1',
  port: 12346 }

[tanuck]

Using port: 0 I got the same on Linux, however the error code changes to EADDRNOTAVAIL on OS X for some reason.

[Trott]

Both of these changes use port 0 in tls.connect(). That doesn't work, does it? (I get how it assigns an arbitrary available port .bind() or .listen() but .connect()?) and we may be breaking the test now. These connections are supposed to fail for other reasons, but now they might be failing because we're giving an invalid port number for a connection.

[tanuck]

@Trott Using port 0 works just the same for connect. I believe linux goes even further and uses separate pools of ports for bind()/listen() and connect()[1]. I guess if we don't replace common.PORT we should at least add the assertions on the error that @thefourtheye mentioned above?

[1] https://github.com/torvalds/linux/blob/master/net/ipv4/inet_connection_sock.c#L278-L280

[Trott]

@Trott Using port 0 works just the same for connect. I

Much to my surprise, that appears to be correct. We should probably document this behavior if it's intentional. (We should also make sure that it works this way on Windows.) I'll clear my review so it doesn't block this. Might be worth adding a comment along the lines of:

// It's not documented in the Node.js docs, but sending port 0 to `tls.connect()`
// results in attempting to connect to an unused port.

Works as advertised.

[Trott]

Argh, hate to flip-flop again, but more testing makes me skeptical this works as advertised again. Port 0 does not attempt to initiate a connection at all. It returns an error immediately. If I provide an unreachable address and a valid port number, I get ENETUNREACH after a pause. If I provide an unreachable address and port 0, I get EADDRNOTAVAIL immediately. Using port 0 with connect() does not seem to return an arbitrary port unless it is special-cased for localhost. (Perhaps it is, but I'd want confirmation in the code or through testing...)

[Trott]

More evidence it does not choose an arbitrary port: If I use localhost and provide a port not in use, I get ECONNREFUSED. If I provide port 0, I get EADDRNOTAVAIL.

[tanuck]

Yup, I was able to reproduce the same. Haven't been able to test on Windows though, so can't say what the situation is there. Probably worth adding more coverage to the errors returned.

[BridgeAR]

As there was no update for a long time and it seems like it is inconclusive if this actually works or not, I am going to close this. @tanuck Thanks a lot for your contribution anyway and please feel free to reopen this if you would like to follow up on it!