net: Validate port in createServer().listen()

Make sure we validate the port number in all kinds of `listen()` calls.

Fixes: https://github.com/nodejs/node/issues/5727
PR-URL: https://github.com/nodejs/node/pull/5732
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

lib/internal/net.js

@@ -1,6 +1,6 @@
 'use strict';
 
-module.exports = { isLegalPort };
+module.exports = { isLegalPort, assertPort };
 
 // 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.
@@ -10,3 +10,9 @@ function isLegalPort(port) {
     return false;
   return +port === (+port >>> 0) && port <= 0xFFFF;
 }
+
+
+function assertPort(port) {
+  if (typeof port !== 'undefined' && !isLegalPort(port))
+    throw new RangeError('"port" argument must be >= 0 and < 65536');
+}

lib/net.js

@@ -24,6 +24,7 @@ var cluster;
 const errnoException = util._errnoException;
 const exceptionWithHostPort = util._exceptionWithHostPort;
 const isLegalPort = internalNet.isLegalPort;
+const assertPort = internalNet.assertPort;
 
 function noop() {}
 
@@ -1352,9 +1353,7 @@ Server.prototype.listen = function() {
           (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))
+        assertPort(h.port);
-          throw new RangeError('"port" option should be >= 0 and < 65536: ' +
-                               h.port);
         if (h.host)
           listenAfterLookup(h.port | 0, h.host, backlog, h.exclusive);
         else
@@ -1375,10 +1374,12 @@ Server.prototype.listen = function() {
              typeof arguments[1] === 'function' ||
              typeof arguments[1] === 'number') {
     // The first argument is the port, no IP given.
+    assertPort(port);
     listen(self, null, port, 4, backlog);
 
   } else {
     // The first argument is the port, the second an IP.
+    assertPort(port);
     listenAfterLookup(port, arguments[1], backlog);
   }
 

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

@@ -18,7 +18,7 @@ net.Server().listen({ port: '' + common.PORT }, close);
 ].forEach(function(port) {
   assert.throws(function() {
     net.Server().listen({ port: port }, assert.fail);
-  }, /"port" option should be >= 0 and < 65536/i);
+  }, /"port" argument must be >= 0 and < 65536/i);
 });
 
 [null, true, false].forEach(function(port) {

test/parallel/test-regress-GH-5727.js

@@ -0,0 +1,27 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const net = require('net');
+
+const invalidPort = -1 >>> 0;
+const errorMessage = /"port" argument must be \>= 0 and \< 65536/;
+
+net.Server().listen(common.PORT, function() {
+  assert.equal(this._connectionKey, '6::::' + common.PORT);
+  this.close();
+});
+
+// The first argument is a configuration object
+assert.throws(() => {
+  net.Server().listen({ port: invalidPort }, common.fail);
+}, errorMessage);
+
+// The first argument is the port, no IP given.
+assert.throws(() => {
+  net.Server().listen(invalidPort, common.fail);
+}, errorMessage);
+
+// The first argument is the port, the second an IP.
+assert.throws(() => {
+  net.Server().listen(invalidPort, '0.0.0.0', common.fail);
+}, errorMessage);

test/sequential/test-net-server-address.js

@@ -87,18 +87,3 @@ server3.listen(0, function() {
   assert.strictEqual(address.family, family_ipv6);
   server3.close();
 });
-
-// Test without hostname, but with port -1
-var server4 = net.createServer();
-
-server4.on('error', function(e) {
-  console.log('Error on ip socket: ' + e.toString());
-});
-
-// Specify -1 as port number
-server4.listen(-1, function() {
-  var address = server4.address();
-  assert.strictEqual(address.address, anycast_ipv6);
-  assert.strictEqual(address.family, family_ipv6);
-  server4.close();
-});