Check zero-length icon files during extension validation.

chrome/common/extensions/extension_file_util.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/common/extensions/extension_file_util.h"
 
 #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"
 #include "base/metrics/histogram.h"
 #include "base/path_service.h"
 #include "base/scoped_temp_dir.h"
 #include "base/stringprintf.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/common/chrome_constants.h"
@@ skipping 202 matching lines @@
 
 bool ValidateExtension(const Extension* extension,
                        std::string* error,
                        Extension::InstallWarningVector* warnings) {
   // Validate icons exist.
   for (ExtensionIconSet::IconMap::const_iterator iter =
            extension->icons().map().begin();
        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));
       return false;
     }
   }
 
   // Theme resource validation.
   if (extension->is_theme()) {
     DictionaryValue* images_value = extension->GetThemeImages();
@@ skipping 57 matching lines @@
     const Extension::PluginInfo& plugin = extension->plugins()[i];
     if (!file_util::PathExists(plugin.path)) {
       *error =
           l10n_util::GetStringFUTF8(
               IDS_EXTENSION_LOAD_PLUGIN_PATH_FAILED,
               plugin.path.LossyDisplayName());
       return false;
     }
   }
 
-  // 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());
     if (!page_action->default_icon_path().empty())
       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,
                 UTF8ToUTF16(*iter));
         return false;
       }
     }
   }
 
-  // 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;
+      }
     }
   }
 
   // Validate that background scripts exist.
   for (size_t i = 0; i < extension->background_scripts().size(); ++i) {
     if (!file_util::PathExists(
             extension->GetResource(
                 extension->background_scripts()[i]).GetFilePath())) {
       *error = l10n_util::GetStringFUTF8(
           IDS_EXTENSION_LOAD_BACKGROUND_SCRIPT_FAILED,
@@ skipping 422 matching lines @@
     return temp_path;
 
   return FilePath();
 }
 
 void DeleteFile(const FilePath& path, bool recursive) {
   file_util::Delete(path, recursive);
 }
 
 }  // namespace extension_file_util