diff options
Diffstat (limited to 'extensions/common')
-rw-r--r-- | extensions/common/file_util.cc | 10 | ||||
-rw-r--r-- | extensions/common/file_util.h | 3 | ||||
-rw-r--r-- | extensions/common/file_util_unittest.cc | 28 | ||||
-rw-r--r-- | extensions/common/manifest_handler_helpers.cc | 24 | ||||
-rw-r--r-- | extensions/common/manifest_handler_helpers.h | 11 | ||||
-rw-r--r-- | extensions/common/manifest_handlers/icons_handler.cc | 2 |
6 files changed, 59 insertions, 19 deletions
diff --git a/extensions/common/file_util.cc b/extensions/common/file_util.cc index 90a7846..3ddf6d0 100644 --- a/extensions/common/file_util.cc +++ b/extensions/common/file_util.cc @@ -56,16 +56,6 @@ enum SafeInstallationFlag { }; SafeInstallationFlag g_use_safe_installation = DEFAULT; -// Returns true if the given file path exists and is not zero-length. -bool ValidateFilePath(const base::FilePath& path) { - int64 size = 0; - if (!base::PathExists(path) || !base::GetFileSize(path, &size) || size == 0) { - return false; - } - - return true; -} - // Returns true if the extension installation should flush all files and the // directory. bool UseSafeInstallation() { diff --git a/extensions/common/file_util.h b/extensions/common/file_util.h index 3fda56a..609b27c 100644 --- a/extensions/common/file_util.h +++ b/extensions/common/file_util.h @@ -72,6 +72,9 @@ scoped_ptr<base::DictionaryValue> LoadManifest( const base::FilePath::CharType* manifest_filename, std::string* error); +// Returns true if the given file path exists and is not zero-length. +bool ValidateFilePath(const base::FilePath& path); + // Returns true if the given extension object is valid and consistent. // May also append a series of warning messages to |warnings|, but they // should not prevent the extension from running. diff --git a/extensions/common/file_util_unittest.cc b/extensions/common/file_util_unittest.cc index 0bfb8b9..afbcf3c 100644 --- a/extensions/common/file_util_unittest.cc +++ b/extensions/common/file_util_unittest.cc @@ -419,19 +419,39 @@ TEST_F(FileUtilTest, WarnOnPrivateKey) { "extension includes the key file.*ext_root.a_key.pem")); } -TEST_F(FileUtilTest, CheckZeroLengthIconFile) { +// Try to install an extension with a zero-length icon file. +TEST_F(FileUtilTest, CheckZeroLengthAndMissingIconFile) { + base::FilePath install_dir; + ASSERT_TRUE(PathService::Get(DIR_TEST_DATA, &install_dir)); + + base::FilePath ext_dir = + install_dir.AppendASCII("file_util").AppendASCII("bad_icon"); + + std::string error; + scoped_refptr<Extension> extension(file_util::LoadExtension( + ext_dir, Manifest::INTERNAL, Extension::NO_FLAGS, &error)); + EXPECT_TRUE(extension); + ASSERT_EQ(2U, extension->install_warnings().size()); + + EXPECT_EQ("Could not load extension icon 'missing-icon.png'.", + extension->install_warnings()[0].message); + EXPECT_EQ("Could not load extension icon 'icon.png'.", + extension->install_warnings()[1].message); +} + +// Try to install an unpacked extension with a zero-length icon file. +TEST_F(FileUtilTest, CheckZeroLengthAndMissingIconFileUnpacked) { base::FilePath install_dir; ASSERT_TRUE(PathService::Get(DIR_TEST_DATA, &install_dir)); - // Try to install an extension with a zero-length icon file. base::FilePath ext_dir = install_dir.AppendASCII("file_util").AppendASCII("bad_icon"); std::string error; scoped_refptr<Extension> extension(file_util::LoadExtension( ext_dir, Manifest::UNPACKED, Extension::NO_FLAGS, &error)); - EXPECT_TRUE(extension.get() == NULL); - EXPECT_STREQ("Could not load extension icon 'icon.png'.", error.c_str()); + EXPECT_FALSE(extension); + EXPECT_EQ("Could not load extension icon 'missing-icon.png'.", error); } TEST_F(FileUtilTest, ExtensionURLToRelativeFilePath) { diff --git a/extensions/common/manifest_handler_helpers.cc b/extensions/common/manifest_handler_helpers.cc index bfc9e33..36af224d 100644 --- a/extensions/common/manifest_handler_helpers.cc +++ b/extensions/common/manifest_handler_helpers.cc @@ -13,7 +13,10 @@ #include "extensions/common/error_utils.h" #include "extensions/common/extension.h" #include "extensions/common/extension_icon_set.h" +#include "extensions/common/file_util.h" #include "extensions/common/manifest_constants.h" +#include "grit/extensions_strings.h" +#include "ui/base/l10n/l10n_util.h" namespace extensions { @@ -32,7 +35,8 @@ bool NormalizeAndValidatePath(std::string* path) { return true; } -bool LoadIconsFromDictionary(const base::DictionaryValue* icons_value, +bool LoadIconsFromDictionary(Extension* extension, + const base::DictionaryValue* icons_value, ExtensionIconSet* icons, base::string16* error) { DCHECK(icons); @@ -49,7 +53,23 @@ bool LoadIconsFromDictionary(const base::DictionaryValue* icons_value, return false; } - icons->Add(size, icon_path); + // For backwards compatibility, only warn (don't error out) if an icon is + // missing. Component extensions can skip this check as their icons are not + // located on disk. Unpacked extensions skip this check and fail later + // during validation if the file isn't present. See crbug.com/570249 + // TODO(estade|devlin): remove this workaround and let install fail in the + // validate step a few releases after M49. See http://crbug.com/571193 + if (Manifest::IsComponentLocation(extension->location()) || + Manifest::IsUnpackedLocation(extension->location()) || + file_util::ValidateFilePath( + extension->GetResource(icon_path).GetFilePath())) { + icons->Add(size, icon_path); + } else { + extension->AddInstallWarning(InstallWarning( + l10n_util::GetStringFUTF8(IDS_EXTENSION_LOAD_ICON_FAILED, + base::UTF8ToUTF16(icon_path)), + std::string())); + } } return true; } diff --git a/extensions/common/manifest_handler_helpers.h b/extensions/common/manifest_handler_helpers.h index b0249c3..e1f901a 100644 --- a/extensions/common/manifest_handler_helpers.h +++ b/extensions/common/manifest_handler_helpers.h @@ -6,9 +6,11 @@ #define EXTENSIONS_COMMON_MANIFEST_HANDLER_HELPERS_H_ #include <string> +#include <vector> #include "base/memory/scoped_ptr.h" #include "base/strings/string16.h" +#include "extensions/common/install_warning.h" class ExtensionIconSet; @@ -17,6 +19,9 @@ class DictionaryValue; } namespace extensions { + +class Extension; + namespace manifest_handler_helpers { // Strips leading slashes from the file path. Returns true iff the final path is @@ -25,8 +30,10 @@ bool NormalizeAndValidatePath(std::string* path); // Loads icon paths defined in dictionary |icons_value| into ExtensionIconSet // |icons|. |icons_value| is a dictionary value {icon size -> icon path}. -// Returns success. If load fails, |error| will be set. -bool LoadIconsFromDictionary(const base::DictionaryValue* icons_value, +// Returns success. If load fails, |error| will be set. Non-failure warnings may +// be added to |extension|. +bool LoadIconsFromDictionary(Extension* extension, + const base::DictionaryValue* icons_value, ExtensionIconSet* icons, base::string16* error); diff --git a/extensions/common/manifest_handlers/icons_handler.cc b/extensions/common/manifest_handlers/icons_handler.cc index 20ad025..52597e5 100644 --- a/extensions/common/manifest_handlers/icons_handler.cc +++ b/extensions/common/manifest_handlers/icons_handler.cc @@ -64,7 +64,7 @@ bool IconsHandler::Parse(Extension* extension, base::string16* error) { } if (!manifest_handler_helpers::LoadIconsFromDictionary( - icons_dict, &icons_info->icons, error)) { + extension, icons_dict, &icons_info->icons, error)) { return false; } |