From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Fri, 10 Feb 2012 00:32:44 +0000 (-0700)
Subject: Do not cache partially loaded entries in shared mem cache (and then serve them)
X-Git-Tag: BumpSslServerFirst.take05~12^2~13
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=449ca8c586a1a206e1b4eae057afa3f4bf49a8d2;p=thirdparty%2Fsquid.git

Do not cache partially loaded entries in shared mem cache (and then serve them)

When handling a conditional request, Squid may load the beginning of a cached
object from disk, realize that the client has the same fresh copy, and respond
with 304 Not Modified. After that, Squid was checking whether the partially
loaded object should be kept in shared memory cache (if enabled). There were
no checks preventing memory caching of the partially loaded object.

Later, partially cached objects were served to clients, resulting in truncated
responses. I believe this happens because shared memory cache does not keep
all the StoreEntry data (just like a disk cache does not do that) so the fact
that only a part of the object was available was lost.
---

diff --git a/src/MemStore.cc b/src/MemStore.cc
index 5bf7cd3880..c66560165b 100644
--- a/src/MemStore.cc
+++ b/src/MemStore.cc
@@ -259,8 +259,25 @@ MemStore::considerKeeping(StoreEntry &e)
         return; // cannot keep due to entry state or properties
     }
 
+    // since we copy everything at once, we can only keep complete entries
+    if (e.store_status != STORE_OK) {
+        debugs(20, 7, HERE << "Incomplete: " << e);
+        return;
+    }
+
     assert(e.mem_obj);
-    if (!willFit(e.mem_obj->endOffset())) {
+
+    const int64_t loadedSize = e.mem_obj->endOffset();
+    const int64_t expectedSize = e.mem_obj->expectedReplySize();
+
+    // since we copy everything at once, we can only keep fully loaded entries
+    if (loadedSize != expectedSize) {
+        debugs(20, 7, HERE << "partially loaded: " << loadedSize << " != " <<
+               expectedSize);
+        return;
+    }
+
+    if (!willFit(expectedSize)) {
         debugs(20, 5, HERE << "No mem-cache space for " << e);
         return; // failed to free enough space
     }