Skip to content

Commit

Permalink
timers: do not use user object call/apply
Browse files Browse the repository at this point in the history
Timers should work even if the user has monkey-patched `.call()` and
`.apply()` to undesirable values.

PR-URL: #12960
Ref: #12956
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
  • Loading branch information
Trott authored and MylesBorins committed Aug 16, 2017
1 parent 74d5cba commit e8438c1
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 6 deletions.
12 changes: 6 additions & 6 deletions lib/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -383,20 +383,20 @@ function ontimeout(timer) {
var args = timer._timerArgs;
var callback = timer._onTimeout;
if (!args)
callback.call(timer);
timer._onTimeout();
else {
switch (args.length) {
case 1:
callback.call(timer, args[0]);
timer._onTimeout(args[0]);
break;
case 2:
callback.call(timer, args[0], args[1]);
timer._onTimeout(args[0], args[1]);
break;
case 3:
callback.call(timer, args[0], args[1], args[2]);
timer._onTimeout(args[0], args[1], args[2]);
break;
default:
callback.apply(timer, args);
Function.prototype.apply.call(callback, timer, args);
}
}
if (timer._repeat)
Expand Down Expand Up @@ -678,7 +678,7 @@ function runCallback(timer) {
return timer._callback(argv[0], argv[1], argv[2]);
// more than 3 arguments run slower with .apply
default:
return timer._callback.apply(timer, argv);
return Function.prototype.apply.call(timer._callback, timer, argv);
}
}

Expand Down
40 changes: 40 additions & 0 deletions test/parallel/test-timers-user-call.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Make sure `setTimeout()` and friends don't throw if the user-supplied
// function has .call() and .apply() monkey-patched to undesirable values.

// Refs: https://github.com/nodejs/node/issues/12956

'use strict';

const common = require('../common');

{
const fn = common.mustCall(10);
fn.call = 'not a function';
fn.apply = 'also not a function';
setTimeout(fn, 1);
setTimeout(fn, 1, 'oneArg');
setTimeout(fn, 1, 'two', 'args');
setTimeout(fn, 1, 'three', '(3)', 'args');
setTimeout(fn, 1, 'more', 'than', 'three', 'args');

setImmediate(fn, 1);
setImmediate(fn, 1, 'oneArg');
setImmediate(fn, 1, 'two', 'args');
setImmediate(fn, 1, 'three', '(3)', 'args');
setImmediate(fn, 1, 'more', 'than', 'three', 'args');
}

{
const testInterval = (...args) => {
const fn = common.mustCall(() => { clearInterval(interval); });
fn.call = 'not a function';
fn.apply = 'also not a function';
const interval = setInterval(fn, 1, ...args);
};

testInterval();
testInterval('oneArg');
testInterval('two', 'args');
testInterval('three', '(3)', 'args');
testInterval('more', 'than', 'three', 'args');
}

0 comments on commit e8438c1

Please sign in to comment.