Do not allow redirecting to data: or about: URLs

https://bugs.webkit.org/show_bug.cgi?id=230158 <rdar://83244357> Reviewed by Brent Fulgham. Do not allow redirecting to data: or about: URLs, as per: - whatwg/html#7042 This aligns our behavior with Blink and gets us closer to Gecko. * LayoutTests/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/navigating-across-documents/redirect-to-about.window-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/navigating-across-documents/redirect-to-data-expected.txt: * Source/WebCore/loader/DocumentLoader.cpp: (WebCore::DocumentLoader::willSendRequest): Canonical link: https://commits.webkit.org/254619@main
WebKit · Sep 18, 2022 · cf4ebbe5d88a1c6b84cde33660c6a693e0cdfddd · cf4ebbe
1 parent 61b6842
commit cf4ebbe5d88a1c6b84cde33660c6a693e0cdfddd
Show file tree

Hide file tree

Showing 8 changed files with 25 additions and 107 deletions.
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
@@ -425,6 +425,8 @@ imported/w3c/web-platform-tests/FileAPI/url/sandboxed-iframe.html [ DumpJSConsol
 imported/w3c/web-platform-tests/eventsource/format-mime-bogus.any.html [ DumpJSConsoleLogInStdErr ]
 imported/w3c/web-platform-tests/html/anonymous-iframe/embedding.tentative.https.window.html [ DumpJSConsoleLogInStdErr ]
 imported/w3c/web-platform-tests/html/browsers/browsing-the-web/navigating-across-documents/failure-check-sequence.https.html [ DumpJSConsoleLogInStdErr ]
+imported/w3c/web-platform-tests/html/browsers/browsing-the-web/navigating-across-documents/redirect-to-data.html [ DumpJSConsoleLogInStdErr ]
+imported/w3c/web-platform-tests/html/browsers/browsing-the-web/navigating-across-documents/top-level-data-url.window.html [ DumpJSConsoleLogInStdErr ]
 imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken.html [ DumpJSConsoleLogInStdErr Failure Pass ]
 imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-weird.html [ DumpJSConsoleLogInStdErr ]
 imported/w3c/web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html [ DumpJSConsoleLogInStdErr ]

diff --git a/LayoutTests/http/tests/misc/redirect-to-about-blank-expected.txt b/LayoutTests/http/tests/misc/redirect-to-about-blank-expected.txt
diff --git a/LayoutTests/http/tests/misc/redirect-to-about-blank.html b/LayoutTests/http/tests/misc/redirect-to-about-blank.html
diff --git a/...owsers/browsing-the-web/navigating-across-documents/redirect-to-about.window-expected.txt b/...owsers/browsing-the-web/navigating-across-documents/redirect-to-about.window-expected.txt
@@ -1,16 +1,14 @@
+CONSOLE MESSAGE: Not allowed to redirect to about:blank due to its scheme
+CONSOLE MESSAGE: Not allowed to redirect to about:blank due to its scheme
+CONSOLE MESSAGE: Not allowed to redirect to about:srcdoc due to its scheme
+CONSOLE MESSAGE: Not allowed to redirect to about:srcdoc due to its scheme
+CONSOLE MESSAGE: Not allowed to redirect to about:nonstandard due to its scheme
+CONSOLE MESSAGE: Not allowed to redirect to about:nonstandard due to its scheme
 
-FAIL An iframe with src set to a redirect to about:blank assert_throws_dom: function "() => {
-      iframe.contentWindow.document;
-    }" did not throw
-FAIL An iframe that is navigated to a redirect to about:blank assert_throws_dom: function "() => {
-      iframe.contentWindow.document;
-    }" did not throw
-FAIL An iframe with src set to a redirect to about:srcdoc assert_throws_dom: function "() => {
-      iframe.contentWindow.document;
-    }" did not throw
-FAIL An iframe that is navigated to a redirect to about:srcdoc assert_throws_dom: function "() => {
-      iframe.contentWindow.document;
-    }" did not throw
+PASS An iframe with src set to a redirect to about:blank
+PASS An iframe that is navigated to a redirect to about:blank
+PASS An iframe with src set to a redirect to about:srcdoc
+PASS An iframe that is navigated to a redirect to about:srcdoc
 PASS An iframe with src set to a redirect to about:nonstandard
 PASS An iframe that is navigated to a redirect to about:nonstandard
 
diff --git a/.../html/browsers/browsing-the-web/navigating-across-documents/redirect-to-data-expected.txt b/.../html/browsers/browsing-the-web/navigating-across-documents/redirect-to-data-expected.txt
@@ -1,7 +1,7 @@
 
 
-FAIL Loading an iframe with src=redirecting URL assert_unreached: must not be messaged Reached unreachable code
-FAIL Navigating an iframe to a redirecting URL assert_unreached: must not be messaged Reached unreachable code
+PASS Loading an iframe with src=redirecting URL
+PASS Navigating an iframe to a redirecting URL
 PASS Loading a popup directly to the redirecting URL
 PASS Loading a popup that eventually goes to the redirecting URL
 
diff --git a/Source/WebCore/loader/DocumentLoader.cpp b/Source/WebCore/loader/DocumentLoader.cpp
@@ -657,6 +657,17 @@ void DocumentLoader::willSendRequest(ResourceRequest&& newRequest, const Resourc
 
     ASSERT(timing().startTime());
     if (didReceiveRedirectResponse) {
+        if (newRequest.url().protocolIsAbout() || newRequest.url().protocolIsData()) {
+            DOCUMENTLOADER_RELEASE_LOG("willSendRequest: canceling - redirecting URL scheme is not allowed");
+            if (m_frame && m_frame->document()) {
+                m_frame->document()->enforceSandboxFlags(SandboxOrigin);
+                m_frame->document()->addConsoleMessage(MessageSource::Security, MessageLevel::Error, "Not allowed to redirect to " + newRequest.url().stringCenterEllipsizedToLength() + " due to its scheme");
+            }
+            if (auto* ownerElement = m_frame ? m_frame->ownerElement() : nullptr)
+                ownerElement->dispatchEvent(Event::create(eventNames().loadEvent, Event::CanBubble::No, Event::IsCancelable::No));
+            cancelMainResourceLoad(frameLoader()->blockedError(newRequest));
+            return completionHandler(WTFMove(newRequest));
+        }
         // If the redirecting url is not allowed to display content from the target origin,
         // then block the redirect.
         Ref<SecurityOrigin> redirectingOrigin(SecurityOrigin::create(redirectResponse.url()));

diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
@@ -1755,55 +1755,6 @@ static void runSameSiteWindowOpenNoOpenerTest(WindowHasName windowHasName, Expec
     EXPECT_WK_STREQ(@"pson://www.webkit.org/main1.html", [[webView URL] absoluteString]);
 }
 
-TEST(ProcessSwap, ServerRedirectToAboutBlank)
-{
-    auto processPoolConfiguration = psonProcessPoolConfiguration();
-    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
-
-    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
-    [webViewConfiguration setProcessPool:processPool.get()];
-    auto handler = adoptNS([[PSONScheme alloc] init]);
-    [handler addRedirectFromURLString:@"pson://www.webkit.org/main.html" toURLString:@"about:blank"];
-    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"pson"];
-
-    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
-    auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
-    [webView setNavigationDelegate:delegate.get()];
-
-    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.google.com/main.html"]];
-    [webView loadRequest:request];
-
-    TestWebKitAPI::Util::run(&done);
-    done = false;
-
-    auto pidAfterFirstLoad = [webView _webProcessIdentifier];
-
-    EXPECT_EQ(1, numberOfDecidePolicyCalls);
-    EXPECT_EQ(1u, seenPIDs.size());
-    EXPECT_TRUE(*seenPIDs.begin() == pidAfterFirstLoad);
-
-    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
-    [webView loadRequest:request];
-
-    TestWebKitAPI::Util::run(&serverRedirected);
-    serverRedirected = false;
-
-    seenPIDs.add([webView _webProcessIdentifier]);
-    if (auto provisionalPID = [webView _provisionalWebProcessIdentifier])
-        seenPIDs.add(provisionalPID);
-
-    TestWebKitAPI::Util::run(&done);
-    done = false;
-
-    seenPIDs.add([webView _webProcessIdentifier]);
-    if (auto provisionalPID = [webView _provisionalWebProcessIdentifier])
-        seenPIDs.add(provisionalPID);
-
-    EXPECT_FALSE(serverRedirected);
-    EXPECT_EQ(3, numberOfDecidePolicyCalls);
-    EXPECT_EQ(2u, seenPIDs.size());
-}
-
 enum class ShouldCacheProcessFirst { No, Yes };
 static void runSameOriginServerRedirectTest(ShouldCacheProcessFirst shouldCacheProcessFirst)
 {

diff --git a/Tools/TestWebKitAPI/Tests/WebKitGLib/TestLoaderClient.cpp b/Tools/TestWebKitAPI/Tests/WebKitGLib/TestLoaderClient.cpp
@@ -682,18 +682,6 @@ static void testURIResponseHTTPHeaders(WebViewTest* test, gconstpointer)
     g_assert_cmpstr(soup_message_headers_get_one(headers, "Foo"), ==, "bar");
 }
 
-static void testRedirectToDataURI(WebViewTest* test, gconstpointer)
-{
-    test->loadURI(kServer->getURIForPath("/redirect-to-data").data());
-    test->waitUntilLoadFinished();
-
-    static const char* expectedData = "data-uri";
-    size_t mainResourceDataSize = 0;
-    const char* mainResourceData = test->mainResourceData(mainResourceDataSize);
-    g_assert_cmpint(mainResourceDataSize, ==, strlen(expectedData));
-    g_assert_cmpint(strncmp(mainResourceData, expectedData, mainResourceDataSize), ==, 0);
-}
-
 static HashMap<CString, CString> s_userAgentMap;
 
 static void testUserAgent(WebViewTest* test, gconstpointer)
@@ -866,7 +854,6 @@ void beforeAll()
     WebViewTest::add("WebKitURIRequest", "http-headers", testURIRequestHTTPHeaders);
     WebViewTest::add("WebKitURIRequest", "http-method", testURIRequestHTTPMethod);
     WebViewTest::add("WebKitURIResponse", "http-headers", testURIResponseHTTPHeaders);
-    WebViewTest::add("WebKitWebPage", "redirect-to-data-uri", testRedirectToDataURI);
     WebViewTest::add("WebKitWebView", "user-agent", testUserAgent);
 }