src/patches/squid/squid-3.5-14169.patch

   1 ------------------------------------------------------------
   2 revno: 14169
   3 revision-id: squid3@treenet.co.nz-20170614213720-3qmiohlx4zr2jnqq
   4 parent: squid3@treenet.co.nz-20170601134753-6u64sl2rzmbfs67l
   5 fixes bug: http://bugs.squid-cache.org/show_bug.cgi?id=2833
   6 author: Eduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
   7 committer: Amos Jeffries <squid3@treenet.co.nz>
   8 branch nick: 3.5
   9 timestamp: Thu 2017-06-15 09:37:20 +1200
  10 message:
  11   Bug 2833 pt2: Collapse internal revalidation requests (SMP-unaware caches), again.
  12
  13   The security fix in v5 r14979 had a negative effect on collapsed
  14   forwarding. All "private" entries were considered automatically
  15   non-shareable among collapsed clients. However this is not true: there
  16   are many situations when collapsed forwarding should work despite of
  17   "private" entry status: 304/5xx responses are good examples of that.
  18   This patch fixes that by means of a new StoreEntry::shareableWhenPrivate
  19   flag.
  20
  21   The suggested fix is not complete: To cover all possible situations, we
  22   need to decide whether StoreEntry::shareableWhenPrivate is true or not
  23   for all contexts where StoreEntry::setPrivateKey() is used. This patch
  24   fixes only few important cases inside http.cc, making CF (as well
  25   collapsed revalidation) work for some [non-cacheable] response status
  26   codes, including 3xx, 5xx and some others.
  27
  28   The original support for internal revalidation requests collapsing
  29   was in trink r14755 and referred to Squid bugs 2833, 4311, and 4471.
  30 ------------------------------------------------------------
  31 # Bazaar merge directive format 2 (Bazaar 0.90)
  32 # revision_id: squid3@treenet.co.nz-20170614213720-3qmiohlx4zr2jnqq
  33 # target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
  34 # testament_sha1: 9e248e2e9d2f1defe1070eb808177df978fb4146
  35 # timestamp: 2017-06-14 21:51:05 +0000
  36 # source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
  37 # base_revision_id: squid3@treenet.co.nz-20170601134753-\
  38 #   6u64sl2rzmbfs67l
  39 #
  40 # Begin patch
  41 === modified file 'src/HttpHdrCc.cc'
  42 --- src/HttpHdrCc.cc    2017-01-01 00:16:45 +0000
  43 +++ src/HttpHdrCc.cc    2017-06-14 21:37:20 +0000
  44 @@ -262,8 +262,8 @@
  45              case CC_PUBLIC:
  46                  break;
  47              case CC_PRIVATE:
  48 -                if (Private().size())
  49 -                    packerPrintf(p, "=\"" SQUIDSTRINGPH "\"", SQUIDSTRINGPRINT(Private()));
  50 +                if (private_.size())
  51 +                    packerPrintf(p, "=\"" SQUIDSTRINGPH "\"", SQUIDSTRINGPRINT(private_));
  52                  break;
  53
  54              case CC_NO_CACHE:
  55
  56 === modified file 'src/MemStore.cc'
  57 --- src/MemStore.cc     2017-01-01 00:16:45 +0000
  58 +++ src/MemStore.cc     2017-06-14 21:37:20 +0000
  59 @@ -299,7 +299,7 @@
  60      e.ping_status = PING_NONE;
  61
  62      EBIT_CLR(e.flags, RELEASE_REQUEST);
  63 -    EBIT_CLR(e.flags, KEY_PRIVATE);
  64 +    e.clearPrivate();
  65      EBIT_SET(e.flags, ENTRY_VALIDATED);
  66
  67      MemObject::MemCache &mc = e.mem_obj->memCache;
  68
  69 === modified file 'src/Store.h'
  70 --- src/Store.h 2017-01-01 00:16:45 +0000
  71 +++ src/Store.h 2017-06-14 21:37:20 +0000
  72 @@ -95,15 +95,19 @@
  73      void abort();
  74      void unlink();
  75      void makePublic(const KeyScope keyScope = ksDefault);
  76 -    void makePrivate();
  77 +    void makePrivate(const bool shareable);
  78 +    /// A low-level method just resetting "private key" flags.
  79 +    /// To avoid key inconsistency please use forcePublicKey()
  80 +    /// or similar instead.
  81 +    void clearPrivate();
  82      void setPublicKey(const KeyScope keyScope = ksDefault);
  83      /// Resets existing public key to a public key with default scope,
  84      /// releasing the old default-scope entry (if any).
  85      /// Does nothing if the existing public key already has default scope.
  86      void clearPublicKeyScope();
  87 -    void setPrivateKey();
  88 +    void setPrivateKey(const bool shareable);
  89      void expireNow();
  90 -    void releaseRequest();
  91 +    void releaseRequest(const bool shareable = false);
  92      void negativeCache();
  93      void cacheNegatively();     /** \todo argh, why both? */
  94      void invokeHandlers();
  95 @@ -230,7 +234,13 @@
  96      /// update last reference timestamp and related Store metadata
  97      void touch();
  98
  99 -    virtual void release();
 100 +    virtual void release(const bool shareable = false);
 101 +
 102 +    /// May the caller commit to treating this [previously locked]
 103 +    /// entry as a cache hit?
 104 +    bool mayStartHitting() const {
 105 +        return !EBIT_TEST(flags, KEY_PRIVATE) || shareableWhenPrivate;
 106 +    }
 107
 108  #if USE_ADAPTATION
 109      /// call back producer when more buffer space is available
 110 @@ -252,6 +262,13 @@
 111
 112      unsigned short lock_count;      /* Assume < 65536! */
 113
 114 +    /// Nobody can find/lock KEY_PRIVATE entries, but some transactions
 115 +    /// (e.g., collapsed requests) find/lock a public entry before it becomes
 116 +    /// private. May such transactions start using the now-private entry
 117 +    /// they previously locked? This member should not affect transactions
 118 +    /// that already started reading from the entry.
 119 +    bool shareableWhenPrivate;
 120 +
 121  #if USE_ADAPTATION
 122      /// producer callback registered with deferProducer
 123      AsyncCall::Pointer deferredProducer;
 124 @@ -259,6 +276,8 @@
 125
 126      bool validLength() const;
 127      bool hasOneOfEtags(const String &reqETags, const bool allowWeakMatch) const;
 128 +
 129 +    friend std::ostream &operator <<(std::ostream &os, const StoreEntry &e);
 130  };
 131
 132  std::ostream &operator <<(std::ostream &os, const StoreEntry &e);
 133
 134 === modified file 'src/client_side_reply.cc'
 135 --- src/client_side_reply.cc    2017-05-29 13:15:55 +0000
 136 +++ src/client_side_reply.cc    2017-06-14 21:37:20 +0000
 137 @@ -396,8 +396,8 @@
 138      if (result.flags.error && !EBIT_TEST(http->storeEntry()->flags, ENTRY_ABORTED))
 139          return;
 140
 141 -    if (collapsedRevalidation == crSlave && EBIT_TEST(http->storeEntry()->flags, KEY_PRIVATE)) {
 142 -        debugs(88, 3, "CF slave hit private " << *http->storeEntry() << ". MISS");
 143 +    if (collapsedRevalidation == crSlave && !http->storeEntry()->mayStartHitting()) {
 144 +        debugs(88, 3, "CF slave hit private non-shareable " << *http->storeEntry() << ". MISS");
 145          // restore context to meet processMiss() expectations
 146          restoreState();
 147          http->logType = LOG_TCP_MISS;
 148 @@ -530,7 +530,7 @@
 149      // The previously identified hit suddenly became unsharable!
 150      // This is common for collapsed forwarding slaves but might also
 151      // happen to regular hits because we are called asynchronously.
 152 -    if (EBIT_TEST(e->flags, KEY_PRIVATE)) {
 153 +    if (!e->mayStartHitting()) {
 154          debugs(88, 3, "unsharable " << *e << ". MISS");
 155          http->logType = LOG_TCP_MISS;
 156          processMiss();
 157
 158 === modified file 'src/fs/rock/RockSwapDir.cc'
 159 --- src/fs/rock/RockSwapDir.cc  2017-01-01 00:16:45 +0000
 160 +++ src/fs/rock/RockSwapDir.cc  2017-06-14 21:37:20 +0000
 161 @@ -149,7 +149,7 @@
 162      e.ping_status = PING_NONE;
 163
 164      EBIT_CLR(e.flags, RELEASE_REQUEST);
 165 -    EBIT_CLR(e.flags, KEY_PRIVATE);
 166 +    e.clearPrivate();
 167      EBIT_SET(e.flags, ENTRY_VALIDATED);
 168
 169      e.swap_dirn = index;
 170
 171 === modified file 'src/fs/ufs/UFSSwapDir.cc'
 172 --- src/fs/ufs/UFSSwapDir.cc    2017-01-01 00:16:45 +0000
 173 +++ src/fs/ufs/UFSSwapDir.cc    2017-06-14 21:37:20 +0000
 174 @@ -809,7 +809,7 @@
 175      e->refcount = refcount;
 176      e->flags = newFlags;
 177      EBIT_CLR(e->flags, RELEASE_REQUEST);
 178 -    EBIT_CLR(e->flags, KEY_PRIVATE);
 179 +    e->clearPrivate();
 180      e->ping_status = PING_NONE;
 181      EBIT_CLR(e->flags, ENTRY_VALIDATED);
 182      mapBitSet(e->swap_filen);
 183
 184 === modified file 'src/http.cc'
 185 --- src/http.cc 2017-01-01 00:16:45 +0000
 186 +++ src/http.cc 2017-06-14 21:37:20 +0000
 187 @@ -290,7 +290,9 @@
 188                      (Config.onoff.surrogate_is_remote
 189                       && sctusable->noStoreRemote())) {
 190                  surrogateNoStore = true;
 191 -                entry->makePrivate();
 192 +                // Be conservative for now and make it non-shareable because
 193 +                // there is no enough information here to make the decision.
 194 +                entry->makePrivate(false);
 195              }
 196
 197              /* The HttpHeader logic cannot tell if the header it's parsing is a reply to an
 198 @@ -315,12 +317,13 @@
 199      }
 200  }
 201
 202 -int
 203 -HttpStateData::cacheableReply()
 204 +HttpStateData::ReuseDecision::Answers
 205 +HttpStateData::reusableReply(HttpStateData::ReuseDecision &decision)
 206  {
 207      HttpReply const *rep = finalReply();
 208      HttpHeader const *hdr = &rep->header;
 209      const char *v;
 210 +
 211  #if USE_HTTP_VIOLATIONS
 212
 213      const RefreshPattern *R = NULL;
 214 @@ -337,24 +340,19 @@
 215  #define REFRESH_OVERRIDE(flag) 0
 216  #endif
 217
 218 -    if (EBIT_TEST(entry->flags, RELEASE_REQUEST)) {
 219 -        debugs(22, 3, "NO because " << *entry << " has been released.");
 220 -        return 0;
 221 -    }
 222 +    if (EBIT_TEST(entry->flags, RELEASE_REQUEST))
 223 +        return decision.make(ReuseDecision::reuseNot, "the entry has been released");
 224
 225      // RFC 7234 section 4: a cache MUST use the most recent response
 226      // (as determined by the Date header field)
 227 -    if (sawDateGoBack) {
 228 -        debugs(22, 3, "NO because " << *entry << " has an older date header.");
 229 -        return 0;
 230 -    }
 231 +    // TODO: whether such responses could be shareable?
 232 +    if (sawDateGoBack)
 233 +        return decision.make(ReuseDecision::reuseNot, "the response has an older date header");
 234
 235      // Check for Surrogate/1.0 protocol conditions
 236      // NP: reverse-proxy traffic our parent server has instructed us never to cache
 237 -    if (surrogateNoStore) {
 238 -        debugs(22, 3, HERE << "NO because Surrogate-Control:no-store");
 239 -        return 0;
 240 -    }
 241 +    if (surrogateNoStore)
 242 +        return decision.make(ReuseDecision::reuseNot, "Surrogate-Control:no-store");
 243
 244      // RFC 2616: HTTP/1.1 Cache-Control conditions
 245      if (!ignoreCacheControl) {
 246 @@ -363,11 +361,10 @@
 247          // for now we are not reliably doing that so we waste CPU re-checking request CC
 248
 249          // RFC 2616 section 14.9.2 - MUST NOT cache any response with request CC:no-store
 250 -        if (request && request->cache_control && request->cache_control->noStore() &&
 251 -                !REFRESH_OVERRIDE(ignore_no_store)) {
 252 -            debugs(22, 3, HERE << "NO because client request Cache-Control:no-store");
 253 -            return 0;
 254 -        }
 255 +        if (request && request->cache_control && request->cache_control->hasNoStore() &&
 256 +                !REFRESH_OVERRIDE(ignore_no_store))
 257 +            return decision.make(ReuseDecision::reuseNot,
 258 +                                 "client request Cache-Control:no-store");
 259
 260          // NP: request CC:no-cache only means cache READ is forbidden. STORE is permitted.
 261          if (rep->cache_control && rep->cache_control->hasNoCache() && rep->cache_control->noCache().size() > 0) {
 262 @@ -376,19 +373,18 @@
 263               * successfully (ie, must revalidate AND these headers are prohibited on stale replies).
 264               * That is a bit tricky for squid right now so we avoid caching entirely.
 265               */
 266 -            debugs(22, 3, HERE << "NO because server reply Cache-Control:no-cache has parameters");
 267 -            return 0;
 268 +            return decision.make(ReuseDecision::reuseNot,
 269 +                                 "server reply Cache-Control:no-cache has parameters");
 270          }
 271
 272          // NP: request CC:private is undefined. We ignore.
 273          // NP: other request CC flags are limiters on HIT/MISS. We don't care about here.
 274
 275          // RFC 2616 section 14.9.2 - MUST NOT cache any response with CC:no-store
 276 -        if (rep->cache_control && rep->cache_control->noStore() &&
 277 -                !REFRESH_OVERRIDE(ignore_no_store)) {
 278 -            debugs(22, 3, HERE << "NO because server reply Cache-Control:no-store");
 279 -            return 0;
 280 -        }
 281 +        if (rep->cache_control && rep->cache_control->hasNoStore() &&
 282 +                !REFRESH_OVERRIDE(ignore_no_store))
 283 +            return decision.make(ReuseDecision::reuseNot,
 284 +                                 "server reply Cache-Control:no-store");
 285
 286          // RFC 2616 section 14.9.1 - MUST NOT cache any response with CC:private in a shared cache like Squid.
 287          // CC:private overrides CC:public when both are present in a response.
 288 @@ -401,27 +397,25 @@
 289               * successfully (ie, must revalidate AND these headers are prohibited on stale replies).
 290               * That is a bit tricky for squid right now so we avoid caching entirely.
 291               */
 292 -            debugs(22, 3, HERE << "NO because server reply Cache-Control:private");
 293 -            return 0;
 294 +            return decision.make(ReuseDecision::reuseNot,
 295 +                                 "server reply Cache-Control:private");
 296          }
 297      }
 298
 299      // RFC 2068, sec 14.9.4 - MUST NOT cache any response with Authentication UNLESS certain CC controls are present
 300      // allow HTTP violations to IGNORE those controls (ie re-block caching Auth)
 301      if (request && (request->flags.auth || request->flags.authSent) && !REFRESH_OVERRIDE(ignore_auth)) {
 302 -        if (!rep->cache_control) {
 303 -            debugs(22, 3, HERE << "NO because Authenticated and server reply missing Cache-Control");
 304 -            return 0;
 305 -        }
 306 +        if (!rep->cache_control)
 307 +            return decision.make(ReuseDecision::reuseNot,
 308 +                                 "authenticated and server reply missing Cache-Control");
 309
 310 -        if (ignoreCacheControl) {
 311 -            debugs(22, 3, HERE << "NO because Authenticated and ignoring Cache-Control");
 312 -            return 0;
 313 -        }
 314 +        if (ignoreCacheControl)
 315 +            return decision.make(ReuseDecision::reuseNot,
 316 +                                 "authenticated and ignoring Cache-Control");
 317
 318          bool mayStore = false;
 319          // HTTPbis pt6 section 3.2: a response CC:public is present
 320 -        if (rep->cache_control->Public()) {
 321 +        if (rep->cache_control->hasPublic()) {
 322              debugs(22, 3, HERE << "Authenticated but server reply Cache-Control:public");
 323              mayStore = true;
 324
 325 @@ -441,15 +435,13 @@
 326  #endif
 327
 328              // HTTPbis pt6 section 3.2: a response CC:s-maxage is present
 329 -        } else if (rep->cache_control->sMaxAge()) {
 330 +        } else if (rep->cache_control->hasSMaxAge()) {
 331              debugs(22, 3, HERE << "Authenticated but server reply Cache-Control:s-maxage");
 332              mayStore = true;
 333          }
 334
 335 -        if (!mayStore) {
 336 -            debugs(22, 3, HERE << "NO because Authenticated transaction");
 337 -            return 0;
 338 -        }
 339 +        if (!mayStore)
 340 +            return decision.make(ReuseDecision::reuseNot, "authenticated transaction");
 341
 342          // NP: response CC:no-cache is equivalent to CC:must-revalidate,max-age=0. We MAY cache, and do so.
 343          // NP: other request CC flags are limiters on HIT/MISS/REFRESH. We don't care about here.
 344 @@ -460,12 +452,26 @@
 345       * probably should not be cachable
 346       */
 347      if ((v = hdr->getStr(HDR_CONTENT_TYPE)))
 348 -        if (!strncasecmp(v, "multipart/x-mixed-replace", 25)) {
 349 -            debugs(22, 3, HERE << "NO because Content-Type:multipart/x-mixed-replace");
 350 -            return 0;
 351 -        }
 352 +        if (!strncasecmp(v, "multipart/x-mixed-replace", 25))
 353 +            return decision.make(ReuseDecision::reuseNot, "Content-Type:multipart/x-mixed-replace");
 354 +
 355 +    // TODO: if possible, provide more specific message for each status code
 356 +    static const char *shareableError = "shareable error status code";
 357 +    static const char *nonShareableError = "non-shareable error status code";
 358 +    ReuseDecision::Answers statusAnswer = ReuseDecision::reuseNot;
 359 +    const char *statusReason = nonShareableError;
 360
 361      switch (rep->sline.status()) {
 362 +
 363 +    /* There are several situations when a non-cacheable response may be
 364 +     * still shareable (e.g., among collapsed clients). We assume that these
 365 +     * are 3xx and 5xx responses, indicating server problems and some of
 366 +     * 4xx responses, common for all clients with a given cache key (e.g.,
 367 +     * 404 Not Found or 414 URI Too Long). On the other hand, we should not
 368 +     * share non-cacheable client-specific errors, such as 400 Bad Request
 369 +     * or 406 Not Acceptable.
 370 +     */
 371 +
 372      /* Responses that are cacheable */
 373
 374      case Http::scOkay:
 375 @@ -482,112 +488,90 @@
 376           * Don't cache objects that need to be refreshed on next request,
 377           * unless we know how to refresh it.
 378           */
 379 +        if (refreshIsCachable(entry) || REFRESH_OVERRIDE(store_stale))
 380 +            decision.make(ReuseDecision::cachePositively, "refresh check returned cacheable");
 381 +        else
 382 +            decision.make(ReuseDecision::doNotCacheButShare, "refresh check returned non-cacheable");
 383
 384 -        if (!refreshIsCachable(entry) && !REFRESH_OVERRIDE(store_stale)) {
 385 -            debugs(22, 3, "NO because refreshIsCachable() returned non-cacheable..");
 386 -            return 0;
 387 -        } else {
 388 -            debugs(22, 3, HERE << "YES because HTTP status " << rep->sline.status());
 389 -            return 1;
 390 -        }
 391 -        /* NOTREACHED */
 392          break;
 393
 394      /* Responses that only are cacheable if the server says so */
 395
 396      case Http::scFound:
 397      case Http::scTemporaryRedirect:
 398 -        if (rep->date <= 0) {
 399 -            debugs(22, 3, HERE << "NO because HTTP status " << rep->sline.status() << " and Date missing/invalid");
 400 -            return 0;
 401 -        }
 402 -        if (rep->expires > rep->date) {
 403 -            debugs(22, 3, HERE << "YES because HTTP status " << rep->sline.status() << " and Expires > Date");
 404 -            return 1;
 405 -        } else {
 406 -            debugs(22, 3, HERE << "NO because HTTP status " << rep->sline.status() << " and Expires <= Date");
 407 -            return 0;
 408 -        }
 409 -        /* NOTREACHED */
 410 +
 411 +        if (rep->date <= 0)
 412 +            decision.make(ReuseDecision::doNotCacheButShare, "Date is missing/invalid");
 413 +        else if (rep->expires > rep->date)
 414 +            decision.make(ReuseDecision::cachePositively, "Expires > Date");
 415 +        else
 416 +            decision.make(ReuseDecision::doNotCacheButShare, "Expires <= Date");
 417          break;
 418
 419 -    /* Errors can be negatively cached */
 420 -
 421 +    /* These responses can be negatively cached. Most can also be shared. */
 422      case Http::scNoContent:
 423 -
 424      case Http::scUseProxy:
 425 -
 426 -    case Http::scBadRequest:
 427 -
 428      case Http::scForbidden:
 429 -
 430      case Http::scNotFound:
 431 -
 432      case Http::scMethodNotAllowed:
 433 -
 434      case Http::scUriTooLong:
 435 -
 436      case Http::scInternalServerError:
 437 -
 438      case Http::scNotImplemented:
 439 -
 440      case Http::scBadGateway:
 441 -
 442      case Http::scServiceUnavailable:
 443 -
 444      case Http::scGatewayTimeout:
 445      case Http::scMisdirectedRequest:
 446 -
 447 -        debugs(22, 3, "MAYBE because HTTP status " << rep->sline.status());
 448 -        return -1;
 449 -
 450 -        /* NOTREACHED */
 451 +        statusAnswer = ReuseDecision::doNotCacheButShare;
 452 +        statusReason = shareableError;
 453 +        // fall through to the actual decision making below
 454 +
 455 +    case Http::scBadRequest: // no sharing; perhaps the server did not like something specific to this request
 456 +
 457 +#if USE_HTTP_VIOLATIONS
 458 +        if (Config.negativeTtl > 0)
 459 +            decision.make(ReuseDecision::cacheNegatively, "Config.negativeTtl > 0");
 460 +        else
 461 +#endif
 462 +            decision.make(statusAnswer, statusReason);
 463          break;
 464
 465 -    /* Some responses can never be cached */
 466 -
 467 -    case Http::scPartialContent:    /* Not yet supported */
 468 -
 469 +    /* these responses can never be cached, some
 470 +       of them can be shared though */
 471      case Http::scSeeOther:
 472 -
 473      case Http::scNotModified:
 474 -
 475      case Http::scUnauthorized:
 476 -
 477      case Http::scProxyAuthenticationRequired:
 478 -
 479 -    case Http::scInvalidHeader: /* Squid header parsing error */
 480 -
 481 -    case Http::scHeaderTooLarge:
 482 -
 483      case Http::scPaymentRequired:
 484 +    case Http::scInsufficientStorage:
 485 +        // TODO: use more specific reason for non-error status codes
 486 +        decision.make(ReuseDecision::doNotCacheButShare, shareableError);
 487 +        break;
 488 +
 489 +    case Http::scPartialContent: /* Not yet supported. TODO: make shareable for suitable ranges */
 490      case Http::scNotAcceptable:
 491 -    case Http::scRequestTimeout:
 492 -    case Http::scConflict:
 493 +    case Http::scRequestTimeout: // TODO: is this shareable?
 494 +    case Http::scConflict: // TODO: is this shareable?
 495      case Http::scLengthRequired:
 496      case Http::scPreconditionFailed:
 497      case Http::scPayloadTooLarge:
 498      case Http::scUnsupportedMediaType:
 499      case Http::scUnprocessableEntity:
 500 -    case Http::scLocked:
 501 +    case Http::scLocked: // TODO: is this shareable?
 502      case Http::scFailedDependency:
 503 -    case Http::scInsufficientStorage:
 504      case Http::scRequestedRangeNotSatisfied:
 505      case Http::scExpectationFailed:
 506 -
 507 -        debugs(22, 3, HERE << "NO because HTTP status " << rep->sline.status());
 508 -        return 0;
 509 -
 510 +    case Http::scInvalidHeader: /* Squid header parsing error */
 511 +    case Http::scHeaderTooLarge:
 512 +        decision.make(ReuseDecision::reuseNot, nonShareableError);
 513 +        break;
 514      default:
 515          /* RFC 2616 section 6.1.1: an unrecognized response MUST NOT be cached. */
 516 -        debugs (11, 3, HERE << "NO because unknown HTTP status code " << rep->sline.status());
 517 -        return 0;
 518
 519 -        /* NOTREACHED */
 520 +        decision.make(ReuseDecision::reuseNot, "unknown status code");
 521          break;
 522      }
 523
 524 -    /* NOTREACHED */
 525 +    return decision.answer;
 526  }
 527
 528  /// assemble a variant key (vary-mark) from the given Vary header and HTTP request
 529 @@ -898,11 +882,12 @@
 530
 531      Ctx ctx = ctx_enter(entry->mem_obj->urlXXX());
 532      HttpReply *rep = finalReply();
 533 +    const Http::StatusCode statusCode = rep->sline.status();
 534
 535      entry->timestampsSet();
 536
 537      /* Check if object is cacheable or not based on reply code */
 538 -    debugs(11, 3, "HTTP CODE: " << rep->sline.status());
 539 +    debugs(11, 3, "HTTP CODE: " << statusCode);
 540
 541      if (const StoreEntry *oldEntry = findPreviouslyCachedEntry(entry))
 542          sawDateGoBack = rep->olderThan(oldEntry->getReply());
 543 @@ -919,7 +904,9 @@
 544          const SBuf vary(httpMakeVaryMark(request, rep));
 545
 546          if (vary.isEmpty()) {
 547 -            entry->makePrivate();
 548 +            // TODO: check whether such responses are shareable.
 549 +            // Do not share for now.
 550 +            entry->makePrivate(false);
 551              if (!fwd->reforwardableStatus(rep->sline.status()))
 552                  EBIT_CLR(entry->flags, ENTRY_FWD_HDR_WAIT);
 553              varyFailure = true;
 554 @@ -942,30 +929,31 @@
 555          if (!fwd->reforwardableStatus(rep->sline.status()))
 556              EBIT_CLR(entry->flags, ENTRY_FWD_HDR_WAIT);
 557
 558 -        switch (cacheableReply()) {
 559 -
 560 -        case 1:
 561 +        ReuseDecision decision(entry, statusCode);
 562 +
 563 +        switch (reusableReply(decision)) {
 564 +
 565 +        case ReuseDecision::reuseNot:
 566 +            entry->makePrivate(false);
 567 +            break;
 568 +
 569 +        case ReuseDecision::cachePositively:
 570              entry->makePublic();
 571              break;
 572
 573 -        case 0:
 574 -            entry->makePrivate();
 575 +        case ReuseDecision::cacheNegatively:
 576 +            entry->cacheNegatively();
 577              break;
 578
 579 -        case -1:
 580 -
 581 -#if USE_HTTP_VIOLATIONS
 582 -            if (Config.negativeTtl > 0)
 583 -                entry->cacheNegatively();
 584 -            else
 585 -#endif
 586 -                entry->makePrivate();
 587 +        case ReuseDecision::doNotCacheButShare:
 588 +            entry->makePrivate(true);
 589              break;
 590
 591          default:
 592              assert(0);
 593              break;
 594          }
 595 +        debugs(11, 3, "decided: " << decision);
 596      }
 597
 598      if (!ignoreCacheControl) {
 599 @@ -2429,3 +2417,29 @@
 600      mustStop(reason);
 601  }
 602
 603 +HttpStateData::ReuseDecision::ReuseDecision(const StoreEntry *e, const Http::StatusCode code)
 604 +    : answer(HttpStateData::ReuseDecision::reuseNot), reason(nullptr), entry(e), statusCode(code) {}
 605 +
 606 +HttpStateData::ReuseDecision::Answers
 607 +HttpStateData::ReuseDecision::make(const HttpStateData::ReuseDecision::Answers ans, const char *why)
 608 +{
 609 +    answer = ans;
 610 +    reason = why;
 611 +    return answer;
 612 +}
 613 +
 614 +std::ostream &operator <<(std::ostream &os, const HttpStateData::ReuseDecision &d)
 615 +{
 616 +    static const char *ReuseMessages[] = {
 617 +        "do not cache and do not share", // reuseNot
 618 +        "cache positively and share", // cachePositively
 619 +        "cache negatively and share", // cacheNegatively
 620 +        "do not cache but share" // doNotCacheButShare
 621 +    };
 622 +
 623 +    assert(d.answer >= HttpStateData::ReuseDecision::reuseNot &&
 624 +            d.answer <= HttpStateData::ReuseDecision::doNotCacheButShare);
 625 +    return os << ReuseMessages[d.answer] << " because " << d.reason <<
 626 +        "; HTTP status " << d.statusCode << " " << *(d.entry);
 627 +}
 628 +
 629
 630 === modified file 'src/http.h'
 631 --- src/http.h  2017-01-01 00:16:45 +0000
 632 +++ src/http.h  2017-06-14 21:37:20 +0000
 633 @@ -22,6 +22,23 @@
 634  {
 635
 636  public:
 637 +
 638 +    /// assists in making and relaying entry caching/sharing decision
 639 +    class ReuseDecision
 640 +    {
 641 +    public:
 642 +        enum Answers { reuseNot = 0, cachePositively, cacheNegatively, doNotCacheButShare };
 643 +
 644 +        ReuseDecision(const StoreEntry *e, const Http::StatusCode code);
 645 +        /// stores the corresponding decision
 646 +        Answers make(const Answers ans, const char *why);
 647 +
 648 +        Answers answer; ///< the decision id
 649 +        const char *reason; ///< the decision reason
 650 +        const StoreEntry *entry; ///< entry for debugging
 651 +        const Http::StatusCode statusCode; ///< HTTP status for debugging
 652 +    };
 653 +
 654      HttpStateData(FwdState *);
 655      ~HttpStateData();
 656
 657 @@ -39,8 +56,8 @@
 658      void readReply(const CommIoCbParams &io);
 659      virtual void maybeReadVirginBody(); // read response data from the network
 660
 661 -    // Determine whether the response is a cacheable representation
 662 -    int cacheableReply();
 663 +    // Checks whether the response is cacheable/shareable.
 664 +    ReuseDecision::Answers reusableReply(ReuseDecision &decision);
 665
 666      CachePeer *_peer;       /* CachePeer request made to */
 667      int eof;            /* reached end-of-object? */
 668 @@ -119,6 +136,8 @@
 669      CBDATA_CLASS2(HttpStateData);
 670  };
 671
 672 +std::ostream &operator <<(std::ostream &os, const HttpStateData::ReuseDecision &d);
 673 +
 674  int httpCachable(const HttpRequestMethod&);
 675  void httpStart(FwdState *);
 676  SBuf httpMakeVaryMark(HttpRequest * request, HttpReply const * reply);
 677
 678 === modified file 'src/store.cc'
 679 --- src/store.cc        2017-01-01 00:16:45 +0000
 680 +++ src/store.cc        2017-06-14 21:37:20 +0000
 681 @@ -171,11 +171,18 @@
 682  }
 683
 684  void
 685 -StoreEntry::makePrivate()
 686 +StoreEntry::makePrivate(const bool shareable)
 687  {
 688      /* This object should never be cached at all */
 689      expireNow();
 690 -    releaseRequest(); /* delete object when not used */
 691 +    releaseRequest(shareable); /* delete object when not used */
 692 +}
 693 +
 694 +void
 695 +StoreEntry::clearPrivate()
 696 +{
 697 +    EBIT_CLR(flags, KEY_PRIVATE);
 698 +    shareableWhenPrivate = false;
 699  }
 700
 701  void
 702 @@ -365,7 +372,8 @@
 703      ping_status(PING_NONE),
 704      store_status(STORE_PENDING),
 705      swap_status(SWAPOUT_NONE),
 706 -    lock_count(0)
 707 +    lock_count(0),
 708 +    shareableWhenPrivate(false)
 709  {
 710      debugs(20, 5, "StoreEntry constructed, this=" << this);
 711  }
 712 @@ -504,14 +512,14 @@
 713  }
 714
 715  void
 716 -StoreEntry::releaseRequest()
 717 +StoreEntry::releaseRequest(const bool shareable)
 718  {
 719      if (EBIT_TEST(flags, RELEASE_REQUEST))
 720          return;
 721
 722      setReleaseFlag(); // makes validToSend() false, preventing future hits
 723
 724 -    setPrivateKey();
 725 +    setPrivateKey(shareable);
 726  }
 727
 728  int
 729 @@ -623,12 +631,16 @@
 730   * concept'.
 731   */
 732  void
 733 -StoreEntry::setPrivateKey()
 734 +StoreEntry::setPrivateKey(const bool shareable)
 735  {
 736      const cache_key *newkey;
 737
 738 -    if (key && EBIT_TEST(flags, KEY_PRIVATE))
 739 -        return;                 /* is already private */
 740 +    if (key && EBIT_TEST(flags, KEY_PRIVATE)) {
 741 +        // The entry is already private, but it may be still shareable.
 742 +        if (!shareable)
 743 +            shareableWhenPrivate = false;
 744 +        return;
 745 +    }
 746
 747      if (key) {
 748          setReleaseFlag(); // will markForUnlink(); all caches/workers will know
 749 @@ -649,6 +661,7 @@
 750
 751      assert(hash_lookup(store_table, newkey) == NULL);
 752      EBIT_SET(flags, KEY_PRIVATE);
 753 +    shareableWhenPrivate = shareable;
 754      hashInsert(newkey);
 755  }
 756
 757 @@ -705,14 +718,17 @@
 758      if (StoreEntry *e2 = (StoreEntry *)hash_lookup(store_table, newkey)) {
 759          assert(e2 != this);
 760          debugs(20, 3, "Making old " << *e2 << " private.");
 761 -        e2->setPrivateKey();
 762 -        e2->release();
 763 +
 764 +        // TODO: check whether there is any sense in keeping old entry
 765 +        // shareable here. Leaving it non-shareable for now.
 766 +        e2->setPrivateKey(false);
 767 +        e2->release(false);
 768      }
 769
 770      if (key)
 771          hashDelete();
 772
 773 -    EBIT_CLR(flags, KEY_PRIVATE);
 774 +    clearPrivate();
 775
 776      hashInsert(newkey);
 777
 778 @@ -830,7 +846,7 @@
 779      e->lock("storeCreateEntry");
 780
 781      if (neighbors_do_private_keys || !flags.hierarchical)
 782 -        e->setPrivateKey();
 783 +        e->setPrivateKey(false);
 784      else
 785          e->setPublicKey();
 786
 787 @@ -1264,7 +1280,7 @@
 788
 789  /* release an object from a cache */
 790  void
 791 -StoreEntry::release()
 792 +StoreEntry::release(const bool shareable)
 793  {
 794      PROF_start(storeRelease);
 795      debugs(20, 3, "releasing " << *this << ' ' << getMD5Text());
 796 @@ -1274,7 +1290,7 @@
 797      if (locked()) {
 798          expireNow();
 799          debugs(20, 3, "storeRelease: Only setting RELEASE_REQUEST bit");
 800 -        releaseRequest();
 801 +        releaseRequest(shareable);
 802          PROF_stop(storeRelease);
 803          return;
 804      }
 805 @@ -1282,7 +1298,7 @@
 806      Store::Root().memoryUnlink(*this);
 807
 808      if (StoreController::store_dirs_rebuilding && swap_filen > -1) {
 809 -        setPrivateKey();
 810 +        setPrivateKey(shareable);
 811
 812          if (swap_filen > -1) {
 813              // lock the entry until rebuilding is done
 814 @@ -2181,7 +2197,11 @@
 815          if (EBIT_TEST(e.flags, REFRESH_REQUEST)) os << 'F';
 816          if (EBIT_TEST(e.flags, ENTRY_REVALIDATE_STALE)) os << 'E';
 817          if (EBIT_TEST(e.flags, ENTRY_DISPATCHED)) os << 'D';
 818 -        if (EBIT_TEST(e.flags, KEY_PRIVATE)) os << 'I';
 819 +        if (EBIT_TEST(e.flags, KEY_PRIVATE)) {
 820 +            os << 'I';
 821 +            if (e.shareableWhenPrivate)
 822 +                os << 'H';
 823 +        }
 824          if (EBIT_TEST(e.flags, ENTRY_FWD_HDR_WAIT)) os << 'W';
 825          if (EBIT_TEST(e.flags, ENTRY_NEGCACHED)) os << 'N';
 826          if (EBIT_TEST(e.flags, ENTRY_VALIDATED)) os << 'V';
 827
 828 === modified file 'src/tests/stub_store.cc'
 829 --- src/tests/stub_store.cc     2017-01-01 00:16:45 +0000
 830 +++ src/tests/stub_store.cc     2017-06-14 21:37:20 +0000
 831 @@ -43,11 +43,11 @@
 832  void StoreEntry::abort() STUB
 833  void StoreEntry::unlink() STUB
 834  void StoreEntry::makePublic(const KeyScope keyScope) STUB
 835 -void StoreEntry::makePrivate() STUB
 836 +void StoreEntry::makePrivate(const bool shareable) STUB
 837  void StoreEntry::setPublicKey(const KeyScope keyScope) STUB
 838 -void StoreEntry::setPrivateKey() STUB
 839 +void StoreEntry::setPrivateKey(const bool shareable) STUB
 840  void StoreEntry::expireNow() STUB
 841 -void StoreEntry::releaseRequest() STUB
 842 +void StoreEntry::releaseRequest(const bool shareable) STUB
 843  void StoreEntry::negativeCache() STUB
 844  void StoreEntry::cacheNegatively() STUB
 845  void StoreEntry::purgeMem() STUB
 846 @@ -99,7 +99,7 @@
 847  int64_t StoreEntry::contentLen() const STUB_RETVAL(0)
 848  void StoreEntry::lock(const char *) STUB
 849  void StoreEntry::touch() STUB
 850 -void StoreEntry::release() STUB
 851 +void StoreEntry::release(const bool shareable) STUB
 852
 853  NullStoreEntry *NullStoreEntry::getInstance() STUB_RETVAL(NULL)
 854  const char *NullStoreEntry::getMD5Text() const STUB_RETVAL(NULL)
 855
 856 === modified file 'src/tests/testStoreController.cc'
 857 --- src/tests/testStoreController.cc    2017-01-01 00:16:45 +0000
 858 +++ src/tests/testStoreController.cc    2017-06-14 21:37:20 +0000
 859 @@ -116,7 +116,7 @@
 860      e->lastModified(squid_curtime);
 861      e->refcount = 1;
 862      EBIT_CLR(e->flags, RELEASE_REQUEST);
 863 -    EBIT_CLR(e->flags, KEY_PRIVATE);
 864 +    e->clearPrivate();
 865      e->ping_status = PING_NONE;
 866      EBIT_CLR(e->flags, ENTRY_VALIDATED);
 867      e->hashInsert((const cache_key *)name.termedBuf()); /* do it after we clear KEY_PRIVATE */
 868
 869 === modified file 'src/tests/testStoreHashIndex.cc'
 870 --- src/tests/testStoreHashIndex.cc     2017-01-01 00:16:45 +0000
 871 +++ src/tests/testStoreHashIndex.cc     2017-06-14 21:37:20 +0000
 872 @@ -97,7 +97,7 @@
 873      e->lastModified(squid_curtime);
 874      e->refcount = 1;
 875      EBIT_CLR(e->flags, RELEASE_REQUEST);
 876 -    EBIT_CLR(e->flags, KEY_PRIVATE);
 877 +    e->clearPrivate();
 878      e->ping_status = PING_NONE;
 879      EBIT_CLR(e->flags, ENTRY_VALIDATED);
 880      e->hashInsert((const cache_key *)name.termedBuf()); /* do it after we clear KEY_PRIVATE */
 881