0

Remove some url::Origin unit tests in favor of AbstractOriginTests.

This CL adds some extra test coverage via:

1. Extra test assertions in AbstractOriginTests (symmetry of
   IsSameOrigin, properties of a *copy* of an origin, comparing against
   `different_tuple_origin`, comparing against opaque origin recreated
   from the same test input, covering more derived origins).

2. Extra test cases in AbstractOriginTests ("unknown-scheme").

3. Extra test assertions in UrlOriginTestTraits::Serialize to verify
   ExpectParsedUrlsEqual for `GURL(origin.Serialize())` vs
   `origin.GetURL()'.

The extra test coverage described above makes it possible to remove
some test cases from //url/origin_unittest.cc, because these cases
are already covered by AbstractOriginTests.

Bug: 1164416
Change-Id: I74b73d62ffb93177a0cfa219c420b7668b4f8d0a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2641237
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#848406}
This commit is contained in:
Lukasz Anforowicz
2021-01-29 04:22:13 +00:00
committed by Chromium LUCI CQ
parent ba75d2d3f2
commit 0b29aa4558
3 changed files with 83 additions and 237 deletions

@ -6,6 +6,28 @@
namespace url {
void ExpectParsedUrlsEqual(const GURL& a, const GURL& b) {
EXPECT_EQ(a, b);
const Parsed& a_parsed = a.parsed_for_possibly_invalid_spec();
const Parsed& b_parsed = b.parsed_for_possibly_invalid_spec();
EXPECT_EQ(a_parsed.scheme.begin, b_parsed.scheme.begin);
EXPECT_EQ(a_parsed.scheme.len, b_parsed.scheme.len);
EXPECT_EQ(a_parsed.username.begin, b_parsed.username.begin);
EXPECT_EQ(a_parsed.username.len, b_parsed.username.len);
EXPECT_EQ(a_parsed.password.begin, b_parsed.password.begin);
EXPECT_EQ(a_parsed.password.len, b_parsed.password.len);
EXPECT_EQ(a_parsed.host.begin, b_parsed.host.begin);
EXPECT_EQ(a_parsed.host.len, b_parsed.host.len);
EXPECT_EQ(a_parsed.port.begin, b_parsed.port.begin);
EXPECT_EQ(a_parsed.port.len, b_parsed.port.len);
EXPECT_EQ(a_parsed.path.begin, b_parsed.path.begin);
EXPECT_EQ(a_parsed.path.len, b_parsed.path.len);
EXPECT_EQ(a_parsed.query.begin, b_parsed.query.begin);
EXPECT_EQ(a_parsed.query.len, b_parsed.query.len);
EXPECT_EQ(a_parsed.ref.begin, b_parsed.ref.begin);
EXPECT_EQ(a_parsed.ref.len, b_parsed.ref.len);
}
// static
Origin UrlOriginTestTraits::CreateOriginFromString(base::StringPiece s) {
return Origin::Create(GURL(s));
@ -62,7 +84,13 @@ bool UrlOriginTestTraits::IsSameOrigin(const Origin& a, const Origin& b) {
// static
std::string UrlOriginTestTraits::Serialize(const Origin& origin) {
return origin.Serialize();
std::string serialized = origin.Serialize();
// Extra test assertion for GetURL (which doesn't have an equivalent in
// blink::SecurityOrigin).
ExpectParsedUrlsEqual(GURL(serialized), origin.GetURL());
return serialized;
}
// static

@ -19,6 +19,8 @@
namespace url {
void ExpectParsedUrlsEqual(const GURL& a, const GURL& b);
// AbstractOriginTest below abstracts away differences between url::Origin and
// blink::SecurityOrigin by parametrizing the tests with a class that has to
// expose the same public members as UrlOriginTestTraits below.
@ -123,7 +125,10 @@ class AbstractOriginTest : public testing::Test {
return TOriginTraits::GetTupleOrPrecursorTupleIfOpaque(origin);
}
bool IsSameOrigin(const OriginType& a, const OriginType& b) {
return TOriginTraits::IsSameOrigin(a, b);
bool is_a_same_with_b = TOriginTraits::IsSameOrigin(a, b);
bool is_b_same_with_a = TOriginTraits::IsSameOrigin(b, a);
EXPECT_EQ(is_a_same_with_b, is_b_same_with_a);
return is_a_same_with_b;
}
std::string Serialize(const OriginType& origin) {
return TOriginTraits::Serialize(origin);
@ -146,22 +151,21 @@ class AbstractOriginTest : public testing::Test {
// An origin is always same-origin with itself.
EXPECT_SAME_ORIGIN(origin, origin);
// A copy of |origin| should be same-origin as well.
auto origin_copy = origin;
EXPECT_EQ(this->GetScheme(origin), this->GetScheme(origin_copy));
EXPECT_EQ(this->GetHost(origin), this->GetHost(origin_copy));
EXPECT_EQ(this->GetPort(origin), this->GetPort(origin_copy));
EXPECT_EQ(this->IsOpaque(origin), this->IsOpaque(origin_copy));
EXPECT_SAME_ORIGIN(origin, origin_copy);
// An origin is always cross-origin from another, unique, opaque origin.
EXPECT_CROSS_ORIGIN(origin, this->CreateUniqueOpaqueOrigin());
// Derived opaque origins.
auto derived_opaque = this->DeriveNewOpaqueOrigin(origin);
auto derived_via_data_url =
this->CreateWithReferenceOrigin("data:text/html,baz", origin);
EXPECT_TRUE(this->IsOpaque(derived_opaque));
EXPECT_TRUE(this->IsOpaque(derived_via_data_url));
EXPECT_CROSS_ORIGIN(origin, derived_opaque);
EXPECT_CROSS_ORIGIN(origin, derived_via_data_url);
EXPECT_CROSS_ORIGIN(derived_opaque, derived_via_data_url);
EXPECT_EQ(this->GetTupleOrPrecursorTupleIfOpaque(origin),
this->GetTupleOrPrecursorTupleIfOpaque(derived_opaque));
EXPECT_EQ(this->GetTupleOrPrecursorTupleIfOpaque(origin),
this->GetTupleOrPrecursorTupleIfOpaque(derived_via_data_url));
// An origin is always cross-origin from another tuple origin.
auto different_tuple_origin =
this->CreateOriginFromString("https://not-in-the-list.test/");
EXPECT_CROSS_ORIGIN(origin, different_tuple_origin);
// Deriving an origin for "about:blank".
auto about_blank_origin1 =
@ -170,6 +174,22 @@ class AbstractOriginTest : public testing::Test {
this->CreateWithReferenceOrigin("about:blank?bar#foo", origin);
EXPECT_SAME_ORIGIN(origin, about_blank_origin1);
EXPECT_SAME_ORIGIN(origin, about_blank_origin2);
// Derived opaque origins.
std::vector<OriginType> derived_origins = {
this->DeriveNewOpaqueOrigin(origin),
this->CreateWithReferenceOrigin("data:text/html,baz", origin),
this->DeriveNewOpaqueOrigin(about_blank_origin1),
};
for (size_t i = 0; i < derived_origins.size(); i++) {
SCOPED_TRACE(testing::Message() << "Derived origin #" << i);
const OriginType& derived_origin = derived_origins[i];
EXPECT_TRUE(this->IsOpaque(derived_origin));
EXPECT_SAME_ORIGIN(derived_origin, derived_origin);
EXPECT_CROSS_ORIGIN(origin, derived_origin);
EXPECT_EQ(this->GetTupleOrPrecursorTupleIfOpaque(origin),
this->GetTupleOrPrecursorTupleIfOpaque(derived_origin));
}
}
void VerifyUniqueOpaqueOriginInvariants(const OriginType& origin) {
@ -194,8 +214,18 @@ class AbstractOriginTest : public testing::Test {
VerifyOriginInvariants(origin);
}
void VerifyTupleOrigin(const OriginType& origin,
const SchemeHostPort& expected_tuple) {
void TestUniqueOpaqueOrigin(base::StringPiece test_input) {
auto origin = this->CreateOriginFromString(test_input);
this->VerifyUniqueOpaqueOriginInvariants(origin);
// Re-creating from the URL should be cross-origin.
auto origin_recreated_from_same_input =
this->CreateOriginFromString(test_input);
EXPECT_CROSS_ORIGIN(origin, origin_recreated_from_same_input);
}
void VerifyTupleOriginInvariants(const OriginType& origin,
const SchemeHostPort& expected_tuple) {
if (this->IsOpaque(origin)) {
ADD_FAILURE() << "Got unexpectedly opaque origin";
return; // Skip other test assertions.
@ -274,8 +304,7 @@ TYPED_TEST_P(AbstractOriginTest, OpaqueOriginsFromValidUrls) {
// technicality like having no host in a noaccess-std-with-host: scheme).
EXPECT_TRUE(this->IsValidUrl(test_input));
auto origin = this->CreateOriginFromString(test_input);
this->VerifyUniqueOpaqueOriginInvariants(origin);
this->TestUniqueOpaqueOrigin(test_input);
}
}
@ -311,6 +340,9 @@ TYPED_TEST_P(AbstractOriginTest, OpaqueOriginsFromInvalidUrls) {
"http://[]/",
"http://[2001:0db8:0000:0000:0000:0000:0000:0000:0001]/", // 9 groups.
// Unknown scheme without a colon character (":") gives an invalid URL.
"unknown-scheme",
// Standard schemes require a hostname (and result in an opaque origin if
// the hostname is missing).
"local-std-with-host:",
@ -326,8 +358,7 @@ TYPED_TEST_P(AbstractOriginTest, OpaqueOriginsFromInvalidUrls) {
EXPECT_FALSE(this->IsValidUrl(test_input));
// Invalid URLs should always result in an opaque origin.
auto origin = this->CreateOriginFromString(test_input);
this->VerifyUniqueOpaqueOriginInvariants(origin);
this->TestUniqueOpaqueOrigin(test_input);
}
}
@ -406,7 +437,7 @@ TYPED_TEST_P(AbstractOriginTest, TupleOrigins) {
EXPECT_TRUE(this->IsValidUrl(test.input));
auto origin = this->CreateOriginFromString(test.input);
this->VerifyTupleOrigin(origin, test.expected_tuple);
this->VerifyTupleOriginInvariants(origin, test.expected_tuple);
}
}
@ -439,8 +470,7 @@ TYPED_TEST_P(AbstractOriginTest, CustomSchemes_OpaqueOrigins) {
// technicality like having no host in a noaccess-std-with-host: scheme).
EXPECT_TRUE(this->IsValidUrl(test_input));
auto origin = this->CreateOriginFromString(test_input);
this->VerifyUniqueOpaqueOriginInvariants(origin);
this->TestUniqueOpaqueOrigin(test_input);
}
}
@ -480,7 +510,7 @@ TYPED_TEST_P(AbstractOriginTest, CustomSchemes_TupleOrigins) {
EXPECT_TRUE(this->IsValidUrl(test.input));
auto origin = this->CreateOriginFromString(test.input);
this->VerifyTupleOrigin(origin, test.expected_tuple);
this->VerifyTupleOriginInvariants(origin, test.expected_tuple);
}
}

@ -15,32 +15,6 @@
namespace url {
namespace {
void ExpectParsedUrlsEqual(const GURL& a, const GURL& b) {
EXPECT_EQ(a, b);
const Parsed& a_parsed = a.parsed_for_possibly_invalid_spec();
const Parsed& b_parsed = b.parsed_for_possibly_invalid_spec();
EXPECT_EQ(a_parsed.scheme.begin, b_parsed.scheme.begin);
EXPECT_EQ(a_parsed.scheme.len, b_parsed.scheme.len);
EXPECT_EQ(a_parsed.username.begin, b_parsed.username.begin);
EXPECT_EQ(a_parsed.username.len, b_parsed.username.len);
EXPECT_EQ(a_parsed.password.begin, b_parsed.password.begin);
EXPECT_EQ(a_parsed.password.len, b_parsed.password.len);
EXPECT_EQ(a_parsed.host.begin, b_parsed.host.begin);
EXPECT_EQ(a_parsed.host.len, b_parsed.host.len);
EXPECT_EQ(a_parsed.port.begin, b_parsed.port.begin);
EXPECT_EQ(a_parsed.port.len, b_parsed.port.len);
EXPECT_EQ(a_parsed.path.begin, b_parsed.path.begin);
EXPECT_EQ(a_parsed.path.len, b_parsed.path.len);
EXPECT_EQ(a_parsed.query.begin, b_parsed.query.begin);
EXPECT_EQ(a_parsed.query.len, b_parsed.query.len);
EXPECT_EQ(a_parsed.ref.begin, b_parsed.ref.begin);
EXPECT_EQ(a_parsed.ref.len, b_parsed.ref.len);
}
} // namespace
class OriginTest : public ::testing::Test {
public:
void SetUp() override {
@ -202,72 +176,6 @@ TEST_F(OriginTest, OpaqueOriginComparison) {
EXPECT_EQ(opaque_b, url::Origin::Resolve(GURL("about:srcdoc"), opaque_b));
EXPECT_EQ(opaque_b,
url::Origin::Resolve(GURL("about:blank?hello#whee"), opaque_b));
const char* const urls[] = {
"data:text/html,Hello!",
"javascript:alert(1)",
"about:blank",
"file://example.com:443/etc/passwd",
"unknown-scheme:foo",
"unknown-scheme://bar",
"http",
"http:",
"http:/",
"http://",
"http://:",
"http://:1",
"yay",
"http::///invalid.example.com/",
"blob:null/foo", // blob:null (actually a valid URL)
"blob:data:foo", // blob + data (which is nonstandard)
"blob:about://blank/", // blob + about (which is nonstandard)
"blob:about:blank/", // blob + about (which is nonstandard)
"filesystem:http://example.com/", // Invalid (missing /type/)
"filesystem:local-but-nonstandard:baz./type/", // fs requires standard
"filesystem:local-but-nonstandard://hostname/type/",
"filesystem:unknown-scheme://hostname/type/",
"local-but-nonstandar:foo", // Prefix of registered scheme.
"but-nonstandard:foo", // Suffix of registered scheme.
"local-and-standard:", // Standard scheme needs a hostname.
"blob:blob:http://www.example.com/guid-goes-here", // Double blob.
// Scheme (registered in SetUp()) that's standard but marked as noaccess.
// See also SecurityOriginTest.StandardNoAccessScheme and
// NavigationUrlRewriteBrowserTest.RewriteToNoAccess.
"standard-but-noaccess:", // Standard scheme needs a hostname.
"standard-but-noaccess:foo", // Standard scheme needs a hostname.
"standard-but-noaccess://bar",
};
for (auto* test_url : urls) {
SCOPED_TRACE(test_url);
GURL url(test_url);
const url::Origin opaque_origin;
// Opaque origins returned by Origin::Create().
{
Origin origin = Origin::Create(url);
EXPECT_EQ("", origin.scheme());
EXPECT_EQ("", origin.host());
EXPECT_EQ(0, origin.port());
EXPECT_TRUE(origin.opaque());
// An origin is always same-origin with itself.
EXPECT_EQ(origin, origin);
EXPECT_NE(origin, url::Origin());
EXPECT_EQ(SchemeHostPort(), origin.GetTupleOrPrecursorTupleIfOpaque());
// A copy of |origin| should be same-origin as well.
Origin origin_copy = origin;
EXPECT_EQ("", origin_copy.scheme());
EXPECT_EQ("", origin_copy.host());
EXPECT_EQ(0, origin_copy.port());
EXPECT_TRUE(origin_copy.opaque());
EXPECT_EQ(origin, origin_copy);
// Re-creating from the URL should also be cross-origin.
EXPECT_NE(origin, Origin::Create(url));
ExpectParsedUrlsEqual(GURL(origin.Serialize()), origin.GetURL());
}
}
}
TEST_F(OriginTest, ConstructFromTuple) {
@ -295,126 +203,6 @@ TEST_F(OriginTest, ConstructFromTuple) {
}
}
TEST_F(OriginTest, ConstructFromGURL) {
Origin different_origin =
Origin::Create(GURL("https://not-in-the-list.test/"));
struct TestCases {
const char* const url;
const char* const expected_scheme;
const char* const expected_host;
const uint16_t expected_port;
} cases[] = {
// IP Addresses
{"http://192.168.9.1/", "http", "192.168.9.1", 80},
{"http://[2001:db8::1]/", "http", "[2001:db8::1]", 80},
{"http://1/", "http", "0.0.0.1", 80},
{"http://1:1/", "http", "0.0.0.1", 1},
{"http://3232237825/", "http", "192.168.9.1", 80},
// Punycode
{"http://☃.net/", "http", "xn--n3h.net", 80},
{"blob:http://☃.net/", "http", "xn--n3h.net", 80},
// Generic URLs
{"http://example.com/", "http", "example.com", 80},
{"http://example.com:123/", "http", "example.com", 123},
{"https://example.com/", "https", "example.com", 443},
{"https://example.com:123/", "https", "example.com", 123},
{"http://user:pass@example.com/", "http", "example.com", 80},
{"http://example.com:123/?query", "http", "example.com", 123},
{"https://example.com/#1234", "https", "example.com", 443},
{"https://u:p@example.com:123/?query#1234", "https", "example.com", 123},
// Registered URLs
{"ftp://example.com/", "ftp", "example.com", 21},
{"ws://example.com/", "ws", "example.com", 80},
{"wss://example.com/", "wss", "example.com", 443},
{"wss://user:pass@example.com/", "wss", "example.com", 443},
// Scheme (registered in SetUp()) that's both local and standard.
// TODO: Is it really appropriate to do network-host canonicalization of
// schemes without ports?
{"local-and-standard:20", "local-and-standard", "0.0.0.20", 0},
{"local-and-standard:20.", "local-and-standard", "0.0.0.20", 0},
{"local-and-standard:↑↑↓↓←→←→ba.↑↑↓↓←→←→ba.0.bg", "local-and-standard",
"xn--ba-rzuadaibfa.xn--ba-rzuadaibfa.0.bg", 0},
{"local-and-standard:foo", "local-and-standard", "foo", 0},
{"local-and-standard://bar:20", "local-and-standard", "bar", 0},
{"local-and-standard:baz.", "local-and-standard", "baz.", 0},
{"local-and-standard:baz..", "local-and-standard", "baz..", 0},
{"local-and-standard:baz..bar", "local-and-standard", "baz..bar", 0},
{"local-and-standard:baz...", "local-and-standard", "baz...", 0},
// Scheme (registered in SetUp()) that's local but nonstandard. These
// always have empty hostnames, but are allowed to be url::Origins.
{"local-but-nonstandard:", "local-but-nonstandard", "", 0},
{"local-but-nonstandard:foo", "local-but-nonstandard", "", 0},
{"local-but-nonstandard://bar", "local-but-nonstandard", "", 0},
{"also-local-but-nonstandard://bar", "also-local-but-nonstandard", "", 0},
// file: URLs
{"file:///etc/passwd", "file", "", 0},
{"file://example.com/etc/passwd", "file", "example.com", 0},
// Filesystem:
{"filesystem:http://example.com/type/", "http", "example.com", 80},
{"filesystem:http://example.com:123/type/", "http", "example.com", 123},
{"filesystem:https://example.com/type/", "https", "example.com", 443},
{"filesystem:https://example.com:123/type/", "https", "example.com", 123},
{"filesystem:local-and-standard:baz./type/", "local-and-standard", "baz.",
0},
// Blob:
{"blob:http://example.com/guid-goes-here", "http", "example.com", 80},
{"blob:http://example.com:123/guid-goes-here", "http", "example.com",
123},
{"blob:https://example.com/guid-goes-here", "https", "example.com", 443},
{"blob:http://u:p@example.com/guid-goes-here", "http", "example.com", 80},
};
for (const auto& test_case : cases) {
SCOPED_TRACE(test_case.url);
GURL url(test_case.url);
EXPECT_TRUE(url.is_valid());
Origin origin = Origin::Create(url);
EXPECT_EQ(test_case.expected_scheme, origin.scheme());
EXPECT_EQ(test_case.expected_host, origin.host());
EXPECT_EQ(test_case.expected_port, origin.port());
EXPECT_FALSE(origin.opaque());
EXPECT_EQ(origin, origin);
EXPECT_NE(different_origin, origin);
EXPECT_NE(origin, different_origin);
EXPECT_EQ(origin, Origin::Resolve(GURL("about:blank"), origin));
EXPECT_EQ(origin, Origin::Resolve(GURL("about:blank?bar#foo"), origin));
ExpectParsedUrlsEqual(GURL(origin.Serialize()), origin.GetURL());
url::Origin derived_opaque =
Origin::Resolve(GURL("about:blank?bar#foo"), origin)
.DeriveNewOpaqueOrigin();
EXPECT_TRUE(derived_opaque.opaque());
EXPECT_NE(origin, derived_opaque);
EXPECT_TRUE(derived_opaque.GetTupleOrPrecursorTupleIfOpaque().IsValid());
EXPECT_EQ(origin.GetTupleOrPrecursorTupleIfOpaque(),
derived_opaque.GetTupleOrPrecursorTupleIfOpaque());
EXPECT_EQ(derived_opaque, derived_opaque);
url::Origin derived_opaque_via_data_url =
Origin::Resolve(GURL("data:text/html,baz"), origin);
EXPECT_TRUE(derived_opaque_via_data_url.opaque());
EXPECT_NE(origin, derived_opaque_via_data_url);
EXPECT_TRUE(derived_opaque_via_data_url.GetTupleOrPrecursorTupleIfOpaque()
.IsValid());
EXPECT_EQ(origin.GetTupleOrPrecursorTupleIfOpaque(),
derived_opaque_via_data_url.GetTupleOrPrecursorTupleIfOpaque());
EXPECT_NE(derived_opaque, derived_opaque_via_data_url);
EXPECT_NE(derived_opaque_via_data_url, derived_opaque);
EXPECT_NE(derived_opaque.DeriveNewOpaqueOrigin(), derived_opaque);
EXPECT_EQ(derived_opaque_via_data_url, derived_opaque_via_data_url);
}
}
TEST_F(OriginTest, Serialization) {
struct TestCases {
const char* const url;