Double-check safe-browsing database validity on update failure.

chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc

Index: chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc
diff --git a/chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc b/chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc
index f62b261392015d11197a9a80886e943611590e9a..405e23a641c84d973ff2daf8aefeb33f90a96c05 100644
--- a/chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_store_file_unittest.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/safe_browsing/safe_browsing_store_file.h"
 
 #include "base/bind.h"
+#include "base/md5.h"
 #include "base/scoped_temp_dir.h"
 #include "chrome/browser/safe_browsing/safe_browsing_store_unittest_helper.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -26,7 +27,10 @@ class SafeBrowsingStoreFileTest : public PlatformTest {
     filename_ = filename_.AppendASCII("SafeBrowsingTestStore");
 
     store_.reset(new SafeBrowsingStoreFile());
-    store_->Init(filename_, base::Closure());
+    store_->Init(filename_,
+                 base::Bind(&SafeBrowsingStoreFileTest::OnCorruptionDetected,
+                            base::Unretained(this)));
+    corruption_detected_ = false;
   }
   virtual void TearDown() {
     if (store_.get())
@@ -78,22 +82,14 @@ TEST_F(SafeBrowsingStoreFileTest, DetectsCorruption) {
   // Load a store with some data.
   SafeBrowsingStoreTestStorePrefix(store_.get());
 
-  SafeBrowsingStoreFile test_store;
-  test_store.Init(
-      filename_,
-      base::Bind(&SafeBrowsingStoreFileTest::OnCorruptionDetected,
-                 base::Unretained(this)));
-
-  corruption_detected_ = false;
-
   // Can successfully open and read the store.
   std::vector<SBAddFullHash> pending_adds;
   std::set<SBPrefix> prefix_misses;
   SBAddPrefixes orig_prefixes;
   std::vector<SBAddFullHash> orig_hashes;
-  EXPECT_TRUE(test_store.BeginUpdate());
-  EXPECT_TRUE(test_store.FinishUpdate(pending_adds, prefix_misses,
-                                      &orig_prefixes, &orig_hashes));
+  EXPECT_TRUE(store_->BeginUpdate());
+  EXPECT_TRUE(store_->FinishUpdate(pending_adds, prefix_misses,
+                                   &orig_prefixes, &orig_hashes));
   EXPECT_GT(orig_prefixes.size(), 0U);
   EXPECT_GT(orig_hashes.size(), 0U);
   EXPECT_FALSE(corruption_detected_);
@@ -114,9 +110,9 @@ TEST_F(SafeBrowsingStoreFileTest, DetectsCorruption) {
   SBAddPrefixes add_prefixes;
   std::vector<SBAddFullHash> add_hashes;
   corruption_detected_ = false;
-  EXPECT_TRUE(test_store.BeginUpdate());
-  EXPECT_FALSE(test_store.FinishUpdate(pending_adds, prefix_misses,
-                                       &add_prefixes, &add_hashes));
+  EXPECT_TRUE(store_->BeginUpdate());
+  EXPECT_FALSE(store_->FinishUpdate(pending_adds, prefix_misses,
+                                    &add_prefixes, &add_hashes));
   EXPECT_TRUE(corruption_detected_);
   EXPECT_EQ(add_prefixes.size(), 0U);
   EXPECT_EQ(add_hashes.size(), 0U);
@@ -131,8 +127,70 @@ TEST_F(SafeBrowsingStoreFileTest, DetectsCorruption) {
 
   // Detects corruption and fails to even begin the update.
   corruption_detected_ = false;
-  EXPECT_FALSE(test_store.BeginUpdate());
+  EXPECT_FALSE(store_->BeginUpdate());
+  EXPECT_TRUE(corruption_detected_);
+}
+
+TEST_F(SafeBrowsingStoreFileTest, CheckValidity) {
+  // Empty store is valid.
+  EXPECT_FALSE(file_util::PathExists(filename_));
+  ASSERT_TRUE(store_->BeginUpdate());
+  EXPECT_FALSE(corruption_detected_);
+  EXPECT_TRUE(store_->CheckValidity());
+  EXPECT_FALSE(corruption_detected_);
+  EXPECT_TRUE(store_->CancelUpdate());
+
+  // A store with some data is valid.
+  EXPECT_FALSE(file_util::PathExists(filename_));
+  SafeBrowsingStoreTestStorePrefix(store_.get());
+  EXPECT_TRUE(file_util::PathExists(filename_));
+  ASSERT_TRUE(store_->BeginUpdate());
+  EXPECT_FALSE(corruption_detected_);
+  EXPECT_TRUE(store_->CheckValidity());
+  EXPECT_FALSE(corruption_detected_);
+  EXPECT_TRUE(store_->CancelUpdate());
+}
+
+// Corrupt the payload.
+TEST_F(SafeBrowsingStoreFileTest, CheckValidityPayload) {
+  SafeBrowsingStoreTestStorePrefix(store_.get());
+  EXPECT_TRUE(file_util::PathExists(filename_));
+
+  // 37 is the most random prime number.  It's also past the header,
+  // as corrupting the header would fail BeginUpdate() in which case
+  // CheckValidity() cannot be called.
+  const size_t kOffset = 37;
+
+  {
+    file_util::ScopedFILE file(file_util::OpenFile(filename_, "rb+"));
+    EXPECT_EQ(0, fseek(file.get(), kOffset, SEEK_SET));
+    EXPECT_GE(fputs("hello", file.get()), 0);
+  }
+  ASSERT_TRUE(store_->BeginUpdate());
+  EXPECT_FALSE(corruption_detected_);
+  EXPECT_FALSE(store_->CheckValidity());
+  EXPECT_TRUE(corruption_detected_);
+  EXPECT_TRUE(store_->CancelUpdate());
+}
+
+// Corrupt the checksum.
+TEST_F(SafeBrowsingStoreFileTest, CheckValidityChecksum) {
+  SafeBrowsingStoreTestStorePrefix(store_.get());
+  EXPECT_TRUE(file_util::PathExists(filename_));
+
+  // An offset from the end of the file which is in the checksum.
+  const int kOffset = -static_cast<int>(sizeof(base::MD5Digest));
+
+  {
+    file_util::ScopedFILE file(file_util::OpenFile(filename_, "rb+"));
+    EXPECT_EQ(0, fseek(file.get(), kOffset, SEEK_END));
+    EXPECT_GE(fputs("hello", file.get()), 0);
+  }
+  ASSERT_TRUE(store_->BeginUpdate());
+  EXPECT_FALSE(corruption_detected_);
+  EXPECT_FALSE(store_->CheckValidity());
   EXPECT_TRUE(corruption_detected_);
+  EXPECT_TRUE(store_->CancelUpdate());
 }
 
 }  // namespace