Skip to content

GitLab

T
tangled
Commit 42fe0860 authored by Fergal Daly's avatar Fergal Daly Committed by Commit Bot
Browse files
Options

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

This reverts commit 29392d00.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> 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
> Change-Id: I20734fe68b6919c06849a1ce1439624d6332898c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1896863
> Commit-Queue: Aaron Colwell <acolwell@chromium.org>
> Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#712415}

TBR=acolwell@chromium.org,alexmos@chromium.org

Change-Id: I6ac033436b2719090e9fd198c90c410f509c845c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1016711,1021779
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1900513Reviewed-by: default avatarFergal Daly <fergal@chromium.org>
Commit-Queue: Fergal Daly <fergal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712902}
parent 114fd674
Hide whitespace changes
Inline Side-by-side
Showing with 0 additions and 118 deletions
content/browser/child_process_security_policy_impl.cc
View file @ 42fe0860
......@@ -1306,19 +1306,6 @@ CanCommitStatus ChildProcessSecurityPolicyImpl::CanCommitOriginAndUrl(
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;
}
......
content/browser/navigation_browsertest.cc
View file @ 42fe0860
......@@ -17,7 +17,6 @@
#include "base/threading/thread_restrictions.h"
#include "build/build_config.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/frame_host/navigation_request.h"
#include "content/browser/web_contents/web_contents_impl.h"
......@@ -56,7 +55,6 @@
#include "content/shell/browser/shell_download_manager_delegate.h"
#include "content/test/content_browser_test_utils_internal.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 "mojo/public/cpp/bindings/pending_associated_remote.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
......@@ -3071,109 +3069,6 @@ IN_PROC_BROWSER_TEST_P(NavigationCookiesBrowserTest, CookiesInheritedDataUrl) {
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://foo.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 {
NavigationBaseBrowserTest::TearDownOnMainThread();
SetBrowserClientForTesting(old_browser_client_);
}
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("foo.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
// page. Regression test https://crbug.com/1018385
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