summaryrefslogtreecommitdiffstats
path: root/extensions
diff options
context:
space:
mode:
Diffstat (limited to 'extensions')
-rw-r--r--extensions/common/file_util.cc17
-rw-r--r--extensions/common/file_util.h3
-rw-r--r--extensions/common/file_util_unittest.cc28
-rw-r--r--extensions/common/manifest_handler_helpers.cc25
-rw-r--r--extensions/common/manifest_handler_helpers.h11
-rw-r--r--extensions/common/manifest_handlers/icons_handler.cc2
-rw-r--r--extensions/test/data/file_util/bad_icon/good-icon.pngbin0 -> 160 bytes
-rw-r--r--extensions/test/data/file_util/bad_icon/manifest.json4
-rw-r--r--extensions/utility/unpacker.cc5
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
new file mode 100644
index 0000000..d89ed60
--- /dev/null
+++ b/extensions/test/data/file_util/bad_icon/good-icon.png
Binary files differ
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.
}