Skip to content

Commit

Permalink
Use inner origin of blob: URLs to determine their trustworthiness.
Browse files Browse the repository at this point in the history
This CL is removing special-casing of blob URLs from
content::IsOriginSecure.  After the CL, the inner origin of blob URLs
will be extracted by network::IsUrlPotentiallyTrustworthy and passed to
network::IsOriginPotentiallyTrustworthy (both of these network-layer
functions follow https://www.w3.org/TR/powerful-features).

The change above is desirable (blob URLs which were previously
incorrectly always considered untrustworthy will be correctly judged
based on their inner origin).  OTOH, the change caused a test failure in
PaymentRequestBlobUrlTest.ConnectionTerminated.  As suggested by
rouslan@ the CL just tweaks the test expectations of this test.

I note that before this CL,
PaymentRequestBlobUrlTest.ConnectionTerminated was exercising a scenario
where the URL is insecure (and after this CL some blob URLs are
considered secure based on their inner origin).  Similar scenario is
already tested by PaymentRequestDataUrlTest.SecurityError (pointed out
by rouslan@ - thanks!) and therefore no test coverage is lost by
tweaking the expectations of the PaymentRequestBlobUrlTest.

Bug: 937451
Change-Id: I9cec22a13fe821eb39ea789e1128bc592267d986
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1506941
Reviewed-by: Rouslan Solomakhin <[email protected]>
Reviewed-by: Alex Moshchuk <[email protected]>
Auto-Submit: Łukasz Anforowicz <[email protected]>
Commit-Queue: Łukasz Anforowicz <[email protected]>
Cr-Commit-Position: refs/heads/master@{#643579}
  • Loading branch information
anforowicz authored and Commit Bot committed Mar 22, 2019
1 parent d672d83 commit 16586e1
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ class PaymentRequestBlobUrlTest : public PaymentRequestBrowserTestBase {

IN_PROC_BROWSER_TEST_F(PaymentRequestBlobUrlTest, ConnectionTerminated) {
NavigateTo("/payment_request_blob_url_test.html");
ResetEventWaiter(DialogEvent::DIALOG_CLOSED);
ResetEventWaiter(DialogEvent::NOT_SUPPORTED_ERROR);
ASSERT_TRUE(content::ExecuteScript(
GetActiveWebContents(),
"(function() { document.getElementById('buy').click(); })();"));
WaitForObservedEvent();
ExpectBodyContains({"Rejected: UnknownError: Request failed"});
ExpectBodyContains({"Rejected: NotSupportedError"});
}

} // namespace payments
7 changes: 0 additions & 7 deletions content/common/origin_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,6 @@ bool IsOriginSecure(const GURL& url) {
if (url.SchemeIs(url::kDataScheme))
return true;

// TODO(lukasza): trustworthiness of blob: URLs should depend on their inner
// origin (just as it does for filesystem: URLs). Changing this behavior of
// content::IsOriginSecure breaks some tests, so fixing this is postponed to a
// follow-up CL. WIP CL @ https://crrev.com/c/1506941.
if (url.SchemeIs(url::kBlobScheme))
return false;

return network::IsUrlPotentiallyTrustworthy(url);
}

Expand Down
9 changes: 5 additions & 4 deletions content/common/origin_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,12 @@ TEST(OriginUtilTest, IsOriginSecure) {
// that https + data = mixed content).
EXPECT_TRUE(IsOriginSecure(GURL("data:test/plain;blah")));

// TODO(lukasza): trustworthiness of blob: URLs should depend on their inner
// origin (just as it does for filesystem: URLs). Changing this behavior of
// content::IsOriginSecure breaks some tests, so fixing this is postponed to a
// follow-up CL.
EXPECT_FALSE(
IsOriginSecure(GURL("blob:http://www.example.com/guid-goes-here")));
EXPECT_FALSE(
IsOriginSecure(GURL("blob:ftp://www.example.com/guid-goes-here")));
EXPECT_TRUE(IsOriginSecure(GURL("blob:ftp://127.0.0.1/guid-goes-here")));
EXPECT_TRUE(
IsOriginSecure(GURL("blob:https://www.example.com/guid-goes-here")));
}

Expand Down

0 comments on commit 16586e1

Please sign in to comment.