diff --git a/content/browser/renderer_host/navigation_request.h b/content/browser/renderer_host/navigation_request.h index 05a80065f7db1..656e35cf00bde 100644 --- a/content/browser/renderer_host/navigation_request.h +++ b/content/browser/renderer_host/navigation_request.h @@ -1848,6 +1848,8 @@ class CONTENT_EXPORT NavigationRequest private: friend class NavigationRequestTest; FRIEND_TEST_ALL_PREFIXES(NavigationRequestTest, SanitizeRedirectsForCommit); + FRIEND_TEST_ALL_PREFIXES(NavigationRequestTest, + SanitizeRedirectsForCommitResponseHeaders); FRIEND_TEST_ALL_PREFIXES(NavigationRequestTest, SanitizeRedirectsForCommitRelativeLocation); FRIEND_TEST_ALL_PREFIXES(NavigationRequestTest, diff --git a/content/browser/renderer_host/navigation_request_unittest.cc b/content/browser/renderer_host/navigation_request_unittest.cc index 27d3ffd6dd466..9a7b35f8dff23 100644 --- a/content/browser/renderer_host/navigation_request_unittest.cc +++ b/content/browser/renderer_host/navigation_request_unittest.cc @@ -243,7 +243,11 @@ class NavigationRequestTest : public RenderViewHostImplTestHarness { common_params->url = url; common_params->method = "GET"; auto commit_params = blink::CreateCommitNavigationParams(); - commit_params->original_url = url; + commit_params->original_url = + base::FeatureList::IsEnabled( + features::kSanitizeOriginalUrlDuringNavigation) + ? url.DeprecatedGetOriginAsURL() + : url; commit_params->frame_policy = child_node->pending_frame_policy(); return NavigationRequest::CreateBrowserInitiated( child_node, std::move(common_params), std::move(commit_params), @@ -912,6 +916,61 @@ TEST_F(NavigationRequestTest, SanitizeRedirectsForCommit) { EXPECT_EQ(GURL("https://c.com"), commit_params->redirects[2]); } +// Test to ensure that, by default, the Location headers in redirect_response +// and the original_url are sanitized in addition to redirects/redirect_infos. +TEST_F(NavigationRequestTest, SanitizeRedirectsForCommitResponseHeaders) { + const GURL start_url("https://a.com/start?param=1"); + const GURL url_2("https://b.com/path?param=2"); + const GURL final_url("https://c.com/final?param=3"); + + std::unique_ptr navigation = + NavigationSimulator::CreateRendererInitiated(start_url, main_test_rfh()); + navigation->Start(); + + // Redirect to b.com (cross-origin to final URL). + auto headers1 = + base::MakeRefCounted("HTTP/1.1 302 Found"); + headers1->SetHeader("Location", url_2.spec()); + navigation->SetRedirectHeaders(headers1); + navigation->Redirect(url_2); + + // Redirect to c.com (same-origin to final URL). + auto headers2 = + base::MakeRefCounted("HTTP/1.1 302 Found"); + headers2->SetHeader("Location", final_url.spec()); + navigation->SetRedirectHeaders(headers2); + navigation->Redirect(final_url); + + NavigationRequest* request = + NavigationRequest::From(navigation->GetNavigationHandle()); + auto commit_params = request->commit_params().Clone(); + request->SanitizeRedirectsForCommit(commit_params); + + ASSERT_EQ(2u, commit_params->redirect_response.size()); + + // The Location header pointing to b.com (cross-origin to the final URL) + // should be reduced to the origin. + size_t iter = 0; + std::optional location = + commit_params->redirect_response[0]->headers->EnumerateHeader(&iter, + "Location"); + ASSERT_TRUE(location.has_value()); + EXPECT_EQ("https://b.com/", location.value()); + + // The Location header pointing to the final URL is same-origin and should be + // left as-is. + iter = 0; + location = commit_params->redirect_response[1]->headers->EnumerateHeader( + &iter, "Location"); + ASSERT_TRUE(location.has_value()); + EXPECT_EQ(final_url.spec(), location.value()); + + // The original URL sent to the renderer should be reduced to the origin, + // while the browser-side copy keeps the full URL. + EXPECT_EQ(GURL("https://a.com/"), commit_params->original_url); + EXPECT_EQ(start_url, request->original_url()); +} + // Test to ensure that relative Location headers are handled correctly during // sanitization (not cleared if same-origin, and sanitized to origin if // cross-origin). diff --git a/content/common/features.cc b/content/common/features.cc index bb75e229a833b..cca6b372cb0b8 100644 --- a/content/common/features.cc +++ b/content/common/features.cc @@ -681,13 +681,13 @@ BASE_FEATURE(kScrollAfterOSKViewportShrinkFix, // to only include the origin when cross-origin to the final URL. // See https://crbug.com/495463654. BASE_FEATURE(kSanitizeLocationHeadersDuringNavigation, - base::FEATURE_DISABLED_BY_DEFAULT); + base::FEATURE_ENABLED_BY_DEFAULT); // Controls whether the `original_url` contains the full URL or just the // sanitized origin when sent to the renderer on commit. // See https://crbug.com/495463654. BASE_FEATURE(kSanitizeOriginalUrlDuringNavigation, - base::FEATURE_DISABLED_BY_DEFAULT); + base::FEATURE_ENABLED_BY_DEFAULT); BASE_FEATURE(kServiceWorkerAvoidMainThreadForInitialization, base::FEATURE_ENABLED_BY_DEFAULT);