am 1ab02c36: Part of fix for bug 5523834, backporting cache fixes

* commit '1ab02c36bee95e85558f6815b2836e9ee6a26b19':
  Part of fix for bug 5523834, backporting cache fixes

4 files changed, 65 insertions, 12 deletions

diff --git a/net/disk_cache/backend_impl.cc b/net/disk_cache/backend_impl.cc
index c5b7724..e5d889f 100644
--- a/net/disk_cache/backend_impl.cc
+++ b/net/disk_cache/backend_impl.cc
@@ -723,14 +723,7 @@ EntryImpl* BackendImpl::CreateEntryImpl(const std::string& key) {
     }
   }
 
-  int num_blocks;
-  size_t key1_len = sizeof(EntryStore) - offsetof(EntryStore, key);
-  if (key.size() < key1_len ||
-      key.size() > static_cast<size_t>(kMaxInternalKeyLength))
-    num_blocks = 1;
-  else
-    num_blocks = static_cast<int>((key.size() - key1_len) / 256 + 2);
-
+  int num_blocks = EntryImpl::NumBlocksForEntry(key.size());
   if (!block_files_.CreateBlock(BLOCK_256, num_blocks, &entry_address)) {
     LOG(ERROR) << "Create entry failed " << key.c_str();
     stats_.OnEvent(Stats::CREATE_ERROR);
diff --git a/net/disk_cache/entry_impl.cc b/net/disk_cache/entry_impl.cc
index 73c67df..0dbfc94 100644
--- a/net/disk_cache/entry_impl.cc
+++ b/net/disk_cache/entry_impl.cc
@@ -584,12 +584,26 @@ bool EntryImpl::SanityCheck() {
     return false;
 
   Addr key_addr(stored->long_key);
-  if (stored->key_len <= kMaxInternalKeyLength && key_addr.is_initialized())
+  if ((stored->key_len <= kMaxInternalKeyLength && key_addr.is_initialized()) ||
+      (stored->key_len > kMaxInternalKeyLength && !key_addr.is_initialized()))
     return false;
 
   if (!key_addr.SanityCheck())
     return false;
 
+  if (key_addr.is_initialized() &&
+      ((stored->key_len <= kMaxBlockSize && key_addr.is_separate_file()) ||
+      (stored->key_len > kMaxBlockSize && key_addr.is_block_file())))
+    return false;
+
+  int num_blocks = NumBlocksForEntry(stored->key_len);
+  if (entry_.address().num_blocks() != num_blocks)
+    return false;
+
+  // The key must be NULL terminated.
+  if (!key_addr.is_initialized() && stored->key[stored->key_len])
+    return false;
+
   if (stored->hash != Hash(GetKey()))
     return false;
 
@@ -663,6 +677,20 @@ const net::BoundNetLog& EntryImpl::net_log() const {
   return net_log_;
 }
 
+// static
+int EntryImpl::NumBlocksForEntry(int key_size) {
+  // The longest key that can be stored using one block.
+  int key1_len =
+      static_cast<int>(sizeof(EntryStore) - offsetof(EntryStore, key));
+
+  if (key_size < key1_len || key_size > kMaxInternalKeyLength)
+    return 1;
+
+  return ((key_size - key1_len) / 256 + 2);
+}
+
+// ------------------------------------------------------------------------
+
 void EntryImpl::Doom() {
   backend_->background_queue()->DoomEntryImpl(this);
 }
@@ -673,7 +701,8 @@ void EntryImpl::Close() {
 
 std::string EntryImpl::GetKey() const {
   CacheEntryBlock* entry = const_cast<CacheEntryBlock*>(&entry_);
-  if (entry->Data()->key_len <= kMaxInternalKeyLength)
+  int key_len = entry->Data()->key_len;
+  if (key_len <= kMaxInternalKeyLength)
     return std::string(entry->Data()->key);
 
   // We keep a copy of the key so that we can always return it, even if the
@@ -691,9 +720,11 @@ std::string EntryImpl::GetKey() const {
   File* key_file = const_cast<EntryImpl*>(this)->GetBackingFile(address,
                                                                 kKeyFileIndex);
 
+  if (!offset && key_file->GetLength() != static_cast<size_t>(key_len + 1))
+    return std::string();
+
   if (!key_file ||
-      !key_file->Read(WriteInto(&key_, entry->Data()->key_len + 1),
-                      entry->Data()->key_len + 1, offset))
+      !key_file->Read(WriteInto(&key_, key_len + 1), key_len + 1, offset))
     key_.clear();
   return key_;
 }
diff --git a/net/disk_cache/entry_impl.h b/net/disk_cache/entry_impl.h
index fab676a..16f9e07 100644
--- a/net/disk_cache/entry_impl.h
+++ b/net/disk_cache/entry_impl.h
@@ -124,6 +124,9 @@ class EntryImpl : public Entry, public base::RefCounted<EntryImpl> {
 
   const net::BoundNetLog& net_log() const;
 
+  // Returns the number of blocks needed to store an EntryStore.
+  static int NumBlocksForEntry(int key_size);
+
   // Entry interface.
   virtual void Doom();
   virtual void Close();
diff --git a/net/disk_cache/entry_unittest.cc b/net/disk_cache/entry_unittest.cc
index 9c6b28a..ce3696c 100644
--- a/net/disk_cache/entry_unittest.cc
+++ b/net/disk_cache/entry_unittest.cc
@@ -1944,3 +1944,29 @@ TEST_F(DiskCacheEntryTest, CancelSparseIO) {
   EXPECT_EQ(0, cb5.GetResult(rv));
   entry->Close();
 }
+
+// Tests that we perform sanity checks on an entry's key. Note that there are
+// other tests that exercise sanity checks by using saved corrupt files.
+TEST_F(DiskCacheEntryTest, KeySanityCheck) {
+  UseCurrentThread();
+  InitCache();
+  std::string key("the first key");
+  disk_cache::Entry* entry;
+  ASSERT_EQ(net::OK, CreateEntry(key, &entry));
+
+  disk_cache::EntryImpl* entry_impl =
+      static_cast<disk_cache::EntryImpl*>(entry);
+  disk_cache::EntryStore* store = entry_impl->entry()->Data();
+
+  // We have reserved space for a short key (one block), let's say that the key
+  // takes more than one block, and remove the NULLs after the actual key.
+  store->key_len = 800;
+  memset(store->key + key.size(), 'k', sizeof(store->key) - key.size());
+  entry_impl->entry()->set_modified();
+  entry->Close();
+
+  // We have a corrupt entry. Now reload it. We should NOT read beyond the
+  // allocated buffer here.
+  ASSERT_NE(net::OK, OpenEntry(key, &entry));
+  DisableIntegrityCheck();
+}