From 480b48244f14d64d6361c4b8e9bb3e479b5f7fa1 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 10 Mar 2015 12:33:38 +0100 Subject: [PATCH] lib: allow server.listen({ port: "1234" }) net.connect() accepts `{ port: "1234" }` (i.e. a string) as of commit 9d2b89d06 ("net: allow port 0 in connect()") but net.Server#listen() did not, creating a minor inconsistency. This commit rectifies that. Fixes: https://github.com/iojs/io.js/issues/1111 PR-URL: https://github.com/iojs/io.js/pull/1116 Reviewed-By: Colin Ihrig --- lib/net.js | 38 +++++++++++++------- test/parallel/test-net-listen-port-option.js | 26 ++++++++++++++ 2 files changed, 51 insertions(+), 13 deletions(-) create mode 100644 test/parallel/test-net-listen-port-option.js diff --git a/lib/net.js b/lib/net.js index 081e71d78fa1f1..aafb707ca45654 100644 --- a/lib/net.js +++ b/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); diff --git a/test/parallel/test-net-listen-port-option.js b/test/parallel/test-net-listen-port-option.js new file mode 100644 index 00000000000000..1b9938f7c7f155 --- /dev/null +++ b/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); +});