Error diagnostics for Blacklist IO...

chrome/browser/privacy_blacklist/blacklist_store.cc

Index: chrome/browser/privacy_blacklist/blacklist_store.cc
===================================================================
--- chrome/browser/privacy_blacklist/blacklist_store.cc	(revision 25875)
+++ chrome/browser/privacy_blacklist/blacklist_store.cc	(working copy)
@@ -5,6 +5,7 @@
 #include "chrome/browser/privacy_blacklist/blacklist_store.h"
 
 #include <cstdio>
+#include <limits>
 
 #include "base/basictypes.h"
 #include "base/file_util.h"
@@ -14,83 +15,83 @@
 
 const char cookie[] = "GCPBL100";

[M-A Ruel, 2009/09/10 18:59:41]

const char* const cookie = "..";

 
+const size_t kMaxBlockedTypes = 256;
+const size_t kMaxStringSize = 8192;
+
 }
 
-void BlacklistStoreOutput::WriteUInt(uint32 i) {
-  if (fwrite(reinterpret_cast<char*>(&i), 1, sizeof(uint32), file_) !=
-      sizeof(uint32)) {
-    LOG(ERROR) << "fwrite failed to write uint32";
-  }
+bool BlacklistStoreOutput::WriteUInt(uint32 i) {
+  return fwrite(reinterpret_cast<char*>(&i), 1, sizeof(uint32), file_) ==
+      sizeof(uint32);
 }
 
-void BlacklistStoreOutput::WriteString(const std::string& s) {
+bool BlacklistStoreOutput::WriteString(const std::string& s) {
   uint32 n = s.size();
-  if (fwrite(reinterpret_cast<char*>(&n), 1, sizeof(uint32), file_) !=
-      sizeof(uint32)) {
-    LOG(ERROR) << "fwrite failed to write string size";
-  }
-  if (fwrite(s.c_str(), 1, s.size(), file_) != s.size())
-    LOG(ERROR) << "fwrite failed to write string data";
+  return WriteUInt(n) && fwrite(s.c_str(), 1, n, file_) == n;
 }
 
-BlacklistStoreOutput::BlacklistStoreOutput(FILE* file) : file_(file) {
-  if (fwrite(cookie, 1, sizeof(cookie), file_) != sizeof(cookie))
-    LOG(ERROR) << "fwrite failed to write cookie";
+BlacklistStoreOutput::BlacklistStoreOutput(FILE* file)
+    : file_(file), is_good_(false) {
+  is_good_ = fwrite(cookie, 1, sizeof(cookie), file_) == sizeof(cookie);
 }
 
 BlacklistStoreOutput::~BlacklistStoreOutput() {
   file_util::CloseFile(file_);
 }
 
-void BlacklistStoreOutput::ReserveProviders(uint32 num) {
-  WriteUInt(num);
+bool BlacklistStoreOutput::ReserveProviders(uint32 num) {
+  return WriteUInt(num);
 }
 
-void BlacklistStoreOutput::StoreProvider(const std::string& name,
+bool BlacklistStoreOutput::StoreProvider(const std::string& name,
                                          const std::string& url) {
-  WriteString(name);
-  WriteString(url);
+  return WriteString(name) && WriteString(url);
 }
 
-void BlacklistStoreOutput::ReserveEntries(uint32 num) {
-  WriteUInt(num);
+bool BlacklistStoreOutput::ReserveEntries(uint32 num) {
+  return WriteUInt(num);
 }
 
-void BlacklistStoreOutput::StoreEntry(const std::string& pattern,
+bool BlacklistStoreOutput::StoreEntry(const std::string& pattern,
                                       uint32 attributes,
                                       const std::vector<std::string>& types,
                                       uint32 provider) {
-  WriteString(pattern);
-  WriteUInt(attributes);
-  WriteUInt(types.size());
-  for (uint32 i = 0; i < types.size(); ++i)
-    WriteString(types[i]);
-  WriteUInt(provider);
+  if (WriteString(pattern) &&
+      WriteUInt(attributes) &&
+      WriteUInt(types.size())) {
+    for (uint32 i = 0; i < types.size(); ++i) {
+      if (!WriteString(types[i]))
+        return false;
+    }
+    return WriteUInt(provider);
+  }
+  return false;
 }
 
 uint32 BlacklistStoreInput::ReadUInt() {
   uint32 buf;
   if (fread(&buf, 1, sizeof(uint32), file_) != sizeof(uint32))
-    return 0;
+    return std::numeric_limits<uint32>::max();
   return buf;
 }
 
 std::string BlacklistStoreInput::ReadString() {
   uint32 size = ReadUInt();
 
-  // Too long strings are not allowed.
-  if (size > 8192) {
+  // Too long strings are not allowed. Covers the case of ReadUInt failing.
+  if (size > kMaxStringSize) {
     return std::string();
   }
 
-  char buf[8192];
+  char buf[kMaxStringSize];
   if (fread(buf, 1, size, file_) != size)
     return std::string();
   return std::string(buf, size);
 }
 
-BlacklistStoreInput::BlacklistStoreInput(FILE* file) : file_(file) {
-  fseek(file_, sizeof(cookie), SEEK_CUR);
+BlacklistStoreInput::BlacklistStoreInput(FILE* file)
+    : file_(file), is_good_(false) {
+  is_good_ = !fseek(file_, sizeof(cookie), SEEK_CUR);
 }
 
 BlacklistStoreInput::~BlacklistStoreInput() {
@@ -101,24 +102,40 @@
   return ReadUInt();
 }
 
-void BlacklistStoreInput::ReadProvider(std::string* name, std::string* url) {
+bool BlacklistStoreInput::ReadProvider(std::string* name, std::string* url) {
   *name = ReadString();
+  if (name->empty())
+    return false;
   *url = ReadString();
+  return !url->empty();
 }
 
 uint32 BlacklistStoreInput::ReadNumEntries() {
   return ReadUInt();
 }
 
-void BlacklistStoreInput::ReadEntry(std::string* pattern,
+bool BlacklistStoreInput::ReadEntry(std::string* pattern,
                                     uint32* attributes,
                                     std::vector<std::string>* types,
                                     uint32* provider) {
   *pattern = ReadString();
+  if (pattern->empty())
+    return false;
+
   *attributes = ReadUInt();
+  if (*attributes == std::numeric_limits<uint32>::max())
+    return false;
+
   if (uint32 n = ReadUInt()) {
-    while (n--)
-      types->push_back(ReadString());
+    if (n >= kMaxBlockedTypes)
+      return false;
+    while (n--) {
+      std::string type = ReadString();
+      if (type.empty())
+        return false;
+      types->push_back(type);
+    }
   }
   *provider = ReadUInt();
+  return *provider != std::numeric_limits<uint32>::max();
 }