Sharing IsPotentiallyTrustworthy tests across Blink and "network" layers

Bug: 1164416
Change-Id: I3834ffb77ff1ef23f39c8886049b563753ab6cf0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2630625
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Auto-Submit: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845905}

services/network/public/cpp/BUILD.gn

@@ -300,6 +300,21 @@ mojom("test_interfaces") {
   public_deps = [ "//services/network/public/mojom" ]
 }
 
+source_set("test_support") {
+  testonly = true
+
+  sources = [ "is_potentially_trustworthy_unittest.h" ]
+
+  public_deps = [
+    ":cpp",
+    "//base",
+    "//base/test:test_support",
+    "//testing/gmock",
+    "//testing/gtest",
+    "//url:url_test_support",
+  ]
+}
+
 source_set("tests") {
   testonly = true
 
@@ -355,6 +370,7 @@ source_set("tests") {
   deps = [
     ":cpp",
     ":test_interfaces",
+    ":test_support",
     "//base",
     "//mojo/public/cpp/bindings",
     "//mojo/public/cpp/test_support:test_utils",

services/network/public/cpp/is_potentially_trustworthy_unittest.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "services/network/public/cpp/is_potentially_trustworthy.h"
+#include "services/network/public/cpp/is_potentially_trustworthy_unittest.h"
 
 #include "base/test/scoped_command_line.h"
 #include "services/network/public/cpp/network_switches.h"
@@ -13,6 +13,7 @@
 #include "url/url_util.h"
 
 namespace network {
+namespace test {
 
 bool IsOriginAllowlisted(const url::Origin& origin) {
   return SecureOriginAllowlist::GetInstance().IsOriginAllowlisted(origin);
@@ -22,12 +23,8 @@ bool IsOriginAllowlisted(const char* str) {
   return IsOriginAllowlisted(url::Origin::Create(GURL(str)));
 }
 
-bool IsOriginPotentiallyTrustworthy(const char* str) {
-  return IsOriginPotentiallyTrustworthy(url::Origin::Create(GURL(str)));
-}
-
 bool IsUrlPotentiallyTrustworthy(const char* str) {
-  return IsUrlPotentiallyTrustworthy(GURL(str));
+  return network::IsUrlPotentiallyTrustworthy(GURL(str));
 }
 
 std::vector<std::string> CanonicalizeAllowlist(
@@ -37,51 +34,18 @@ std::vector<std::string> CanonicalizeAllowlist(
       allowlist, rejected_patterns);
 }
 
-TEST(IsPotentiallyTrustworthy, Origin) {
-  const url::Origin unique_origin;
-  EXPECT_FALSE(IsOriginPotentiallyTrustworthy(unique_origin));
-  const url::Origin opaque_origin =
-      url::Origin::Create(GURL("https://www.example.com"))
-          .DeriveNewOpaqueOrigin();
-  EXPECT_FALSE(IsOriginPotentiallyTrustworthy(opaque_origin));
-
-  EXPECT_FALSE(IsOriginPotentiallyTrustworthy("about:blank"));
-  EXPECT_FALSE(IsOriginPotentiallyTrustworthy("about:blank#ref"));
-  EXPECT_FALSE(IsOriginPotentiallyTrustworthy("about:srcdoc"));
-  EXPECT_FALSE(IsOriginPotentiallyTrustworthy("javascript:alert('blah')"));
-  EXPECT_FALSE(IsOriginPotentiallyTrustworthy("data:test/plain;blah"));
-
-  EXPECT_TRUE(
-      IsOriginPotentiallyTrustworthy("quic-transport://example.com/counter"));
-}
-
+// TODO(crbug.com/1153336 and crbug.com/1164416): Fix product behavior, so that
+// blink::SecurityOrigin::IsSecure(const KURL&) is compatible with
+// network::IsUrlPotentiallyTrustworthy(const GURL&) and then move the tests
+// below to the AbstractTrustworthinessTest.UrlFromString test case in
+// //services/network/public/cpp/is_potentially_trustworthy_unittest.h
+// See also SecurityOriginTest.IsSecure test.
 TEST(IsPotentiallyTrustworthy, Url) {
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy("about:blank"));
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy("about:blank?x=2"));
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy("about:blank#ref"));
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy("about:blank?x=2#ref"));
-
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy("about:srcdoc"));
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy("about:srcdoc?x=2"));
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy("about:srcdoc#ref"));
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy("about:srcdoc?x=2#ref"));
-
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy("about:mumble"));
-
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy("data:test/plain;blah"));
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy("javascript:alert('blah')"));
-
   EXPECT_TRUE(IsUrlPotentiallyTrustworthy("file:///test/fun.html"));
   EXPECT_TRUE(IsUrlPotentiallyTrustworthy("file:///test/"));
   EXPECT_TRUE(IsUrlPotentiallyTrustworthy("file://localhost/test/"));
   EXPECT_TRUE(IsUrlPotentiallyTrustworthy("file://otherhost/test/"));
 
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy("https://example.com/fun.html"));
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://example.com/fun.html"));
-
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy("wss://example.com/fun.html"));
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy("ws://example.com/fun.html"));
-
   EXPECT_TRUE(IsUrlPotentiallyTrustworthy("http://localhost/fun.html"));
   EXPECT_TRUE(IsUrlPotentiallyTrustworthy("http://localhost./fun.html"));
   EXPECT_TRUE(IsUrlPotentiallyTrustworthy("http://pumpkin.localhost/fun.html"));
@@ -91,99 +55,20 @@ TEST(IsPotentiallyTrustworthy, Url) {
       IsUrlPotentiallyTrustworthy("http://pumpkin.localhost:8080/fun.html"));
   EXPECT_TRUE(IsUrlPotentiallyTrustworthy(
       "http://crumpet.pumpkin.localhost:3000/fun.html"));
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://localhost.com/fun.html"));
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy("https://localhost.com/fun.html"));
 
   EXPECT_TRUE(IsUrlPotentiallyTrustworthy("http://127.0.0.1/fun.html"));
   EXPECT_TRUE(IsUrlPotentiallyTrustworthy("ftp://127.0.0.1/fun.html"));
   EXPECT_TRUE(IsUrlPotentiallyTrustworthy("http://127.3.0.1/fun.html"));
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://127.example.com/fun.html"));
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy("https://127.example.com/fun.html"));
 
   EXPECT_TRUE(IsUrlPotentiallyTrustworthy("http://[::1]/fun.html"));
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://[::2]/fun.html"));
-  EXPECT_FALSE(
-      IsUrlPotentiallyTrustworthy("http://[::1].example.com/fun.html"));
-
-  // IPv4 mapped IPv6 literals for loopback.
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://[::ffff:127.0.0.1]/"));
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://[::ffff:7f00:1]"));
-
-  // IPv4 compatible IPv6 literal for loopback.
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://[::127.0.0.1]"));
-
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://loopback"));
-
-  // Legacy localhost names.
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://localhost.localdomain"));
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy("http://localhost6"));
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy("ftp://localhost6.localdomain6"));
 
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy(
-      "filesystem:http://www.example.com/temporary/"));
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy(
-      "filesystem:ftp://www.example.com/temporary/"));
   EXPECT_TRUE(
       IsUrlPotentiallyTrustworthy("filesystem:ftp://127.0.0.1/temporary/"));
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy(
-      "filesystem:https://www.example.com/temporary/"));
-
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy(
-      "blob:http://www.example.com/guid-goes-here"));
-  EXPECT_FALSE(
-      IsUrlPotentiallyTrustworthy("blob:ftp://www.example.com/guid-goes-here"));
   EXPECT_TRUE(
       IsUrlPotentiallyTrustworthy("blob:ftp://127.0.0.1/guid-goes-here"));
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy(
-      "blob:https://www.example.com/guid-goes-here"));
 
   EXPECT_FALSE(IsUrlPotentiallyTrustworthy("blob:data:text/html,Hello"));
   EXPECT_FALSE(IsUrlPotentiallyTrustworthy("blob:about:blank"));
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy("filesystem:data:text/html,Hello"));
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy("filesystem:about:blank"));
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy(
-      "blob:blob:https://example.com/578223a1-8c13-17b3-84d5-eca045ae384a"));
-  EXPECT_FALSE(
-      IsUrlPotentiallyTrustworthy("filesystem:blob:https://example.com/"
-                                  "578223a1-8c13-17b3-84d5-eca045ae384a"));
-
-  EXPECT_TRUE(
-      IsUrlPotentiallyTrustworthy("quic-transport://example.com/counter"));
-}
-
-TEST(IsPotentiallyTrustworthy, CustomSchemes) {
-  url::ScopedSchemeRegistryForTests scoped_registry;
-  url::AddSecureScheme("sec-nonstd-scheme");
-  url::AddSecureScheme("sec-std-scheme");
-  url::AddStandardScheme("sec-std-scheme", url::SCHEME_WITH_HOST);
-  url::AddSecureScheme("sec-noaccess-scheme");
-  url::AddNoAccessScheme("sec-noaccess-scheme");
-  url::AddNoAccessScheme("nonsec-noaccess-scheme");
-
-  // Unrecognized / unknown schemes are not trustworthy.
-  EXPECT_FALSE(IsOriginPotentiallyTrustworthy("unknown-scheme://example.com"));
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy("unknown-scheme://example.com"));
-
-  // Secure URLs are trustworthy, even if their scheme is also marked as
-  // no-access, or are not marked as standard.  See also //chrome-layer
-  // ChromeContentClientTest.AdditionalSchemes test and
-  // https://crbug.com/734581.
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy("sec-nonstd-scheme://blah/x.js"));
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy("sec-std-scheme://blah/x.js"));
-  EXPECT_TRUE(IsUrlPotentiallyTrustworthy("sec-noaccess-scheme://blah/x.js"));
-  EXPECT_TRUE(IsOriginPotentiallyTrustworthy("sec-std-scheme://blah/x.js"));
-  // No-access and non-standard/non-local schemes translate into an
-  // untrustworthy, opaque origin.
-  // TODO(lukasza): Maybe if the spec had a notion of an origin *precursor*,
-  // then it could inspect the scheme of the precursor.  After this, it may be
-  // possible to EXPECT_TRUE below...
-  EXPECT_FALSE(IsOriginPotentiallyTrustworthy("sec-nonstd-scheme://blah/x.js"));
-  EXPECT_FALSE(
-      IsOriginPotentiallyTrustworthy("sec-noaccess-scheme://blah/x.js"));
-
-  // No-access, non-secure schemes are untrustworthy.
-  EXPECT_FALSE(IsUrlPotentiallyTrustworthy("nonsec-noaccess-scheme:blah"));
-  EXPECT_FALSE(IsOriginPotentiallyTrustworthy("nonsec-noaccess-scheme:blah"));
 }
 
 class SecureOriginAllowlistTest : public testing::Test {
@@ -330,4 +215,24 @@ TEST_F(SecureOriginAllowlistTest, Canonicalization) {
   EXPECT_THAT(canonicalized, ::testing::ElementsAre("*.example.com"));
 }
 
+class TrustworthinessTestTraits final
+    : public virtual url::UrlOriginTestTraits,
+      public virtual TrustworthinessTraitsBase<url::Origin> {
+ public:
+  bool IsOriginPotentiallyTrustworthy(const OriginType& origin) override {
+    return network::IsOriginPotentiallyTrustworthy(origin);
+  }
+  bool IsUrlPotentiallyTrustworthy(base::StringPiece str) override {
+    return network::IsUrlPotentiallyTrustworthy(GURL(str));
+  }
+  bool IsOriginOfLocalhost(const OriginType& origin) override {
+    return net::IsLocalhost(origin.GetURL());
+  }
+};
+
+INSTANTIATE_TYPED_TEST_SUITE_P(UrlOrigin,
+                               AbstractTrustworthinessTest,
+                               TrustworthinessTestTraits);
+
+}  // namespace test
 }  // namespace network

third_party/blink/renderer/platform/BUILD.gn

@@ -2174,6 +2174,7 @@ source_set("blink_platform_unittests_sources") {
     "//mojo/public/cpp/bindings/tests:for_blink_tests",
     "//mojo/public/cpp/test_support:test_utils",
     "//mojo/public/interfaces/bindings/tests:test_interfaces_blink",
+    "//services/network/public/cpp:test_support",
     "//services/viz/public/mojom",
     "//skia",
     "//skia:skcms",

url/BUILD.gn

@@ -176,6 +176,7 @@ source_set("url_test_support") {
 
   sources = [
     "gurl_abstract_tests.h",
+    "origin_abstract_tests.cc",
     "origin_abstract_tests.h",
   ]
 

url/origin_abstract_tests.cc

+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "url/origin_abstract_tests.h"
+
+namespace url {
+
+Origin UrlOriginTestTraits::CreateOriginFromString(base::StringPiece s) {
+  return Origin::Create(GURL(s));
+}
+
+Origin UrlOriginTestTraits::CreateUniqueOpaqueOrigin() {
+  return Origin();
+}
+
+Origin UrlOriginTestTraits::CreateWithReferenceOrigin(
+    base::StringPiece url,
+    const Origin& reference_origin) {
+  return Origin::Resolve(GURL(url), reference_origin);
+}
+
+Origin UrlOriginTestTraits::DeriveNewOpaqueOrigin(
+    const Origin& reference_origin) {
+  return reference_origin.DeriveNewOpaqueOrigin();
+}
+
+bool UrlOriginTestTraits::IsOpaque(const Origin& origin) {
+  return origin.opaque();
+}
+
+std::string UrlOriginTestTraits::GetScheme(const Origin& origin) {
+  return origin.scheme();
+}
+
+std::string UrlOriginTestTraits::GetHost(const Origin& origin) {
+  return origin.host();
+}
+
+uint16_t UrlOriginTestTraits::GetPort(const Origin& origin) {
+  return origin.port();
+}
+
+SchemeHostPort UrlOriginTestTraits::GetTupleOrPrecursorTupleIfOpaque(
+    const Origin& origin) {
+  return origin.GetTupleOrPrecursorTupleIfOpaque();
+}
+
+bool UrlOriginTestTraits::IsSameOrigin(const Origin& a, const Origin& b) {
+  return a.IsSameOriginWith(b);
+}
+
+std::string UrlOriginTestTraits::Serialize(const Origin& origin) {
+  return origin.Serialize();
+}
+
+bool UrlOriginTestTraits::IsValidUrl(base::StringPiece str) {
+  return GURL(str).is_valid();
+}
+
+}  // namespace url

url/origin_abstract_tests.h

@@ -8,8 +8,13 @@
 #include <string>
 #include <type_traits>
 
+#include "base/containers/contains.h"
+#include "base/optional.h"
 #include "base/strings/string_piece.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+#include "url/origin.h"
+#include "url/scheme_host_port.h"
 #include "url/url_util.h"
 
 namespace url {
@@ -74,6 +79,9 @@ class AbstractOriginTest : public testing::Test {
         "local-std-with-host",
         "local-noaccess-std-with-host",
         "also-local",
+        "sec",
+        "sec-std-with-host",
+        "sec-noaccess",
     };
     for (const char* kScheme : kSchemesToRegister) {
       std::string scheme(kScheme);
@@ -83,6 +91,8 @@ class AbstractOriginTest : public testing::Test {
         AddStandardScheme(kScheme, SchemeType::SCHEME_WITH_HOST);
       if (base::Contains(scheme, "local"))
         AddLocalScheme(kScheme);
+      if (base::Contains(scheme, "sec"))
+        AddSecureScheme(kScheme);
     }
   }
 
@@ -489,6 +499,25 @@ REGISTER_TYPED_TEST_SUITE_P(AbstractOriginTest,
                             CustomSchemes_OpaqueOrigins,
                             CustomSchemes_TupleOrigins);
 
+class UrlOriginTestTraits : public virtual OriginTraitsBase<Origin> {
+ public:
+  OriginType CreateOriginFromString(base::StringPiece s) override;
+  OriginType CreateUniqueOpaqueOrigin() override;
+  OriginType CreateWithReferenceOrigin(
+      base::StringPiece url,
+      const OriginType& reference_origin) override;
+  OriginType DeriveNewOpaqueOrigin(const OriginType& reference_origin) override;
+  bool IsOpaque(const OriginType& origin) override;
+  std::string GetScheme(const OriginType& origin) override;
+  std::string GetHost(const OriginType& origin) override;
+  uint16_t GetPort(const OriginType& origin) override;
+  SchemeHostPort GetTupleOrPrecursorTupleIfOpaque(
+      const OriginType& origin) override;
+  bool IsSameOrigin(const OriginType& a, const OriginType& b) override;
+  std::string Serialize(const OriginType& origin) override;
+  bool IsValidUrl(base::StringPiece str) override;
+};
+
 }  // namespace url
 
 #endif  // URL_ORIGIN_ABSTRACT_TESTS_H_

url/origin_unittest.cc

@@ -956,55 +956,6 @@ TEST_F(OriginTest, DeserializeValidNonce) {
   EXPECT_EQ(opaque.GetDebugString(), deserialized.value().GetDebugString());
 }
 
-class UrlOriginTestTraits final : public OriginTraitsBase<Origin> {
- public:
-  OriginType CreateOriginFromString(base::StringPiece s) override {
-    return Origin::Create(GURL(s));
-  }
-
-  OriginType CreateUniqueOpaqueOrigin() override { return Origin(); }
-
-  OriginType CreateWithReferenceOrigin(
-      base::StringPiece url,
-      const OriginType& reference_origin) override {
-    return Origin::Resolve(GURL(url), reference_origin);
-  }
-
-  OriginType DeriveNewOpaqueOrigin(
-      const OriginType& reference_origin) override {
-    return reference_origin.DeriveNewOpaqueOrigin();
-  }
-
-  bool IsOpaque(const OriginType& origin) override { return origin.opaque(); }
-
-  std::string GetScheme(const OriginType& origin) override {
-    return origin.scheme();
-  }
-
-  std::string GetHost(const OriginType& origin) override {
-    return origin.host();
-  }
-
-  uint16_t GetPort(const OriginType& origin) override { return origin.port(); }
-
-  SchemeHostPort GetTupleOrPrecursorTupleIfOpaque(
-      const OriginType& origin) override {
-    return origin.GetTupleOrPrecursorTupleIfOpaque();
-  }
-
-  bool IsSameOrigin(const OriginType& a, const OriginType& b) override {
-    return a.IsSameOriginWith(b);
-  }
-
-  std::string Serialize(const OriginType& origin) override {
-    return origin.Serialize();
-  }
-
-  bool IsValidUrl(base::StringPiece str) override {
-    return GURL(str).is_valid();
-  }
-};
-
 INSTANTIATE_TYPED_TEST_SUITE_P(UrlOrigin,
                                AbstractOriginTest,
                                UrlOriginTestTraits);