Do not open clearly invalid entries in the SimpleCache.

In the case of premature kill, the SimpleCache backend can leave
invalid entries on disk without EOF records. Now let's catch these
early and fail to open the entries.

R=pasko@google.com, rdsmith@chromium.org
BUG=236390

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=197172

Review URL: https://codereview.chromium.org/13844016

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

3 files changed, 68 insertions, 7 deletions

diff --git a/net/disk_cache/entry_unittest.cc b/net/disk_cache/entry_unittest.cc
index 68794b2..610ec22 100644
--- a/net/disk_cache/entry_unittest.cc
+++ b/net/disk_cache/entry_unittest.cc
@@ -19,6 +19,7 @@
 #include "net/disk_cache/disk_cache_test_util.h"
 #include "net/disk_cache/entry_impl.h"
 #include "net/disk_cache/mem_entry_impl.h"
+#include "net/disk_cache/simple/simple_entry_format.h"
 #include "net/disk_cache/simple/simple_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -2381,6 +2382,49 @@ TEST_F(DiskCacheEntryTest, SimpleCacheErrorThenDoom) {
   entry->Close();
 }
 
+bool TruncatePath(const base::FilePath& file_path, int64 length)  {
+  const int flags = base::PLATFORM_FILE_WRITE | base::PLATFORM_FILE_OPEN;
+  base::PlatformFile file =
+      base::CreatePlatformFile(file_path, flags, NULL, NULL);
+  if (base::kInvalidPlatformFileValue == file)
+    return false;
+  const bool result = base::TruncatePlatformFile(file, length);
+  base::ClosePlatformFile(file);
+  return result;
+}
+
+TEST_F(DiskCacheEntryTest, SimpleCacheNoEOF) {
+  SetSimpleCacheMode();
+  InitCache();
+
+  const char key[] = "the first key";
+
+  disk_cache::Entry* entry = NULL;
+  ASSERT_EQ(net::OK, CreateEntry(key, &entry));
+  disk_cache::Entry* null = NULL;
+  EXPECT_NE(null, entry);
+  entry->Close();
+  entry = NULL;
+
+  // Force the entry to flush to disk, so subsequent platform file operations
+  // succed.
+  ASSERT_EQ(net::OK, OpenEntry(key, &entry));
+  entry->Close();
+  entry = NULL;
+
+  // Truncate the file such that the length isn't sufficient to have an EOF
+  // record.
+  int kTruncationBytes = -implicit_cast<int>(sizeof(disk_cache::SimpleFileEOF));
+  const base::FilePath entry_path = cache_path_.AppendASCII(
+      disk_cache::simple_util::GetFilenameFromKeyAndIndex(key, 0));
+  const int64 invalid_size =
+      disk_cache::simple_util::GetFileSizeFromKeyAndDataSize(key,
+                                                             kTruncationBytes);
+  EXPECT_TRUE(TruncatePath(entry_path, invalid_size));
+  EXPECT_EQ(net::ERR_FAILED, OpenEntry(key, &entry));
+  DisableIntegrityCheck();
+}
+
 // Tests that old entries are evicted while new entries remain in the index.
 // This test relies on non-mandatory properties of the simple Cache Backend:
 // LRU eviction, specific values of high-watermark and low-watermark etc.
diff --git a/net/disk_cache/simple/simple_entry_impl.cc b/net/disk_cache/simple/simple_entry_impl.cc
index 6847e02..df1125c 100644
--- a/net/disk_cache/simple/simple_entry_impl.cc
+++ b/net/disk_cache/simple/simple_entry_impl.cc
@@ -368,6 +368,10 @@ void SimpleEntryImpl::CreateEntryInternal(Entry** out_entry,
 
   state_ = STATE_IO_PENDING;
 
+  // If creation succeeds, we should mark all streams to be saved on close.
+  for (int i = 0; i < kSimpleEntryFileCount; ++i)
+    have_written_[i] = true;
+
   // We insert the entry in the index before creating the entry files in the
   // SimpleSynchronousEntry, because this way the worst scenario is when we
   // have the entry in the index but we don't have the created files yet, this
@@ -401,10 +405,12 @@ void SimpleEntryImpl::CloseInternal() {
       crc32s_to_write(new std::vector<CRCRecord>());
   for (int i = 0; i < kSimpleEntryFileCount; ++i) {
     if (have_written_[i]) {
-      if (data_size_[i] == crc32s_end_offset_[i])
-        crc32s_to_write->push_back(CRCRecord(i, true, crc32s_[i]));
-      else
+      if (data_size_[i] == crc32s_end_offset_[i]) {
+        int32 crc = data_size_[i] == 0 ? crc32(0, Z_NULL, 0) : crc32s_[i];
+        crc32s_to_write->push_back(CRCRecord(i, true, crc));
+      } else {
         crc32s_to_write->push_back(CRCRecord(i, false, 0));
+      }
     }
   }
   Closure task = base::Bind(&SimpleSynchronousEntry::Close,
@@ -493,7 +499,7 @@ void SimpleEntryImpl::CreationOperationComplete(
   if (creation_failed) {
     completion_callback.Run(net::ERR_FAILED);
     MarkAsDoomed();
-    state_ = STATE_UNINITIALIZED;
+    MakeUninitialized();
     return;
   }
   state_ = STATE_READY;
diff --git a/net/disk_cache/simple/simple_synchronous_entry.cc b/net/disk_cache/simple/simple_synchronous_entry.cc
index d4bd45e..83133f2 100644
--- a/net/disk_cache/simple/simple_synchronous_entry.cc
+++ b/net/disk_cache/simple/simple_synchronous_entry.cc
@@ -326,6 +326,14 @@ bool SimpleSynchronousEntry::OpenOrCreateFiles(bool create) {
     }
   }
 
+  have_open_files_ = true;
+  if (create) {
+    last_modified_ = last_used_ = Time::Now();
+    for (int i = 0; i < kSimpleEntryFileCount; ++i)
+      data_size_[i] = 0;
+    return true;
+  }
+
   for (int i = 0; i < kSimpleEntryFileCount; ++i) {
     PlatformFileInfo file_info;
     bool success = GetPlatformFileInfo(files_[i], &file_info);
@@ -335,11 +343,14 @@ bool SimpleSynchronousEntry::OpenOrCreateFiles(bool create) {
     }
     last_used_ = std::max(last_used_, file_info.last_accessed);
     last_modified_ = std::max(last_modified_, file_info.last_modified);
-    data_size_[i] = create ? 0 : GetDataSizeFromKeyAndFileSize(key_,
-                                                               file_info.size);
+    data_size_[i] = GetDataSizeFromKeyAndFileSize(key_, file_info.size);
+    if (data_size_[i] < 0) {
+      // This entry can't possibly be valid, as it does not enough space to
+      // store a valid SimpleFileEOF record.
+      return false;
+    }
   }
 
-  have_open_files_ = true;
   return true;
 }