diff options
Diffstat (limited to 'chrome/common')
| -rw-r--r-- | chrome/common/extensions/extension.cc | 198 | ||||
| -rw-r--r-- | chrome/common/extensions/extension.h | 17 | ||||
| -rw-r--r-- | chrome/common/extensions/extension_manifests_unittest.cc | 198 |
3 files changed, 213 insertions, 200 deletions
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc index 8d80814..0ad83a1f 100644 --- a/chrome/common/extensions/extension.cc +++ b/chrome/common/extensions/extension.cc @@ -1129,11 +1129,6 @@ bool Extension::LoadLaunchContainer(const DictionaryValue* manifest, bool Extension::LoadAppIsolation(const DictionaryValue* manifest, std::string* error) { - // Only parse app isolation features if this switch is present. - if (!CommandLine::ForCurrentProcess()->HasSwitch( - switches::kEnableExperimentalExtensionApis)) - return true; - Value* temp = NULL; if (!manifest->Get(keys::kIsolation, &temp)) return true; @@ -1496,6 +1491,30 @@ bool Extension::InitFromValue(const DictionaryValue& source, int flags, base::i18n::AdjustStringForLocaleDirection(&localized_name); name_ = UTF16ToUTF8(localized_name); + // Initialize the permissions (optional). + ExtensionAPIPermissionSet api_permissions; + URLPatternSet host_permissions; + if (!ParsePermissions(&source, + keys::kPermissions, + flags, + error, + &api_permissions, + &host_permissions)) { + return false; + } + + // Initialize the optional permissions (optional). + ExtensionAPIPermissionSet optional_api_permissions; + URLPatternSet optional_host_permissions; + if (!ParsePermissions(&source, + keys::kOptionalPermissions, + flags, + error, + &optional_api_permissions, + &optional_host_permissions)) { + return false; + } + // Initialize description (if present). if (source.HasKey(keys::kDescription)) { if (!source.GetString(keys::kDescription, @@ -1783,11 +1802,10 @@ bool Extension::InitFromValue(const DictionaryValue& source, int flags, } } - // Initialize toolstrips. This is deprecated for public use. - // NOTE(erikkay) Although deprecated, we intend to preserve this parsing - // code indefinitely. Please contact me or Joi for details as to why. - if (CommandLine::ForCurrentProcess()->HasSwitch( - switches::kEnableExperimentalExtensionApis) && + // Initialize toolstrips. + // TODO(aa): Remove this and all the related tests, docs, etc. + // See: crbug.com/100488. + if (api_permissions.count(ExtensionAPIPermission::kExperimental) && source.HasKey(keys::kToolstrips)) { ListValue* list_value = NULL; if (!source.GetList(keys::kToolstrips, &list_value)) { @@ -1923,9 +1941,12 @@ bool Extension::InitFromValue(const DictionaryValue& source, int flags, parse_strictness, error) || !EnsureNotHybridApp(manifest_value_.get(), error) || !LoadLaunchURL(manifest_value_.get(), error) || - !LoadLaunchContainer(manifest_value_.get(), error) || - !LoadAppIsolation(manifest_value_.get(), error)) { + !LoadLaunchContainer(manifest_value_.get(), error)) return false; + + if (api_permissions.count(ExtensionAPIPermission::kExperimental)) { + if (!LoadAppIsolation(manifest_value_.get(), error)) + return false; } // Initialize options page url (optional). @@ -1960,30 +1981,6 @@ bool Extension::InitFromValue(const DictionaryValue& source, int flags, } } - // Initialize the permissions (optional). - ExtensionAPIPermissionSet api_permissions; - URLPatternSet host_permissions; - if (!ParsePermissions(&source, - keys::kPermissions, - flags, - error, - &api_permissions, - &host_permissions)) { - return false; - } - - // Initialize the optional permissions (optional). - ExtensionAPIPermissionSet optional_api_permissions; - URLPatternSet optional_host_permissions; - if (!ParsePermissions(&source, - keys::kOptionalPermissions, - flags, - error, - &optional_api_permissions, - &optional_host_permissions)) { - return false; - } - // Initialize background url (optional). if (source.HasKey(keys::kBackground)) { std::string background_str; @@ -2070,8 +2067,7 @@ bool Extension::InitFromValue(const DictionaryValue& source, int flags, } } - if (CommandLine::ForCurrentProcess()->HasSwitch( - switches::kEnableExperimentalExtensionApis) && + if (api_permissions.count(ExtensionAPIPermission::kExperimental) && source.HasKey(keys::kInputComponents)) { ListValue* list_value = NULL; if (!source.GetList(keys::kInputComponents, &list_value)) { @@ -2598,41 +2594,16 @@ bool Extension::ParsePermissions(const DictionaryValue* source, ExtensionAPIPermission* permission = ExtensionPermissionsInfo::GetInstance()->GetByName(permission_str); - // Only COMPONENT extensions can use private APIs. - // TODO(asargent) - We want a more general purpose mechanism for this, - // and better error messages. (http://crbug.com/54013) - if (!IsComponentOnlyPermission(permission) -#ifndef NDEBUG - && !CommandLine::ForCurrentProcess()->HasSwitch( - switches::kExposePrivateExtensionApi) -#endif - ) { - continue; - } - - if (web_extent().is_empty() || location() == Extension::COMPONENT) { - // Check if it's a module permission. If so, enable that permission. - if (permission != NULL) { - // Only allow the experimental API permission if the command line - // flag is present, or if the extension is a component of Chrome. - if (IsDisallowedExperimentalPermission(permission->id()) && - location() != Extension::COMPONENT) { - *error = errors::kExperimentalFlagRequired; - return false; - } - api_permissions->insert(permission->id()); - continue; - } - } else { - // Hosted apps only get access to a subset of the valid permissions. - if (permission != NULL && permission->is_hosted_app()) { - if (IsDisallowedExperimentalPermission(permission->id())) { - *error = errors::kExperimentalFlagRequired; - return false; - } + if (permission != NULL) { + if (CanSpecifyAPIPermission(permission, error)) api_permissions->insert(permission->id()); - continue; - } + + // Sometimes when you can't specify an API permission we ignore it + // silently. This seems like a bug. Specifically, crbug.com/100489. + if (!error->empty()) + return false; + + continue; } // Check if it's a host pattern permission. @@ -2760,17 +2731,6 @@ scoped_refptr<const ExtensionPermissionSet> return runtime_data_.GetActivePermissions(); } -bool Extension::IsComponentOnlyPermission( - const ExtensionAPIPermission* api) const { - if (location() == Extension::COMPONENT) - return true; - - if (api == NULL) - return true; - - return !api->is_component_only(); -} - bool Extension::HasMultipleUISurfaces() const { int num_surfaces = 0; @@ -2842,11 +2802,73 @@ bool Extension::ImplicitlyDelaysNetworkStartup() const { HasAPIPermission(ExtensionAPIPermission::kWebRequest); } -bool Extension::IsDisallowedExperimentalPermission( - ExtensionAPIPermission::ID permission) const { - return permission == ExtensionAPIPermission::kExperimental && - !CommandLine::ForCurrentProcess()->HasSwitch( - switches::kEnableExperimentalExtensionApis); +bool Extension::CanSpecifyAPIPermission( + const ExtensionAPIPermission* permission, + std::string* error) const { + if (permission->is_component_only()) { + if (!CanSpecifyComponentOnlyPermission()) + return false; + } + + if (permission->id() == ExtensionAPIPermission::kExperimental) { + if (!CanSpecifyExperimentalPermission()) { + *error = errors::kExperimentalFlagRequired; + return false; + } + } + + if (is_hosted_app()) { + if (!CanSpecifyPermissionForHostedApp(permission)) + return false; + } + + return true; +} + +bool Extension::CanSpecifyComponentOnlyPermission() const { + // Only COMPONENT extensions can use private APIs. + // TODO(asargent) - We want a more general purpose mechanism for this, + // and better error messages. (http://crbug.com/54013) + if (location_ == Extension::COMPONENT) + return true; + +#ifndef NDEBUG + if (CommandLine::ForCurrentProcess()->HasSwitch( + switches::kExposePrivateExtensionApi)) { + return true; + } +#endif + + return false; +} + +bool Extension::CanSpecifyExperimentalPermission() const { + if (location_ == Extension::COMPONENT) + return true; + + if (CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableExperimentalExtensionApis)) { + return true; + } + + // We rely on the webstore to check access to experimental. This way we can + // whitelist extensions to have access to experimental in just the store, and + // not have to push a new version of the client. + if (from_webstore()) + return true; + + return false; +} + +bool Extension::CanSpecifyPermissionForHostedApp( + const ExtensionAPIPermission* permission) const { + if (location_ == Extension::COMPONENT) + return true; + + if (permission->is_hosted_app()) + return true; + + return false; } bool Extension::CanExecuteScriptEverywhere() const { diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h index bdf493f..a97f858 100644 --- a/chrome/common/extensions/extension.h +++ b/chrome/common/extensions/extension.h @@ -384,6 +384,14 @@ class Extension : public base::RefCountedThreadSafe<Extension> { // mechanisms that are implicitly trusted. bool CanSilentlyIncreasePermissions() const; + // Returns true if this extension can specify |api|. + bool CanSpecifyAPIPermission(const ExtensionAPIPermission* api, + std::string* error) const; + bool CanSpecifyComponentOnlyPermission() const; + bool CanSpecifyExperimentalPermission() const; + bool CanSpecifyPermissionForHostedApp( + const ExtensionAPIPermission* api) const; + // Whether or not the extension is allowed permission for a URL pattern from // the manifest. http, https, and chrome://favicon/ is allowed for all // extensions, while component extensions are allowed access to @@ -679,15 +687,6 @@ class Extension : public base::RefCountedThreadSafe<Extension> { // don't want to allow scripts and such to be bundled with themes. bool ContainsNonThemeKeys(const base::DictionaryValue& source) const; - // Only allow the experimental API permission if the command line - // flag is present. - bool IsDisallowedExperimentalPermission( - ExtensionAPIPermission::ID permission) const; - - // Returns true if this is a component, or we are not attempting to access a - // component-private permission. - bool IsComponentOnlyPermission(const ExtensionAPIPermission* api) const; - // Updates the launch URL and extents for the extension using the given // |override_url|. void OverrideLaunchUrl(const GURL& override_url); diff --git a/chrome/common/extensions/extension_manifests_unittest.cc b/chrome/common/extensions/extension_manifests_unittest.cc index 367cd3c..7c41fd5 100644 --- a/chrome/common/extensions/extension_manifests_unittest.cc +++ b/chrome/common/extensions/extension_manifests_unittest.cc @@ -48,8 +48,8 @@ class ExtensionManifestTest : public testing::Test { ExtensionManifestTest() : enable_apps_(true) {} protected: - DictionaryValue* LoadManifestFile(const std::string& filename, - std::string* error) { + static DictionaryValue* LoadManifestFile(const std::string& filename, + std::string* error) { FilePath path; PathService::Get(chrome::DIR_TEST_DATA, &path); path = path.AppendASCII("extensions") @@ -61,70 +61,59 @@ class ExtensionManifestTest : public testing::Test { return static_cast<DictionaryValue*>(serializer.Deserialize(NULL, error)); } - scoped_refptr<Extension> LoadExtensionWithLocation( - DictionaryValue* value, - Extension::Location location, - bool strict_error_checks, - std::string* error) { - FilePath path; - PathService::Get(chrome::DIR_TEST_DATA, &path); - path = path.AppendASCII("extensions").AppendASCII("manifest_tests"); - int flags = Extension::NO_FLAGS; - if (strict_error_checks) - flags |= Extension::STRICT_ERROR_CHECKS; - return Extension::Create(path.DirName(), location, *value, flags, error); - } + // Helper class that simplifies creating methods that take either a filename + // to a manifest or the manifest itself. + class Manifest { + public: + // Purposely not marked explicit for convenience. The vast majority of + // callers pass string literal. + Manifest(const char* name) + : name_(name), manifest_(NULL) { + } + Manifest(DictionaryValue* manifest, const char* name) + : name_(name), manifest_(manifest) { + } - scoped_refptr<Extension> LoadExtension(const std::string& name, - std::string* error) { - return LoadExtensionWithLocation(name, Extension::INTERNAL, false, error); - } + const std::string& name() const { return name_; } - scoped_refptr<Extension> LoadExtensionStrict(const std::string& name, - std::string* error) { - return LoadExtensionWithLocation(name, Extension::INTERNAL, true, error); - } + DictionaryValue* GetManifest(std::string* error) const { + if (manifest_) + return manifest_; - scoped_refptr<Extension> LoadExtension(DictionaryValue* value, - std::string* error) { - // Loading as an installed extension disables strict error checks. - return LoadExtensionWithLocation(value, Extension::INTERNAL, false, error); - } - - scoped_refptr<Extension> LoadExtensionWithLocation( - const std::string& name, - Extension::Location location, - bool strict_error_checks, - std::string* error) { - scoped_ptr<DictionaryValue> value(LoadManifestFile(name, error)); - if (!value.get()) - return NULL; - return LoadExtensionWithLocation(value.get(), location, - strict_error_checks, error); - } + manifest_ = LoadManifestFile(name_, error); + manifest_holder_.reset(manifest_); + return manifest_; + } - scoped_refptr<Extension> LoadAndExpectSuccess(const std::string& name) { - std::string error; - scoped_refptr<Extension> extension = LoadExtension(name, &error); - EXPECT_TRUE(extension) << name; - EXPECT_EQ("", error) << name; - return extension; - } + private: + std::string name_; + mutable DictionaryValue* manifest_; + mutable scoped_ptr<DictionaryValue> manifest_holder_; + }; - scoped_refptr<Extension> LoadStrictAndExpectSuccess(const std::string& name) { - std::string error; - scoped_refptr<Extension> extension = LoadExtensionStrict(name, &error); - EXPECT_TRUE(extension) << name; - EXPECT_EQ("", error) << name; - return extension; + scoped_refptr<Extension> LoadExtension( + const Manifest& manifest, + std::string* error, + Extension::Location location = Extension::INTERNAL, + int flags = Extension::NO_FLAGS) { + DictionaryValue* value = manifest.GetManifest(error); + if (!value) + return NULL; + FilePath path; + PathService::Get(chrome::DIR_TEST_DATA, &path); + path = path.AppendASCII("extensions").AppendASCII("manifest_tests"); + return Extension::Create(path.DirName(), location, *value, flags, error); } - scoped_refptr<Extension> LoadAndExpectSuccess(DictionaryValue* manifest, - const std::string& name) { + scoped_refptr<Extension> LoadAndExpectSuccess( + const Manifest& manifest, + Extension::Location location = Extension::INTERNAL, + int flags = Extension::NO_FLAGS) { std::string error; - scoped_refptr<Extension> extension = LoadExtension(manifest, &error); - EXPECT_TRUE(extension) << "Unexpected failure for " << name; - EXPECT_EQ("", error) << "Unexpected error for " << name; + scoped_refptr<Extension> extension = + LoadExtension(manifest, &error, location, flags); + EXPECT_TRUE(extension) << manifest.name(); + EXPECT_EQ("", error) << manifest.name(); return extension; } @@ -139,26 +128,15 @@ class ExtensionManifestTest : public testing::Test { " expected '" << expected_error << "' but got '" << error << "'"; } - void LoadAndExpectError(const std::string& name, - const std::string& expected_error) { + void LoadAndExpectError(const Manifest& manifest, + const std::string& expected_error, + Extension::Location location = Extension::INTERNAL, + int flags = Extension::NO_FLAGS) { std::string error; - scoped_refptr<Extension> extension(LoadExtension(name, &error)); - VerifyExpectedError(extension.get(), name, error, expected_error); - } - - void LoadAndExpectErrorStrict(const std::string& name, - const std::string& expected_error) { - std::string error; - scoped_refptr<Extension> extension(LoadExtensionStrict(name, &error)); - VerifyExpectedError(extension.get(), name, error, expected_error); - } - - void LoadAndExpectError(DictionaryValue* manifest, - const std::string& name, - const std::string& expected_error) { - std::string error; - scoped_refptr<Extension> extension(LoadExtension(manifest, &error)); - VerifyExpectedError(extension.get(), name, error, expected_error); + scoped_refptr<Extension> extension( + LoadExtension(manifest, &error, location, flags)); + VerifyExpectedError(extension.get(), manifest.name(), error, + expected_error); } struct Testcase { @@ -168,7 +146,8 @@ class ExtensionManifestTest : public testing::Test { void RunTestcases(const Testcase* testcases, size_t num_testcases) { for (size_t i = 0; i < num_testcases; ++i) { - LoadAndExpectError(testcases[i].manifest, testcases[i].expected_error); + LoadAndExpectError(testcases[i].manifest.c_str(), + testcases[i].expected_error); } } @@ -295,25 +274,30 @@ TEST_F(ExtensionManifestTest, InitFromValueValidNameInRTL) { TEST_F(ExtensionManifestTest, UpdateUrls) { // Test several valid update urls - LoadStrictAndExpectSuccess("update_url_valid_1.json"); - LoadStrictAndExpectSuccess("update_url_valid_2.json"); - LoadStrictAndExpectSuccess("update_url_valid_3.json"); - LoadStrictAndExpectSuccess("update_url_valid_4.json"); + LoadAndExpectSuccess("update_url_valid_1.json", Extension::INTERNAL, + Extension::STRICT_ERROR_CHECKS); + LoadAndExpectSuccess("update_url_valid_2.json", Extension::INTERNAL, + Extension::STRICT_ERROR_CHECKS); + LoadAndExpectSuccess("update_url_valid_3.json", Extension::INTERNAL, + Extension::STRICT_ERROR_CHECKS); + LoadAndExpectSuccess("update_url_valid_4.json", Extension::INTERNAL, + Extension::STRICT_ERROR_CHECKS); // Test some invalid update urls - LoadAndExpectErrorStrict("update_url_invalid_1.json", - errors::kInvalidUpdateURL); - LoadAndExpectErrorStrict("update_url_invalid_2.json", - errors::kInvalidUpdateURL); - LoadAndExpectErrorStrict("update_url_invalid_3.json", - errors::kInvalidUpdateURL); + LoadAndExpectError("update_url_invalid_1.json", errors::kInvalidUpdateURL, + Extension::INTERNAL, Extension::STRICT_ERROR_CHECKS); + LoadAndExpectError("update_url_invalid_2.json", errors::kInvalidUpdateURL, + Extension::INTERNAL, Extension::STRICT_ERROR_CHECKS); + LoadAndExpectError("update_url_invalid_3.json", errors::kInvalidUpdateURL, + Extension::INTERNAL, Extension::STRICT_ERROR_CHECKS); } // Tests that the old permission name "unlimited_storage" still works for // backwards compatibility (we renamed it to "unlimitedStorage"). TEST_F(ExtensionManifestTest, OldUnlimitedStoragePermission) { - scoped_refptr<Extension> extension = LoadStrictAndExpectSuccess( - "old_unlimited_storage.json"); + scoped_refptr<Extension> extension = LoadAndExpectSuccess( + "old_unlimited_storage.json", Extension::INTERNAL, + Extension::STRICT_ERROR_CHECKS); EXPECT_TRUE(extension->HasAPIPermission( ExtensionAPIPermission::kUnlimitedStorage)); } @@ -370,13 +354,14 @@ TEST_F(ExtensionManifestTest, AppWebUrls) { // Ports in app.urls only raise an error when loading as a // developer would. LoadAndExpectSuccess("web_urls_invalid_has_port.json"); - LoadAndExpectErrorStrict( + LoadAndExpectError( "web_urls_invalid_has_port.json", ExtensionErrorUtils::FormatErrorMessage( errors::kInvalidWebURL, base::IntToString(1), - URLPattern::GetParseResultString(URLPattern::PARSE_ERROR_HAS_COLON))); - + URLPattern::GetParseResultString(URLPattern::PARSE_ERROR_HAS_COLON)), + Extension::INTERNAL, + Extension::STRICT_ERROR_CHECKS); scoped_refptr<Extension> extension( LoadAndExpectSuccess("web_urls_default.json")); @@ -477,11 +462,11 @@ TEST_F(ExtensionManifestTest, ChromeResourcesPermissionValidOnlyForComponents) { errors::kInvalidPermissionScheme); std::string error; scoped_refptr<Extension> extension; - extension = LoadExtensionWithLocation( + extension = LoadExtension( "permission_chrome_resources_url.json", + &error, Extension::COMPONENT, - true, // Strict error checking - &error); + Extension::STRICT_ERROR_CHECKS); EXPECT_EQ("", error); } @@ -511,14 +496,16 @@ TEST_F(ExtensionManifestTest, InvalidContentScriptMatchPattern) { LoadAndExpectSuccess("forbid_ports_in_content_scripts.json"); // Loading as a developer would should give an error. - LoadAndExpectErrorStrict( + LoadAndExpectError( "forbid_ports_in_content_scripts.json", ExtensionErrorUtils::FormatErrorMessage( errors::kInvalidMatch, base::IntToString(1), base::IntToString(0), URLPattern::GetParseResultString( - URLPattern::PARSE_ERROR_HAS_COLON))); + URLPattern::PARSE_ERROR_HAS_COLON)), + Extension::INTERNAL, + Extension::STRICT_ERROR_CHECKS); } TEST_F(ExtensionManifestTest, ExcludeMatchPatterns) { @@ -536,6 +523,9 @@ TEST_F(ExtensionManifestTest, ExcludeMatchPatterns) { TEST_F(ExtensionManifestTest, ExperimentalPermission) { LoadAndExpectError("experimental.json", errors::kExperimentalFlagRequired); + LoadAndExpectSuccess("experimental.json", Extension::COMPONENT); + LoadAndExpectSuccess("experimental.json", Extension::INTERNAL, + Extension::FROM_WEBSTORE); CommandLine old_command_line = *CommandLine::ForCurrentProcess(); CommandLine::ForCurrentProcess()->AppendSwitch( switches::kEnableExperimentalExtensionApis); @@ -659,7 +649,8 @@ TEST_F(ExtensionManifestTest, AllowUnrecognizedPermissions) { // Extensions are allowed to contain unrecognized API permissions, // so there shouldn't be any errors. scoped_refptr<Extension> extension; - extension = LoadAndExpectSuccess(manifest.get(), message_name); + extension = LoadAndExpectSuccess( + Manifest(manifest.get(), message_name.c_str())); } *CommandLine::ForCurrentProcess() = old_command_line; } @@ -808,7 +799,8 @@ TEST_F(ExtensionManifestTest, ForbidPortsInPermissions) { // To ensure that we do not error out on a valid permission // in a future version of chrome, validation is to loose // to flag this case. - LoadStrictAndExpectSuccess("forbid_ports_in_permissions.json"); + LoadAndExpectSuccess("forbid_ports_in_permissions.json", + Extension::INTERNAL, Extension::STRICT_ERROR_CHECKS); } TEST_F(ExtensionManifestTest, IsolatedApps) { @@ -864,11 +856,11 @@ TEST_F(ExtensionManifestTest, FileManagerURLOverride) { // A component extention can override chrome://files/ URL. std::string error; scoped_refptr<Extension> extension; - extension = LoadExtensionWithLocation( + extension = LoadExtension( "filebrowser_url_override.json", + &error, Extension::COMPONENT, - true, // Strict error checking - &error); + Extension::STRICT_ERROR_CHECKS); #if defined(FILE_MANAGER_EXTENSION) EXPECT_EQ("", error); #else |