From 586ae16a9901051dee5be902e49ef95f6ec8b25d Mon Sep 17 00:00:00 2001 From: "yoz@chromium.org" Date: Fri, 1 Nov 2013 22:54:15 +0000 Subject: Move ManifestHandler to top-level extensions. BUG=298586 R=benwells@chromium.org Review URL: https://codereview.chromium.org/52593012 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@232499 0039d316-1c4b-4281-b951-d872f2087c98 --- extensions/DEPS | 10 +- extensions/common/manifest_handler.cc | 205 ++++++++++++++++++ extensions/common/manifest_handler.h | 138 ++++++++++++ extensions/common/manifest_handler_unittest.cc | 281 +++++++++++++++++++++++++ extensions/extensions.gyp | 2 + 5 files changed, 633 insertions(+), 3 deletions(-) create mode 100644 extensions/common/manifest_handler.cc create mode 100644 extensions/common/manifest_handler.h create mode 100644 extensions/common/manifest_handler_unittest.cc (limited to 'extensions') diff --git a/extensions/DEPS b/extensions/DEPS index 14eae2f..29ebf1e 100644 --- a/extensions/DEPS +++ b/extensions/DEPS @@ -7,11 +7,15 @@ include_rules = [ # More specific rules for what we are allowed to include. specific_include_rules = { - ".*test\.cc": [ + ".*test\.cc$": [ "+content/public/test", ], - "api_permission_set_unittest\.cc": [ - # Temporary include for tests. + # Temporary includes for tests. + "^api_permission_set_unittest\.cc$": [ "+chrome/common/extensions/extension_messages.h", ], + "^manifest_handler_unittest\.cc$": [ + "+chrome/common/extensions/extension_builder.h", + "+chrome/common/extensions/value_builder.h", + ], } diff --git a/extensions/common/manifest_handler.cc b/extensions/common/manifest_handler.cc new file mode 100644 index 0000000..43e6bd8 --- /dev/null +++ b/extensions/common/manifest_handler.cc @@ -0,0 +1,205 @@ +// Copyright (c) 2013 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 "extensions/common/manifest_handler.h" + +#include + +#include "base/logging.h" +#include "base/stl_util.h" +#include "chrome/common/extensions/extension.h" + +namespace extensions { + +namespace { + +static base::LazyInstance g_registry = + LAZY_INSTANCE_INITIALIZER; +static ManifestHandlerRegistry* g_registry_override = NULL; + +ManifestHandlerRegistry* GetRegistry() { + if (!g_registry_override) + return g_registry.Pointer(); + return g_registry_override; +} + +} // namespace + +ManifestHandler::ManifestHandler() { +} + +ManifestHandler::~ManifestHandler() { +} + +bool ManifestHandler::Validate(const Extension* extension, + std::string* error, + std::vector* warnings) const { + return true; +} + +bool ManifestHandler::AlwaysParseForType(Manifest::Type type) const { + return false; +} + +bool ManifestHandler::AlwaysValidateForType(Manifest::Type type) const { + return false; +} + +const std::vector ManifestHandler::PrerequisiteKeys() const { + return std::vector(); +} + +void ManifestHandler::Register() { + linked_ptr this_linked(this); + const std::vector keys = Keys(); + for (size_t i = 0; i < keys.size(); ++i) + GetRegistry()->RegisterManifestHandler(keys[i], this_linked); +} + +// static +void ManifestHandler::FinalizeRegistration() { + GetRegistry()->Finalize(); +} + +// static +bool ManifestHandler::IsRegistrationFinalized() { + return GetRegistry()->is_finalized_; +} + +// static +bool ManifestHandler::ParseExtension(Extension* extension, string16* error) { + return GetRegistry()->ParseExtension(extension, error); +} + +// static +bool ManifestHandler::ValidateExtension(const Extension* extension, + std::string* error, + std::vector* warnings) { + return GetRegistry()->ValidateExtension(extension, error, warnings); +} + +// static +const std::vector ManifestHandler::SingleKey( + const std::string& key) { + return std::vector(1, key); +} + +ManifestHandlerRegistry::ManifestHandlerRegistry() : is_finalized_(false) { +} + +ManifestHandlerRegistry::~ManifestHandlerRegistry() { +} + +void ManifestHandlerRegistry::Finalize() { + CHECK(!is_finalized_); + SortManifestHandlers(); + is_finalized_ = true; +} + +void ManifestHandlerRegistry::RegisterManifestHandler( + const std::string& key, linked_ptr handler) { + CHECK(!is_finalized_); + handlers_[key] = handler; +} + +bool ManifestHandlerRegistry::ParseExtension(Extension* extension, + string16* error) { + std::map handlers_by_priority; + for (ManifestHandlerMap::iterator iter = handlers_.begin(); + iter != handlers_.end(); ++iter) { + ManifestHandler* handler = iter->second.get(); + if (extension->manifest()->HasPath(iter->first) || + handler->AlwaysParseForType(extension->GetType())) { + handlers_by_priority[priority_map_[handler]] = handler; + } + } + for (std::map::iterator iter = + handlers_by_priority.begin(); + iter != handlers_by_priority.end(); ++iter) { + if (!(iter->second)->Parse(extension, error)) + return false; + } + return true; +} + +bool ManifestHandlerRegistry::ValidateExtension( + const Extension* extension, + std::string* error, + std::vector* warnings) { + std::set handlers; + for (ManifestHandlerMap::iterator iter = handlers_.begin(); + iter != handlers_.end(); ++iter) { + ManifestHandler* handler = iter->second.get(); + if (extension->manifest()->HasPath(iter->first) || + handler->AlwaysValidateForType(extension->GetType())) { + handlers.insert(handler); + } + } + for (std::set::iterator iter = handlers.begin(); + iter != handlers.end(); ++iter) { + if (!(*iter)->Validate(extension, error, warnings)) + return false; + } + return true; +} + +// static +ManifestHandlerRegistry* ManifestHandlerRegistry::SetForTesting( + ManifestHandlerRegistry* new_registry) { + ManifestHandlerRegistry* old_registry = GetRegistry(); + if (new_registry != g_registry.Pointer()) + g_registry_override = new_registry; + else + g_registry_override = NULL; + return old_registry; +} + +void ManifestHandlerRegistry::SortManifestHandlers() { + std::set unsorted_handlers; + for (ManifestHandlerMap::const_iterator iter = handlers_.begin(); + iter != handlers_.end(); ++iter) { + unsorted_handlers.insert(iter->second.get()); + } + + int priority = 0; + while (true) { + std::set next_unsorted_handlers; + for (std::set::const_iterator iter = + unsorted_handlers.begin(); + iter != unsorted_handlers.end(); ++iter) { + ManifestHandler* handler = *iter; + const std::vector& prerequisites = + handler->PrerequisiteKeys(); + int unsatisfied = prerequisites.size(); + for (size_t i = 0; i < prerequisites.size(); ++i) { + ManifestHandlerMap::const_iterator prereq_iter = + handlers_.find(prerequisites[i]); + // If the prerequisite does not exist, crash. + CHECK(prereq_iter != handlers_.end()) + << "Extension manifest handler depends on unrecognized key " + << prerequisites[i]; + // Prerequisite is in our map. + if (ContainsKey(priority_map_, prereq_iter->second.get())) + unsatisfied--; + } + if (unsatisfied == 0) { + priority_map_[handler] = priority; + priority++; + } else { + // Put in the list for next time. + next_unsorted_handlers.insert(handler); + } + } + if (next_unsorted_handlers.size() == unsorted_handlers.size()) + break; + unsorted_handlers.swap(next_unsorted_handlers); + } + + // If there are any leftover unsorted handlers, they must have had + // circular dependencies. + CHECK(unsorted_handlers.size() == 0) << "Extension manifest handlers have " + << "circular dependencies!"; +} + +} // namespace extensions diff --git a/extensions/common/manifest_handler.h b/extensions/common/manifest_handler.h new file mode 100644 index 0000000..fd7b571 --- /dev/null +++ b/extensions/common/manifest_handler.h @@ -0,0 +1,138 @@ +// Copyright (c) 2013 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. + +#ifndef EXTENSIONS_COMMON_MANIFEST_HANDLER_H_ +#define EXTENSIONS_COMMON_MANIFEST_HANDLER_H_ + +#include +#include +#include + +#include "base/lazy_instance.h" +#include "base/memory/linked_ptr.h" +#include "base/strings/string16.h" +#include "extensions/common/manifest.h" + +namespace extensions { +class Extension; + +// An interface for clients that recognize and parse keys in extension +// manifests. +class ManifestHandler { + public: + ManifestHandler(); + virtual ~ManifestHandler(); + + // Attempts to parse the extension's manifest. + // Returns true on success or false on failure; if false, |error| will + // be set to a failure message. + virtual bool Parse(Extension* extension, string16* error) = 0; + + // Validate that files associated with this manifest key exist. + // Validation takes place after parsing. May also append a series of + // warning messages to |warnings|. + // + // Otherwise, returns false, and a description of the error is + // returned in |error|. + // TODO(yoz): Change error to string16. See crbug.com/71980. + virtual bool Validate(const Extension* extension, + std::string* error, + std::vector* warnings) const; + + // If false (the default), only parse the manifest if a registered + // key is present in the manifest. If true, always attempt to parse + // the manifest for this extension type, even if no registered keys + // are present. This allows specifying a default parsed value for + // extensions that don't declare our key in the manifest. + // TODO(yoz): Use Feature availability instead. + virtual bool AlwaysParseForType(Manifest::Type type) const; + + // Same as AlwaysParseForType, but for Validate instead of Parse. + virtual bool AlwaysValidateForType(Manifest::Type type) const; + + // The list of keys that, if present, should be parsed before calling our + // Parse (typically, because our Parse needs to read those keys). + // Defaults to empty. + virtual const std::vector PrerequisiteKeys() const; + + // Associate us with our keys() in the manifest. A handler can register + // for multiple keys. The global registry takes ownership of this; + // if it has an existing handler for |key|, it replaces it with this. + // Manifest handlers must be registered at process startup in + // chrome_manifest_handlers.cc: + // (new MyManifestHandler)->Register(); + void Register(); + + // Calling FinalizeRegistration indicates that there are no more + // manifest handlers to be registered. + static void FinalizeRegistration(); + + static bool IsRegistrationFinalized(); + + // Call Parse on all registered manifest handlers that should parse + // this extension. + static bool ParseExtension(Extension* extension, string16* error); + + // Call Validate on all registered manifest handlers for this extension. + static bool ValidateExtension(const Extension* extension, + std::string* error, + std::vector* warnings); + + protected: + // A convenience method for handlers that only register for 1 key, + // so that they can define keys() { return SingleKey(kKey); } + static const std::vector SingleKey(const std::string& key); + + private: + // The keys to register us for (in Register). + virtual const std::vector Keys() const = 0; +}; + +// The global registry for manifest handlers. +class ManifestHandlerRegistry { + private: + friend class ManifestHandler; + friend class ScopedTestingManifestHandlerRegistry; + friend struct base::DefaultLazyInstanceTraits; + + ManifestHandlerRegistry(); + ~ManifestHandlerRegistry(); + + void Finalize(); + + void RegisterManifestHandler(const std::string& key, + linked_ptr handler); + bool ParseExtension(Extension* extension, string16* error); + bool ValidateExtension(const Extension* extension, + std::string* error, + std::vector* warnings); + + // Overrides the current global ManifestHandlerRegistry with + // |registry|, returning the current one. + static ManifestHandlerRegistry* SetForTesting( + ManifestHandlerRegistry* new_registry); + + typedef std::map > + ManifestHandlerMap; + typedef std::map ManifestHandlerPriorityMap; + + // Puts the manifest handlers in order such that each handler comes after + // any handlers for their PrerequisiteKeys. If there is no handler for + // a prerequisite key, that dependency is simply ignored. + // CHECKs that there are no manifest handlers with circular dependencies. + void SortManifestHandlers(); + + // All registered manifest handlers. + ManifestHandlerMap handlers_; + + // The priority for each manifest handler. Handlers with lower priority + // values are evaluated first. + ManifestHandlerPriorityMap priority_map_; + + bool is_finalized_; +}; + +} // namespace extensions + +#endif // EXTENSIONS_COMMON_MANIFEST_HANDLER_H_ diff --git a/extensions/common/manifest_handler_unittest.cc b/extensions/common/manifest_handler_unittest.cc new file mode 100644 index 0000000..f5e012e --- /dev/null +++ b/extensions/common/manifest_handler_unittest.cc @@ -0,0 +1,281 @@ +// Copyright (c) 2013 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 +#include + +#include "base/files/file_path.h" +#include "base/memory/scoped_ptr.h" +#include "base/stl_util.h" +#include "base/strings/utf_string_conversions.h" +#include "chrome/common/extensions/extension.h" +#include "chrome/common/extensions/extension_builder.h" +#include "chrome/common/extensions/value_builder.h" +#include "extensions/common/install_warning.h" +#include "extensions/common/manifest_handler.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace extensions { + +namespace { + +std::vector SingleKey(const std::string& key) { + return std::vector(1, key); +} + +} // namespace + +class ScopedTestingManifestHandlerRegistry { + public: + ScopedTestingManifestHandlerRegistry() { + old_registry_ = ManifestHandlerRegistry::SetForTesting(®istry_); + } + + ~ScopedTestingManifestHandlerRegistry() { + ManifestHandlerRegistry::SetForTesting(old_registry_); + } + + ManifestHandlerRegistry registry_; + ManifestHandlerRegistry* old_registry_; +}; + +class ManifestHandlerTest : public testing::Test { + public: + class ParsingWatcher { + public: + // Called when a manifest handler parses. + void Record(const std::string& name) { + parsed_names_.push_back(name); + } + + const std::vector& parsed_names() { + return parsed_names_; + } + + // Returns true if |name_before| was parsed before |name_after|. + bool ParsedBefore(const std::string& name_before, + const std::string& name_after) { + size_t i_before = parsed_names_.size(); + size_t i_after = 0; + for (size_t i = 0; i < parsed_names_.size(); ++i) { + if (parsed_names_[i] == name_before) + i_before = i; + if (parsed_names_[i] == name_after) + i_after = i; + } + if (i_before < i_after) + return true; + return false; + } + + private: + // The order of manifest handlers that we watched parsing. + std::vector parsed_names_; + }; + + class TestManifestHandler : public ManifestHandler { + public: + TestManifestHandler(const std::string& name, + const std::vector& keys, + const std::vector& prereqs, + ParsingWatcher* watcher) + : name_(name), keys_(keys), prereqs_(prereqs), watcher_(watcher) { + } + + virtual bool Parse(Extension* extension, string16* error) OVERRIDE { + watcher_->Record(name_); + return true; + } + + virtual const std::vector PrerequisiteKeys() const OVERRIDE { + return prereqs_; + } + + protected: + std::string name_; + std::vector keys_; + std::vector prereqs_; + ParsingWatcher* watcher_; + + virtual const std::vector Keys() const OVERRIDE { + return keys_; + } + }; + + class FailingTestManifestHandler : public TestManifestHandler { + public: + FailingTestManifestHandler(const std::string& name, + const std::vector& keys, + const std::vector& prereqs, + ParsingWatcher* watcher) + : TestManifestHandler(name, keys, prereqs, watcher) { + } + virtual bool Parse(Extension* extension, string16* error) OVERRIDE { + *error = ASCIIToUTF16(name_); + return false; + } + }; + + class AlwaysParseTestManifestHandler : public TestManifestHandler { + public: + AlwaysParseTestManifestHandler(const std::string& name, + const std::vector& keys, + const std::vector& prereqs, + ParsingWatcher* watcher) + : TestManifestHandler(name, keys, prereqs, watcher) { + } + + virtual bool AlwaysParseForType(Manifest::Type type) const OVERRIDE { + return true; + } + }; + + class TestManifestValidator : public ManifestHandler { + public: + TestManifestValidator(bool return_value, + bool always_validate, + std::vector keys) + : return_value_(return_value), + always_validate_(always_validate), + keys_(keys) { + } + + virtual bool Parse(Extension* extension, string16* error) OVERRIDE { + return true; + } + + virtual bool Validate( + const Extension* extension, + std::string* error, + std::vector* warnings) const OVERRIDE { + return return_value_; + } + + virtual bool AlwaysValidateForType(Manifest::Type type) const OVERRIDE { + return always_validate_; + } + + private: + virtual const std::vector Keys() const OVERRIDE { + return keys_; + } + + protected: + bool return_value_; + bool always_validate_; + std::vector keys_; + }; +}; + +TEST_F(ManifestHandlerTest, DependentHandlers) { + ScopedTestingManifestHandlerRegistry registry; + ParsingWatcher watcher; + std::vector prereqs; + (new TestManifestHandler("A", SingleKey("a"), prereqs, &watcher))->Register(); + (new TestManifestHandler("B", SingleKey("b"), prereqs, &watcher))->Register(); + (new TestManifestHandler("J", SingleKey("j"), prereqs, &watcher))->Register(); + (new AlwaysParseTestManifestHandler("K", SingleKey("k"), prereqs, &watcher))-> + Register(); + prereqs.push_back("c.d"); + std::vector keys; + keys.push_back("c.e"); + keys.push_back("c.z"); + (new TestManifestHandler("C.EZ", keys, prereqs, &watcher))->Register(); + prereqs.clear(); + prereqs.push_back("b"); + prereqs.push_back("k"); + (new TestManifestHandler("C.D", SingleKey("c.d"), prereqs, &watcher))-> + Register(); + ManifestHandler::FinalizeRegistration(); + + scoped_refptr extension = ExtensionBuilder() + .SetManifest(DictionaryBuilder() + .Set("name", "no name") + .Set("version", "0") + .Set("manifest_version", 2) + .Set("a", 1) + .Set("b", 2) + .Set("c", DictionaryBuilder() + .Set("d", 3) + .Set("e", 4) + .Set("f", 5)) + .Set("g", 6)) + .Build(); + + // A, B, C.EZ, C.D, K + EXPECT_EQ(5u, watcher.parsed_names().size()); + EXPECT_TRUE(watcher.ParsedBefore("B", "C.D")); + EXPECT_TRUE(watcher.ParsedBefore("K", "C.D")); + EXPECT_TRUE(watcher.ParsedBefore("C.D", "C.EZ")); +} + +TEST_F(ManifestHandlerTest, FailingHandlers) { + ScopedTestingManifestHandlerRegistry registry; + // Can't use ExtensionBuilder, because this extension will fail to + // be parsed. + scoped_ptr manifest_a( + DictionaryBuilder() + .Set("name", "no name") + .Set("version", "0") + .Set("manifest_version", 2) + .Set("a", 1) + .Build()); + + // Succeeds when "a" is not recognized. + std::string error; + scoped_refptr extension = Extension::Create( + base::FilePath(), + Manifest::INVALID_LOCATION, + *manifest_a, + Extension::NO_FLAGS, + &error); + EXPECT_TRUE(extension.get()); + + // Register a handler for "a" that fails. + ParsingWatcher watcher; + (new FailingTestManifestHandler( + "A", SingleKey("a"), std::vector(), &watcher))->Register(); + ManifestHandler::FinalizeRegistration(); + + extension = Extension::Create( + base::FilePath(), + Manifest::INVALID_LOCATION, + *manifest_a, + Extension::NO_FLAGS, + &error); + EXPECT_FALSE(extension.get()); + EXPECT_EQ("A", error); +} + +TEST_F(ManifestHandlerTest, Validate) { + ScopedTestingManifestHandlerRegistry registry; + scoped_refptr extension = ExtensionBuilder() + .SetManifest(DictionaryBuilder() + .Set("name", "no name") + .Set("version", "0") + .Set("manifest_version", 2) + .Set("a", 1) + .Set("b", 2)) + .Build(); + EXPECT_TRUE(extension.get()); + + std::string error; + std::vector warnings; + // Always validates and fails. + (new TestManifestValidator(false, true, SingleKey("c")))->Register(); + EXPECT_FALSE( + ManifestHandler::ValidateExtension(extension.get(), &error, &warnings)); + + // This overrides the registered handler for "c". + (new TestManifestValidator(false, false, SingleKey("c")))->Register(); + EXPECT_TRUE( + ManifestHandler::ValidateExtension(extension.get(), &error, &warnings)); + + // Validates "a" and fails. + (new TestManifestValidator(false, true, SingleKey("a")))->Register(); + EXPECT_FALSE( + ManifestHandler::ValidateExtension(extension.get(), &error, &warnings)); +} + +} // namespace extensions diff --git a/extensions/extensions.gyp b/extensions/extensions.gyp index f58a442..a29d9c5 100644 --- a/extensions/extensions.gyp +++ b/extensions/extensions.gyp @@ -59,6 +59,8 @@ 'common/manifest.h', 'common/manifest_constants.cc', 'common/manifest_constants.h', + 'common/manifest_handler.cc', + 'common/manifest_handler.h', 'common/matcher/regex_set_matcher.cc', 'common/matcher/regex_set_matcher.h', 'common/matcher/string_pattern.cc', -- cgit v1.1