diff options
7 files changed, 67 insertions, 5 deletions
diff --git a/chrome/browser/extensions/extension_file_util.cc b/chrome/browser/extensions/extension_file_util.cc index 32c4819..1b6f474 100644 --- a/chrome/browser/extensions/extension_file_util.cc +++ b/chrome/browser/extensions/extension_file_util.cc @@ -101,14 +101,26 @@ Extension* LoadExtension(const FilePath& extension_path, FilePath manifest_path = extension_path.AppendASCII(Extension::kManifestFilename); if (!file_util::PathExists(manifest_path)) { - *error = extension_manifest_errors::kInvalidManifest; + *error = extension_manifest_errors::kManifestUnreadable; return NULL; } JSONFileValueSerializer serializer(manifest_path); scoped_ptr<Value> root(serializer.Deserialize(error)); - if (!root.get()) + if (!root.get()) { + if (error->empty()) { + // If |error| is empty, than the file could not be read. + // It would be cleaner to have the JSON reader give a specific error + // in this case, but other code tests for a file error with + // error->empty(). For now, be consistent. + *error = extension_manifest_errors::kManifestUnreadable; + } else { + *error = StringPrintf("%s %s", + extension_manifest_errors::kManifestParseError, + error->c_str()); + } return NULL; + } if (!root->IsType(Value::TYPE_DICTIONARY)) { *error = extension_manifest_errors::kInvalidManifest; diff --git a/chrome/browser/extensions/extension_file_util_unittest.cc b/chrome/browser/extensions/extension_file_util_unittest.cc index 7230ecd..c462032 100644 --- a/chrome/browser/extensions/extension_file_util_unittest.cc +++ b/chrome/browser/extensions/extension_file_util_unittest.cc @@ -185,6 +185,41 @@ TEST(ExtensionFileUtil, CheckIllegalFilenamesReservedAndIllegal) { &error)); } +TEST(ExtensionFileUtil, LoadExtensionGivesHelpfullErrorOnMissingManifest) { + FilePath install_dir; + ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &install_dir)); + install_dir = install_dir.AppendASCII("extensions") + .AppendASCII("bad") + .AppendASCII("Extensions") + .AppendASCII("dddddddddddddddddddddddddddddddd") + .AppendASCII("1.0"); + + std::string error; + scoped_ptr<Extension> extension( + extension_file_util::LoadExtension(install_dir, false, &error)); + ASSERT_TRUE(extension == NULL); + ASSERT_FALSE(error.empty()); + ASSERT_STREQ("Manifest file is missing or unreadable.", error.c_str()); +} + +TEST(ExtensionFileUtil, LoadExtensionGivesHelpfullErrorOnBadManifest) { + FilePath install_dir; + ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &install_dir)); + install_dir = install_dir.AppendASCII("extensions") + .AppendASCII("bad") + .AppendASCII("Extensions") + .AppendASCII("eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee") + .AppendASCII("1.0"); + + std::string error; + scoped_ptr<Extension> extension( + extension_file_util::LoadExtension(install_dir, false, &error)); + ASSERT_TRUE(extension == NULL); + ASSERT_FALSE(error.empty()); + ASSERT_STREQ("Manifest is not valid JSON. " + "Line: 2, column: 16, Syntax error.", error.c_str()); +} + // TODO(aa): More tests as motivation allows. Maybe steal some from // ExtensionsService? Many of them could probably be tested here without the // MessageLoop shenanigans. diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc index 1eaa7159..f1a1ede 100644 --- a/chrome/browser/extensions/extensions_service_unittest.cc +++ b/chrome/browser/extensions/extensions_service_unittest.cc @@ -611,7 +611,7 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectoryFail) { EXPECT_TRUE(MatchPattern(GetErrors()[1], std::string("Could not load extension from '*'. ") + - extension_manifest_errors::kInvalidManifest)) << GetErrors()[1]; + extension_manifest_errors::kManifestUnreadable)) << GetErrors()[1]; EXPECT_TRUE(MatchPattern(GetErrors()[2], std::string("Could not load extension from '*'. ") + @@ -619,7 +619,7 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectoryFail) { EXPECT_TRUE(MatchPattern(GetErrors()[3], std::string("Could not load extension from '*'. ") + - extension_manifest_errors::kInvalidManifest)) << GetErrors()[3]; + extension_manifest_errors::kManifestUnreadable)) << GetErrors()[3]; }; // Test that partially deleted extensions are cleaned up during startup diff --git a/chrome/common/extensions/extension_constants.cc b/chrome/common/extensions/extension_constants.cc index 228197b..b269848 100644 --- a/chrome/common/extensions/extension_constants.cc +++ b/chrome/common/extensions/extension_constants.cc @@ -82,7 +82,7 @@ const char* kInvalidJsList = const char* kInvalidKey = "Value 'key' is missing or invalid."; const char* kInvalidManifest = - "Manifest is missing or invalid."; + "Manifest file is invalid."; const char* kInvalidMatchCount = "Invalid value for 'content_scripts[*].matches. There must be at least one " "match specified."; @@ -142,6 +142,10 @@ const char* kInvalidVersion = "dot-separated integers."; const char* kInvalidZipHash = "Required key 'zip_hash' is missing or invalid."; +const char* kManifestParseError = + "Manifest is not valid JSON."; +const char* kManifestUnreadable = + "Manifest file is missing or unreadable."; const char* kMissingFile = "At least one js or css file is required for 'content_scripts[*]'."; const char* kInvalidTheme = diff --git a/chrome/common/extensions/extension_constants.h b/chrome/common/extensions/extension_constants.h index 125a47f..61f8092 100644 --- a/chrome/common/extensions/extension_constants.h +++ b/chrome/common/extensions/extension_constants.h @@ -104,6 +104,8 @@ namespace extension_manifest_errors { extern const char* kInvalidThemeColors; extern const char* kInvalidThemeTints; extern const char* kThemesCannotContainExtensions; + extern const char* kManifestParseError; + extern const char* kManifestUnreadable; extern const char* kMissingFile; extern const char* kInvalidUpdateURL; extern const char* kInvalidDefaultLocale; diff --git a/chrome/test/data/extensions/bad/Extensions/dddddddddddddddddddddddddddddddd/1.0/git_hates_empty_dirs b/chrome/test/data/extensions/bad/Extensions/dddddddddddddddddddddddddddddddd/1.0/git_hates_empty_dirs new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/chrome/test/data/extensions/bad/Extensions/dddddddddddddddddddddddddddddddd/1.0/git_hates_empty_dirs diff --git a/chrome/test/data/extensions/bad/Extensions/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee/1.0/manifest.json b/chrome/test/data/extensions/bad/Extensions/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee/1.0/manifest.json new file mode 100644 index 0000000..65e7ab28 --- /dev/null +++ b/chrome/test/data/extensions/bad/Extensions/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee/1.0/manifest.json @@ -0,0 +1,9 @@ +{ + "version": ""1.0", + "name": "My extension 3", + "content_scripts": [ + { + "matches": ["http://*/*"] + } + ] +} |