From 188827c86eca6fa38e6cef5ac9852a26826a658a Mon Sep 17 00:00:00 2001 From: "bauerb@chromium.org" Date: Thu, 30 Jun 2011 08:47:58 +0000 Subject: Use extension match pattern syntax in content settings extension API. This requires adding a port to a URLPattern, but that shouldn't change existing behavior, as we already have a lenient parsing mode there. BUG=71067 Review URL: http://codereview.chromium.org/7229012 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@91099 0039d316-1c4b-4281-b951-d872f2087c98 --- .../extensions/extension_content_settings_api.cc | 30 ++++----- .../extension_content_settings_helpers.cc | 75 ++++++++++++++++++++++ .../extension_content_settings_helpers.h | 9 +++ .../extension_content_settings_unittest.cc | 51 +++++++++++++++ .../extensions/extension_context_menu_api.cc | 2 +- chrome/browser/extensions/extension_prefs.cc | 2 +- .../browser/extensions/extension_webrequest_api.cc | 2 +- chrome/browser/extensions/user_script_master.cc | 2 +- .../render_view_context_menu_unittest.cc | 2 +- 9 files changed, 154 insertions(+), 21 deletions(-) create mode 100644 chrome/browser/extensions/extension_content_settings_unittest.cc (limited to 'chrome/browser') diff --git a/chrome/browser/extensions/extension_content_settings_api.cc b/chrome/browser/extensions/extension_content_settings_api.cc index eaed7c0..58f8343 100644 --- a/chrome/browser/extensions/extension_content_settings_api.cc +++ b/chrome/browser/extensions/extension_content_settings_api.cc @@ -169,29 +169,27 @@ bool SetContentSettingFunction::RunImpl() { DictionaryValue* details = NULL; EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(1, &details)); - DictionaryValue* top_level_pattern_dict = NULL; - EXTENSION_FUNCTION_VALIDATE( - details->GetDictionary(keys::kTopLevelPatternKey, - &top_level_pattern_dict)); std::string top_level_pattern_str; + std::string top_level_error; EXTENSION_FUNCTION_VALIDATE( - top_level_pattern_dict->GetString(keys::kPatternKey, - &top_level_pattern_str)); + details->GetString(keys::kTopLevelPatternKey, &top_level_pattern_str)); ContentSettingsPattern top_level_pattern = - ContentSettingsPattern::FromString(top_level_pattern_str); - EXTENSION_FUNCTION_VALIDATE(top_level_pattern.IsValid()); + helpers::ParseExtensionPattern(top_level_pattern_str, &top_level_error); + if (!top_level_pattern.IsValid()) { + error_ = top_level_error; + return false; + } - DictionaryValue* embedded_pattern_dict = NULL; - EXTENSION_FUNCTION_VALIDATE( - details->GetDictionary(keys::kEmbeddedPatternKey, - &embedded_pattern_dict)); std::string embedded_pattern_str; + std::string embedded_error; EXTENSION_FUNCTION_VALIDATE( - embedded_pattern_dict->GetString(keys::kPatternKey, - &embedded_pattern_str)); + details->GetString(keys::kEmbeddedPatternKey, &embedded_pattern_str)); ContentSettingsPattern embedded_pattern = - ContentSettingsPattern::FromString(embedded_pattern_str); - EXTENSION_FUNCTION_VALIDATE(embedded_pattern.IsValid()); + helpers::ParseExtensionPattern(embedded_pattern_str, &embedded_error); + if (!embedded_pattern.IsValid()) { + error_ = embedded_error; + return false; + } std::string resource_identifier; if (details->HasKey(keys::kResourceIdentifierKey)) { diff --git a/chrome/browser/extensions/extension_content_settings_helpers.cc b/chrome/browser/extensions/extension_content_settings_helpers.cc index c250e7b1..bb24955 100644 --- a/chrome/browser/extensions/extension_content_settings_helpers.cc +++ b/chrome/browser/extensions/extension_content_settings_helpers.cc @@ -6,9 +6,17 @@ #include "base/basictypes.h" #include "base/logging.h" +#include "base/scoped_ptr.h" +#include "chrome/common/extensions/url_pattern.h" +#include "content/common/url_constants.h" namespace { +const char kNoPathWildcardsError[] = + "Path wildcards in file URL patterns are not allowed."; +const char kNoPathsError[] = "Specific paths are not allowed."; +const char kInvalidPatternError[] = "The pattern \"*\" is invalid."; + const char* const kContentSettingsTypeNames[] = { "cookies", "images", @@ -33,10 +41,77 @@ COMPILE_ASSERT(arraysize(kContentSettingNames) <= CONTENT_SETTING_NUM_SETTINGS, content_setting_names_size_invalid); +// TODO(bauerb): Move this someplace where it can be reused. +std::string GetDefaultPort(const std::string& scheme) { + if (scheme == chrome::kHttpScheme) + return "80"; + if (scheme == chrome::kHttpsScheme) + return "443"; + NOTREACHED(); + return ""; +} + } // namespace namespace extension_content_settings_helpers { +ContentSettingsPattern ParseExtensionPattern(const std::string& pattern_str, + std::string* error) { + URLPattern url_pattern(URLPattern::SCHEME_HTTP | + URLPattern::SCHEME_HTTPS | + URLPattern::SCHEME_FILE); + URLPattern::ParseResult result = + url_pattern.Parse(pattern_str, URLPattern::USE_PORTS); + if (result != URLPattern::PARSE_SUCCESS) { + *error = URLPattern::GetParseResultString(result); + return ContentSettingsPattern(); + } else { + scoped_ptr builder( + ContentSettingsPattern::CreateBuilder(false)); + builder->WithHost(url_pattern.host()); + if (url_pattern.match_subdomains()) + builder->WithDomainWildcard(); + + std::string scheme = url_pattern.scheme(); + if (scheme == "*") + builder->WithSchemeWildcard(); + else + builder->WithScheme(scheme); + + std::string port = url_pattern.port(); + if (port.empty() && scheme != "file") { + if (scheme == "*") + port = "*"; + else + port = GetDefaultPort(scheme); + } + if (port == "*") + builder->WithPortWildcard(); + else + builder->WithPort(port); + + std::string path = url_pattern.path(); + if (scheme == "file") { + // For file URLs we allow only exact path matches. + if (path.find_first_of("*?") != std::string::npos) { + *error = kNoPathWildcardsError; + return ContentSettingsPattern(); + } else { + builder->WithPath(path); + } + } else if (path != "/*") { + // For other URLs we allow only paths which match everything. + *error = kNoPathsError; + return ContentSettingsPattern(); + } + + ContentSettingsPattern pattern = builder->Build(); + if (!pattern.IsValid()) + *error = kInvalidPatternError; + return pattern; + } +} + ContentSettingsType StringToContentSettingsType( const std::string& content_type) { for (size_t type = 0; type < arraysize(kContentSettingsTypeNames); ++type) { diff --git a/chrome/browser/extensions/extension_content_settings_helpers.h b/chrome/browser/extensions/extension_content_settings_helpers.h index c7c6f25..b3e4ce7 100644 --- a/chrome/browser/extensions/extension_content_settings_helpers.h +++ b/chrome/browser/extensions/extension_content_settings_helpers.h @@ -8,10 +8,19 @@ #include +#include "chrome/browser/content_settings/content_settings_pattern.h" #include "chrome/common/content_settings.h" namespace extension_content_settings_helpers { +// Parses an extension match pattern and returns a corresponding +// content settings pattern object. +// If |pattern_str| is invalid or can't be converted to a content settings +// pattern, |error| is set to the parsing error and an invalid pattern +// is returned. +ContentSettingsPattern ParseExtensionPattern(const std::string& pattern_str, + std::string* error); + // Converts a content settings type string to the corresponding // ContentSettingsType. Returns CONTENT_SETTINGS_TYPE_DEFAULT if the string // didn't specify a valid content settings type. diff --git a/chrome/browser/extensions/extension_content_settings_unittest.cc b/chrome/browser/extensions/extension_content_settings_unittest.cc new file mode 100644 index 0000000..500afc2 --- /dev/null +++ b/chrome/browser/extensions/extension_content_settings_unittest.cc @@ -0,0 +1,51 @@ +// Copyright (c) 2011 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 "testing/gtest/include/gtest/gtest.h" + +#include "chrome/browser/extensions/extension_content_settings_helpers.h" + +namespace helpers = extension_content_settings_helpers; + +TEST(ExtensionContentSettingsHelpersTest, ParseExtensionPattern) { + const struct { + const char* extension_pattern; + const char* content_settings_pattern; + } kTestPatterns[] = { + { "", "*" }, + { "*://*.google.com/*", "[*.]google.com" }, + { "http://www.example.com/*", "http://www.example.com" }, + { "*://www.example.com/*", "www.example.com" }, + { "http://www.example.com:8080/*", "http://www.example.com:8080" }, + { "https://*/*", "https://*" }, + { "file:///foo/bar/baz", "file:///foo/bar/baz" }, + }; + for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kTestPatterns); ++i) { + std::string error; + std::string pattern_str = helpers::ParseExtensionPattern( + kTestPatterns[i].extension_pattern, &error).ToString(); + EXPECT_EQ(kTestPatterns[i].content_settings_pattern, pattern_str) + << "Unexpected error parsing " << kTestPatterns[i].extension_pattern + << ": " << error; + } + + const struct { + const char* extension_pattern; + const char* expected_error; + } kInvalidTestPatterns[] = { + { "http://www.example.com/path", "Specific paths are not allowed." }, + { "file:///foo/bar/*", + "Path wildcards in file URL patterns are not allowed." }, + }; + for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kInvalidTestPatterns); ++i) { + std::string error; + ContentSettingsPattern pattern = helpers::ParseExtensionPattern( + kInvalidTestPatterns[i].extension_pattern, &error); + EXPECT_FALSE(pattern.IsValid()); + EXPECT_EQ(kInvalidTestPatterns[i].expected_error, error) + << "Unexpected error parsing " + << kInvalidTestPatterns[i].extension_pattern; + } + +} diff --git a/chrome/browser/extensions/extension_context_menu_api.cc b/chrome/browser/extensions/extension_context_menu_api.cc index 50d08b7..d5f1e01 100644 --- a/chrome/browser/extensions/extension_context_menu_api.cc +++ b/chrome/browser/extensions/extension_context_menu_api.cc @@ -143,7 +143,7 @@ bool ExtensionContextMenuFunction::ParseURLPatterns( // TODO(skerner): Consider enabling strict pattern parsing // if this extension's location indicates that it is under development. if (URLPattern::PARSE_SUCCESS != pattern.Parse(tmp, - URLPattern::PARSE_LENIENT)) { + URLPattern::IGNORE_PORTS)) { error_ = ExtensionErrorUtils::FormatErrorMessage(kInvalidURLPatternError, tmp); return false; diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc index d7b63dc..600dea7 100644 --- a/chrome/browser/extensions/extension_prefs.cc +++ b/chrome/browser/extensions/extension_prefs.cc @@ -451,7 +451,7 @@ bool ExtensionPrefs::ReadExtensionPrefURLPatternSet( if (!value->GetString(i, &item)) return false; URLPattern pattern(valid_schemes); - if (pattern.Parse(item, URLPattern::PARSE_LENIENT) != + if (pattern.Parse(item, URLPattern::IGNORE_PORTS) != URLPattern::PARSE_SUCCESS) { NOTREACHED(); return false; diff --git a/chrome/browser/extensions/extension_webrequest_api.cc b/chrome/browser/extensions/extension_webrequest_api.cc index ce46469..df3a9ee 100644 --- a/chrome/browser/extensions/extension_webrequest_api.cc +++ b/chrome/browser/extensions/extension_webrequest_api.cc @@ -238,7 +238,7 @@ bool ExtensionWebRequestEventRouter::RequestFilter::InitFromValue( std::string url; URLPattern pattern(URLPattern::SCHEME_ALL); if (!urls_value->GetString(i, &url) || - pattern.Parse(url, URLPattern::PARSE_STRICT) != + pattern.Parse(url, URLPattern::ERROR_ON_PORTS) != URLPattern::PARSE_SUCCESS) { *error = ExtensionErrorUtils::FormatErrorMessage( keys::kInvalidRequestFilterUrl, url); diff --git a/chrome/browser/extensions/user_script_master.cc b/chrome/browser/extensions/user_script_master.cc index 60f8be0..3772ac6 100644 --- a/chrome/browser/extensions/user_script_master.cc +++ b/chrome/browser/extensions/user_script_master.cc @@ -107,7 +107,7 @@ bool UserScriptMaster::ScriptReloader::ParseMetadataHeader( } else if (GetDeclarationValue(line, kMatchDeclaration, &value)) { URLPattern pattern(UserScript::kValidUserScriptSchemes); if (URLPattern::PARSE_SUCCESS != - pattern.Parse(value, URLPattern::PARSE_LENIENT)) + pattern.Parse(value, URLPattern::IGNORE_PORTS)) return false; script->add_url_pattern(pattern); } else if (GetDeclarationValue(line, kRunAtDeclaration, &value)) { diff --git a/chrome/browser/tab_contents/render_view_context_menu_unittest.cc b/chrome/browser/tab_contents/render_view_context_menu_unittest.cc index 02ffe30..5589354 100644 --- a/chrome/browser/tab_contents/render_view_context_menu_unittest.cc +++ b/chrome/browser/tab_contents/render_view_context_menu_unittest.cc @@ -73,7 +73,7 @@ static ContextMenuParams CreateParams(int contexts) { // Generates a URLPatternSet with a single pattern static URLPatternSet CreatePatternSet(const std::string& pattern) { URLPattern target(URLPattern::SCHEME_HTTP); - target.Parse(pattern, URLPattern::PARSE_LENIENT); + target.Parse(pattern, URLPattern::IGNORE_PORTS); URLPatternSet rv; rv.AddPattern(target); -- cgit v1.1