------------------------------------------------------------
revno: 14956
revision-id: squid3@treenet.co.nz-20161216024339-gi2ivuoul8mj8170
parent: squid3@treenet.co.nz-20161215075442-py6yxdvpfwhohsev
author: Eduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: trunk
timestamp: Fri 2016-12-16 15:43:39 +1300
message:
  Do not share private responses with collapsed client(s).
  
  This excessive sharing problem with collapsed forwarding code has
  several layers. In most cases, the core CF code does not share
  uncachable or private response with collapsed clients because of the
  refreshCheckHTTP() check. However, some responses might not be subject
  to that (or equivalent) check. More importantly, collapsed revalidation
  code does not check its responses at all and, hence, easily shares
  private responses.
  
  This short-term fix incorrectly assumes that an entry may become private
  (KEY_PRIVATE) only when it cannot be shared among multiple clients
  (e.g., because of a Cache-Control:private response header). However,
  there are a few other cases when an entry becomes private. One of them
  is a DISK_NO_SPACE_LEFT error inside storeSwapOutFileClosed() where
  StoreEntry::releaseRequest() sets KEY_PRIVATE for a sharable entry [that
  may still be perfectly preserved in the memory cache]. Consequently, the
  short-term fix reduces CF effectiveness. The extent of this reduction is
  probably environment-dependent.
  
  Also: do not re-use SET_COOKIE headers for collapsed revalidation slaves,
  i.e., adhere to the same requirement as for regular response HITs.
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20161216024339-gi2ivuoul8mj8170
# target_branch: http://bzr.squid-cache.org/bzr/squid3/trunk/
# testament_sha1: 997111346157c0704ad0d8b4388c2212515e2b6e
# timestamp: 2016-12-16 02:51:10 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/4
# base_revision_id: squid3@treenet.co.nz-20161215075442-\
#   py6yxdvpfwhohsev
# 
# Begin patch
=== modified file 'src/client_side_reply.cc'
--- src/client_side_reply.cc	2016-12-11 09:46:22 +0000
+++ src/client_side_reply.cc	2016-12-16 02:43:39 +0000
@@ -415,6 +415,15 @@
     if (result.flags.error && !EBIT_TEST(http->storeEntry()->flags, ENTRY_ABORTED))
         return;
 
+    if (collapsedRevalidation == crSlave && EBIT_TEST(http->storeEntry()->flags, KEY_PRIVATE)) {
+        debugs(88, 3, "CF slave hit private " << *http->storeEntry() << ". MISS");
+        // restore context to meet processMiss() expectations
+        restoreState();
+        http->logType = LOG_TCP_MISS;
+        processMiss();
+        return;
+    }
+
     /* update size of the request */
     reqsize = result.length + reqofs;
 
@@ -536,6 +545,16 @@
         return;
     }
 
+    // The previously identified hit suddenly became unsharable!
+    // This is common for collapsed forwarding slaves but might also
+    // happen to regular hits because we are called asynchronously.
+    if (EBIT_TEST(e->flags, KEY_PRIVATE)) {
+        debugs(88, 3, "unsharable " << *e << ". MISS");
+        http->logType = LOG_TCP_MISS;
+        processMiss();
+        return;
+    }
+
     if (result.length == 0) {
         debugs(88, 5, "store IO buffer has no content. MISS");
         /* the store couldn't get enough data from the file for us to id the
@@ -1351,7 +1370,7 @@
     hdr->delById(HDR_ETAG);
 #endif
 
-    if (is_hit)
+    if (is_hit || collapsedRevalidation == crSlave)
         hdr->delById(Http::HdrType::SET_COOKIE);
     // TODO: RFC 2965 : Must honour Cache-Control: no-cache="set-cookie2" and remove header.