Commit 217fd27e authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

Correctly handle blob:file:///... URIs in SiteInstance::GetSiteForURL.

file URIs should map to "file:///" site.  The same site needs to also
be used for blob:file:///... URIs - this is what is fixed by this CL.

Bug: 697111
Change-Id: Iff9e9a1552eb747ecd576aefdc5085009588690b
Reviewed-on: https://chromium-review.googlesource.com/953129Reviewed-by: default avatarNick Carter <nick@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541589}
parent 69412614
......@@ -412,10 +412,8 @@ GURL SiteInstance::GetSiteForURL(BrowserContext* browser_context,
// origin lookup.
auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
url::Origin isolated_origin;
if (policy->GetMatchingIsolatedOrigin(url::Origin::Create(url),
&isolated_origin)) {
if (policy->GetMatchingIsolatedOrigin(origin, &isolated_origin))
return isolated_origin.GetURL();
}
// If the url has a host, then determine the site. Skip file URLs to avoid a
// situation where site URL of file://localhost/ would mismatch Blink's origin
......@@ -433,11 +431,16 @@ GURL SiteInstance::GetSiteForURL(BrowserContext* browser_context,
// If there is no host but there is a scheme, return the scheme.
// This is useful for cases like file URLs.
if (url.has_scheme())
if (!origin.unique()) {
// Prefer to use the scheme of |origin| rather than |url|, to correctly
// cover blob: and filesystem: URIs (see also https://crbug.com/697111).
DCHECK(origin.GetURL().has_scheme()) << origin;
return GURL(origin.GetURL().scheme() + ":");
} else if (url.has_scheme())
return GURL(url.scheme() + ":");
// Otherwise the URL should be invalid; return an empty site.
DCHECK(!url.is_valid());
DCHECK(!url.is_valid()) << url;
return GURL();
}
......
......@@ -361,6 +361,13 @@ TEST_F(SiteInstanceTest, GetSiteForURL) {
site_url = SiteInstanceImpl::GetSiteForURL(nullptr, test_url);
EXPECT_EQ(GURL("gopher://chromium.org"), site_url);
// Blob URLs with file origin also extract the site from the origin.
test_url = GURL("blob:file:///1029e5a4-2983-4b90-a585-ed217563acfeb");
site_url = SiteInstanceImpl::GetSiteForURL(nullptr, test_url);
EXPECT_EQ(GURL("file:"), site_url);
EXPECT_EQ("file", site_url.scheme());
EXPECT_FALSE(site_url.has_host());
// Private domains are preserved, appspot being such a site.
test_url = GURL(
"blob:http://www.example.appspot.com:44/"
......
......@@ -27,9 +27,6 @@ crbug.com/765779 http/tests/loading/bad-server-subframe.html [ Failure ]
crbug.com/393285 http/tests/text-autosizing/narrow-iframe.html [ Failure Crash ]
crbug.com/393285 http/tests/text-autosizing/wide-iframe.html [ Failure Crash ]
# https://crbug.com/697111: assert failures in test added in r451938.
crbug.com/697111 svg/dom/getScreenCTM-ancestor-transform.html [ Failure ]
# https://crbug.com/669083: console messages mismatch (origin-only VS full-URI)
crbug.com/669083 http/tests/security/frameNavigation/xss-DENIED-top-navigation-without-user-gesture.html [ Failure ]
......@@ -229,9 +226,7 @@ Bug(none) external/wpt/webmessaging/without-ports/020.html [ Failure ]
Bug(none) external/wpt/webusb/usb-disabled-by-feature-policy.https.sub.html [ Timeout ]
Bug(none) external/wpt/webvr/webvr-enabled-by-feature-policy-attribute-redirect-on-load.https.sub.html [ Timeout ]
Bug(none) jquery/manipulation.html [ Timeout ]
Bug(none) storage/indexeddb/blob-valid-after-deletion.html [ Failure ]
Bug(none) storage/indexeddb/blob-valid-before-commit.html [ Failure ]
Bug(none) storage/indexeddb/empty-crash.html [ Timeout ]
Bug(none) virtual/outofblink-cors/external/wpt/fetch/api/basic/keepalive.html [ Timeout ]
crbug.com/801992 external/wpt/content-security-policy/frame-ancestors/frame-ancestors-nested-cross-in-cross-url-allow.html [ Pass Timeout ]
......
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