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.

TBR=rogerta@chromium.org,fukino@chromium.org,sdefresne@chromium.org

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 <nick@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547316}
  • Loading branch information
nick-chromium authored and Commit Bot committed Mar 30, 2018
1 parent 9b4794e commit 123ca19
Show file tree
Hide file tree
Showing 15 changed files with 109 additions and 71 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,7 @@ TEST(ChromeOSFileSystemBackendTest, GetRootDirectories) {
}

TEST(ChromeOSFileSystemBackendTest, AccessPermissions) {
url::AddStandardScheme(extensions::kExtensionScheme,
url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme(extensions::kExtensionScheme, url::SCHEME_WITH_HOST);

scoped_refptr<storage::ExternalMountPoints> mount_points(
storage::ExternalMountPoints::CreateRefCounted());
Expand Down
12 changes: 7 additions & 5 deletions components/test/components_test_suite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ class ComponentsTestSuite : public base::TestSuite {

mojo::edk::Init();

// Before registering any schemes, clear GURL's internal state.
url::Shutdown();

#if !defined(OS_IOS)
gl::GLSurfaceTestSupport::InitializeOneOff();

Expand Down Expand Up @@ -79,10 +82,10 @@ class ComponentsTestSuite : public base::TestSuite {

// These schemes need to be added globally to pass tests of
// autocomplete_input_unittest.cc and content_settings_pattern*
url::AddStandardScheme("chrome", url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme("chrome-extension", url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme("chrome-devtools", url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme("chrome-search", url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme("chrome", url::SCHEME_WITH_HOST);
url::AddStandardScheme("chrome-extension", url::SCHEME_WITH_HOST);
url::AddStandardScheme("chrome-devtools", url::SCHEME_WITH_HOST);
url::AddStandardScheme("chrome-search", url::SCHEME_WITH_HOST);

ContentSettingsPattern::SetNonWildcardDomainNonPortSchemes(
kNonWildcardDomainNonPortSchemes,
Expand All @@ -91,7 +94,6 @@ class ComponentsTestSuite : public base::TestSuite {

void Shutdown() override {
ui::ResourceBundle::CleanupSharedInstance();

base::TestSuite::Shutdown();
}

Expand Down
6 changes: 4 additions & 2 deletions content/browser/site_instance_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ class SiteInstanceTest : public testing::Test {

void SetUp() override {
old_browser_client_ = SetBrowserClientForTesting(&browser_client_);
url::AddStandardScheme(kPrivilegedScheme, url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme(kChromeUIScheme, url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme(kPrivilegedScheme, url::SCHEME_WITH_HOST);
url::AddStandardScheme(kChromeUIScheme, url::SCHEME_WITH_HOST);

RenderProcessHostImpl::set_render_process_host_factory(&rph_factory_);
}
Expand All @@ -127,6 +127,8 @@ class SiteInstanceTest : public testing::Test {
// scheduled for deletion. Here, call DrainMessageLoop() again so the
// AppCacheDatabase actually gets deleted.
DrainMessageLoop();

ResetSchemesAndOriginsWhitelist();
}

void set_privileged_process_id(int process_id) {
Expand Down
10 changes: 5 additions & 5 deletions content/common/url_schemes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@ void RegisterContentSchemes(bool lock_schemes) {
ContentClient::Schemes schemes;
GetContentClient()->AddAdditionalSchemes(&schemes);

url::AddStandardScheme(kChromeDevToolsScheme, url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme(kChromeUIScheme, url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme(kGuestScheme, url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme(kChromeDevToolsScheme, url::SCHEME_WITH_HOST);
url::AddStandardScheme(kChromeUIScheme, url::SCHEME_WITH_HOST);
url::AddStandardScheme(kGuestScheme, url::SCHEME_WITH_HOST);

for (auto& scheme : schemes.standard_schemes)
url::AddStandardScheme(scheme.c_str(), url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme(scheme.c_str(), url::SCHEME_WITH_HOST);

for (auto& scheme : schemes.referrer_schemes)
url::AddReferrerScheme(scheme.c_str(), url::SCHEME_WITHOUT_PORT);
url::AddReferrerScheme(scheme.c_str(), url::SCHEME_WITH_HOST);

schemes.secure_schemes.push_back(kChromeUIScheme);
schemes.secure_schemes.push_back(kChromeErrorScheme);
Expand Down
7 changes: 5 additions & 2 deletions google_apis/drive/drive_api_url_generator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ class DriveApiUrlGeneratorTest : public testing::Test {
TEAM_DRIVES_INTEGRATION_DISABLED),
team_drives_url_generator_(GURL(kBaseUrlForTesting),
GURL(kBaseThumbnailUrlForTesting),
TEAM_DRIVES_INTEGRATION_ENABLED) {}
TEAM_DRIVES_INTEGRATION_ENABLED) {
url::AddStandardScheme("chrome-extension", url::SCHEME_WITH_HOST);
}

~DriveApiUrlGeneratorTest() override { url::Shutdown(); }

protected:
DriveApiUrlGenerator url_generator_;
Expand Down Expand Up @@ -75,7 +79,6 @@ TEST_F(DriveApiUrlGeneratorTest, GetFilesGetUrl) {
url_generator_.GetFilesGetUrl("0ADK06pfg", true, GURL()).spec());

// If |embed_origin| is not empty, it should be added as a query parameter.
url::AddStandardScheme("chrome-extension", url::SCHEME_WITHOUT_PORT);
EXPECT_EQ(
"https://www.example.com/drive/v2/files/0ADK06pfg"
"?embedOrigin=chrome-extension%3A%2F%2Ftest",
Expand Down
2 changes: 1 addition & 1 deletion ios/chrome/browser/u2f/u2f_controller_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
class U2FControllerTest : public PlatformTest {
protected:
U2FControllerTest() : _U2FController([[U2FController alloc] init]) {
url::AddStandardScheme("chromium", url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme("chromium", url::SCHEME_WITH_HOST);
[[ChromeAppConstants sharedInstance]
setCallbackSchemeForTesting:@"chromium"];
}
Expand Down
2 changes: 1 addition & 1 deletion ios/chrome/test/ios_chrome_unit_test_suite.mm
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,6 @@ void OnTestEnd(const testing::TestInfo& test_info) override {

ios::RegisterPathProvider();
ui::RegisterPathProvider();
url::AddStandardScheme(kChromeUIScheme, url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme(kChromeUIScheme, url::SCHEME_WITH_HOST);
ContentSettingsPattern::SetNonWildcardDomainNonPortSchemes(nullptr, 0);
}
2 changes: 1 addition & 1 deletion ios/web/navigation/navigation_manager_impl_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ void SetSessionController(CRWSessionController* session_controller) {

// Setup rewriter.
BrowserURLRewriter::GetInstance()->AddURLRewriter(UrlRewriter);
url::AddStandardScheme(kSchemeToRewrite, url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme(kSchemeToRewrite, url::SCHEME_WITH_HOST);

manager_->SetDelegate(&delegate_);
manager_->SetBrowserState(&browser_state_);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ void RemoveWebView() override {
manager_->SetBrowserState(&browser_state_);

BrowserURLRewriter::GetInstance()->AddURLRewriter(WebUIUrlRewriter);
url::AddStandardScheme(kSchemeToRewrite, url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme(kSchemeToRewrite, url::SCHEME_WITH_HOST);
}

std::unique_ptr<NavigationManagerImpl> manager_;
Expand Down
2 changes: 1 addition & 1 deletion ios/web/public/url_schemes.mm
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ void RegisterWebSchemes(bool lock_schemes) {
web::WebClient::Schemes schemes;
GetWebClient()->AddAdditionalSchemes(&schemes);
for (const auto& scheme : schemes.standard_schemes)
url::AddStandardScheme(scheme.c_str(), url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme(scheme.c_str(), url::SCHEME_WITH_HOST);

for (const auto& scheme : schemes.secure_schemes)
url::AddSecureScheme(scheme.c_str());
Expand Down
6 changes: 3 additions & 3 deletions url/scheme_host_port.cc
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 url/scheme_host_port_unittest.cc
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/url_util.cc
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/url_util.h
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
Loading

0 comments on commit 123ca19

Please sign in to comment.