lib: allow server.listen({ port: "1234" })

net.connect() accepts `{ port: "1234" }` (i.e. a string) as of commit
9d2b89d ("net: allow port 0 in connect()") but net.Server#listen()
did not, creating a minor inconsistency.  This commit rectifies that.

Fixes: #1111
PR-URL: #1116
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

lib/net.js

@@ -834,6 +834,15 @@ function connect(self, address, port, addressType, localAddress, localPort) {
 }
 
 
+// Check that the port number is not NaN when coerced to a number,
+// is an integer and that it falls within the legal range of port numbers.
+function isLegalPort(port) {
+  if (typeof port === 'string' && port.trim() === '')
+    return false;
+  return +port === (port >>> 0) && port >= 0 && port <= 0xFFFF;
+}
+
+
 Socket.prototype.connect = function(options, cb) {
   if (this.write !== Socket.prototype.write)
     this.write = Socket.prototype.write;
@@ -896,16 +905,14 @@ Socket.prototype.connect = function(options, cb) {
     if (localPort && typeof localPort !== 'number')
       throw new TypeError('localPort should be a number: ' + localPort);
 
-    if (typeof options.port === 'number')
-      port = options.port;
-    else if (typeof options.port === 'string')
-      port = options.port.trim() === '' ? -1 : +options.port;
-    else if (options.port !== undefined)
-      throw new TypeError('port should be a number or string: ' + options.port);
-
-    if (port < 0 || port > 65535 || isNaN(port))
-      throw new RangeError('port should be >= 0 and < 65536: ' +
-                           options.port);
+    port = options.port;
+    if (typeof port !== 'undefined') {
+      if (typeof port !== 'number' && typeof port !== 'string')
+        throw new TypeError('port should be a number or string: ' + port);
+      if (!isLegalPort(port))
+        throw new RangeError('port should be >= 0 and < 65536: ' + port);
+    }
+    port |= 0;
 
     if (dnsopts.family !== 4 && dnsopts.family !== 6)
       dnsopts.hints = dns.ADDRCONFIG | dns.V4MAPPED;
@@ -1266,11 +1273,16 @@ Server.prototype.listen = function() {
       if (h.backlog)
         backlog = h.backlog;
 
-      if (typeof h.port === 'number') {
+      if (typeof h.port === 'number' || typeof h.port === 'string' ||
+          (typeof h.port === 'undefined' && 'port' in h)) {
+        // Undefined is interpreted as zero (random port) for consistency
+        // with net.connect().
+        if (typeof h.port !== 'undefined' && !isLegalPort(h.port))
+          throw new RangeError('port should be >= 0 and < 65536: ' + h.port);
         if (h.host)
-          listenAfterLookup(h.port, h.host, backlog, h.exclusive);
+          listenAfterLookup(h.port | 0, h.host, backlog, h.exclusive);
         else
-          listen(self, null, h.port, 4, backlog, undefined, h.exclusive);
+          listen(self, null, h.port | 0, 4, backlog, undefined, h.exclusive);
       } else if (h.path && isPipeName(h.path)) {
         var pipeName = self._pipeName = h.path;
         listen(self, pipeName, -1, -1, backlog, undefined, h.exclusive);

test/parallel/test-net-listen-port-option.js

@@ -0,0 +1,26 @@
+var common = require('../common');
+var assert = require('assert');
+var net = require('net');
+
+function close() { this.close(); }
+net.Server().listen({ port: undefined }, close);
+net.Server().listen({ port: '' + common.PORT }, close);
+
+[ 'nan',
+  -1,
+  123.456,
+  0x10000,
+  1 / 0,
+  -1 / 0,
+  '+Infinity',
+  '-Infinity' ].forEach(function(port) {
+  assert.throws(function() {
+    net.Server().listen({ port: port }, assert.fail);
+  }, /port should be >= 0 and < 65536/i);
+});
+
+[null, true, false].forEach(function(port) {
+  assert.throws(function() {
+    net.Server().listen({ port: port }, assert.fail);
+  }, /invalid listen argument/i);
+});