Skip to content

Commit 98a7e49

Browse files
basukeJarred-Sumner
basuke
authored and
Jarred-Sumner
committed
Navigation API: Don't allow the javascript: protocol in navigation.navigate()
https://bugs.webkit.org/show_bug.cgi?id=297650 rdar://158867866 Reviewed by Tim Nguyen. navigation.navigate("javascript:alert(1)") is a new script execution sink that was newly added. We believe that it would be more useful for new APIs to not support the legacy javascript: protocol, compared to just keeping it because other APIs like location.href support it. whatwg/html#11533 * LayoutTests/http/wpt/navigation-api/navigation-methods/return-value/navigate-javascript-url-expected.txt: Added. * LayoutTests/http/wpt/navigation-api/navigation-methods/return-value/navigate-javascript-url.html: Added. * LayoutTests/http/wpt/navigation-api/navigation-methods/return-value/resources/helpers.js: Added. (window.assertReturnValue): (window.assertNeverSettles): (window.assertBothFulfillEntryNotAvailable.async t): (window.assertBothFulfill.async t): (window.assertCommittedFulfillsFinishedRejectsExactly.async t): (window.assertCommittedFulfillsFinishedRejectsDOM.async t): (window.waitForAllLenient): (window.assertBothRejectExactly.async t): (window.assertBothRejectDOM.async t): * Source/WebCore/page/Navigation.cpp: (WebCore::Navigation::navigate): Canonical link: https://commits.webkit.org/299235@main
1 parent 7411ea7 commit 98a7e49

File tree

4 files changed

+198
-2
lines changed

4 files changed

+198
-2
lines changed

‎LayoutTests/http/wpt/navigation-api/navigation-methods/return-value/navigate-javascript-url-expected.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
2+
PASS navigate() to a javascript: URL
3+

‎LayoutTests/http/wpt/navigation-api/navigation-methods/return-value/navigate-javascript-url.html‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<!doctype html>
2+
<script src="/resources/testharness.js"></script>
3+
<script src="/resources/testharnessreport.js"></script>
4+
<script src="resources/helpers.js"></script>
5+
6+
<script type="module">
7+
8+
promise_test(async t => {
9+
async function ensureWindowLoadEventFired(t) {
10+
return new Promise(resolve => {
11+
const callback = () => t.step_timeout(resolve, 0);
12+
if (document.readyState === 'complete') {
13+
callback();
14+
} else {
15+
window.onload = callback;
16+
}
17+
});
18+
}
19+
20+
// Wait for after the load event so that we are definitely testing the
21+
// javascript: URL as the cause of the rejections.
22+
await ensureWindowLoadEventFired(t);
23+
24+
navigation.onnavigate = t.unreached_func("onnavigate should not be called");
25+
navigation.onnavigatesuccess = t.unreached_func("onnavigatesuccess should not be called");
26+
navigation.onnavigateerror = t.unreached_func("onnavigateerror should not be called");
27+
28+
let result;
29+
result = navigation.navigate("javascript:'foo'");
30+
await assertBothRejectDOM(t, result, "NotSupportedError");
31+
32+
result = navigation.navigate("javascript:'foo'", { history: "replace" });
33+
await assertBothRejectDOM(t, result, "NotSupportedError");
34+
35+
result = navigation.navigate("javascript:'foo'", { history: "push" });
36+
await assertBothRejectDOM(t, result, "NotSupportedError");
37+
}, "navigate() to a javascript: URL");
38+
</script>

‎LayoutTests/http/wpt/navigation-api/navigation-methods/return-value/resources/helpers.js‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
window.assertReturnValue = (result, w = window) => {
2+
assert_equals(Object.getPrototypeOf(result), w.Object.prototype, "result object must be from the right realm");
3+
assert_array_equals(Reflect.ownKeys(result), ["committed", "finished"]);
4+
assert_true(result.committed instanceof w.Promise);
5+
assert_true(result.finished instanceof w.Promise);
6+
assert_not_equals(result.committed, result.finished);
7+
};
8+
9+
window.assertNeverSettles = (t, result, w = window) => {
10+
assertReturnValue(result, w);
11+
result.committed.then(
12+
t.unreached_func("committed must not fulfill"),
13+
t.unreached_func("committed must not reject")
14+
);
15+
16+
result.finished.then(
17+
t.unreached_func("finished must not fulfill"),
18+
t.unreached_func("finished must not reject")
19+
);
20+
};
21+
22+
window.assertBothFulfillEntryNotAvailable = async (t, result, w = window) => {
23+
assertReturnValue(result, w);
24+
25+
// Don't use await here so that we can catch out-of-order settlements.
26+
let committedValue;
27+
result.committed.then(
28+
t.step_func(v => { committedValue = v;}),
29+
t.unreached_func("committed must not reject")
30+
);
31+
32+
const finishedValue = await result.finished;
33+
34+
assert_not_equals(committedValue, undefined, "committed must fulfill before finished");
35+
assert_equals(finishedValue, committedValue, "committed and finished must fulfill with the same value");
36+
assert_true(finishedValue instanceof w.NavigationHistoryEntry, "fulfillment value must be a NavigationHistoryEntry");
37+
};
38+
39+
window.assertBothFulfill = async (t, result, expected, w = window) => {
40+
assertReturnValue(result, w);
41+
42+
// Don't use await here so that we can catch out-of-order settlements.
43+
let committedValue;
44+
result.committed.then(
45+
t.step_func(v => { committedValue = v; }),
46+
t.unreached_func("committed must not reject")
47+
);
48+
49+
const finishedValue = await result.finished;
50+
51+
assert_not_equals(committedValue, undefined, "committed must fulfill before finished");
52+
assert_equals(finishedValue, committedValue, "committed and finished must fulfill with the same value");
53+
assert_true(finishedValue instanceof w.NavigationHistoryEntry, "fulfillment value must be a NavigationHistoryEntry");
54+
assert_equals(finishedValue, expected);
55+
};
56+
57+
window.assertCommittedFulfillsFinishedRejectsExactly = async (t, result, expectedEntry, expectedRejection, w = window) => {
58+
assertReturnValue(result, w);
59+
60+
// Don't use await here so that we can catch out-of-order settlements.
61+
let committedValue;
62+
result.committed.then(
63+
t.step_func(v => { committedValue = v; }),
64+
t.unreached_func("committed must not reject")
65+
);
66+
67+
await promise_rejects_exactly(t, expectedRejection, result.finished);
68+
69+
assert_not_equals(committedValue, undefined, "committed must fulfill before finished rejects");
70+
assert_true(committedValue instanceof w.NavigationHistoryEntry, "fulfillment value must be a NavigationHistoryEntry");
71+
assert_equals(committedValue, expectedEntry);
72+
};
73+
74+
window.assertCommittedFulfillsFinishedRejectsDOM = async (t, result, expectedEntry, expectedDOMExceptionCode, w = window, domExceptionConstructor = w.DOMException, navigationHistoryEntryConstuctor = w.NavigationHistoryEntry) => {
75+
assertReturnValue(result, w);
76+
77+
let committedValue;
78+
result.committed.then(
79+
t.step_func(v => { committedValue = v; }),
80+
t.unreached_func("committed must not reject")
81+
);
82+
83+
await promise_rejects_dom(t, expectedDOMExceptionCode, domExceptionConstructor, result.finished);
84+
85+
assert_not_equals(committedValue, undefined, "committed must fulfill before finished rejects");
86+
assert_true(committedValue instanceof navigationHistoryEntryConstuctor, "fulfillment value must be an NavigationHistoryEntry");
87+
assert_equals(committedValue, expectedEntry);
88+
};
89+
90+
// We cannot use Promise.all() because the automatic coercion behavior when
91+
// promises from multiple realms are involved causes it to hang if one of the
92+
// promises is from a detached iframe's realm. See discussion at
93+
// https://github.com/whatwg/html/issues/11252#issuecomment-2984143855.
94+
window.waitForAllLenient = (iterable) => {
95+
const { promise: all, resolve, reject } = Promise.withResolvers();
96+
let remaining = 0;
97+
let results = [];
98+
for (const promise of iterable) {
99+
let index = remaining++;
100+
promise.then(v => {
101+
results[index] = v;
102+
--remaining;
103+
if (!remaining) {
104+
resolve(results);
105+
}
106+
return v;
107+
}, v => reject(v));
108+
}
109+
110+
if (!remaining) {
111+
resolve(results);
112+
}
113+
114+
return all;
115+
}
116+
117+
window.assertBothRejectExactly = async (t, result, expectedRejection, w = window) => {
118+
assertReturnValue(result, w);
119+
120+
let committedReason, finishedReason;
121+
await waitForAllLenient([
122+
result.committed.then(
123+
t.unreached_func("committed must not fulfill"),
124+
t.step_func(r => { committedReason = r; })
125+
),
126+
result.finished.then(
127+
t.unreached_func("finished must not fulfill"),
128+
t.step_func(r => { finishedReason = r; })
129+
)
130+
]);
131+
132+
assert_equals(committedReason, finishedReason, "committed and finished must reject with the same value");
133+
assert_equals(expectedRejection, committedReason);
134+
};
135+
136+
window.assertBothRejectDOM = async (t, result, expectedDOMExceptionCode, w = window, domExceptionConstructor = w.DOMException) => {
137+
assertReturnValue(result, w);
138+
139+
// Don't use await here so that we can catch out-of-order settlements.
140+
let committedReason, finishedReason;
141+
await waitForAllLenient([
142+
result.committed.then(
143+
t.unreached_func("committed must not fulfill"),
144+
t.step_func(r => { committedReason = r; })
145+
),
146+
result.finished.then(
147+
t.unreached_func("finished must not fulfill"),
148+
t.step_func(r => { finishedReason = r; })
149+
)
150+
]);
151+
152+
assert_equals(committedReason, finishedReason, "committed and finished must reject with the same value");
153+
assert_throws_dom(expectedDOMExceptionCode, domExceptionConstructor, () => { throw committedReason; });
154+
};

‎Source/WebCore/page/Navigation.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,8 +430,9 @@ Navigation::Result Navigation::navigate(const String& url, NavigateOptions&& opt
430430
if (!newURL.isValid())
431431
return createErrorResult(WTFMove(committed), WTFMove(finished), ExceptionCode::SyntaxError, "Invalid URL"_s);
432432

433-
if (options.history == HistoryBehavior::Push && newURL.protocolIsJavaScript())
434-
return createErrorResult(WTFMove(committed), WTFMove(finished), ExceptionCode::NotSupportedError, "A \"push\" navigation was explicitly requested, but only a \"replace\" navigation is possible when navigating to a javascript: URL."_s);
433+
// Reject all JavaScript URLs in Navigation API.
434+
if (newURL.protocolIsJavaScript())
435+
return createErrorResult(WTFMove(committed), WTFMove(finished), ExceptionCode::NotSupportedError, "Navigation API does not support javascript: URLs."_s);
435436

436437
if (options.history == HistoryBehavior::Push && currentURL.isAboutBlank())
437438
return createErrorResult(WTFMove(committed), WTFMove(finished), ExceptionCode::NotSupportedError, "A \"push\" navigation was explicitly requested, but only a \"replace\" navigation is possible while on an about:blank document."_s);

0 commit comments

Comments
 (0)