lib: validate AbortSignal.timeout delay per its WebIDL definition

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

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(

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,

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');
+  }
+});