diff --git a/doc/api/globals.md b/doc/api/globals.md index 8e002eb897a221..c10f138c0f24b2 100644 --- a/doc/api/globals.md +++ b/doc/api/globals.md @@ -117,6 +117,10 @@ Returns a new already aborted `AbortSignal`. added: - v17.3.0 - v16.14.0 +changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/58648 + description: Validate `delay` per its WebIDL definition. --> * `delay` {number} The number of milliseconds to wait before triggering diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index 4fbed3bfa7399a..be8b4a102e2e13 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -46,11 +46,11 @@ const { converters, createInterfaceConverter, createSequenceConverter, + convertToInt, } = require('internal/webidl'); const { validateObject, - validateUint32, kValidateObjectAllowObjects, } = require('internal/validators'); @@ -60,7 +60,6 @@ const { const { clearTimeout, - setTimeout, } = require('timers'); const assert = require('internal/assert'); @@ -70,6 +69,7 @@ const { kTransferList, markTransferMode, } = require('internal/worker/js_transferable'); +const { insert, InternalTimeout } = require('internal/timers'); let _MessageChannel; @@ -145,7 +145,7 @@ function validateThisAbortSignal(obj) { // the created timer object. Separately, we add the signal to a // FinalizerRegistry that will clear the timeout when the signal is gc'd. function setWeakAbortSignalTimeout(weakRef, delay) { - const timeout = setTimeout(() => { + const timeout = new InternalTimeout(() => { const signal = weakRef.deref(); if (signal !== undefined) { gcPersistentSignals.delete(signal); @@ -155,8 +155,8 @@ function setWeakAbortSignalTimeout(weakRef, delay) { 'The operation was aborted due to timeout', 'TimeoutError')); } - }, delay); - timeout.unref(); + }, delay, undefined, false, false); + insert(timeout, timeout._idleTimeout); return timeout; } @@ -236,7 +236,8 @@ class AbortSignal extends EventTarget { * @returns {AbortSignal} */ static timeout(delay) { - validateUint32(delay, 'delay', false); + const opts = { __proto__: null, enforceRange: true }; + delay = convertToInt('delay', delay, 64, opts); const signal = new AbortSignal(kDontThrowSymbol); signal[kTimeout] = true; clearTimeoutRegistry.register( diff --git a/lib/internal/timers.js b/lib/internal/timers.js index f603725a7bfa41..ba012138c53a16 100644 --- a/lib/internal/timers.js +++ b/lib/internal/timers.js @@ -174,36 +174,9 @@ function initAsyncResource(resource, type) { let warnedNegativeNumber = false; let warnedNotNumber = false; -class Timeout { - // Timer constructor function. - // The entire prototype is defined in lib/timers.js +// Internal Timeout skips the `after` parameter validation, so we can use it for uint64 in `AbortSignal` +class InternalTimeout { constructor(callback, after, args, isRepeat, isRefed) { - if (after === undefined) { - after = 1; - } else { - after *= 1; // Coalesce to number or NaN - } - - if (!(after >= 1 && after <= TIMEOUT_MAX)) { - if (after > TIMEOUT_MAX) { - process.emitWarning(`${after} does not fit into` + - ' a 32-bit signed integer.' + - '\nTimeout duration was set to 1.', - 'TimeoutOverflowWarning'); - } else if (after < 0 && !warnedNegativeNumber) { - warnedNegativeNumber = true; - process.emitWarning(`${after} is a negative number.` + - '\nTimeout duration was set to 1.', - 'TimeoutNegativeWarning'); - } else if (NumberIsNaN(after) && !warnedNotNumber) { - warnedNotNumber = true; - process.emitWarning(`${after} is not a number.` + - '\nTimeout duration was set to 1.', - 'TimeoutNaNWarning'); - } - after = 1; // Schedule on next tick, follows browser behavior - } - this._idleTimeout = after; this._idlePrev = this; this._idleNext = this; @@ -264,6 +237,39 @@ class Timeout { } } +class Timeout extends InternalTimeout { + // Timer constructor function. + // The entire prototype is defined in lib/timers.js + constructor(callback, after, args, isRepeat, isRefed) { + if (after === undefined) { + after = 1; + } else { + after *= 1; // Coalesce to number or NaN + } + + if (!(after >= 1 && after <= TIMEOUT_MAX)) { + if (after > TIMEOUT_MAX) { + process.emitWarning(`${after} does not fit into` + + ' a 32-bit signed integer.' + + '\nTimeout duration was set to 1.', + 'TimeoutOverflowWarning'); + } else if (after < 0 && !warnedNegativeNumber) { + warnedNegativeNumber = true; + process.emitWarning(`${after} is a negative number.` + + '\nTimeout duration was set to 1.', + 'TimeoutNegativeWarning'); + } else if (NumberIsNaN(after) && !warnedNotNumber) { + warnedNotNumber = true; + process.emitWarning(`${after} is not a number.` + + '\nTimeout duration was set to 1.', + 'TimeoutNaNWarning'); + } + after = 1; // Schedule on next tick, follows browser behavior + } + super(callback, after, args, isRepeat, isRefed); + } +} + class TimersList { constructor(expiry, msecs) { this._idleNext = this; // Create the list with the linkedlist properties to @@ -702,6 +708,7 @@ module.exports = { kTimeout: Symbol('timeout'), // For hiding Timeouts on other internals. async_id_symbol, trigger_async_id_symbol, + InternalTimeout, Timeout, Immediate, kRefed, diff --git a/test/parallel/test-abortcontroller.js b/test/parallel/test-abortcontroller.js index 87781f849ffb52..95acb8f2463caa 100644 --- a/test/parallel/test-abortcontroller.js +++ b/test/parallel/test-abortcontroller.js @@ -274,3 +274,33 @@ test('abortSignal.throwIfAobrted() works as expected (3)', () => { throws(() => AbortSignal.abort(actualReason).throwIfAborted(), actualReason); Reflect.defineProperty(AbortSignal.prototype, 'reason', originalDesc); }); + +test('abortSignal.timeout() works with special \'delay\' value', async () => { + const inputs = [ + null, + true, + false, + '', + [], + 0, + 0.0, + 0.1, + 0.5, + 1, + 1.0, + ]; + + const signals = []; + + for (let i = 0; i < inputs.length; i++) { + signals[i] = AbortSignal.timeout(inputs[i]); + } + + await sleep(2); + + for (let i = 0; i < inputs.length; i++) { + ok(signals[i].aborted); + ok(signals[i].reason instanceof DOMException); + strictEqual(signals[i].reason.name, 'TimeoutError'); + } +});