Skip to content

Commit

Permalink
[GURL] (1 of 2) Prep for stripping "username:password" from internal …
Browse files Browse the repository at this point in the history 
…schemes

Rename SCHEME_WITHOUT_PORT to SCHEME_WITH_HOST.
Rename SCHEME_WITH_PORT to SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION.

Add calls to url::Shutdown() in a few places, to reset the registration
info.

This is done in preparation for stripping "username:password@" from
URLs with schemes that don't support it.

[email protected],[email protected],[email protected]

Bug: 606001
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I0eeef29a19b1b6d842b946f705c0f1442ce2c17c
Reviewed-on: https://chromium-review.googlesource.com/978450
Commit-Queue: Nick Carter <[email protected]>
Reviewed-by: Mike West <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#547316}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 123ca19dd4e7c2a12b2ad215a8f32fb2236c83d6
  • Loading branch information
@nick-chromium
nick-chromium authored and Commit Bot committed Mar 30, 2018
1 parent 773be4c commit 9b53d43
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 50 deletions.
6 changes: 3 additions & 3 deletions scheme_host_port.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ bool IsValidInput(const base::StringPiece& scheme,
const base::StringPiece& host,
uint16_t port,
SchemeHostPort::ConstructPolicy policy) {
SchemeType scheme_type = SCHEME_WITH_PORT;
SchemeType scheme_type = SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION;
bool is_standard = GetStandardSchemeType(
scheme.data(),
Component(0, base::checked_cast<int>(scheme.length())),
Expand All @@ -60,7 +60,7 @@ bool IsValidInput(const base::StringPiece& scheme,
return false;

switch (scheme_type) {
case SCHEME_WITH_PORT:
case SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION:
// A URL with |scheme| is required to have the host and port (may be
// omitted in a serialization if it's the same as the default value).
// Return an invalid instance if either of them is not given.
Expand All @@ -78,7 +78,7 @@ bool IsValidInput(const base::StringPiece& scheme,

return true;

case SCHEME_WITHOUT_PORT:
case SCHEME_WITH_HOST:
if (port != 0) {
// Return an invalid object if a URL with the scheme never represents
// the port data but the given |port| is non-zero.
Expand Down
34 changes: 23 additions & 11 deletions scheme_host_port_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,18 @@

namespace {

class SchemeHostPortTest : public testing::Test {
public:
SchemeHostPortTest() = default;
~SchemeHostPortTest() override {
// Reset any added schemes.
url::Shutdown();
}

private:
DISALLOW_COPY_AND_ASSIGN(SchemeHostPortTest);
};

void ExpectParsedUrlsEqual(const GURL& a, const GURL& b) {
EXPECT_EQ(a, b);
const url::Parsed& a_parsed = a.parsed_for_possibly_invalid_spec();
Expand All @@ -35,7 +47,7 @@ void ExpectParsedUrlsEqual(const GURL& a, const GURL& b) {
EXPECT_EQ(a_parsed.ref.len, b_parsed.ref.len);
}

TEST(SchemeHostPortTest, Invalid) {
TEST_F(SchemeHostPortTest, Invalid) {
url::SchemeHostPort invalid;
EXPECT_EQ("", invalid.scheme());
EXPECT_EQ("", invalid.host());
Expand Down Expand Up @@ -72,7 +84,7 @@ TEST(SchemeHostPortTest, Invalid) {
}
}

TEST(SchemeHostPortTest, ExplicitConstruction) {
TEST_F(SchemeHostPortTest, ExplicitConstruction) {
struct TestCases {
const char* scheme;
const char* host;
Expand All @@ -99,7 +111,7 @@ TEST(SchemeHostPortTest, ExplicitConstruction) {
}
}

TEST(SchemeHostPortTest, InvalidConstruction) {
TEST_F(SchemeHostPortTest, InvalidConstruction) {
struct TestCases {
const char* scheme;
const char* host;
Expand Down Expand Up @@ -135,7 +147,7 @@ TEST(SchemeHostPortTest, InvalidConstruction) {
}
}

TEST(SchemeHostPortTest, InvalidConstructionWithEmbeddedNulls) {
TEST_F(SchemeHostPortTest, InvalidConstructionWithEmbeddedNulls) {
struct TestCases {
const char* scheme;
size_t scheme_length;
Expand Down Expand Up @@ -163,7 +175,7 @@ TEST(SchemeHostPortTest, InvalidConstructionWithEmbeddedNulls) {
}
}

TEST(SchemeHostPortTest, GURLConstruction) {
TEST_F(SchemeHostPortTest, GURLConstruction) {
struct TestCases {
const char* url;
const char* scheme;
Expand Down Expand Up @@ -199,7 +211,7 @@ TEST(SchemeHostPortTest, GURLConstruction) {
}
}

TEST(SchemeHostPortTest, Serialization) {
TEST_F(SchemeHostPortTest, Serialization) {
struct TestCases {
const char* url;
const char* expected;
Expand All @@ -224,7 +236,7 @@ TEST(SchemeHostPortTest, Serialization) {
}
}

TEST(SchemeHostPortTest, Comparison) {
TEST_F(SchemeHostPortTest, Comparison) {
// These tuples are arranged in increasing order:
struct SchemeHostPorts {
const char* scheme;
Expand Down Expand Up @@ -256,10 +268,10 @@ TEST(SchemeHostPortTest, Comparison) {
// Some schemes have optional authority. Make sure that GURL conversion from
// SchemeHostPort is not opinionated in that regard. For more info, See
// crbug.com/820194, where we considered all SchemeHostPorts with
// SCHEME_WITHOUT_PORT as valid with empty hosts, even though some are not (e.g.
// chrome URLs).
TEST(SchemeHostPortTest, EmptyHostGurlConversion) {
url::AddStandardScheme("chrome", url::SCHEME_WITHOUT_PORT);
// SCHEME_WITH_HOST (i.e., without ports) as valid with empty hosts, even though
// most are not (e.g. chrome URLs).
TEST_F(SchemeHostPortTest, EmptyHostGurlConversion) {
url::AddStandardScheme("chrome", url::SCHEME_WITH_HOST);

GURL chrome_url("chrome:");
EXPECT_FALSE(chrome_url.is_valid());
Expand Down
29 changes: 16 additions & 13 deletions url_util.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,25 @@ enum WhitespaceRemovalPolicy {
};

const SchemeWithType kStandardURLSchemes[] = {
{kHttpsScheme, SCHEME_WITH_PORT},
{kHttpScheme, SCHEME_WITH_PORT},
{kHttpsScheme, SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION},
{kHttpScheme, SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION},
// Yes, file URLs can have a hostname, so file URLs should be handled as
// "standard". File URLs never have a port as specified by the SchemeType
// field.
{kFileScheme, SCHEME_WITHOUT_PORT},
{kFtpScheme, SCHEME_WITH_PORT},
{kGopherScheme, SCHEME_WITH_PORT},
{kWssScheme, SCHEME_WITH_PORT}, // WebSocket secure.
{kWsScheme, SCHEME_WITH_PORT}, // WebSocket.
// field. Unlike other SCHEME_WITH_HOST schemes, the 'host' in a file
// URL may be empty, a behavior which is special-cased during
// canonicalization.
{kFileScheme, SCHEME_WITH_HOST},
{kFtpScheme, SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION},
{kGopherScheme, SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION},
{kWssScheme,
SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION}, // WebSocket secure.
{kWsScheme, SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION}, // WebSocket.
{kFileSystemScheme, SCHEME_WITHOUT_AUTHORITY},
};

const SchemeWithType kReferrerURLSchemes[] = {
{kHttpsScheme, SCHEME_WITH_PORT},
{kHttpScheme, SCHEME_WITH_PORT},
{kHttpsScheme, SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION},
{kHttpScheme, SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION},
};

const char* kSecureSchemes[] = {
Expand Down Expand Up @@ -241,7 +244,7 @@ bool DoCanonicalize(const CHAR* spec,
// This is the parsed version of the input URL, we have to canonicalize it
// before storing it in our object.
bool success;
SchemeType unused_scheme_type = SCHEME_WITH_PORT;
SchemeType unused_scheme_type = SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION;
if (DoCompareSchemeComponent(spec, scheme, url::kFileScheme)) {
// File URLs are special.
ParseFileURL(spec, spec_len, &parsed_input);
Expand Down Expand Up @@ -304,7 +307,7 @@ bool DoResolveRelative(const char* base_spec,
base_is_hierarchical = num_slashes > 0;
}

SchemeType unused_scheme_type = SCHEME_WITH_PORT;
SchemeType unused_scheme_type = SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION;
bool standard_base_scheme =
base_parsed.scheme.is_nonempty() &&
DoIsStandard(base_spec, base_parsed.scheme, &unused_scheme_type);
Expand Down Expand Up @@ -439,7 +442,7 @@ bool DoReplaceComponents(const char* spec,
return ReplaceFileSystemURL(spec, parsed, replacements, charset_converter,
output, out_parsed);
}
SchemeType unused_scheme_type = SCHEME_WITH_PORT;
SchemeType unused_scheme_type = SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION;
if (DoIsStandard(spec, parsed.scheme, &unused_scheme_type)) {
return ReplaceStandardURL(spec, parsed, replacements, charset_converter,
output, out_parsed);
Expand Down
17 changes: 11 additions & 6 deletions url_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,17 @@ URL_EXPORT void Shutdown();
// Types of a scheme representing the requirements on the data represented by
// the authority component of a URL with the scheme.
enum SchemeType {
// The authority component of a URL with the scheme, if any, has the port
// (the default values may be omitted in a serialization).
SCHEME_WITH_PORT,
// The authority component of a URL with the scheme, if any, doesn't have a
// port.
SCHEME_WITHOUT_PORT,
// The authority component of a URL with the scheme, if any, has the form
// "username:password@host:port". The username and password entries are
// optional; the host may not be empty. The default value of the port
// can be omitted in serialization. This type occurs with network schemes
// like http, https, and ftp.
SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION,
// The authority component of a URL with this scheme, if any, consists only
// of a host. It does not contain port, username, or password. Schemes used
// internally by browser features usually work this way, as hostnames do not
// correspond to network hosts.
SCHEME_WITH_HOST,
// A URL with the scheme doesn't have the authority component.
SCHEME_WITHOUT_AUTHORITY,
};
Expand Down
46 changes: 29 additions & 17 deletions url_util_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,19 @@

namespace url {

TEST(URLUtilTest, FindAndCompareScheme) {
class URLUtilTest : public testing::Test {
public:
URLUtilTest() = default;
~URLUtilTest() override {
// Reset any added schemes.
Shutdown();
}

private:
DISALLOW_COPY_AND_ASSIGN(URLUtilTest);
};

TEST_F(URLUtilTest, FindAndCompareScheme) {
Component found_scheme;

// Simple case where the scheme is found and matches.
Expand Down Expand Up @@ -63,54 +75,54 @@ TEST(URLUtilTest, FindAndCompareScheme) {
EXPECT_TRUE(found_scheme == Component(1, 11));
}

TEST(URLUtilTest, IsStandard) {
TEST_F(URLUtilTest, IsStandard) {
const char kHTTPScheme[] = "http";
EXPECT_TRUE(IsStandard(kHTTPScheme, Component(0, strlen(kHTTPScheme))));

const char kFooScheme[] = "foo";
EXPECT_FALSE(IsStandard(kFooScheme, Component(0, strlen(kFooScheme))));
}

TEST(URLUtilTest, IsReferrerScheme) {
TEST_F(URLUtilTest, IsReferrerScheme) {
const char kHTTPScheme[] = "http";
EXPECT_TRUE(IsReferrerScheme(kHTTPScheme, Component(0, strlen(kHTTPScheme))));

const char kFooScheme[] = "foo";
EXPECT_FALSE(IsReferrerScheme(kFooScheme, Component(0, strlen(kFooScheme))));
}

TEST(URLUtilTest, AddReferrerScheme) {
TEST_F(URLUtilTest, AddReferrerScheme) {
const char kFooScheme[] = "foo";
EXPECT_FALSE(IsReferrerScheme(kFooScheme, Component(0, strlen(kFooScheme))));
AddReferrerScheme(kFooScheme, url::SCHEME_WITHOUT_PORT);
AddReferrerScheme(kFooScheme, url::SCHEME_WITH_HOST);
EXPECT_TRUE(IsReferrerScheme(kFooScheme, Component(0, strlen(kFooScheme))));
}

TEST(URLUtilTest, GetStandardSchemeType) {
TEST_F(URLUtilTest, GetStandardSchemeType) {
url::SchemeType scheme_type;

const char kHTTPScheme[] = "http";
scheme_type = url::SCHEME_WITHOUT_AUTHORITY;
EXPECT_TRUE(GetStandardSchemeType(kHTTPScheme,
Component(0, strlen(kHTTPScheme)),
&scheme_type));
EXPECT_EQ(url::SCHEME_WITH_PORT, scheme_type);
EXPECT_EQ(url::SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION, scheme_type);

const char kFilesystemScheme[] = "filesystem";
scheme_type = url::SCHEME_WITH_PORT;
scheme_type = url::SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION;
EXPECT_TRUE(GetStandardSchemeType(kFilesystemScheme,
Component(0, strlen(kFilesystemScheme)),
&scheme_type));
EXPECT_EQ(url::SCHEME_WITHOUT_AUTHORITY, scheme_type);

const char kFooScheme[] = "foo";
scheme_type = url::SCHEME_WITH_PORT;
scheme_type = url::SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION;
EXPECT_FALSE(GetStandardSchemeType(kFooScheme,
Component(0, strlen(kFooScheme)),
&scheme_type));
}

TEST(URLUtilTest, ReplaceComponents) {
TEST_F(URLUtilTest, ReplaceComponents) {
Parsed parsed;
RawCanonOutputT<char> output;
Parsed new_parsed;
Expand Down Expand Up @@ -153,7 +165,7 @@ static std::string CheckReplaceScheme(const char* base_url,
return output_string;
}

TEST(URLUtilTest, ReplaceScheme) {
TEST_F(URLUtilTest, ReplaceScheme) {
EXPECT_EQ("https://google.com/",
CheckReplaceScheme("http://google.com/", "https"));
EXPECT_EQ("file://google.com/",
Expand Down Expand Up @@ -183,7 +195,7 @@ TEST(URLUtilTest, ReplaceScheme) {
CheckReplaceScheme("myscheme:example.com/ hello # world ", "http"));
}

TEST(URLUtilTest, DecodeURLEscapeSequences) {
TEST_F(URLUtilTest, DecodeURLEscapeSequences) {
struct DecodeCase {
const char* input;
const char* output;
Expand Down Expand Up @@ -254,7 +266,7 @@ TEST(URLUtilTest, DecodeURLEscapeSequences) {
}
}

TEST(URLUtilTest, TestEncodeURIComponent) {
TEST_F(URLUtilTest, TestEncodeURIComponent) {
struct EncodeCase {
const char* input;
const char* output;
Expand Down Expand Up @@ -287,7 +299,7 @@ TEST(URLUtilTest, TestEncodeURIComponent) {
}
}

TEST(URLUtilTest, TestResolveRelativeWithNonStandardBase) {
TEST_F(URLUtilTest, TestResolveRelativeWithNonStandardBase) {
// This tests non-standard (in the sense that IsStandard() == false)
// hierarchical schemes.
struct ResolveRelativeCase {
Expand Down Expand Up @@ -372,7 +384,7 @@ TEST(URLUtilTest, TestResolveRelativeWithNonStandardBase) {
}
}

TEST(URLUtilTest, TestNoRefComponent) {
TEST_F(URLUtilTest, TestNoRefComponent) {
// The hash-mark must be ignored when mailto: scheme is parsed,
// even if the URL has a base and relative part.
const char* base = "mailto://to/";
Expand All @@ -393,7 +405,7 @@ TEST(URLUtilTest, TestNoRefComponent) {
EXPECT_FALSE(resolved_parsed.ref.is_valid());
}

TEST(URLUtilTest, PotentiallyDanglingMarkup) {
TEST_F(URLUtilTest, PotentiallyDanglingMarkup) {
struct ResolveRelativeCase {
const char* base;
const char* rel;
Expand Down Expand Up @@ -441,7 +453,7 @@ TEST(URLUtilTest, PotentiallyDanglingMarkup) {
}
}

TEST(URLUtilTest, TestDomainIs) {
TEST_F(URLUtilTest, TestDomainIs) {
const struct {
const char* canonicalized_host;
const char* lower_ascii_domain;
Expand Down

0 comments on commit 9b53d43

Please sign in to comment.