Skip to content

Commit

Permalink
fix(lib/runnable.js): Fix timeout range issue for good
Browse files Browse the repository at this point in the history
Found that previous "fix" hadn't actually fixed the correct value, as the wrong upper bound value
had been used. Further checks indicated that IE had problems (surprise, surprise) with negative
timeout values. New code clamps to nonnegative range ending at `INT_MAX`.

Closes mochajs#1652
  • Loading branch information
plroebuck committed Nov 9, 2018
1 parent 20083cb commit 0a88350
Showing 1 changed file with 11 additions and 3 deletions.
14 changes: 11 additions & 3 deletions lib/runnable.js
Expand Up @@ -60,10 +60,12 @@ utils.inherits(Runnable, EventEmitter);
* Set timeout threshold value (msecs).
*
* @description
* A string argument can use shorthand (such as "2s") and will be converted.
* If value is `0` or exceeds `2^<sup>31</sup>-1`, timeouts will be disabled.
* A string argument can use shorthand (e.g., "2s") and will be converted.
* The value will be clamped to range [<code>0</code>, <code>2^<sup>31</sup>-1</code>].
* If clamped value matches either range endpoint, timeouts will be disabled.
*
* @private
* @see {@link https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setTimeout#Maximum_delay_value}
* @param {number|string} ms - Timeout threshold value.
* @returns {Runnable} this
* @chainable
Expand All @@ -75,8 +77,14 @@ Runnable.prototype.timeout = function(ms) {
if (typeof ms === 'string') {
ms = milliseconds(ms);
}

// Clamp to range
var INT_MAX = Math.pow(2, 31) - 1;
var range = [0, INT_MAX];
ms = utils.clamp(ms, range);

// see #1652 for reasoning
if (ms === 0 || ms > Math.pow(2, 31) - 1) {
if (ms === range[0] || ms === range[1]) {
this._enableTimeouts = false;
}
debug('timeout %d', ms);
Expand Down

0 comments on commit 0a88350

Please sign in to comment.