diff options
10 files changed, 150 insertions, 30 deletions
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc index a84296a..d405a5e 100644 --- a/chrome/browser/extensions/extension_service_unittest.cc +++ b/chrome/browser/extensions/extension_service_unittest.cc @@ -4384,6 +4384,23 @@ TEST_F(ExtensionServiceTest, GenerateID) { ASSERT_EQ(previous_id, loaded_[0]->id()); } +TEST_F(ExtensionServiceTest, UnpackedValidatesLocales) { + InitializeEmptyExtensionService(); + + base::FilePath bad_locale = data_dir_.AppendASCII("unpacked"). + AppendASCII("bad_messages_file"); + extensions::UnpackedInstaller::Create(service_)->Load(bad_locale); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(1u, GetErrors().size()); + base::FilePath ms_messages_file = bad_locale.AppendASCII("_locales") + .AppendASCII("ms") + .AppendASCII("messages.json"); + EXPECT_THAT(UTF16ToUTF8(GetErrors()[0]), testing::AllOf( + testing::HasSubstr(UTF16ToUTF8(ms_messages_file.LossyDisplayName())), + testing::HasSubstr("Dictionary keys must be quoted."))); + ASSERT_EQ(0u, loaded_.size()); +} + void ExtensionServiceTest::TestExternalProvider( MockExtensionProvider* provider, Manifest::Location location) { // Verify that starting with no providers loads no extensions. diff --git a/chrome/browser/extensions/unpacked_installer.cc b/chrome/browser/extensions/unpacked_installer.cc index b36afaa..13db0f5 100644 --- a/chrome/browser/extensions/unpacked_installer.cc +++ b/chrome/browser/extensions/unpacked_installer.cc @@ -18,6 +18,7 @@ #include "chrome/common/extensions/api/plugins/plugins_handler.h" #include "chrome/common/extensions/extension.h" #include "chrome/common/extensions/extension_file_util.h" +#include "chrome/common/extensions/extension_l10n_util.h" #include "content/public/browser/browser_thread.h" #include "extensions/common/id_util.h" #include "extensions/common/manifest.h" @@ -135,7 +136,11 @@ bool UnpackedInstaller::LoadFromCommandLine(const base::FilePath& path_in, installer_.set_extension(extension_file_util::LoadExtension( extension_path_, Manifest::COMMAND_LINE, GetFlags(), &error).get()); - if (!installer_.extension().get()) { + if (!installer_.extension().get() || + !extension_l10n_util::ValidateExtensionLocales( + extension_path_, + installer_.extension()->manifest()->value(), + &error)) { ReportExtensionLoadError(error); return false; } @@ -251,7 +256,11 @@ void UnpackedInstaller::LoadWithFileAccess(int flags) { installer_.set_extension(extension_file_util::LoadExtension( extension_path_, Manifest::UNPACKED, flags, &error).get()); - if (!installer_.extension().get()) { + if (!installer_.extension().get() || + !extension_l10n_util::ValidateExtensionLocales( + extension_path_, + installer_.extension()->manifest()->value(), + &error)) { BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, diff --git a/chrome/common/extensions/extension_l10n_util.cc b/chrome/common/extensions/extension_l10n_util.cc index 6a1256c..5353190 100644 --- a/chrome/common/extensions/extension_l10n_util.cc +++ b/chrome/common/extensions/extension_l10n_util.cc @@ -15,11 +15,13 @@ #include "base/logging.h" #include "base/memory/linked_ptr.h" #include "base/strings/stringprintf.h" +#include "base/strings/utf_string_conversions.h" #include "base/values.h" #include "chrome/common/extensions/extension_file_util.h" #include "chrome/common/extensions/message_bundle.h" #include "chrome/common/url_constants.h" #include "extensions/common/constants.h" +#include "extensions/common/error_utils.h" #include "extensions/common/manifest_constants.h" #include "third_party/icu/source/common/unicode/uloc.h" #include "ui/base/l10n/l10n_util.h" @@ -27,6 +29,37 @@ namespace errors = extensions::manifest_errors; namespace keys = extensions::manifest_keys; +namespace { + +// Loads contents of the messages file for given locale. If file is not found, +// or there was parsing error we return NULL and set |error|. +// Caller owns the returned object. +base::DictionaryValue* LoadMessageFile(const base::FilePath& locale_path, + const std::string& locale, + std::string* error) { + base::FilePath file = locale_path.AppendASCII(locale) + .Append(extensions::kMessagesFilename); + JSONFileValueSerializer messages_serializer(file); + base::Value* dictionary = messages_serializer.Deserialize(NULL, error); + if (!dictionary) { + if (error->empty()) { + // JSONFileValueSerializer just returns NULL if file cannot be found. It + // doesn't set the error, so we have to do it. + *error = base::StringPrintf("Catalog file is missing for locale %s.", + locale.c_str()); + } else { + *error = extensions::ErrorUtils::FormatErrorMessage( + errors::kLocalesInvalidLocale, + UTF16ToUTF8(file.LossyDisplayName()), + *error); + } + } + + return static_cast<base::DictionaryValue*>(dictionary); +} + +} // namespace + static std::string& GetProcessLocale() { CR_DEFINE_STATIC_LOCAL(std::string, locale, ()); return locale; @@ -46,7 +79,6 @@ std::string GetDefaultLocaleFromManifest(const base::DictionaryValue& manifest, *error = errors::kInvalidDefaultLocale; return std::string(); - } bool ShouldRelocalizeManifest(const base::DictionaryValue* manifest) { @@ -291,35 +323,13 @@ bool GetValidLocales(const base::FilePath& locale_path, } if (valid_locales->empty()) { - *error = extensions::manifest_errors::kLocalesNoValidLocaleNamesListed; + *error = errors::kLocalesNoValidLocaleNamesListed; return false; } return true; } -// Loads contents of the messages file for given locale. If file is not found, -// or there was parsing error we return NULL and set |error|. -// Caller owns the returned object. -static base::DictionaryValue* LoadMessageFile( - const base::FilePath& locale_path, - const std::string& locale, - std::string* error) { - std::string extension_locale = locale; - base::FilePath file = locale_path.AppendASCII(extension_locale) - .Append(extensions::kMessagesFilename); - JSONFileValueSerializer messages_serializer(file); - base::Value *dictionary = messages_serializer.Deserialize(NULL, error); - if (!dictionary && error->empty()) { - // JSONFileValueSerializer just returns NULL if file cannot be found. It - // doesn't set the error, so we have to do it. - *error = base::StringPrintf("Catalog file is missing for locale %s.", - extension_locale.c_str()); - } - - return static_cast<base::DictionaryValue*>(dictionary); -} - extensions::MessageBundle* LoadMessageCatalogs( const base::FilePath& locale_path, const std::string& default_locale, @@ -349,6 +359,37 @@ extensions::MessageBundle* LoadMessageCatalogs( return extensions::MessageBundle::Create(catalogs, error); } +bool ValidateExtensionLocales(const base::FilePath& extension_path, + const base::DictionaryValue* manifest, + std::string* error) { + std::string default_locale = GetDefaultLocaleFromManifest(*manifest, error); + + if (default_locale.empty()) + return true; + + base::FilePath locale_path = + extension_path.Append(extensions::kLocaleFolder); + + std::set<std::string> valid_locales; + if (!GetValidLocales(locale_path, &valid_locales, error)) + return false; + + for (std::set<std::string>::const_iterator locale = valid_locales.begin(); + locale != valid_locales.end(); ++locale) { + std::string locale_error; + scoped_ptr<DictionaryValue> catalog( + LoadMessageFile(locale_path, *locale, &locale_error)); + + if (!locale_error.empty()) { + if (!error->empty()) + error->append(" "); + error->append(locale_error); + } + } + + return error->empty(); +} + bool ShouldSkipValidation(const base::FilePath& locales_path, const base::FilePath& locale_path, const std::set<std::string>& all_locales) { diff --git a/chrome/common/extensions/extension_l10n_util.h b/chrome/common/extensions/extension_l10n_util.h index 684a198..29f1831 100644 --- a/chrome/common/extensions/extension_l10n_util.h +++ b/chrome/common/extensions/extension_l10n_util.h @@ -97,6 +97,11 @@ extensions::MessageBundle* LoadMessageCatalogs( const std::set<std::string>& valid_locales, std::string* error); +// Loads message catalogs for all locales to check for validity. +bool ValidateExtensionLocales(const base::FilePath& extension_path, + const base::DictionaryValue* manifest, + std::string* error); + // Returns true if directory has "." in the name (for .svn) or if it doesn't // belong to Chrome locales. // |locales_path| is extension_id/_locales diff --git a/chrome/common/extensions/extension_l10n_util_unittest.cc b/chrome/common/extensions/extension_l10n_util_unittest.cc index fbb5c63..78148a5 100644 --- a/chrome/common/extensions/extension_l10n_util_unittest.cc +++ b/chrome/common/extensions/extension_l10n_util_unittest.cc @@ -8,12 +8,15 @@ #include "base/memory/linked_ptr.h" #include "base/memory/scoped_ptr.h" #include "base/path_service.h" +#include "base/strings/utf_string_conversions.h" #include "base/values.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/extensions/extension_l10n_util.h" #include "chrome/common/extensions/message_bundle.h" #include "extensions/common/constants.h" +#include "extensions/common/error_utils.h" #include "extensions/common/manifest_constants.h" +#include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "ui/base/l10n/l10n_util.h" @@ -26,6 +29,28 @@ namespace keys = extensions::manifest_keys; namespace { +TEST(ExtensionL10nUtil, ValidateLocalesWithBadLocale) { + base::ScopedTempDir temp; + ASSERT_TRUE(temp.CreateUniqueTempDir()); + + base::FilePath src_path = temp.path().Append(kLocaleFolder); + base::FilePath locale = src_path.AppendASCII("ms"); + ASSERT_TRUE(file_util::CreateDirectory(locale)); + + base::FilePath messages_file = locale.Append(kMessagesFilename); + std::string data = "{ \"name\":"; + ASSERT_TRUE(file_util::WriteFile(messages_file, data.c_str(), data.length())); + + base::DictionaryValue manifest; + manifest.SetString(keys::kDefaultLocale, "en"); + std::string error; + EXPECT_FALSE(extension_l10n_util::ValidateExtensionLocales( + temp.path(), &manifest, &error)); + EXPECT_THAT(error, + testing::HasSubstr( + UTF16ToUTF8(messages_file.LossyDisplayName()))); +} + TEST(ExtensionL10nUtil, GetValidLocalesEmptyLocaleFolder) { base::ScopedTempDir temp; ASSERT_TRUE(temp.CreateUniqueTempDir()); @@ -161,9 +186,8 @@ TEST(ExtensionL10nUtil, LoadMessageCatalogsBadJSONFormat) { ASSERT_TRUE(file_util::CreateDirectory(locale)); std::string data = "{ \"name\":"; - ASSERT_TRUE( - file_util::WriteFile(locale.Append(kMessagesFilename), - data.c_str(), data.length())); + base::FilePath messages_file = locale.Append(kMessagesFilename); + ASSERT_TRUE(file_util::WriteFile(messages_file, data.c_str(), data.length())); std::set<std::string> valid_locales; valid_locales.insert("sr"); @@ -174,7 +198,12 @@ TEST(ExtensionL10nUtil, LoadMessageCatalogsBadJSONFormat) { "sr", valid_locales, &error)); - EXPECT_EQ("Line: 1, column: 10, Unexpected token.", error); + EXPECT_EQ( + extensions::ErrorUtils::FormatErrorMessage( + errors::kLocalesInvalidLocale, + base::UTF16ToUTF8(messages_file.LossyDisplayName()), + "Line: 1, column: 10, Unexpected token."), + error); } TEST(ExtensionL10nUtil, LoadMessageCatalogsDuplicateKeys) { diff --git a/chrome/test/data/extensions/unpacked/bad_messages_file/_locales/en/messages.json b/chrome/test/data/extensions/unpacked/bad_messages_file/_locales/en/messages.json new file mode 100644 index 0000000..3502076 --- /dev/null +++ b/chrome/test/data/extensions/unpacked/bad_messages_file/_locales/en/messages.json @@ -0,0 +1,6 @@ +{ + "application_title": { + "message": "Test Localized Extension", + "description": "The title of the extensions, displayed in the web store." + } +} diff --git a/chrome/test/data/extensions/unpacked/bad_messages_file/_locales/ms/messages.json b/chrome/test/data/extensions/unpacked/bad_messages_file/_locales/ms/messages.json new file mode 100644 index 0000000..05ec136 --- /dev/null +++ b/chrome/test/data/extensions/unpacked/bad_messages_file/_locales/ms/messages.json @@ -0,0 +1,3 @@ +{ + "application_title": { + diff --git a/chrome/test/data/extensions/unpacked/bad_messages_file/manifest.json b/chrome/test/data/extensions/unpacked/bad_messages_file/manifest.json new file mode 100644 index 0000000..71993e7 --- /dev/null +++ b/chrome/test/data/extensions/unpacked/bad_messages_file/manifest.json @@ -0,0 +1,7 @@ +{ + "name": "__MSG_application_title__", + "description": "Tests if loading an unpacked extension tests all of its locales", + "version": "0.2", + "default_locale": "en", + "manifest_version": 2 + } diff --git a/extensions/common/manifest_constants.cc b/extensions/common/manifest_constants.cc index caf22e1..7ab1d23 100644 --- a/extensions/common/manifest_constants.cc +++ b/extensions/common/manifest_constants.cc @@ -598,6 +598,8 @@ const char kLaunchPathAndURLAreExclusive[] = "both be set."; const char kLaunchURLRequired[] = "Either 'app.launch.local_path' or 'app.launch.web_url' is required."; +const char kLocalesInvalidLocale[] = + "Invalid locale file '*': *"; const char kLocalesMessagesFileMissing[] = "Messages file is missing for locale."; const char kLocalesNoDefaultLocaleSpecified[] = diff --git a/extensions/common/manifest_constants.h b/extensions/common/manifest_constants.h index f87fdf2..53fb1ef 100644 --- a/extensions/common/manifest_constants.h +++ b/extensions/common/manifest_constants.h @@ -400,6 +400,7 @@ extern const char kInsecureContentSecurityPolicy[]; extern const char kLaunchPathAndExtentAreExclusive[]; extern const char kLaunchPathAndURLAreExclusive[]; extern const char kLaunchURLRequired[]; +extern const char kLocalesInvalidLocale[]; extern const char kLocalesMessagesFileMissing[]; extern const char kLocalesNoDefaultLocaleSpecified[]; extern const char kLocalesNoDefaultMessages[]; |