Actually run directory validation code on install.

BUG=23084
TEST=Install extensions attached to referenced bug. You should get an error dialog, not a crash.

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

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

4 files changed, 18 insertions, 3 deletions

diff --git a/chrome/browser/extensions/extension_file_util.cc b/chrome/browser/extensions/extension_file_util.cc
index 1692c67..ac34470 100644
--- a/chrome/browser/extensions/extension_file_util.cc
+++ b/chrome/browser/extensions/extension_file_util.cc
@@ -366,8 +366,8 @@ bool CheckForIllegalFilenames(const FilePath& extension_path,
     if (reserved_underscore_names.find(filename) ==
           reserved_underscore_names.end()) {
       *error = StringPrintf(
-        "Cannot load extension with file or directory name %s."
-        "Filenames starting with \"_\" are reserved for use by the system",
+        "Cannot load extension with file or directory name %s. "
+        "Filenames starting with \"_\" are reserved for use by the system.",
         filename.c_str());
       return false;
     }
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index 28969d2..d380c90 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -715,6 +715,14 @@ TEST_F(ExtensionsServiceTest, InstallExtension) {
   InstallExtension(path, false);
   ValidatePrefKeyCount(pref_count);
 
+  // Extensions cannot have folders or files that have underscores except ofr in
+  // certain whitelisted cases (eg _locales). This is an example of a broader
+  // class of validation that we do to the directory structure of the extension.
+  // We did not used to handle this correctly for installation.
+  path = extensions_path.AppendASCII("bad_underscore.crx");
+  InstallExtension(path, false);
+  ValidatePrefKeyCount(pref_count);
+
   // TODO(erikkay): add more tests for many of the failure cases.
   // TODO(erikkay): add tests for upgrade cases.
 }
diff --git a/chrome/common/extensions/extension_unpacker.cc b/chrome/common/extensions/extension_unpacker.cc
index 19b8d9c..95c7e0d 100644
--- a/chrome/common/extensions/extension_unpacker.cc
+++ b/chrome/common/extensions/extension_unpacker.cc
@@ -11,6 +11,7 @@
 #include "base/thread.h"
 #include "base/values.h"
 #include "net/base/file_stream.h"
+#include "chrome/browser/extensions/extension_file_util.h"
 #include "chrome/common/common_param_traits.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
@@ -131,7 +132,7 @@ bool ExtensionUnpacker::Run() {
   // InitFromValue is allowed to generate a temporary id for the extension.
   // ANY CODE THAT FOLLOWS SHOULD NOT DEPEND ON THE CORRECT ID OF THIS
   // EXTENSION.
-  Extension extension;
+  Extension extension(temp_install_dir_);
   std::string error;
   if (!extension.InitFromValue(*parsed_manifest_,
                                false,
@@ -139,6 +140,12 @@ bool ExtensionUnpacker::Run() {
     SetError(error);
     return false;
   }
+
+  if (!extension_file_util::ValidateExtension(&extension, &error)) {
+    SetError(error);
+    return false;
+  }
+
   // Decode any images that the browser needs to display.
   std::set<FilePath> image_paths = extension.GetBrowserImages();
   for (std::set<FilePath>::iterator it = image_paths.begin();
diff --git a/chrome/test/data/extensions/bad_underscore.crx b/chrome/test/data/extensions/bad_underscore.crx
new file mode 100755
index 0000000..9230799
--- /dev/null
+++ b/chrome/test/data/extensions/bad_underscore.crx