From 1e1b82baea0a490158c495beea717b801536dfe4 Mon Sep 17 00:00:00 2001 From: dnalborczyk Date: Mon, 31 Oct 2022 08:47:38 -0400 Subject: [PATCH] lib: fix `AbortSignal.timeout` parameter validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/42856 Reviewed-By: Michaƫl Zasso Reviewed-By: Benjamin Gruenbaum Reviewed-By: Colin Ihrig Reviewed-By: Antoine du Hamel Reviewed-By: Zijian Liu --- lib/internal/abort_controller.js | 2 +- test/fixtures/wpt/README.md | 2 +- .../fixtures/wpt/dom/abort/AbortSignal.any.js | 28 ++++ .../wpt/dom/abort/abort-signal-timeout.html | 19 +++ .../dom/abort/crashtests/timeout-close.html | 22 ++++ test/fixtures/wpt/dom/abort/event.any.js | 123 ++++++++++++++++++ .../wpt/dom/abort/reason-constructor.html | 12 ++ test/fixtures/wpt/versions.json | 2 +- 8 files changed, 207 insertions(+), 3 deletions(-) create mode 100644 test/fixtures/wpt/dom/abort/abort-signal-timeout.html create mode 100644 test/fixtures/wpt/dom/abort/crashtests/timeout-close.html create mode 100644 test/fixtures/wpt/dom/abort/reason-constructor.html diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index a3fb544b615e3e..7fbc4ed756c3c5 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -170,7 +170,7 @@ class AbortSignal extends EventTarget { * @returns {AbortSignal} */ static timeout(delay) { - validateUint32(delay, 'delay', true); + validateUint32(delay, 'delay', false); const signal = createAbortSignal(); signal[kTimeout] = true; clearTimeoutRegistry.register( diff --git a/test/fixtures/wpt/README.md b/test/fixtures/wpt/README.md index 515a769b4392ff..9bfa7bc8668273 100644 --- a/test/fixtures/wpt/README.md +++ b/test/fixtures/wpt/README.md @@ -12,7 +12,7 @@ Last update: - common: https://github.com/web-platform-tests/wpt/tree/03c5072aff/common - console: https://github.com/web-platform-tests/wpt/tree/767ae35464/console -- dom/abort: https://github.com/web-platform-tests/wpt/tree/c49cafb491/dom/abort +- dom/abort: https://github.com/web-platform-tests/wpt/tree/8fadb38120/dom/abort - dom/events: https://github.com/web-platform-tests/wpt/tree/f8821adb28/dom/events - encoding: https://github.com/web-platform-tests/wpt/tree/c1b24fce6e/encoding - fetch/data-urls/resources: https://github.com/web-platform-tests/wpt/tree/7c79d998ff/fetch/data-urls/resources diff --git a/test/fixtures/wpt/dom/abort/AbortSignal.any.js b/test/fixtures/wpt/dom/abort/AbortSignal.any.js index 1d7d7678eb1a6e..3bbdc11a92f90d 100644 --- a/test/fixtures/wpt/dom/abort/AbortSignal.any.js +++ b/test/fixtures/wpt/dom/abort/AbortSignal.any.js @@ -10,3 +10,31 @@ async_test(t => { s.onabort = t.unreached_func("abort event handler called"); t.step_timeout(() => { t.done(); }, 2000); }, "signal returned by AbortSignal.abort() should not fire abort event"); + +test(t => { + const signal = AbortSignal.timeout(0); + assert_true(signal instanceof AbortSignal, "returned object is an AbortSignal"); + assert_false(signal.aborted, "returned signal is not already aborted"); +}, "AbortSignal.timeout() returns a non-aborted signal"); + +async_test(t => { + const signal = AbortSignal.timeout(5); + signal.onabort = t.step_func_done(() => { + assert_true(signal.aborted, "signal is aborted"); + assert_true(signal.reason instanceof DOMException, "signal.reason is a DOMException"); + assert_equals(signal.reason.name, "TimeoutError", "signal.reason is a TimeoutError"); + }); +}, "Signal returned by AbortSignal.timeout() times out"); + +async_test(t => { + let result = ""; + for (const value of ["1", "2", "3"]) { + const signal = AbortSignal.timeout(5); + signal.onabort = t.step_func(() => { result += value; }); + } + + const signal = AbortSignal.timeout(5); + signal.onabort = t.step_func_done(() => { + assert_equals(result, "123", "Timeout order should be 123"); + }); +}, "AbortSignal timeouts fire in order"); diff --git a/test/fixtures/wpt/dom/abort/abort-signal-timeout.html b/test/fixtures/wpt/dom/abort/abort-signal-timeout.html new file mode 100644 index 00000000000000..2a9c13d61434b4 --- /dev/null +++ b/test/fixtures/wpt/dom/abort/abort-signal-timeout.html @@ -0,0 +1,19 @@ + + +AbortSignal.timeout frame detach + + + + diff --git a/test/fixtures/wpt/dom/abort/crashtests/timeout-close.html b/test/fixtures/wpt/dom/abort/crashtests/timeout-close.html new file mode 100644 index 00000000000000..ee8544a7f57edb --- /dev/null +++ b/test/fixtures/wpt/dom/abort/crashtests/timeout-close.html @@ -0,0 +1,22 @@ + + + + + diff --git a/test/fixtures/wpt/dom/abort/event.any.js b/test/fixtures/wpt/dom/abort/event.any.js index a67e6f400fcf1d..34af8ee5c560ae 100644 --- a/test/fixtures/wpt/dom/abort/event.any.js +++ b/test/fixtures/wpt/dom/abort/event.any.js @@ -4,6 +4,8 @@ test(t => { let state = "begin"; assert_false(s.aborted); + assert_true("reason" in s, "signal has reason property"); + assert_equals(s.reason, undefined, "signal.reason is initially undefined"); s.addEventListener("abort", t.step_func(e => { @@ -15,6 +17,8 @@ test(t => { assert_equals(state, "aborted"); assert_true(s.aborted); + assert_true(s.reason instanceof DOMException, "signal.reason is DOMException"); + assert_equals(s.reason.name, "AbortError", "signal.reason is AbortError"); c.abort(); }, "AbortController abort() should fire event synchronously"); @@ -64,4 +68,123 @@ test(t => { controller.abort(); }, "the abort event should have the right properties"); +test(t => { + const controller = new AbortController(); + const signal = controller.signal; + + assert_true("reason" in signal, "signal has reason property"); + assert_equals(signal.reason, undefined, "signal.reason is initially undefined"); + + const reason = Error("hello"); + controller.abort(reason); + + assert_true(signal.aborted, "signal.aborted"); + assert_equals(signal.reason, reason, "signal.reason"); +}, "AbortController abort(reason) should set signal.reason"); + +test(t => { + const controller = new AbortController(); + const signal = controller.signal; + + assert_true("reason" in signal, "signal has reason property"); + assert_equals(signal.reason, undefined, "signal.reason is initially undefined"); + + controller.abort(); + + assert_true(signal.aborted, "signal.aborted"); + assert_true(signal.reason instanceof DOMException, "signal.reason is DOMException"); + assert_equals(signal.reason.name, "AbortError", "signal.reason is AbortError"); +}, "aborting AbortController without reason creates an \"AbortError\" DOMException"); + +test(t => { + const controller = new AbortController(); + const signal = controller.signal; + + assert_true("reason" in signal, "signal has reason property"); + assert_equals(signal.reason, undefined, "signal.reason is initially undefined"); + + controller.abort(undefined); + + assert_true(signal.aborted, "signal.aborted"); + assert_true(signal.reason instanceof DOMException, "signal.reason is DOMException"); + assert_equals(signal.reason.name, "AbortError", "signal.reason is AbortError"); +}, "AbortController abort(undefined) creates an \"AbortError\" DOMException"); + +test(t => { + const controller = new AbortController(); + const signal = controller.signal; + + assert_true("reason" in signal, "signal has reason property"); + assert_equals(signal.reason, undefined, "signal.reason is initially undefined"); + + controller.abort(null); + + assert_true(signal.aborted, "signal.aborted"); + assert_equals(signal.reason, null, "signal.reason"); +}, "AbortController abort(null) should set signal.reason"); + +test(t => { + const signal = AbortSignal.abort(); + + assert_true(signal.aborted, "signal.aborted"); + assert_true(signal.reason instanceof DOMException, "signal.reason is DOMException"); + assert_equals(signal.reason.name, "AbortError", "signal.reason is AbortError"); +}, "static aborting signal should have right properties"); + +test(t => { + const reason = Error("hello"); + const signal = AbortSignal.abort(reason); + + assert_true(signal.aborted, "signal.aborted"); + assert_equals(signal.reason, reason, "signal.reason"); +}, "static aborting signal with reason should set signal.reason"); + +test(t => { + const reason = new Error('boom'); + const signal = AbortSignal.abort(reason); + assert_true(signal.aborted); + assert_throws_exactly(reason, () => signal.throwIfAborted()); +}, "throwIfAborted() should throw abort.reason if signal aborted"); + +test(t => { + const signal = AbortSignal.abort('hello'); + assert_true(signal.aborted); + assert_throws_exactly('hello', () => signal.throwIfAborted()); +}, "throwIfAborted() should throw primitive abort.reason if signal aborted"); + +test(t => { + const controller = new AbortController(); + assert_false(controller.signal.aborted); + controller.signal.throwIfAborted(); +}, "throwIfAborted() should not throw if signal not aborted"); + +test(t => { + const signal = AbortSignal.abort(); + + assert_true( + signal.reason instanceof DOMException, + "signal.reason is a DOMException" + ); + assert_equals( + signal.reason, + signal.reason, + "signal.reason returns the same DOMException" + ); +}, "AbortSignal.reason returns the same DOMException"); + +test(t => { + const controller = new AbortController(); + controller.abort(); + + assert_true( + controller.signal.reason instanceof DOMException, + "signal.reason is a DOMException" + ); + assert_equals( + controller.signal.reason, + controller.signal.reason, + "signal.reason returns the same DOMException" + ); +}, "AbortController.signal.reason returns the same DOMException"); + done(); diff --git a/test/fixtures/wpt/dom/abort/reason-constructor.html b/test/fixtures/wpt/dom/abort/reason-constructor.html new file mode 100644 index 00000000000000..0515165a0f6788 --- /dev/null +++ b/test/fixtures/wpt/dom/abort/reason-constructor.html @@ -0,0 +1,12 @@ + + +AbortSignal.reason constructor + + + + diff --git a/test/fixtures/wpt/versions.json b/test/fixtures/wpt/versions.json index cd1ef803ef0cfa..dcf27842b67ceb 100644 --- a/test/fixtures/wpt/versions.json +++ b/test/fixtures/wpt/versions.json @@ -8,7 +8,7 @@ "path": "console" }, "dom/abort": { - "commit": "c49cafb491d99d6318f8f24a26936cc66501f412", + "commit": "8fadb381209a215280dc3ad96d0f135b6005f176", "path": "dom/abort" }, "dom/events": {