Skip to content

Commit

Permalink
net: throw on invalid socket timeouts
Browse files Browse the repository at this point in the history
This commit restricts socket timeouts non-negative, finite
numbers. Any other value throws a TypeError or RangeError.
This prevents subtle bugs that can happen due to type
coercion.

Fixes: nodejs/node-v0.x-archive#8618
PR-URL: nodejs/node-v0.x-archive#8884
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Timothy J Fontaine <tjfontaine@gmail.com>

Conflicts:
	lib/timers.js
	test/simple/test-net-settimeout.js
	test/simple/test-net-socket-timeout.js
  • Loading branch information
cjihrig committed Feb 13, 2015
1 parent ba40942 commit 0cff052
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 13 deletions.
12 changes: 6 additions & 6 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -300,17 +300,17 @@ Socket.prototype.listen = function() {


Socket.prototype.setTimeout = function(msecs, callback) {
if (msecs > 0 && isFinite(msecs)) {
if (msecs === 0) {
timers.unenroll(this);
if (callback) {
this.removeListener('timeout', callback);
}
} else {
timers.enroll(this, msecs);
timers._unrefActive(this);
if (callback) {
this.once('timeout', callback);
}
} else if (msecs === 0) {
timers.unenroll(this);
if (callback) {
this.removeListener('timeout', callback);
}
}
};

Expand Down
18 changes: 12 additions & 6 deletions lib/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,13 @@
const Timer = process.binding('timer_wrap').Timer;
const L = require('_linklist');
const assert = require('assert').ok;

const util = require('util');
const debug = util.debuglog('timer');
const kOnTimeout = Timer.kOnTimeout | 0;

// Timeout values > TIMEOUT_MAX are set to 1.
const TIMEOUT_MAX = 2147483647; // 2^31-1

const debug = require('util').debuglog('timer');


// IDLE TIMEOUTS
//
// Because often many sockets will have the same idle timeout we will not
Expand Down Expand Up @@ -132,13 +130,21 @@ const unenroll = exports.unenroll = function(item) {

// Does not start the time, just sets up the members needed.
exports.enroll = function(item, msecs) {
if (typeof msecs !== 'number') {
throw new TypeError('msecs must be a number');
}

if (msecs < 0 || !isFinite(msecs)) {
throw new RangeError('msecs must be a non-negative finite number');
}

// if this item was already in a list somewhere
// then we should unenroll it from that
if (item._idleNext) unenroll(item);

// Ensure that msecs fits into signed int32
if (msecs > 0x7fffffff) {
msecs = 0x7fffffff;
if (msecs > TIMEOUT_MAX) {
msecs = TIMEOUT_MAX;
}

item._idleTimeout = msecs;
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-net-settimeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ var server = net.createServer(function(c) {
});
server.listen(common.PORT);

var killers = [0, Infinity, NaN];
var killers = [0];

var left = killers.length;
killers.forEach(function(killer) {
Expand Down
25 changes: 25 additions & 0 deletions test/parallel/test-net-socket-timeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,31 @@ var common = require('../common');
var net = require('net');
var assert = require('assert');

// Verify that invalid delays throw
var noop = function() {};
var s = new net.Socket();
var nonNumericDelays = ['100', true, false, undefined, null, '', {}, noop, []];
var badRangeDelays = [-0.001, -1, -Infinity, Infinity, NaN];
var validDelays = [0, 0.001, 1, 1e6];

for (var i = 0; i < nonNumericDelays.length; i++) {
assert.throws(function() {
s.setTimeout(nonNumericDelays[i], noop);
}, TypeError);
}

for (var i = 0; i < badRangeDelays.length; i++) {
assert.throws(function() {
s.setTimeout(badRangeDelays[i], noop);
}, RangeError);
}

for (var i = 0; i < validDelays.length; i++) {
assert.doesNotThrow(function() {
s.setTimeout(validDelays[i], noop);
});
}

var timedout = false;

var server = net.Server();
Expand Down

0 comments on commit 0cff052

Please sign in to comment.