- Revision
- 230919
- Author
- [email protected]
- Date
- 2018-04-23 13:49:57 -0700 (Mon, 23 Apr 2018)
Log Message
HTML String load cannot be prevented by responding 'Cancel' asynchronously in decidePolicyForNavigationAction
https://bugs.webkit.org/show_bug.cgi?id=184848
<rdar://problem/39145306>
Reviewed by Brady Eidson.
Source/WebCore:
When calling loadHTMLString on a WebView, we end up doing a load for 'about:blank'
with substitute data. In such case, we want to do a regular asynchronous policy
delegate check, there is no reason we need it to be synchronous. Update our check
to make sure we only do a synchronous policy check for initial 'about:blank' loads
that do not have substitute data.
* loader/PolicyChecker.cpp:
(WebCore::PolicyChecker::checkNavigationPolicy):
Tools:
Add API test coverage.
* TestWebKitAPI/Tests/WebKitCocoa/DecidePolicyForNavigationAction.mm:
(-[DecidePolicyForNavigationActionController webView:decidePolicyForNavigationAction:decisionHandler:]):
(TEST):
LayoutTests:
Update layout tests that wrongly expected 'about:blank' to load synchronously even
when it is not the initial empty document of an iframe. I have checked that our
behavior is now consistent with Chrome.
* fast/events/beforeunload-alert-user-interaction2.html:
* http/tests/security/cross-origin-reified-window-location-setting-expected.txt:
* http/tests/security/cross-origin-reified-window-location-setting.html:
* webarchive/loading/_javascript_-url-iframe-crash-expected.txt:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (230918 => 230919)
--- trunk/LayoutTests/ChangeLog 2018-04-23 20:29:36 UTC (rev 230918)
+++ trunk/LayoutTests/ChangeLog 2018-04-23 20:49:57 UTC (rev 230919)
@@ -1,3 +1,20 @@
+2018-04-23 Chris Dumez <[email protected]>
+
+ HTML String load cannot be prevented by responding 'Cancel' asynchronously in decidePolicyForNavigationAction
+ https://bugs.webkit.org/show_bug.cgi?id=184848
+ <rdar://problem/39145306>
+
+ Reviewed by Brady Eidson.
+
+ Update layout tests that wrongly expected 'about:blank' to load synchronously even
+ when it is not the initial empty document of an iframe. I have checked that our
+ behavior is now consistent with Chrome.
+
+ * fast/events/beforeunload-alert-user-interaction2.html:
+ * http/tests/security/cross-origin-reified-window-location-setting-expected.txt:
+ * http/tests/security/cross-origin-reified-window-location-setting.html:
+ * webarchive/loading/_javascript_-url-iframe-crash-expected.txt:
+
2018-04-23 Wenson Hsieh <[email protected]>
[Extra zoom mode] 100vw is roughly half of the viewport width in extra zoom mode
Modified: trunk/LayoutTests/fast/events/beforeunload-alert-user-interaction2.html (230918 => 230919)
--- trunk/LayoutTests/fast/events/beforeunload-alert-user-interaction2.html 2018-04-23 20:29:36 UTC (rev 230918)
+++ trunk/LayoutTests/fast/events/beforeunload-alert-user-interaction2.html 2018-04-23 20:49:57 UTC (rev 230919)
@@ -16,8 +16,8 @@
const testInput = document.getElementById("testInput");
UIHelper.activateAt(testInput.offsetLeft + 5, testInput.offsetTop + 5).then(function() {
setTimeout(function() {
+ testFrame._onload_ = finishJSTest;
testFrame.src = ""
- setTimeout(finishJSTest, 0);
}, 0);
});
};
Modified: trunk/LayoutTests/http/tests/security/cross-origin-reified-window-location-setting-expected.txt (230918 => 230919)
--- trunk/LayoutTests/http/tests/security/cross-origin-reified-window-location-setting-expected.txt 2018-04-23 20:29:36 UTC (rev 230918)
+++ trunk/LayoutTests/http/tests/security/cross-origin-reified-window-location-setting-expected.txt 2018-04-23 20:49:57 UTC (rev 230919)
@@ -3,7 +3,9 @@
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+PASS crossOriginWindow.location.href threw exception SecurityError: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a cross-origin frame. Protocols, domains, and ports must match..
PASS crossOriginWindow.location = 'about:blank' did not throw exception.
+PASS crossOriginWindow.location.href threw exception SecurityError: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a cross-origin frame. Protocols, domains, and ports must match..
PASS crossOriginWindow.location.href is "about:blank"
PASS successfullyParsed is true
Modified: trunk/LayoutTests/http/tests/security/cross-origin-reified-window-location-setting.html (230918 => 230919)
--- trunk/LayoutTests/http/tests/security/cross-origin-reified-window-location-setting.html 2018-04-23 20:29:36 UTC (rev 230918)
+++ trunk/LayoutTests/http/tests/security/cross-origin-reified-window-location-setting.html 2018-04-23 20:49:57 UTC (rev 230919)
@@ -1,7 +1,7 @@
<!DOCTYPE html>
<html>
<head>
-<script src=""
+<script src=""
</head>
<body _onload_="runTest()">
<iframe id="crossOriginFrame" src=""
@@ -11,15 +11,19 @@
function runTest()
{
- crossOriginWindow = crossOriginFrame.window;
+ crossOriginWindow = document.getElementById("crossOriginFrame").contentWindow;
+ shouldThrow("crossOriginWindow.location.href");
shouldNotThrow("crossOriginWindow.location = 'about:blank'");
-
- setTimeout(function() {
- shouldBeEqualToString("crossOriginWindow.location.href", "about:blank");
- finishJSTest();
- }, 0);
+ shouldThrow("crossOriginWindow.location.href");
+ handle = setInterval(function() {
+ try {
+ crossOriginWindow.location.href;
+ shouldBeEqualToString("crossOriginWindow.location.href", "about:blank");
+ clearInterval(handle);
+ finishJSTest();
+ } catch(e) { }
+ }, 5);
}
</script>
</body>
-<script src=""
</html>
Added: trunk/LayoutTests/platform/wk2/webarchive/loading/_javascript_-url-iframe-crash-expected.txt (0 => 230919)
--- trunk/LayoutTests/platform/wk2/webarchive/loading/_javascript_-url-iframe-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/wk2/webarchive/loading/_javascript_-url-iframe-crash-expected.txt 2018-04-23 20:49:57 UTC (rev 230919)
@@ -0,0 +1,15 @@
+main frame - didStartProvisionalLoadForFrame
+main frame - didCommitLoadForFrame
+main frame - willPerformClientRedirectToURL: resources/_javascript_-url-iframe-crash.webarchive
+main frame - didFinishDocumentLoadForFrame
+main frame - didFinishLoadForFrame
+main frame - didStartProvisionalLoadForFrame
+main frame - didCancelClientRedirectForFrame
+main frame - didCommitLoadForFrame
+frame "<!--framePath //<!--frame0-->-->" - didFinishDocumentLoadForFrame
+frame "<!--framePath //<!--frame0-->-->" - didHandleOnloadEventsForFrame
+main frame - didFinishDocumentLoadForFrame
+main frame - didHandleOnloadEventsForFrame
+main frame - didFinishLoadForFrame
+Loading this webarchive with a "non-empty _javascript_ URL iframe" should not crash.
+
Modified: trunk/Source/WebCore/ChangeLog (230918 => 230919)
--- trunk/Source/WebCore/ChangeLog 2018-04-23 20:29:36 UTC (rev 230918)
+++ trunk/Source/WebCore/ChangeLog 2018-04-23 20:49:57 UTC (rev 230919)
@@ -1,3 +1,20 @@
+2018-04-23 Chris Dumez <[email protected]>
+
+ HTML String load cannot be prevented by responding 'Cancel' asynchronously in decidePolicyForNavigationAction
+ https://bugs.webkit.org/show_bug.cgi?id=184848
+ <rdar://problem/39145306>
+
+ Reviewed by Brady Eidson.
+
+ When calling loadHTMLString on a WebView, we end up doing a load for 'about:blank'
+ with substitute data. In such case, we want to do a regular asynchronous policy
+ delegate check, there is no reason we need it to be synchronous. Update our check
+ to make sure we only do a synchronous policy check for initial 'about:blank' loads
+ that do not have substitute data.
+
+ * loader/PolicyChecker.cpp:
+ (WebCore::PolicyChecker::checkNavigationPolicy):
+
2018-04-23 Wenson Hsieh <[email protected]>
[Extra zoom mode] 100vw is roughly half of the viewport width in extra zoom mode
Modified: trunk/Source/WebCore/loader/PolicyChecker.cpp (230918 => 230919)
--- trunk/Source/WebCore/loader/PolicyChecker.cpp 2018-04-23 20:29:36 UTC (rev 230918)
+++ trunk/Source/WebCore/loader/PolicyChecker.cpp 2018-04-23 20:49:57 UTC (rev 230919)
@@ -123,7 +123,8 @@
loader->setLastCheckedRequest(ResourceRequest(request));
- if (request.url().isBlankURL())
+ // Initial 'about:blank' load needs to happen synchronously so the policy check needs to be synchronous in this case.
+ if (!m_frame.loader().stateMachine().committedFirstRealDocumentLoad() && request.url().isBlankURL() && !substituteData.isValid())
policyDecisionMode = PolicyDecisionMode::Synchronous;
#if USE(QUICK_LOOK)
Modified: trunk/Tools/ChangeLog (230918 => 230919)
--- trunk/Tools/ChangeLog 2018-04-23 20:29:36 UTC (rev 230918)
+++ trunk/Tools/ChangeLog 2018-04-23 20:49:57 UTC (rev 230919)
@@ -1,3 +1,17 @@
+2018-04-23 Chris Dumez <[email protected]>
+
+ HTML String load cannot be prevented by responding 'Cancel' asynchronously in decidePolicyForNavigationAction
+ https://bugs.webkit.org/show_bug.cgi?id=184848
+ <rdar://problem/39145306>
+
+ Reviewed by Brady Eidson.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/DecidePolicyForNavigationAction.mm:
+ (-[DecidePolicyForNavigationActionController webView:decidePolicyForNavigationAction:decisionHandler:]):
+ (TEST):
+
2018-04-23 Michael Catanzaro <[email protected]>
[GTK][WPE] TestSSL fails due to additional TLS errors returned
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/DecidePolicyForNavigationAction.mm (230918 => 230919)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/DecidePolicyForNavigationAction.mm 2018-04-23 20:29:36 UTC (rev 230918)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/DecidePolicyForNavigationAction.mm 2018-04-23 20:49:57 UTC (rev 230919)
@@ -35,6 +35,7 @@
#if WK_API_ENABLED
+static bool shouldCancelNavigation;
static bool createdWebView;
static bool decidedPolicy;
static bool finishedNavigation;
@@ -51,6 +52,16 @@
- (void)webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction decisionHandler:(void (^)(WKNavigationActionPolicy))decisionHandler
{
+ if (shouldCancelNavigation) {
+ int64_t deferredWaitTime = 100 * NSEC_PER_MSEC;
+ dispatch_time_t when = dispatch_time(DISPATCH_TIME_NOW, deferredWaitTime);
+ dispatch_after(when, dispatch_get_main_queue(), ^{
+ decisionHandler(WKNavigationActionPolicyCancel);
+ decidedPolicy = true;
+ });
+ return;
+ }
+
decisionHandler(webView == newWebView.get() ? WKNavigationActionPolicyCancel : WKNavigationActionPolicyAllow);
action = ""
@@ -285,7 +296,7 @@
action = ""
}
-TEST(WebKit, DecidePolicyForNavigationActionForLoadHTMLString)
+TEST(WebKit, DecidePolicyForNavigationActionForLoadHTMLStringAllow)
{
auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
@@ -296,11 +307,33 @@
[webView setNavigationDelegate:controller.get()];
[webView setUIDelegate:controller.get()];
+ finishedNavigation = false;
decidedPolicy = false;
[webView loadHTMLString:@"TEST" baseURL:[NSURL URLWithString:@"about:blank"]];
- TestWebKitAPI::Util::run(&decidedPolicy);
+ TestWebKitAPI::Util::run(&finishedNavigation);
+ EXPECT_TRUE(decidedPolicy);
}
+TEST(WebKit, DecidePolicyForNavigationActionForLoadHTMLStringDeny)
+{
+ auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+
+ auto window = adoptNS([[NSWindow alloc] initWithContentRect:[webView frame] styleMask:NSWindowStyleMaskBorderless backing:NSBackingStoreBuffered defer:YES]);
+ [[window contentView] addSubview:webView.get()];
+
+ auto controller = adoptNS([[DecidePolicyForNavigationActionController alloc] init]);
+ [webView setNavigationDelegate:controller.get()];
+ [webView setUIDelegate:controller.get()];
+
+ shouldCancelNavigation = true;
+ finishedNavigation = false;
+ decidedPolicy = false;
+ [webView loadHTMLString:@"TEST" baseURL:[NSURL URLWithString:@"about:blank"]];
+ TestWebKitAPI::Util::sleep(0.5);
+ EXPECT_FALSE(finishedNavigation);
+ shouldCancelNavigation = false;
+}
+
TEST(WebKit, DecidePolicyForNavigationActionForTargetedWindowOpen)
{
auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
