Implement mimetype sniffing for extensions.

abarth: can you review the changes to mime_sniffer.cc?
paul: everything else?

BUG=13296
TEST=Added unit tests
Review URL: http://codereview.chromium.org/159345

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

8 files changed, 154 insertions, 7 deletions

diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index 01ea2d0..1a3fb53 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -117,6 +117,8 @@ DownloadItem::DownloadItem(const DownloadCreateInfo& info)
     : id_(-1),
       full_path_(info.path),
       url_(info.url),
+      referrer_url_(info.referrer_url),
+      mime_type_(info.mime_type),
       total_bytes_(info.total_bytes),
       received_bytes_(info.received_bytes),
       start_tick_(base::TimeTicks()),
@@ -142,6 +144,7 @@ DownloadItem::DownloadItem(int32 download_id,
                            int path_uniquifier,
                            const GURL& url,
                            const GURL& referrer_url,
+                           const std::string& mime_type,
                            const FilePath& original_name,
                            const base::Time start_time,
                            int64 download_size,
@@ -153,6 +156,7 @@ DownloadItem::DownloadItem(int32 download_id,
       path_uniquifier_(path_uniquifier),
       url_(url),
       referrer_url_(referrer_url),
+      mime_type_(mime_type),
       total_bytes_(download_size),
       received_bytes_(0),
       start_tick_(base::TimeTicks::Now()),
@@ -677,6 +681,7 @@ void DownloadManager::ContinueStartDownload(DownloadCreateInfo* info,
                                 info->path_uniquifier,
                                 info->url,
                                 info->referrer_url,
+                                info->mime_type,
                                 info->original_name,
                                 info->start_time,
                                 info->total_bytes,
@@ -841,7 +846,7 @@ void DownloadManager::ContinueDownloadFinished(DownloadItem* download) {
     extension = extension.substr(1);
 
   // Handle chrome extensions explicitly and skip the shell execute.
-  if (Extension::IsExtension(download->full_path())) {
+  if (download->mime_type() == Extension::kMimeType) {
     OpenChromeExtension(download->full_path(), download->url(),
                         download->referrer_url());
     download->set_auto_opened(true);
@@ -1211,7 +1216,7 @@ void DownloadManager::OpenDownload(const DownloadItem* download,
                                    gfx::NativeView parent_window) {
   // Open Chrome extensions with ExtensionsService. For everything else do shell
   // execute.
-  if (Extension::IsExtension(download->full_path())) {
+  if (download->mime_type() == Extension::kMimeType) {
     OpenChromeExtension(download->full_path(), download->url(),
                         download->referrer_url());
   } else {
diff --git a/chrome/browser/download/download_manager.h b/chrome/browser/download/download_manager.h
index f1b1030..f39308e 100644
--- a/chrome/browser/download/download_manager.h
+++ b/chrome/browser/download/download_manager.h
@@ -109,6 +109,7 @@ class DownloadItem {
                int path_uniquifier,
                const GURL& url,
                const GURL& referrer_url,
+               const std::string& mime_type,
                const FilePath& original_name,
                const base::Time start_time,
                int64 download_size,
@@ -186,6 +187,7 @@ class DownloadItem {
   void set_path_uniquifier(int uniquifier) { path_uniquifier_ = uniquifier; }
   GURL url() const { return url_; }
   GURL referrer_url() const { return referrer_url_; }
+  std::string mime_type() const { return mime_type_; }
   int64 total_bytes() const { return total_bytes_; }
   void set_total_bytes(int64 total_bytes) { total_bytes_ = total_bytes; }
   int64 received_bytes() const { return received_bytes_; }
@@ -238,6 +240,9 @@ class DownloadItem {
   // The URL of the page that initiated the download.
   GURL referrer_url_;
 
+  // The mimetype of the download
+  std::string mime_type_;
+
   // Total bytes expected
   int64 total_bytes_;
 
diff --git a/chrome/browser/download/save_package.cc b/chrome/browser/download/save_package.cc
index 566ed63..dcf8bdb 100644
--- a/chrome/browser/download/save_package.cc
+++ b/chrome/browser/download/save_package.cc
@@ -257,7 +257,7 @@ bool SavePackage::Init() {
 
   // Create the fake DownloadItem and display the view.
   download_ = new DownloadItem(1, saved_main_file_path_, 0, page_url_, GURL(),
-      FilePath(), Time::Now(), 0, -1, -1, false);
+      "", FilePath(), Time::Now(), 0, -1, -1, false);
   download_->set_manager(tab_contents_->profile()->GetDownloadManager());
   tab_contents_->OnStartDownload(download_);
 
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index b0324d6..b5e0684 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -74,6 +74,8 @@ const char* Extension::kExtensionRegistryPath =
 // first 16 bytes of SHA256 hashed public key.
 const size_t Extension::kIdSize = 16;
 
+const char Extension::kMimeType[] = "application/x-chrome-extension";
+
 const int Extension::kKnownIconSizes[] = { 128 };
 
 Extension::~Extension() {
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index 32fc403c..6600da0 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -64,13 +64,13 @@ class Extension {
   // The number of bytes in a legal id.
   static const size_t kIdSize;
 
+  // The mimetype used for extensions.
+  static const char kMimeType[];
+
   Extension() : location_(INVALID), is_theme_(false) {}
   explicit Extension(const FilePath& path);
   virtual ~Extension();
 
-  // Returns true if the specified file is an extension.
-  static bool IsExtension(const FilePath& file_name);
-
   // Resets the id counter. This is only useful for unit tests.
   static void ResetGeneratedIdCounter() {
     id_counter_ = 0;
@@ -79,6 +79,9 @@ class Extension {
   // Checks to see if the extension has a valid ID.
   static bool IdIsValid(const std::string& id);
 
+  // Returns true if the specified file is an extension.
+  static bool IsExtension(const FilePath& file_name);
+
   // Whether the |location| is external or not.
   static inline bool IsExternalLocation(Location location) {
     return location == Extension::EXTERNAL_PREF ||
diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc
index 65ade63..a33f40c 100644
--- a/chrome/common/extensions/extension_unittest.cc
+++ b/chrome/common/extensions/extension_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/file_path.h"
+#include "base/file_util.h"
 #include "base/string_util.h"
 #include "base/path_service.h"
 #include "chrome/common/chrome_paths.h"
@@ -10,6 +11,7 @@
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/extension_error_reporter.h"
 #include "chrome/common/json_value_serializer.h"
+#include "net/base/mime_sniffer.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace keys = extension_manifest_keys;
@@ -429,3 +431,27 @@ TEST(ExtensionTest, UpdateUrls) {
     EXPECT_TRUE(MatchPattern(error, errors::kInvalidUpdateURL));
   }
 }
+
+// This test ensures that the mimetype sniffing code stays in sync with the
+// actual crx files that we test other parts of the system with.
+TEST(ExtensionTest, MimeTypeSniffing) {
+  FilePath path;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &path));
+  path = path.AppendASCII("extensions").AppendASCII("good.crx");
+
+  std::string data;
+  ASSERT_TRUE(file_util::ReadFileToString(path, &data));
+
+  std::string result;
+  EXPECT_TRUE(net::SniffMimeType(data.c_str(), data.size(),
+              GURL("http://www.example.com/foo.crx"), "", &result));
+  EXPECT_EQ(std::string(Extension::kMimeType), result);
+
+  data.clear();
+  result.clear();
+  path = path.DirName().AppendASCII("bad_magic.crx");
+  ASSERT_TRUE(file_util::ReadFileToString(path, &data));
+  EXPECT_TRUE(net::SniffMimeType(data.c_str(), data.size(),
+              GURL("http://www.example.com/foo.crx"), "", &result));
+  EXPECT_EQ("application/octet-stream", result);
+}
diff --git a/net/base/mime_sniffer.cc b/net/base/mime_sniffer.cc
index 0a8bc57..07feb33 100644
--- a/net/base/mime_sniffer.cc
+++ b/net/base/mime_sniffer.cc
@@ -256,7 +256,7 @@ static bool CheckForMagicNumbers(const char* content, size_t size,
                                  Histogram* counter, std::string* result) {
   for (size_t i = 0; i < magic_len; ++i) {
     if (MatchMagicNumber(content, size, &(magic[i]), result)) {
-      counter->Add(static_cast<int>(i));
+      if (counter) counter->Add(static_cast<int>(i));
       return true;
     }
   }
@@ -438,6 +438,43 @@ static bool IsUnknownMimeType(const std::string& mime_type) {
   return false;
 }
 
+// Sniff a crx (chrome extension) file.
+static bool SniffCRX(const char* content, size_t content_size, const GURL& url,
+                     const std::string& type_hint, std::string* result) {
+  static SnifferHistogram counter("mime_sniffer.kSniffCRX", 3);
+
+  // Technically, the crx magic number is just Cr24, but the bytes after that
+  // are a version number which changes infrequently. Including it in the
+  // sniffing gives us less room for error. If the version number ever changes,
+  // we can just add an entry to this list.
+  //
+  // TODO(aa): If we ever have another magic number, we'll want to pass a
+  // histogram into CheckForMagicNumbers(), below, to see which one matched.
+  const struct MagicNumber kCRXMagicNumbers[] = {
+    MAGIC_NUMBER("application/x-chrome-extension", "Cr24\x02\x00\x00\x00")
+  };
+
+  // Only consider files that have the extension ".crx".
+  const char kCRXExtension[] = ".crx";
+  const int kExtensionLength = arraysize(kCRXExtension) - 1;  // ignore null
+  if (url.path().rfind(kCRXExtension, std::string::npos, kExtensionLength) ==
+      url.path().size() - kExtensionLength) {
+    counter.Add(1);
+  } else {
+    return false;
+  }
+
+  if (CheckForMagicNumbers(content, content_size,
+                           kCRXMagicNumbers, arraysize(kCRXMagicNumbers),
+                           NULL, result)) {
+    counter.Add(2);
+  } else {
+    return false;
+  }
+
+  return true;
+}
+
 bool ShouldSniffMimeType(const GURL& url, const std::string& mime_type) {
   static SnifferHistogram should_sniff_counter(
       "mime_sniffer.ShouldSniffMimeType2", 3);
@@ -535,6 +572,11 @@ bool SniffMimeType(const char* content, size_t content_size,
     return content_size >= kMaxBytesToSniff;
   }
 
+  // CRX files (chrome extensions) have a special sniffing algorithm. It is
+  // tighter than the others because we don't have to match legacy behavior.
+  if (SniffCRX(content, content_size, url, type_hint, result))
+    return true;
+
   // Now we look in our large table of magic numbers to see if we can find
   // anything that matches the content.
   if (SniffForMagicNumbers(content, content_size, result))
diff --git a/net/base/mime_sniffer_unittest.cc b/net/base/mime_sniffer_unittest.cc
index 96eb441..ed1634c 100644
--- a/net/base/mime_sniffer_unittest.cc
+++ b/net/base/mime_sniffer_unittest.cc
@@ -90,6 +90,70 @@ TEST(MimeSnifferTest, BasicSniffingTest) {
   TestArray(tests, arraysize(tests));
 }
 
+TEST(MimeSnifferTest, ChromeExtensionsTest) {
+  SnifferTest tests[] = {
+    // schemes
+    { "Cr24\x02\x00\x00\x00", sizeof("Cr24\x02\x00\x00\x00")-1,
+      "http://www.example.com/foo.crx",
+      "", "application/x-chrome-extension" },
+    { "Cr24\x02\x00\x00\x00", sizeof("Cr24\x02\x00\x00\x00")-1,
+      "https://www.example.com/foo.crx",
+      "", "application/x-chrome-extension" },
+    { "Cr24\x02\x00\x00\x00", sizeof("Cr24\x02\x00\x00\x00")-1,
+      "ftp://www.example.com/foo.crx",
+      "", "application/x-chrome-extension" },
+
+    // some other mimetypes that should get converted
+    { "Cr24\x02\x00\x00\x00", sizeof("Cr24\x02\x00\x00\x00")-1,
+      "http://www.example.com/foo.crx",
+      "text/plain", "application/x-chrome-extension" },
+    { "Cr24\x02\x00\x00\x00", sizeof("Cr24\x02\x00\x00\x00")-1,
+      "http://www.example.com/foo.crx",
+      "application/octet-stream", "application/x-chrome-extension" },
+
+    // success edge cases
+    { "Cr24\x02\x00\x00\x00", sizeof("Cr24\x02\x00\x00\x00")-1,
+      "http://www.example.com/foo.crx?query=string",
+      "", "application/x-chrome-extension" },
+    { "Cr24\x02\x00\x00\x00", sizeof("Cr24\x02\x00\x00\x00")-1,
+      "http://www.example.com/foo..crx",
+      "", "application/x-chrome-extension" },
+
+    // wrong file extension
+    { "Cr24\x02\x00\x00\x00", sizeof("Cr24\x02\x00\x00\x00")-1,
+      "http://www.example.com/foo.bin",
+      "", "application/octet-stream" },
+    { "Cr24\x02\x00\x00\x00", sizeof("Cr24\x02\x00\x00\x00")-1,
+      "http://www.example.com/foo.bin?monkey",
+      "", "application/octet-stream" },
+    { "Cr24\x02\x00\x00\x00", sizeof("Cr24\x02\x00\x00\x00")-1,
+      "invalid-url",
+      "", "application/octet-stream" },
+    { "Cr24\x02\x00\x00\x00", sizeof("Cr24\x02\x00\x00\x00")-1,
+      "http://www.example.com",
+      "", "application/octet-stream" },
+    { "Cr24\x02\x00\x00\x00", sizeof("Cr24\x02\x00\x00\x00")-1,
+      "http://www.example.com/",
+      "", "application/octet-stream" },
+    { "Cr24\x02\x00\x00\x00", sizeof("Cr24\x02\x00\x00\x00")-1,
+      "http://www.example.com/foo",
+      "", "application/octet-stream" },
+    { "Cr24\x02\x00\x00\x00", sizeof("Cr24\x02\x00\x00\x00")-1,
+      "http://www.example.com/foocrx",
+      "", "application/octet-stream" },
+    { "Cr24\x02\x00\x00\x00", sizeof("Cr24\x02\x00\x00\x00")-1,
+      "http://www.example.com/foo.crx.blech",
+      "", "application/octet-stream" },
+
+    // wrong magic
+    { "Cr24\x02\x00\x00\x01", sizeof("Cr24\x02\x00\x00\x01")-1,
+      "http://www.example.com/foo.crx?monkey",
+      "", "application/octet-stream" },
+  };
+
+  TestArray(tests, arraysize(tests));
+}
+
 TEST(MimeSnifferTest, MozillaCompatibleTest) {
   SnifferTest tests[] = {
     { " \n <hTmL>\n <hea", sizeof(" \n <hTmL>\n <hea")-1,