Skip to content

Commit

Permalink
lib: fix AbortSignal.timeout parameter validation
Browse files Browse the repository at this point in the history
PR-URL: #42856
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
  • Loading branch information
dnalborczyk authored and danielleadams committed Jan 3, 2023
1 parent ffd0593 commit ba0e7ae
Show file tree
Hide file tree
Showing 8 changed files with 207 additions and 3 deletions.
2 changes: 1 addition & 1 deletion lib/internal/abort_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/wpt/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 28 additions & 0 deletions test/fixtures/wpt/dom/abort/AbortSignal.any.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
19 changes: 19 additions & 0 deletions test/fixtures/wpt/dom/abort/abort-signal-timeout.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<!DOCTYPE HTML>
<meta charset=utf-8>
<title>AbortSignal.timeout frame detach</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<iframe id="iframe"></iframe>
<script>
async_test(t => {
const signal = iframe.contentWindow.AbortSignal.timeout(5);
signal.onabort = t.unreached_func("abort must not fire");

iframe.remove();

t.step_timeout(() => {
assert_false(signal.aborted);
t.done();
}, 10);
}, "Signal returned by AbortSignal.timeout() is not aborted after frame detach");
</script>
22 changes: 22 additions & 0 deletions test/fixtures/wpt/dom/abort/crashtests/timeout-close.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<!DOCTYPE html>
<html class="test-wait">
<meta charset="utf-8">
<iframe id="iframe"></iframe>
<script>
const srcdoc = `
<!DOCTYPE html>
<meta charset="utf-8">
<script>
const xhr = new XMLHttpRequest()
setTimeout(() => {
xhr.open('GET', '/', false)
xhr.send()
AbortSignal.timeout(41.62684667994843)
}, 1)
setTimeout(() => {
location.href = "about:blank"
parent.document.documentElement.classList.remove("test-wait")
}, 0)
</` + "script>";
iframe.srcdoc = srcdoc;
</script>
123 changes: 123 additions & 0 deletions test/fixtures/wpt/dom/abort/event.any.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand All @@ -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");
Expand Down Expand Up @@ -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();
12 changes: 12 additions & 0 deletions test/fixtures/wpt/dom/abort/reason-constructor.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<!DOCTYPE HTML>
<meta charset=utf-8>
<title>AbortSignal.reason constructor</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<iframe id="iframe"></iframe>
<script>
test(() => {
const aborted = iframe.contentWindow.AbortSignal.abort();
assert_equals(aborted.reason.constructor, iframe.contentWindow.DOMException, "DOMException is using the correct global");
}, "AbortSignal.reason.constructor should be from iframe");
</script>
2 changes: 1 addition & 1 deletion test/fixtures/wpt/versions.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"path": "console"
},
"dom/abort": {
"commit": "c49cafb491d99d6318f8f24a26936cc66501f412",
"commit": "8fadb381209a215280dc3ad96d0f135b6005f176",
"path": "dom/abort"
},
"dom/events": {
Expand Down

0 comments on commit ba0e7ae

Please sign in to comment.