Skip to content

lib: validate AbortSignal.timeout delay per its WebIDL definition #58594

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/api/globals.md
Original file line number Diff line number Diff line change
Expand Up @@ -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/58594
description: Change validation of `delay` to allow non-integer.
-->

* `delay` {number} The number of milliseconds to wait before triggering
Expand Down
12 changes: 10 additions & 2 deletions lib/internal/abort_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@
} = require('internal/errors');
const {
converters,
convertToInt,
createInterfaceConverter,
createSequenceConverter,
} = require('internal/webidl');

const {
validateObject,
validateUint32,
kValidateObjectAllowObjects,
} = require('internal/validators');

Expand Down Expand Up @@ -236,7 +236,15 @@
* @returns {AbortSignal}
*/
static timeout(delay) {
validateUint32(delay, 'delay', false);
const opts = { __proto__: null, enforceRange: true };
try {
delay = convertToInt('delay', delay, 32, opts);
} catch (err) {
// this is so that the validation error change is not a semver-major

Check failure on line 243 in lib/internal/abort_controller.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Comments should not begin with a lowercase character
// TODO: remove this try/catch and validateUint32 as semver-major
validateUint32(delay, 'delay', false);

Check failure on line 245 in lib/internal/abort_controller.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

'validateUint32' is not defined
throw err;
}
const signal = new AbortSignal(kDontThrowSymbol);
signal[kTimeout] = true;
clearTimeoutRegistry.register(
Expand Down
32 changes: 32 additions & 0 deletions test/parallel/test-abortcontroller.js
Original file line number Diff line number Diff line change
Expand Up @@ -274,3 +274,35 @@
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,
2147483648,
12345678901234,
];

const signals = [];

for (let i = 0; i < inputs.length; i++) {
signals[i] = AbortSignal.timeout(inputs[i]);

Check failure on line 298 in test/parallel/test-abortcontroller.js

View workflow job for this annotation

GitHub Actions / test-macOS

--- stderr --- (node:56557) TimeoutOverflowWarning: 2147483648 does not fit into a 32-bit signed integer. Timeout duration was set to 1. (Use `node --trace-warnings ...` to show where the warning was created) --- stdout --- Test failure: 'abortSignal.timeout() works with special 'delay' value' Location: test/parallel/test-abortcontroller.js:278:1 ReferenceError: validateUint32 is not defined at AbortSignal.timeout (node:internal/abort_controller:245:7) at TestContext.<anonymous> (/Users/runner/work/node/node/node/test/parallel/test-abortcontroller.js:298:30) at Test.runInAsyncScope (node:async_hooks:214:14) at Test.run (node:internal/test_runner/test:1062:25) at Test.processPendingSubtests (node:internal/test_runner/test:752:18) at Test.postRun (node:internal/test_runner/test:1191:19) at Test.run (node:internal/test_runner/test:1119:12) at async Test.processPendingSubtests (node:internal/test_runner/test:752:7) Command: out/Release/node --expose-gc --test-reporter=./test/common/test-error-reporter.js --test-reporter-destination=stdout /Users/runner/work/node/node/node/test/parallel/test-abortcontroller.js

Check failure on line 298 in test/parallel/test-abortcontroller.js

View workflow job for this annotation

GitHub Actions / test-linux (ubuntu-24.04)

--- stderr --- (node:136032) TimeoutOverflowWarning: 2147483648 does not fit into a 32-bit signed integer. Timeout duration was set to 1. (Use `node --trace-warnings ...` to show where the warning was created) --- stdout --- Test failure: 'abortSignal.timeout() works with special 'delay' value' Location: test/parallel/test-abortcontroller.js:278:1 ReferenceError: validateUint32 is not defined at AbortSignal.timeout (node:internal/abort_controller:245:7) at TestContext.<anonymous> (/home/runner/work/node/node/node/test/parallel/test-abortcontroller.js:298:30) at Test.runInAsyncScope (node:async_hooks:214:14) at Test.run (node:internal/test_runner/test:1062:25) at Test.processPendingSubtests (node:internal/test_runner/test:752:18) at Test.postRun (node:internal/test_runner/test:1191:19) at Test.run (node:internal/test_runner/test:1119:12) at async Test.processPendingSubtests (node:internal/test_runner/test:752:7) Command: out/Release/node --expose-gc --test-reporter=./test/common/test-error-reporter.js --test-reporter-destination=stdout /home/runner/work/node/node/node/test/parallel/test-abortcontroller.js

Check failure on line 298 in test/parallel/test-abortcontroller.js

View workflow job for this annotation

GitHub Actions / test-linux (ubuntu-24.04-arm)

--- stderr --- (node:134373) TimeoutOverflowWarning: 2147483648 does not fit into a 32-bit signed integer. Timeout duration was set to 1. (Use `node --trace-warnings ...` to show where the warning was created) --- stdout --- Test failure: 'abortSignal.timeout() works with special 'delay' value' Location: test/parallel/test-abortcontroller.js:278:1 ReferenceError: validateUint32 is not defined at AbortSignal.timeout (node:internal/abort_controller:245:7) at TestContext.<anonymous> (/home/runner/work/node/node/node/test/parallel/test-abortcontroller.js:298:30) at Test.runInAsyncScope (node:async_hooks:214:14) at Test.run (node:internal/test_runner/test:1062:25) at Test.processPendingSubtests (node:internal/test_runner/test:752:18) at Test.postRun (node:internal/test_runner/test:1191:19) at Test.run (node:internal/test_runner/test:1119:12) at async Test.processPendingSubtests (node:internal/test_runner/test:752:7) Command: out/Release/node --expose-gc --test-reporter=./test/common/test-error-reporter.js --test-reporter-destination=stdout /home/runner/work/node/node/node/test/parallel/test-abortcontroller.js
}

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