Skip to content

Commit

Permalink
net: allow port 0 in connect()
Browse files Browse the repository at this point in the history
The added validation allows non-negative numbers and numeric
strings. All other values result in a thrown exception.

Fixes: nodejs/node-v0.x-archive#9194
PR-URL: nodejs/node-v0.x-archive#9268
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: James M Snell <jasnell@users.noreply.github.com>
  • Loading branch information
cjihrig committed Mar 5, 2015
1 parent b0425f0 commit 1a2a4da
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 33 deletions.
14 changes: 11 additions & 3 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/tcp_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ void TCPWrap::Connect(const FunctionCallbackInfo<Value>& args) {

assert(args[0]->IsObject());
assert(args[1]->IsString());
assert(args[2]->Uint32Value());
assert(args[2]->IsUint32());

Local<Object> req_wrap_obj = args[0].As<Object>();
node::Utf8Value ip_address(args[1]);
Expand Down Expand Up @@ -460,7 +460,7 @@ void TCPWrap::Connect6(const FunctionCallbackInfo<Value>& args) {

assert(args[0]->IsObject());
assert(args[1]->IsString());
assert(args[2]->Uint32Value());
assert(args[2]->IsUint32());

Local<Object> req_wrap_obj = args[0].As<Object>();
node::Utf8Value ip_address(args[1]);
Expand Down
82 changes: 74 additions & 8 deletions test/simple/test-net-create-connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
30 changes: 10 additions & 20 deletions test/simple/test-net-localerror.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit 1a2a4da

Please sign in to comment.