diff options
Diffstat (limited to 'extensions')
-rw-r--r-- | extensions/common/file_util.cc | 17 | ||||
-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 | 25 | ||||
-rw-r--r-- | extensions/common/manifest_handler_helpers.h | 11 | ||||
-rw-r--r-- | extensions/common/manifest_handlers/icons_handler.cc | 2 | ||||
-rw-r--r-- | extensions/test/data/file_util/bad_icon/good-icon.png | bin | 0 -> 160 bytes | |||
-rw-r--r-- | extensions/test/data/file_util/bad_icon/manifest.json | 4 | ||||
-rw-r--r-- | extensions/utility/unpacker.cc | 5 |
9 files changed, 69 insertions, 26 deletions
diff --git a/extensions/common/file_util.cc b/extensions/common/file_util.cc index a187378..a42bdae 100644 --- a/extensions/common/file_util.cc +++ b/extensions/common/file_util.cc @@ -52,18 +52,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() { @@ -271,6 +259,11 @@ scoped_ptr<base::DictionaryValue> LoadManifest( return base::DictionaryValue::From(std::move(root)); } +bool ValidateFilePath(const base::FilePath& path) { + int64 size = 0; + return base::PathExists(path) && base::GetFileSize(path, &size) && size != 0; +} + bool ValidateExtension(const Extension* extension, std::string* error, std::vector<InstallWarning>* warnings) { 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 634b338..5148737 100644 --- a/extensions/common/file_util_unittest.cc +++ b/extensions/common/file_util_unittest.cc @@ -416,19 +416,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 4dc2f6b..f04d0f8 100644 --- a/extensions/common/manifest_handler_helpers.cc +++ b/extensions/common/manifest_handler_helpers.cc @@ -11,8 +11,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 { @@ -31,7 +33,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); @@ -48,7 +51,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; } diff --git a/extensions/test/data/file_util/bad_icon/good-icon.png b/extensions/test/data/file_util/bad_icon/good-icon.png Binary files differnew file mode 100644 index 0000000..d89ed60 --- /dev/null +++ b/extensions/test/data/file_util/bad_icon/good-icon.png diff --git a/extensions/test/data/file_util/bad_icon/manifest.json b/extensions/test/data/file_util/bad_icon/manifest.json index e3846e4..27b7dac 100644 --- a/extensions/test/data/file_util/bad_icon/manifest.json +++ b/extensions/test/data/file_util/bad_icon/manifest.json @@ -2,7 +2,9 @@ "name": "My extension with a zero-length icon", "version": "1.0", "icons": { - "16": "icon.png" + "16": "good-icon.png", + "32": "missing-icon.png", + "64": "icon.png" }, "manifest_version": 2 } diff --git a/extensions/utility/unpacker.cc b/extensions/utility/unpacker.cc index 36c0eba..4763268 100644 --- a/extensions/utility/unpacker.cc +++ b/extensions/utility/unpacker.cc @@ -185,9 +185,8 @@ bool Unpacker::Run() { // Decode any images that the browser needs to display. std::set<base::FilePath> image_paths = ExtensionsClient::Get()->GetBrowserImagePaths(extension.get()); - for (std::set<base::FilePath>::iterator it = image_paths.begin(); - it != image_paths.end(); ++it) { - if (!AddDecodedImage(*it)) + for (const base::FilePath& path : image_paths) { + if (!AddDecodedImage(path)) return false; // Error was already reported. } |