Skip to content

Commit

Permalink
Introduce the concept of web schemes capable of storage
Browse files Browse the repository at this point in the history
Cookies, local storage, IndexedDB, etc. can be used for various URL schemes,
including Chrome-specific and internal ones such as chrome-extension://,
chrome://, chrome-devtools:// etc.

However, in the browsing_data/ codebase, when the user chooses to delete
"site data", we must only delete data from actual websites that user visited,
not data from Chrome's internal modules; that means data associated with
schemes like http[s]://, file://, etc.

Until now, this definition was defined as
ChildProcessSecurityPolicy::IsWebSafeScheme() minus the embedder-specific
schemes chrome-extension:// and chrome-devtools://.

As we're moving parts of browsing_data/ codebase to content/ (crbug.com/668114),
this approach is not satisfactory. We need a definition that can be used
in content/, and embedders can possibly add more schemes, but not remove
them.

This CL introduces such definition to url_utils.h.

BUG=668114

Review-Url: https://codereview.chromium.org/2661543003
Cr-Commit-Position: refs/heads/master@{#448957}
  • Loading branch information
msramek authored and Commit bot committed Feb 8, 2017
1 parent 4b9ec9d commit 7e5c61f
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 22 deletions.
14 changes: 5 additions & 9 deletions chrome/browser/browsing_data/browsing_data_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,18 @@

#include "chrome/browser/browsing_data/browsing_data_helper.h"

#include <vector>

#include "base/strings/utf_string_conversions.h"
#include "chrome/common/url_constants.h"
#include "content/public/browser/child_process_security_policy.h"
#include "extensions/common/constants.h"
#include "storage/browser/quota/special_storage_policy.h"
#include "url/gurl.h"
#include "url/url_util.h"

// Static
bool BrowsingDataHelper::IsWebScheme(const std::string& scheme) {
// All "web safe" schemes are valid, except `chrome-extension://`
// and `chrome-devtools://`.
content::ChildProcessSecurityPolicy* policy =
content::ChildProcessSecurityPolicy::GetInstance();
return (policy->IsWebSafeScheme(scheme) &&
!BrowsingDataHelper::IsExtensionScheme(scheme) &&
scheme != content::kChromeDevToolsScheme);
const std::vector<std::string>& schemes = url::GetWebStorageSchemes();
return std::find(schemes.begin(), schemes.end(), scheme) != schemes.end();
}

// Static
Expand Down
39 changes: 26 additions & 13 deletions chrome/browser/browsing_data/browsing_data_helper_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,16 @@ class BrowsingDataHelperTest : public testing::Test {
DISALLOW_COPY_AND_ASSIGN(BrowsingDataHelperTest);
};

TEST_F(BrowsingDataHelperTest, WebSafeSchemesAreWebSafe) {
TEST_F(BrowsingDataHelperTest, WebStorageSchemesAreWebSchemes) {
EXPECT_TRUE(IsWebScheme(url::kHttpScheme));
EXPECT_TRUE(IsWebScheme(url::kHttpsScheme));
EXPECT_TRUE(IsWebScheme(url::kFileScheme));
EXPECT_TRUE(IsWebScheme(url::kFtpScheme));
EXPECT_TRUE(IsWebScheme(url::kDataScheme));
EXPECT_TRUE(IsWebScheme("feed"));
EXPECT_TRUE(IsWebScheme(url::kBlobScheme));
EXPECT_TRUE(IsWebScheme(url::kFileSystemScheme));
EXPECT_FALSE(IsWebScheme("invalid-scheme-i-just-made-up"));
EXPECT_TRUE(IsWebScheme(url::kWsScheme));
EXPECT_TRUE(IsWebScheme(url::kWssScheme));
}

TEST_F(BrowsingDataHelperTest, ChromeSchemesAreNotWebSafe) {
TEST_F(BrowsingDataHelperTest, ChromeSchemesAreNotWebSchemes) {
EXPECT_FALSE(IsWebScheme(extensions::kExtensionScheme));
EXPECT_FALSE(IsWebScheme(url::kAboutScheme));
EXPECT_FALSE(IsWebScheme(content::kChromeDevToolsScheme));
Expand All @@ -82,15 +80,13 @@ TEST_F(BrowsingDataHelperTest, ChromeSchemesAreNotWebSafe) {
EXPECT_FALSE(IsWebScheme(content::kViewSourceScheme));
}

TEST_F(BrowsingDataHelperTest, WebSafeSchemesAreNotExtensions) {
TEST_F(BrowsingDataHelperTest, WebStorageSchemesAreNotExtensions) {
EXPECT_FALSE(IsExtensionScheme(url::kHttpScheme));
EXPECT_FALSE(IsExtensionScheme(url::kHttpsScheme));
EXPECT_FALSE(IsExtensionScheme(url::kFileScheme));
EXPECT_FALSE(IsExtensionScheme(url::kFtpScheme));
EXPECT_FALSE(IsExtensionScheme(url::kDataScheme));
EXPECT_FALSE(IsExtensionScheme("feed"));
EXPECT_FALSE(IsExtensionScheme(url::kBlobScheme));
EXPECT_FALSE(IsExtensionScheme(url::kFileSystemScheme));
EXPECT_FALSE(IsExtensionScheme("invalid-scheme-i-just-made-up"));
EXPECT_FALSE(IsExtensionScheme(url::kWsScheme));
EXPECT_FALSE(IsExtensionScheme(url::kWssScheme));
}

TEST_F(BrowsingDataHelperTest, ChromeSchemesAreNotAllExtension) {
Expand All @@ -104,6 +100,23 @@ TEST_F(BrowsingDataHelperTest, ChromeSchemesAreNotAllExtension) {
EXPECT_FALSE(IsExtensionScheme(content::kViewSourceScheme));
}

TEST_F(BrowsingDataHelperTest, SchemesThatCantStoreDataDontMatchAnything) {
EXPECT_FALSE(IsWebScheme(url::kDataScheme));
EXPECT_FALSE(IsExtensionScheme(url::kDataScheme));

EXPECT_FALSE(IsWebScheme("feed"));
EXPECT_FALSE(IsExtensionScheme("feed"));

EXPECT_FALSE(IsWebScheme(url::kBlobScheme));
EXPECT_FALSE(IsExtensionScheme(url::kBlobScheme));

EXPECT_FALSE(IsWebScheme(url::kFileSystemScheme));
EXPECT_FALSE(IsExtensionScheme(url::kFileSystemScheme));

EXPECT_FALSE(IsWebScheme("invalid-scheme-i-just-made-up"));
EXPECT_FALSE(IsExtensionScheme("invalid-scheme-i-just-made-up"));
}

#if BUILDFLAG(ENABLE_EXTENSIONS)
TEST_F(BrowsingDataHelperTest, TestMatches) {
scoped_refptr<MockExtensionSpecialStoragePolicy> mock_policy =
Expand Down
24 changes: 24 additions & 0 deletions url/url_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,15 @@ const char* kCORSEnabledSchemes[] = {
kDataScheme,
};

const char* kWebStorageSchemes[] = {
kHttpScheme,
kHttpsScheme,
kFileScheme,
kFtpScheme,
kWsScheme,
kWssScheme,
};

bool initialized = false;

// Lists of the currently installed standard and referrer schemes. These lists
Expand All @@ -86,6 +95,7 @@ std::vector<std::string>* secure_schemes = nullptr;
std::vector<std::string>* local_schemes = nullptr;
std::vector<std::string>* no_access_schemes = nullptr;
std::vector<std::string>* cors_enabled_schemes = nullptr;
std::vector<std::string>* web_storage_schemes = nullptr;

// See the LockSchemeRegistries declaration in the header.
bool scheme_registries_locked = false;
Expand Down Expand Up @@ -512,6 +522,8 @@ void Initialize() {
arraysize(kNoAccessSchemes));
InitSchemes(&cors_enabled_schemes, kCORSEnabledSchemes,
arraysize(kCORSEnabledSchemes));
InitSchemes(&web_storage_schemes, kWebStorageSchemes,
arraysize(kWebStorageSchemes));
initialized = true;
}

Expand All @@ -529,6 +541,8 @@ void Shutdown() {
no_access_schemes = nullptr;
delete cors_enabled_schemes;
cors_enabled_schemes = nullptr;
delete web_storage_schemes;
web_storage_schemes = nullptr;
}

void AddStandardScheme(const char* new_scheme, SchemeType type) {
Expand Down Expand Up @@ -581,6 +595,16 @@ const std::vector<std::string>& GetCORSEnabledSchemes() {
return *cors_enabled_schemes;
}

void AddWebStorageScheme(const char* new_scheme) {
Initialize();
DoAddScheme(new_scheme, web_storage_schemes);
}

const std::vector<std::string>& GetWebStorageSchemes() {
Initialize();
return *web_storage_schemes;
}

void LockSchemeRegistries() {
scheme_registries_locked = true;
}
Expand Down
8 changes: 8 additions & 0 deletions url/url_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,14 @@ URL_EXPORT const std::vector<std::string>& GetNoAccessSchemes();
URL_EXPORT void AddCORSEnabledScheme(const char* new_scheme);
URL_EXPORT const std::vector<std::string>& GetCORSEnabledSchemes();

// Adds an application-defined scheme to the list of web schemes that can be
// used by web to store data (e.g. cookies, local storage, ...). This is
// to differentiate them from schemes that can store data but are not used on
// web (e.g. application's internal schemes) or schemes that are used on web but
// cannot store data.
URL_EXPORT void AddWebStorageScheme(const char* new_scheme);
URL_EXPORT const std::vector<std::string>& GetWebStorageSchemes();

// Sets a flag to prevent future calls to Add*Scheme from succeeding.
//
// This is designed to help prevent errors for multithreaded applications.
Expand Down

0 comments on commit 7e5c61f

Please sign in to comment.