Check zero-length icon files during extension validation.

chrome/common/extensions/extension_file_util.cc

Index: chrome/common/extensions/extension_file_util.cc
diff --git a/chrome/common/extensions/extension_file_util.cc b/chrome/common/extensions/extension_file_util.cc
index 4a6f15002fdf763eb6a6ae296783aace777f335c..c4cb4a0d32af9b15e8b302f5a20d83bfa697ca4b 100644
--- a/chrome/common/extensions/extension_file_util.cc
+++ b/chrome/common/extensions/extension_file_util.cc
@@ -7,6 +7,7 @@
 #include <map>
 #include <vector>
 
+#include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/json/json_file_value_serializer.h"
 #include "base/logging.h"
@@ -229,7 +230,10 @@ bool ValidateExtension(const Extension* extension,
        iter != extension->icons().map().end();
        ++iter) {
     const FilePath path = extension->GetResource(iter->second).GetFilePath();
-    if (!file_util::PathExists(path)) {
+    int64 size;

[asargent_no_longer_on_chrome, 2012/07/03 19:46:49]

(pet peeve nit) please initialize |size| to 0

+    if (!file_util::PathExists(path) ||
+        !file_util::GetFileSize(path, &size) ||
+        size == 0) {
       *error =
           l10n_util::GetStringFUTF8(IDS_EXTENSION_LOAD_ICON_FAILED,
                                     UTF8ToUTF16(iter->second));
@@ -307,7 +311,7 @@ bool ValidateExtension(const Extension* extension,
     }
   }
 
-  // Validate icon location for page actions.
+  // Validate icon location and icon file size for page actions.
   ExtensionAction* page_action = extension->page_action();
   if (page_action) {
     std::vector<std::string> icon_paths(*page_action->icon_paths());
@@ -315,7 +319,11 @@ bool ValidateExtension(const Extension* extension,
       icon_paths.push_back(page_action->default_icon_path());
     for (std::vector<std::string>::iterator iter = icon_paths.begin();
          iter != icon_paths.end(); ++iter) {
-      if (!file_util::PathExists(extension->GetResource(*iter).GetFilePath())) {
+      const FilePath& path = extension->GetResource(*iter).GetFilePath();
+      int64 size;

[asargent_no_longer_on_chrome, 2012/07/03 19:46:49]

initialize to 0

+      if (!file_util::PathExists(path) ||
+          !file_util::GetFileSize(path, &size) ||
+          size == 0) {
         *error =
             l10n_util::GetStringFUTF8(
                 IDS_EXTENSION_LOAD_ICON_FOR_PAGE_ACTION_FAILED,
@@ -325,18 +333,23 @@ bool ValidateExtension(const Extension* extension,
     }
   }
 
-  // Validate icon location for browser actions.
+  // Validate icon location and icon file size for browser actions.
   // Note: browser actions don't use the icon_paths().
   ExtensionAction* browser_action = extension->browser_action();
   if (browser_action) {
     std::string path = browser_action->default_icon_path();
-    if (!path.empty() &&
-        !file_util::PathExists(extension->GetResource(path).GetFilePath())) {
+    if (!path.empty()) {
+      const FilePath& file_path = extension->GetResource(path).GetFilePath();
+      int64 size;
+      if (!file_util::PathExists(file_path) ||
+          file_util::GetFileSize(file_path, &size) ||
+          size == 0) {

[asargent_no_longer_on_chrome, 2012/07/03 19:46:49]

nit: looks like you have this chunk of "does file exist and have >0 size" code
repeated 3 times. Can you factor it out into a helper method named
ValidateFilePath or FileHasContent or something along those lines?

         *error =
             l10n_util::GetStringFUTF8(
                 IDS_EXTENSION_LOAD_ICON_FOR_BROWSER_ACTION_FAILED,
                 UTF8ToUTF16(path));
         return false;
+      }
     }
   }