feat: validate timeout option

PR-URL: nodejs/node#43843 Reviewed-By: Feng Yu <[email protected]> (cherry picked from commit d83446b4c4694322e12d2b7d22592f2be674e580)
aduh95 · Jul 24, 2022 · cf78656 · cf78656
1 parent 3814bf0
commit cf78656
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 5 deletions.
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -1,4 +1,4 @@
-// https://github.com/nodejs/node/blob/2e682f10b6104373fded64b0e364984b85ae2243/lib/internal/test_runner/test.js
+// https://github.com/nodejs/node/blob/d83446b4c4694322e12d2b7d22592f2be674e580/lib/internal/test_runner/test.js
 
 'use strict'
 
@@ -34,8 +34,13 @@ const {
   kEmptyObject
 } = require('#internal/util')
 const { isPromise } = require('#internal/util/types')
-const { isUint32, validateAbortSignal } = require('#internal/validators')
+const {
+  isUint32,
+  validateAbortSignal,
+  validateNumber
+} = require('#internal/validators')
 const { setTimeout } = require('#timers/promises')
+const { TIMEOUT_MAX } = require('#internal/timers')
 const { cpus } = require('os')
 const { bigint: hrtime } = process.hrtime
 const kCallbackAndPromisePresent = 'callbackAndPromisePresent'
@@ -151,7 +156,8 @@ class Test extends AsyncResource {
       this.concurrency = concurrency
     }
 
-    if (isUint32(timeout)) {
+    if (timeout != null && timeout !== Infinity) {
+      validateNumber(timeout, 'options.timeout', 0, TIMEOUT_MAX)
       this.timeout = timeout
     }
 

diff --git a/lib/internal/timers.js b/lib/internal/timers.js
@@ -0,0 +1,7 @@
+'use strict'
+
+const TIMEOUT_MAX = 2 ** 31 - 1
+
+module.exports = {
+  TIMEOUT_MAX
+}
diff --git a/lib/internal/validators.js b/lib/internal/validators.js
@@ -1,8 +1,19 @@
-// https://github.com/nodejs/node/blob/1aab13cad9c800f4121c1d35b554b78c1b17bdbd/lib/internal/validators.js
+// https://github.com/nodejs/node/blob/d83446b4c4694322e12d2b7d22592f2be674e580/lib/internal/validators.js
 function isUint32 (value) {
   return value === (value >>> 0)
 }
 
+function validateNumber (value, name, min = undefined, max) {
+  if (typeof value !== 'number') { throw new TypeError(`Expected ${name} to be a number, got ${value}`) }
+
+  if ((min != null && value < min) || (max != null && value > max) ||
+      ((min != null || max != null) && Number.isNaN(value))) {
+    throw new RangeError(`Expected ${name} to be ${
+      `${min != null ? `>= ${min}` : ''}${min != null && max != null ? ' && ' : ''}${max != null ? `<= ${max}` : ''}`
+    }, got ${value}`)
+  }
+}
+
 const validateAbortSignal = (signal, name) => {
   if (signal !== undefined &&
       (signal === null ||
@@ -14,5 +25,6 @@ const validateAbortSignal = (signal, name) => {
 
 module.exports = {
   isUint32,
-  validateAbortSignal
+  validateAbortSignal,
+  validateNumber
 }
diff --git a/test/parallel/test-runner-option-validation.js b/test/parallel/test-runner-option-validation.js
@@ -0,0 +1,18 @@
+// https://github.com/nodejs/node/blob/d83446b4c4694322e12d2b7d22592f2be674e580/test/parallel/test-runner-option-validation.js
+
+'use strict'
+require('../common')
+const assert = require('assert')
+const test = require('node:test');
+
+// eslint-disable-next-line symbol-description
+[Symbol(), {}, [], () => {}, 1n, true, '1'].forEach((timeout) => {
+  assert.throws(() => test({ timeout }), { code: 'ERR_INVALID_ARG_TYPE' })
+});
+[-1, -Infinity, NaN, 2 ** 33, Number.MAX_SAFE_INTEGER].forEach((timeout) => {
+  assert.throws(() => test({ timeout }), { code: 'ERR_OUT_OF_RANGE' })
+});
+[null, undefined, Infinity, 0, 1, 1.1].forEach((timeout) => {
+  // Valid values should not throw.
+  test({ timeout })
+})