From 1a2a4dac232673987fcea4471b19bb82502f0205 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Mon, 23 Feb 2015 11:23:53 -0500 Subject: [PATCH] net: allow port 0 in connect() The added validation allows non-negative numbers and numeric strings. All other values result in a thrown exception. Fixes: https://github.com/joyent/node/issues/9194 PR-URL: https://github.com/joyent/node/pull/9268 Reviewed-By: Julien Gilli Reviewed-By: Trevor Norris Reviewed-By: James M Snell --- lib/net.js | 14 +++- src/tcp_wrap.cc | 4 +- test/simple/test-net-create-connection.js | 82 ++++++++++++++++++++--- test/simple/test-net-localerror.js | 30 +++------ 4 files changed, 97 insertions(+), 33 deletions(-) diff --git a/lib/net.js b/lib/net.js index 61cb420952..6d12a70aee 100644 --- a/lib/net.js +++ b/lib/net.js @@ -892,7 +892,7 @@ Socket.prototype.connect = function(options, cb) { } else { var dns = require('dns'); var host = options.host || 'localhost'; - var port = options.port | 0; + var port = 0; var localAddress = options.localAddress; var localPort = options.localPort; var dnsopts = { @@ -906,8 +906,16 @@ Socket.prototype.connect = function(options, cb) { if (localPort && !util.isNumber(localPort)) throw new TypeError('localPort should be a number: ' + localPort); - if (port <= 0 || port > 65535) - throw new RangeError('port should be > 0 and < 65536: ' + port); + 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); if (dnsopts.family !== 4 && dnsopts.family !== 6) dnsopts.hints = dns.ADDRCONFIG | dns.V4MAPPED; diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index 6b2408c989..4d66c29126 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -428,7 +428,7 @@ void TCPWrap::Connect(const FunctionCallbackInfo& args) { assert(args[0]->IsObject()); assert(args[1]->IsString()); - assert(args[2]->Uint32Value()); + assert(args[2]->IsUint32()); Local req_wrap_obj = args[0].As(); node::Utf8Value ip_address(args[1]); @@ -460,7 +460,7 @@ void TCPWrap::Connect6(const FunctionCallbackInfo& args) { assert(args[0]->IsObject()); assert(args[1]->IsString()); - assert(args[2]->Uint32Value()); + assert(args[2]->IsUint32()); Local req_wrap_obj = args[0].As(); node::Utf8Value ip_address(args[1]); diff --git a/test/simple/test-net-create-connection.js b/test/simple/test-net-create-connection.js index 12f7f0be6d..2dee029665 100644 --- a/test/simple/test-net-create-connection.js +++ b/test/simple/test-net-create-connection.js @@ -24,33 +24,99 @@ var assert = require('assert'); var net = require('net'); var tcpPort = common.PORT; +var expectedConnections = 7; var clientConnected = 0; var serverConnected = 0; var server = net.createServer(function(socket) { socket.end(); - if (++serverConnected === 4) { + if (++serverConnected === expectedConnections) { server.close(); } }); + server.listen(tcpPort, 'localhost', function() { function cb() { ++clientConnected; } + function fail(opts, errtype, msg) { + assert.throws(function() { + var client = net.createConnection(opts, cb); + }, function (err) { + return err instanceof errtype && msg === err.message; + }); + } + net.createConnection(tcpPort).on('connect', cb); net.createConnection(tcpPort, 'localhost').on('connect', cb); net.createConnection(tcpPort, cb); net.createConnection(tcpPort, 'localhost', cb); + net.createConnection(tcpPort + '', 'localhost', cb); + net.createConnection({port: tcpPort + ''}).on('connect', cb); + net.createConnection({port: '0x' + tcpPort.toString(16)}, cb); + + fail({ + port: true + }, TypeError, 'port should be a number or string: true'); + + fail({ + port: false + }, TypeError, 'port should be a number or string: false'); + + fail({ + port: [] + }, TypeError, 'port should be a number or string: '); + + fail({ + port: {} + }, TypeError, 'port should be a number or string: [object Object]'); + + fail({ + port: null + }, TypeError, 'port should be a number or string: null'); + + fail({ + port: '' + }, RangeError, 'port should be >= 0 and < 65536: '); + + fail({ + port: ' ' + }, RangeError, 'port should be >= 0 and < 65536: '); - assert.throws(function () { - net.createConnection({ - port: 'invalid!' - }, cb); - }); + fail({ + port: '0x' + }, RangeError, 'port should be >= 0 and < 65536: 0x'); + + fail({ + port: '-0x1' + }, RangeError, 'port should be >= 0 and < 65536: -0x1'); + + fail({ + port: NaN + }, RangeError, 'port should be >= 0 and < 65536: NaN'); + + fail({ + port: Infinity + }, RangeError, 'port should be >= 0 and < 65536: Infinity'); + + fail({ + port: -1 + }, RangeError, 'port should be >= 0 and < 65536: -1'); + + fail({ + port: 65536 + }, RangeError, 'port should be >= 0 and < 65536: 65536'); }); -process.on('exit', function() { - assert.equal(clientConnected, 4); +// Try connecting to random ports, but do so once the server is closed +server.on('close', function() { + function nop() {} + + net.createConnection({port: 0}).on('error', nop); + net.createConnection({port: undefined}).on('error', nop); }); +process.on('exit', function() { + assert.equal(clientConnected, expectedConnections); +}); diff --git a/test/simple/test-net-localerror.js b/test/simple/test-net-localerror.js index d04d9c7072..cf113d499a 100644 --- a/test/simple/test-net-localerror.js +++ b/test/simple/test-net-localerror.js @@ -23,27 +23,17 @@ var common = require('../common'); var assert = require('assert'); var net = require('net'); - connect({ - host: 'localhost', - port: common.PORT, - localPort: 'foobar', - }, 'localPort should be a number: foobar'); +connect({ + host: 'localhost', + port: common.PORT, + localPort: 'foobar', +}, 'localPort should be a number: foobar'); - connect({ - host: 'localhost', - port: common.PORT, - localAddress: 'foobar', - }, 'localAddress should be a valid IP: foobar'); - - connect({ - host: 'localhost', - port: 65536 - }, 'port should be > 0 and < 65536: 65536'); - - connect({ - host: 'localhost', - port: 0 - }, 'port should be > 0 and < 65536: 0'); +connect({ + host: 'localhost', + port: common.PORT, + localAddress: 'foobar', +}, 'localAddress should be a valid IP: foobar'); function connect(opts, msg) { assert.throws(function() {