Commit 4696a7b2 authored by Aaron Colwell's avatar Aaron Colwell Committed by Commit Bot

Add CanCommitOriginAndUrl() exception for 'no access' URLs.

Temporary fix that creates an exception for 'no access' URLs like
chrome-native://history. These URLs can cause a origin mismatch in the
commit code because of slight differences between how url::Origin and
blink::SecurityOrigin handle 'no access' URLs. This is intended as a
low risk change that can be easily merged to the M79 branch.

Bug: 1016711, 923144
Change-Id: I8ab833744f45ebfbc489118313c59d7bbfdaff78
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1900358
Auto-Submit: Aaron Colwell <acolwell@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: default avatarAlex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713635}
parent e90fe319
...@@ -1306,6 +1306,19 @@ CanCommitStatus ChildProcessSecurityPolicyImpl::CanCommitOriginAndUrl( ...@@ -1306,6 +1306,19 @@ CanCommitStatus ChildProcessSecurityPolicyImpl::CanCommitOriginAndUrl(
return CanCommitStatus::CAN_COMMIT_ORIGIN_AND_URL; return CanCommitStatus::CAN_COMMIT_ORIGIN_AND_URL;
} }
// Allow "no access" schemes to commit even though |url_origin| and
// |origin| tuples don't match. We have to allow this because Blink's
// SecurityOrigin::CreateWithReferenceOrigin() and url::Origin::Resolve()
// handle "no access" URLs differently. CreateWithReferenceOrigin() treats
// "no access" like data: URLs and returns an opaque origin with |origin|
// as a precursor. Resolve() returns a non-opaque origin consisting of the
// scheme and host portions of the original URL.
//
// TODO(1020201): Make CreateWithReferenceOrigin() & Resolve() consistent
// with each other and then remove this exception.
if (base::Contains(url::GetNoAccessSchemes(), url.scheme()))
return CanCommitStatus::CAN_COMMIT_ORIGIN_AND_URL;
return CanCommitStatus::CANNOT_COMMIT_ORIGIN; return CanCommitStatus::CANNOT_COMMIT_ORIGIN;
} }
......
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "base/threading/thread_restrictions.h" #include "base/threading/thread_restrictions.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "components/network_session_configurator/common/network_switches.h" #include "components/network_session_configurator/common/network_switches.h"
#include "content/browser/browser_url_handler_impl.h"
#include "content/browser/child_process_security_policy_impl.h" #include "content/browser/child_process_security_policy_impl.h"
#include "content/browser/frame_host/navigation_request.h" #include "content/browser/frame_host/navigation_request.h"
#include "content/browser/web_contents/web_contents_impl.h" #include "content/browser/web_contents/web_contents_impl.h"
...@@ -55,6 +56,7 @@ ...@@ -55,6 +56,7 @@
#include "content/shell/browser/shell_download_manager_delegate.h" #include "content/shell/browser/shell_download_manager_delegate.h"
#include "content/test/content_browser_test_utils_internal.h" #include "content/test/content_browser_test_utils_internal.h"
#include "content/test/did_commit_navigation_interceptor.h" #include "content/test/did_commit_navigation_interceptor.h"
#include "content/test/fake_network_url_loader_factory.h"
#include "ipc/ipc_security_test_util.h" #include "ipc/ipc_security_test_util.h"
#include "mojo/public/cpp/bindings/pending_associated_remote.h" #include "mojo/public/cpp/bindings/pending_associated_remote.h"
#include "mojo/public/cpp/bindings/pending_receiver.h" #include "mojo/public/cpp/bindings/pending_receiver.h"
...@@ -3045,6 +3047,112 @@ IN_PROC_BROWSER_TEST_P(NavigationCookiesBrowserTest, CookiesInheritedDataUrl) { ...@@ -3045,6 +3047,112 @@ IN_PROC_BROWSER_TEST_P(NavigationCookiesBrowserTest, CookiesInheritedDataUrl) {
EXPECT_EQ(0u, response_2.http_request()->headers.count("Cookie")); EXPECT_EQ(0u, response_2.http_request()->headers.count("Cookie"));
} }
// Tests for validating URL rewriting behavior like chrome://history to
// chrome-native://history.
class NavigationUrlRewriteBrowserTest : public NavigationBaseBrowserTest {
protected:
static constexpr const char* kRewriteURL = "http://a.com/rewrite";
static constexpr const char* kNoAccessScheme = "no-access";
static constexpr const char* kNoAccessURL = "no-access://testing/";
class BrowserClient : public ContentBrowserClient {
public:
void BrowserURLHandlerCreated(BrowserURLHandler* handler) override {
handler->AddHandlerPair(RewriteUrl,
BrowserURLHandlerImpl::null_handler());
}
void RegisterNonNetworkNavigationURLLoaderFactories(
int frame_tree_node_id,
NonNetworkURLLoaderFactoryMap* factories) override {
auto url_loader_factory = std::make_unique<FakeNetworkURLLoaderFactory>(
"HTTP/1.1 200 OK\nContent-Type: text/html\n\n", "This is a test",
/* network_accessed */ true, net::OK);
factories->emplace(std::string(kNoAccessScheme),
std::move(url_loader_factory));
}
bool ShouldAssignSiteForURL(const GURL& url) override {
return !url.SchemeIs(kNoAccessScheme);
}
static bool RewriteUrl(GURL* url, BrowserContext* browser_context) {
if (*url == GURL(kRewriteURL)) {
*url = GURL(kNoAccessURL);
return true;
}
return false;
}
};
void SetUp() override {
url::AddStandardScheme(kNoAccessScheme, url::SCHEME_WITH_HOST);
url::AddNoAccessScheme(kNoAccessScheme);
NavigationBaseBrowserTest::SetUp();
}
void SetUpOnMainThread() override {
NavigationBaseBrowserTest::SetUpOnMainThread();
ASSERT_TRUE(embedded_test_server()->Start());
browser_client_ = std::make_unique<BrowserClient>();
old_browser_client_ = SetBrowserClientForTesting(browser_client_.get());
}
void TearDownOnMainThread() override {
SetBrowserClientForTesting(old_browser_client_);
old_browser_client_ = nullptr;
browser_client_.reset();
NavigationBaseBrowserTest::TearDownOnMainThread();
}
GURL GetRewriteToNoAccessURL() const { return GURL(kRewriteURL); }
private:
std::unique_ptr<BrowserClient> browser_client_;
ContentBrowserClient* old_browser_client_;
};
INSTANTIATE_TEST_SUITE_P(/* no prefix */,
NavigationUrlRewriteBrowserTest,
::testing::Bool());
// Tests navigating to a URL that gets rewritten to a "no access" URL. This
// mimics the behavior of navigating to special URLs like chrome://newtab and
// chrome://history which get rewritten to "no access" chrome-native:// URLs.
IN_PROC_BROWSER_TEST_P(NavigationUrlRewriteBrowserTest, RewriteToNoAccess) {
// Perform an initial navigation.
{
TestNavigationObserver observer(shell()->web_contents());
GURL url = embedded_test_server()->GetURL("a.com", "/title1.html");
EXPECT_TRUE(NavigateToURL(shell(), url));
EXPECT_EQ(url, observer.last_navigation_url());
EXPECT_TRUE(observer.last_navigation_succeeded());
EXPECT_FALSE(observer.last_initiator_origin().has_value());
}
// Navigate to the URL that will get rewritten to a "no access" URL.
{
auto* web_contents = shell()->web_contents();
TestNavigationObserver observer(web_contents);
// Note: We are using LoadURLParams here because we need to have the
// initiator_origin set and NavigateToURL() does not do that.
NavigationController::LoadURLParams params(GetRewriteToNoAccessURL());
params.initiator_origin =
web_contents->GetMainFrame()->GetLastCommittedOrigin();
web_contents->GetController().LoadURLWithParams(params);
web_contents->Focus();
observer.Wait();
EXPECT_EQ(GURL(kNoAccessURL), observer.last_navigation_url());
EXPECT_TRUE(observer.last_navigation_succeeded());
EXPECT_TRUE(observer.last_initiator_origin().has_value());
}
}
// Update the fragment part of the URL while it is currently displaying an error // Update the fragment part of the URL while it is currently displaying an error
// page. Regression test https://crbug.com/1018385 // page. Regression test https://crbug.com/1018385
IN_PROC_BROWSER_TEST_P(NavigationBrowserTest, IN_PROC_BROWSER_TEST_P(NavigationBrowserTest,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment