Enforce new id format (hex str of sha-1)

Review URL: http://codereview.chromium.org/27236

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@10529 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 34 insertions, 27 deletions

diff --git a/chrome/browser/extensions/extension.cc b/chrome/browser/extensions/extension.cc
index 9efdd37..348cc13 100644
--- a/chrome/browser/extensions/extension.cc
+++ b/chrome/browser/extensions/extension.cc
@@ -69,6 +69,8 @@ const char* Extension::kInvalidZipHashError =
 const char* Extension::kInvalidPluginsDirError =
     "Invalid value for 'plugins_dir'.";
 
+const int Extension::kIdSize = 20;  // SHA1 (160 bits) == 20 bytes
+
 const std::string Extension::VersionString() const {
   return version_->GetString();
 }
@@ -172,19 +174,11 @@ bool Extension::InitFromValue(const DictionaryValue& source,
     *error = kInvalidIdError;
     return false;
   }
-  // Verify that the id is legal.  This test is basically verifying that it
-  // is ASCII and doesn't have any path components in it.
-  // TODO(erikkay): verify the actual id format - it will be more restrictive
-  // than this.  Perhaps just a hex string?
-  if (!IsStringASCII(id_)) {
-    *error = kInvalidIdError;
-    return false;
-  }
-  FilePath id_path;
-  id_path = id_path.AppendASCII(id_);
-  if ((id_path.value() == FilePath::kCurrentDirectory) ||
-      (id_path.value() == FilePath::kParentDirectory) ||
-      !(id_path.BaseName() == id_path)) {
+
+  // Verify that the id is legal.  The id is a hex string of the SHA-1 hash of
+  // the public key.
+  std::vector<uint8> id_bytes;
+  if (!HexStringToBytes(id_, &id_bytes) || id_bytes.size() != kIdSize) {
     *error = kInvalidIdError;
     return false;
   }
diff --git a/chrome/browser/extensions/extension.h b/chrome/browser/extensions/extension.h
index 39e5fff..48833d4 100644
--- a/chrome/browser/extensions/extension.h
+++ b/chrome/browser/extensions/extension.h
@@ -64,6 +64,9 @@ class Extension {
   static const char* kInvalidZipHashError;
   static const char* kInvalidPluginsDirError;
 
+  // The number of bytes in a legal id.
+  static const int kIdSize;
+
   // Creates an absolute url to a resource inside an extension. The
   // |extension_url| argument should be the url() from an Extension object. The
   // |relative_path| can be untrusted user input. The returned URL will either
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index 62df67c..a49399d 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -110,11 +110,15 @@ class ExtensionsServiceTestFrontend
         scoped_refptr<ExtensionsServiceFrontendInterface>(this));
     message_loop_.RunAllPending();
     if (should_succeed) {
-      EXPECT_EQ(1u, installed_.size());
+      EXPECT_EQ(1u, installed_.size()) << path.value();
       EXPECT_EQ(0u, errors_.size()) << path.value();
+      for (std::vector<std::string>::iterator err = errors_.begin();
+        err != errors_.end(); ++err) {
+        LOG(ERROR) << *err;
+      }
     } else {
-      EXPECT_EQ(0u, installed_.size());
-      EXPECT_EQ(1u, errors_.size());
+      EXPECT_EQ(0u, installed_.size()) << path.value();
+      EXPECT_EQ(1u, errors_.size()) << path.value();
     }
     installed_.clear();
     errors_.clear();
@@ -148,9 +152,14 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectorySuccess) {
       scoped_refptr<ExtensionsServiceFrontendInterface>(frontend.get()));
   frontend->GetMessageLoop()->RunAllPending();
 
+  std::vector<std::string>* errors = frontend->errors();
+  for (std::vector<std::string>::iterator err = errors->begin();
+    err != errors->end(); ++err) {
+    LOG(ERROR) << *err;
+  }
   ASSERT_EQ(3u, frontend->extensions()->size());
 
-  EXPECT_EQ(std::string("com.google.myextension1"),
+  EXPECT_EQ(std::string("00123456789ABCDEF0123456789ABCDEF0123456"),
             frontend->extensions()->at(0)->id());
   EXPECT_EQ(std::string("My extension 1"),
             frontend->extensions()->at(0)->name());
@@ -172,7 +181,7 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectorySuccess) {
   EXPECT_EQ(extension->path().AppendASCII("script2.js").value(),
             scripts[1].path().value());
 
-  EXPECT_EQ(std::string("com.google.myextension2"),
+  EXPECT_EQ(std::string("10123456789ABCDEF0123456789ABCDEF0123456"),
             frontend->extensions()->at(1)->id());
   EXPECT_EQ(std::string("My extension 2"),
             frontend->extensions()->at(1)->name());
@@ -182,7 +191,7 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectorySuccess) {
             frontend->extensions()->at(1)->plugins_dir().value());
   ASSERT_EQ(0u, frontend->extensions()->at(1)->content_scripts().size());
 
-  EXPECT_EQ(std::string("com.google.myextension3"),
+  EXPECT_EQ(std::string("20123456789ABCDEF0123456789ABCDEF0123456"),
             frontend->extensions()->at(2)->id());
   EXPECT_EQ(std::string("My extension 3"),
             frontend->extensions()->at(2)->name());
@@ -212,18 +221,19 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectoryFail) {
 
   EXPECT_TRUE(MatchPattern(frontend->errors()->at(0),
       std::string("Could not load extension from '*'. * ") +
-      JSONReader::kBadRootElementType));
+      JSONReader::kBadRootElementType)) << frontend->errors()->at(0);
 
   EXPECT_TRUE(MatchPattern(frontend->errors()->at(1),
       std::string("Could not load extension from '*'. ") +
-      Extension::kInvalidJsListError));
+      Extension::kInvalidJsListError)) << frontend->errors()->at(1);
 
   EXPECT_TRUE(MatchPattern(frontend->errors()->at(2),
       std::string("Could not load extension from '*'. ") +
-      Extension::kInvalidManifestError));
+      Extension::kInvalidManifestError)) << frontend->errors()->at(2);
 
   EXPECT_TRUE(MatchPattern(frontend->errors()->at(3),
-      "Could not load extension from '*'. Could not read '*' file."));
+      "Could not load extension from '*'. Could not read '*' file.")) <<
+      frontend->errors()->at(3);
 };
 
 // Test installing extensions.
diff --git a/chrome/test/data/extensions/bad/missing_content_script/1/manifest.json b/chrome/test/data/extensions/bad/missing_content_script/1/manifest.json
index 83340a8..af1ea06 100644
--- a/chrome/test/data/extensions/bad/missing_content_script/1/manifest.json
+++ b/chrome/test/data/extensions/bad/missing_content_script/1/manifest.json
@@ -1,6 +1,6 @@
 {

   "format_version": 1,

-  "id": "com.google.myextension1",

+  "id": "00123456789ABCDEF0123456789ABCDEF0123456",

   "version": "1.0.0.0",

   "name": "My extension 1",

   "description": "The first extension that I made.",

diff --git a/chrome/test/data/extensions/good.crx b/chrome/test/data/extensions/good.crx
index 7dcab23..1c927cb 100644
--- a/chrome/test/data/extensions/good.crx
+++ b/chrome/test/data/extensions/good.crx
diff --git a/chrome/test/data/extensions/good/extension1/1/manifest.json b/chrome/test/data/extensions/good/extension1/1/manifest.json
index ebf69d0..5a43dd0 100644
--- a/chrome/test/data/extensions/good/extension1/1/manifest.json
+++ b/chrome/test/data/extensions/good/extension1/1/manifest.json
@@ -1,6 +1,6 @@
 {

   "format_version": 1,

-  "id": "com.google.myextension1",

+  "id": "00123456789ABCDEF0123456789ABCDEF0123456",

   "version": "1.0.0.0",

   "name": "My extension 1",

   "description": "The first extension that I made.",

diff --git a/chrome/test/data/extensions/good/extension2/2/manifest.json b/chrome/test/data/extensions/good/extension2/2/manifest.json
index 5e727ec..5e1fb5a 100644
--- a/chrome/test/data/extensions/good/extension2/2/manifest.json
+++ b/chrome/test/data/extensions/good/extension2/2/manifest.json
@@ -1,6 +1,6 @@
 {

   "format_version": 1,

-  "id": "com.google.myextension2",

+  "id": "10123456789ABCDEF0123456789ABCDEF0123456",

   "version": "1.0.0.0",

   "name": "My extension 2",

   "plugins_dir": "npapi"

diff --git a/chrome/test/data/extensions/good/extension3/1.0/manifest.json b/chrome/test/data/extensions/good/extension3/1.0/manifest.json
index 8292d0c..f1390de 100644
--- a/chrome/test/data/extensions/good/extension3/1.0/manifest.json
+++ b/chrome/test/data/extensions/good/extension3/1.0/manifest.json
@@ -1,6 +1,6 @@
 {
   "format_version": 1,
-  "id": "com.google.myextension3",
+  "id": "20123456789ABCDEF0123456789ABCDEF0123456",
   "version": "1.0",
   "name": "My extension 3"
 }