From 9e54cb57bc90560f8f0eb2058ccf583de9c59f7d Mon Sep 17 00:00:00 2001
From: "skerner@chromium.org"
<skerner@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri, 3 Sep 2010 20:08:06 +0000
Subject: Error out if a version in external_extensions.json is invalid.
BUG=53915
TEST=ExtensionsServiceTest.ExternalPrefProvider
Review URL: http://codereview.chromium.org/3233008
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@58531 0039d316-1c4b-4281-b951-d872f2087c98
---
chrome/browser/extensions/extensions_service.cc | 1 +
.../extensions/extensions_service_unittest.cc | 81 ++++++++++++----------
.../extensions/external_pref_extension_provider.cc | 11 ++-
3 files changed, 54 insertions(+), 39 deletions(-)
(limited to 'chrome/browser/extensions')
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 4d1ce3b..06198387 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -1620,6 +1620,7 @@ void ExtensionsServiceBackend::SetProviderForTesting(
void ExtensionsServiceBackend::OnExternalExtensionFileFound(
const std::string& id, const Version* version, const FilePath& path,
Extension::Location location) {
+ DCHECK(version);
ChromeThread::PostTask(
ChromeThread::UI, FROM_HERE,
NewRunnableMethod(
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index bd92b0c..5e64b79 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -2309,17 +2309,17 @@ TEST_F(ExtensionsServiceTest, ExternalPrefProvider) {
InitializeEmptyExtensionsService();
std::string json_data =
"{"
- "\"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\": {"
- "\"external_crx\": \"RandomExtension.crx\","
- "\"external_version\": \"1.0\""
- "},"
- "\"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\": {"
- "\"external_crx\": \"RandomExtension2.crx\","
- "\"external_version\": \"2.0\""
- "},"
- "\"cccccccccccccccccccccccccccccccc\": {"
- "\"external_update_url\": \"http:\\\\foo.com/update\""
- "}"
+ " \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\": {"
+ " \"external_crx\": \"RandomExtension.crx\","
+ " \"external_version\": \"1.0\""
+ " },"
+ " \"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\": {"
+ " \"external_crx\": \"RandomExtension2.crx\","
+ " \"external_version\": \"2.0\""
+ " },"
+ " \"cccccccccccccccccccccccccccccccc\": {"
+ " \"external_update_url\": \"http:\\\\foo.com/update\""
+ " }"
"}";
MockProviderVisitor visitor;
@@ -2332,40 +2332,47 @@ TEST_F(ExtensionsServiceTest, ExternalPrefProvider) {
ignore_list.insert("cccccccccccccccccccccccccccccccc");
EXPECT_EQ(0, visitor.Visit(json_data, ignore_list));
- // Use a json that contains six invalid extensions:
+ // Simulate an external_extensions.json file that contains seven invalid
+ // extensions:
// - One that is missing the 'external_crx' key.
// - One that is missing the 'external_version' key.
// - One that is specifying .. in the path.
// - One that specifies both a file and update URL.
// - One that specifies no file or update URL.
// - One that has an update URL that is not well formed.
- // - Plus one valid extension to make sure the json file is parsed properly.
+ // - One that contains a malformed version.
+ // The final extension is valid, and we check that it is read to make sure
+ // failures don't stop valid records from being read.
json_data =
"{"
- "\"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\": {"
- "\"external_version\": \"1.0\""
- "},"
- "\"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\": {"
- "\"external_crx\": \"RandomExtension.crx\""
- "},"
- "\"cccccccccccccccccccccccccccccccc\": {"
- "\"external_crx\": \"..\\\\foo\\\\RandomExtension2.crx\","
- "\"external_version\": \"2.0\""
- "},"
- "\"dddddddddddddddddddddddddddddddd\": {"
- "\"external_crx\": \"..\\\\foo\\\\RandomExtension2.crx\","
- "\"external_version\": \"2.0\","
- "\"external_update_url\": \"http:\\\\foo.com/update\""
- "},"
- "\"eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\": {"
- "},"
- "\"ffffffffffffffffffffffffffffffff\": {"
- "\"external_update_url\": \"This string is not avalid URL\""
- "},"
- "\"gggggggggggggggggggggggggggggggg\": {"
- "\"external_crx\": \"RandomValidExtension.crx\","
- "\"external_version\": \"1.0\""
- "}"
+ " \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\": {"
+ " \"external_version\": \"1.0\""
+ " },"
+ " \"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\": {"
+ " \"external_crx\": \"RandomExtension.crx\""
+ " },"
+ " \"cccccccccccccccccccccccccccccccc\": {"
+ " \"external_crx\": \"..\\\\foo\\\\RandomExtension2.crx\","
+ " \"external_version\": \"2.0\""
+ " },"
+ " \"dddddddddddddddddddddddddddddddd\": {"
+ " \"external_crx\": \"RandomExtension2.crx\","
+ " \"external_version\": \"2.0\","
+ " \"external_update_url\": \"http:\\\\foo.com/update\""
+ " },"
+ " \"eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\": {"
+ " },"
+ " \"ffffffffffffffffffffffffffffffff\": {"
+ " \"external_update_url\": \"This string is not a valid URL\""
+ " },"
+ " \"gggggggggggggggggggggggggggggggg\": {"
+ " \"external_crx\": \"RandomExtension3.crx\","
+ " \"external_version\": \"This is not a valid version!\""
+ " },"
+ " \"hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh\": {"
+ " \"external_crx\": \"RandomValidExtension.crx\","
+ " \"external_version\": \"1.0\""
+ " }"
"}";
ignore_list.clear();
EXPECT_EQ(1, visitor.Visit(json_data, ignore_list));
diff --git a/chrome/browser/extensions/external_pref_extension_provider.cc b/chrome/browser/extensions/external_pref_extension_provider.cc
index 9020151..3eaafc9 100644
--- a/chrome/browser/extensions/external_pref_extension_provider.cc
+++ b/chrome/browser/extensions/external_pref_extension_provider.cc
@@ -86,7 +86,7 @@ void ExternalPrefExtensionProvider::VisitRegisteredExtension(
continue;
}
- // See if it's an absolute path...
+ // If the path is relative, make it absolute.
FilePath path(external_crx);
if (!path.IsAbsolute()) {
// Try path as relative path from external extension dir.
@@ -97,6 +97,12 @@ void ExternalPrefExtensionProvider::VisitRegisteredExtension(
scoped_ptr<Version> version;
version.reset(Version::GetVersionFromString(external_version));
+ if (!version.get()) {
+ LOG(ERROR) << "Malformed extension dictionary for extension: "
+ << extension_id.c_str() << ". Invalid version string \""
+ << external_version << "\".";
+ continue;
+ }
visitor->OnExternalExtensionFileFound(extension_id, version.get(), path,
Extension::EXTERNAL_PREF);
continue;
@@ -107,7 +113,8 @@ void ExternalPrefExtensionProvider::VisitRegisteredExtension(
if (!update_url.is_valid()) {
LOG(WARNING) << "Malformed extension dictionary for extension: "
<< extension_id.c_str() << ". " << kExternalUpdateUrl
- << " must be a valid URL: " << external_update_url;
+ << " must be a valid URL. Saw \"" << external_update_url
+ << "\".";
continue;
}
visitor->OnExternalExtensionUpdateUrlFound(extension_id, update_url);
--
cgit v1.1