]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Collapse internal revalidation requests (SMP-unaware caches), again.
authorEduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
Thu, 1 Jun 2017 23:34:40 +0000 (17:34 -0600)
committerAlex Rousskov <rousskov@measurement-factory.com>
Thu, 1 Jun 2017 23:34:40 +0000 (17:34 -0600)
The security fix in v5 r14979 had a negative effect on collapsed
forwarding. All "private" entries were considered automatically
non-shareable among collapsed clients. However this is not true: there
are many situations when collapsed forwarding should work despite of
"private" entry status: 304/5xx responses are good examples of that.
This patch fixes that by means of a new StoreEntry::shareableWhenPrivate
flag.

The suggested fix is not complete: To cover all possible situations, we
need to decide whether StoreEntry::shareableWhenPrivate is true or not
for all contexts where StoreEntry::setPrivateKey() is used. This patch
fixes only few important cases inside http.cc, making CF (as well
collapsed revalidation) work for some [non-cacheable] response status
codes, including 3xx, 5xx and some others.

The original support for internal revalidation requests collapsing
was in trink r14755 and referred to Squid bugs 2833, 4311, and 4471.

src/MemStore.cc
src/Store.h
src/client_side_reply.cc
src/fs/rock/RockSwapDir.cc
src/fs/ufs/UFSSwapDir.cc
src/http.cc
src/http.h
src/store.cc
src/tests/stub_store.cc
src/tests/testStoreController.cc
src/tests/testStoreHashIndex.cc

index fed280c389c97da95ba48d67cf5d0067a74ac5e6..ad65db20c9fd866cad2b93f549e77aadb7715f6d 100644 (file)
@@ -474,7 +474,7 @@ MemStore::anchorEntry(StoreEntry &e, const sfileno index, const Ipc::StoreMapAnc
     e.ping_status = PING_NONE;
 
     EBIT_CLR(e.flags, RELEASE_REQUEST);
-    EBIT_CLR(e.flags, KEY_PRIVATE);
+    e.clearPrivate();
     EBIT_SET(e.flags, ENTRY_VALIDATED);
 
     MemObject::MemCache &mc = e.mem_obj->memCache;
index c5ffeb63706bd038d654ecfe3d4abb0184b9f576..cb31b5c33dcd29303054fd0892d0da108d7166ab 100644 (file)
@@ -83,15 +83,19 @@ public:
 
     void abort();
     void makePublic(const KeyScope keyScope = ksDefault);
-    void makePrivate();
+    void makePrivate(const bool shareable);
+    /// A low-level method just resetting "private key" flags.
+    /// To avoid key inconsistency please use forcePublicKey()
+    /// or similar instead.
+    void clearPrivate();
     void setPublicKey(const KeyScope keyScope = ksDefault);
     /// Resets existing public key to a public key with default scope,
     /// releasing the old default-scope entry (if any).
     /// Does nothing if the existing public key already has default scope.
     void clearPublicKeyScope();
-    void setPrivateKey();
+    void setPrivateKey(const bool shareable);
     void expireNow();
-    void releaseRequest();
+    void releaseRequest(const bool shareable = false);
     void negativeCache();
     void cacheNegatively();     /** \todo argh, why both? */
     void invokeHandlers();
@@ -212,7 +216,13 @@ public:
     /// update last reference timestamp and related Store metadata
     void touch();
 
-    virtual void release();
+    virtual void release(const bool shareable = false);
+
+    /// May the caller commit to treating this [previously locked]
+    /// entry as a cache hit?
+    bool mayStartHitting() const {
+        return !EBIT_TEST(flags, KEY_PRIVATE) || shareableWhenPrivate;
+    }
 
 #if USE_ADAPTATION
     /// call back producer when more buffer space is available
@@ -240,6 +250,13 @@ private:
 
     unsigned short lock_count;      /* Assume < 65536! */
 
+    /// Nobody can find/lock KEY_PRIVATE entries, but some transactions
+    /// (e.g., collapsed requests) find/lock a public entry before it becomes
+    /// private. May such transactions start using the now-private entry
+    /// they previously locked? This member should not affect transactions
+    /// that already started reading from the entry.
+    bool shareableWhenPrivate;
+
 #if USE_ADAPTATION
     /// producer callback registered with deferProducer
     AsyncCall::Pointer deferredProducer;
@@ -247,6 +264,8 @@ private:
 
     bool validLength() const;
     bool hasOneOfEtags(const String &reqETags, const bool allowWeakMatch) const;
+
+    friend std::ostream &operator <<(std::ostream &os, const StoreEntry &e);
 };
 
 std::ostream &operator <<(std::ostream &os, const StoreEntry &e);
index 395b366950eb20a643b71df76c8613e1002aee15..a367b5b67481db3d9ae68651cc6aa64d90e72410 100644 (file)
@@ -415,8 +415,8 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result)
     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");
+    if (collapsedRevalidation == crSlave && !http->storeEntry()->mayStartHitting()) {
+        debugs(88, 3, "CF slave hit private non-shareable " << *http->storeEntry() << ". MISS");
         // restore context to meet processMiss() expectations
         restoreState();
         http->logType = LOG_TCP_MISS;
@@ -548,7 +548,7 @@ clientReplyContext::cacheHit(StoreIOBuffer result)
     // 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)) {
+    if (!e->mayStartHitting()) {
         debugs(88, 3, "unsharable " << *e << ". MISS");
         http->logType = LOG_TCP_MISS;
         processMiss();
index a9bab4eb6c10e595038267df2becbe3e4c0e1f1f..52a1c211fb9c00013847c9df516ea71ea0c7c1e0 100644 (file)
@@ -137,7 +137,7 @@ Rock::SwapDir::anchorEntry(StoreEntry &e, const sfileno filen, const Ipc::StoreM
     e.ping_status = PING_NONE;
 
     EBIT_CLR(e.flags, RELEASE_REQUEST);
-    EBIT_CLR(e.flags, KEY_PRIVATE);
+    e.clearPrivate();
     EBIT_SET(e.flags, ENTRY_VALIDATED);
 
     e.swap_dirn = index;
index 48170703cb1d4d2c740353aca06be5eac57e394b..cb19b41b876d8c9f3624ef8452684d733836484d 100644 (file)
@@ -816,7 +816,7 @@ Fs::Ufs::UFSSwapDir::addDiskRestore(const cache_key * key,
     e->refcount = refcount;
     e->flags = newFlags;
     EBIT_CLR(e->flags, RELEASE_REQUEST);
-    EBIT_CLR(e->flags, KEY_PRIVATE);
+    e->clearPrivate();
     e->ping_status = PING_NONE;
     EBIT_CLR(e->flags, ENTRY_VALIDATED);
     mapBitSet(e->swap_filen);
index c19971a323cd9ecaf7f5f44a2d385da40cf91a94..3e685d0e52b92fd2dc3c4029b708379832e03408 100644 (file)
@@ -288,7 +288,9 @@ HttpStateData::processSurrogateControl(HttpReply *reply)
                     (Config.onoff.surrogate_is_remote
                      && sctusable->noStoreRemote())) {
                 surrogateNoStore = true;
-                entry->makePrivate();
+                // Be conservative for now and make it non-shareable because
+                // there is no enough information here to make the decision.
+                entry->makePrivate(false);
             }
 
             /* The HttpHeader logic cannot tell if the header it's parsing is a reply to an
@@ -313,8 +315,8 @@ HttpStateData::processSurrogateControl(HttpReply *reply)
     }
 }
 
-int
-HttpStateData::cacheableReply()
+HttpStateData::ReuseDecision::Answers
+HttpStateData::reusableReply(HttpStateData::ReuseDecision &decision)
 {
     HttpReply const *rep = finalReply();
     HttpHeader const *hdr = &rep->header;
@@ -335,24 +337,19 @@ HttpStateData::cacheableReply()
 #define REFRESH_OVERRIDE(flag) 0
 #endif
 
-    if (EBIT_TEST(entry->flags, RELEASE_REQUEST)) {
-        debugs(22, 3, "NO because " << *entry << " has been released.");
-        return 0;
-    }
+    if (EBIT_TEST(entry->flags, RELEASE_REQUEST))
+        return decision.make(ReuseDecision::reuseNot, "the entry has been released");
 
     // RFC 7234 section 4: a cache MUST use the most recent response
     // (as determined by the Date header field)
-    if (sawDateGoBack) {
-        debugs(22, 3, "NO because " << *entry << " has an older date header.");
-        return 0;
-    }
+    // TODO: whether such responses could be shareable?
+    if (sawDateGoBack)
+        return decision.make(ReuseDecision::reuseNot, "the response has an older date header");
 
     // Check for Surrogate/1.0 protocol conditions
     // NP: reverse-proxy traffic our parent server has instructed us never to cache
-    if (surrogateNoStore) {
-        debugs(22, 3, HERE << "NO because Surrogate-Control:no-store");
-        return 0;
-    }
+    if (surrogateNoStore)
+        return decision.make(ReuseDecision::reuseNot, "Surrogate-Control:no-store");
 
     // RFC 2616: HTTP/1.1 Cache-Control conditions
     if (!ignoreCacheControl) {
@@ -362,10 +359,9 @@ HttpStateData::cacheableReply()
 
         // RFC 2616 section 14.9.2 - MUST NOT cache any response with request CC:no-store
         if (request && request->cache_control && request->cache_control->hasNoStore() &&
-                !REFRESH_OVERRIDE(ignore_no_store)) {
-            debugs(22, 3, HERE << "NO because client request Cache-Control:no-store");
-            return 0;
-        }
+                !REFRESH_OVERRIDE(ignore_no_store))
+            return decision.make(ReuseDecision::reuseNot,
+                                 "client request Cache-Control:no-store");
 
         // NP: request CC:no-cache only means cache READ is forbidden. STORE is permitted.
         if (rep->cache_control && rep->cache_control->hasNoCacheWithParameters()) {
@@ -374,8 +370,8 @@ HttpStateData::cacheableReply()
              * successfully (ie, must revalidate AND these headers are prohibited on stale replies).
              * That is a bit tricky for squid right now so we avoid caching entirely.
              */
-            debugs(22, 3, HERE << "NO because server reply Cache-Control:no-cache has parameters");
-            return 0;
+            return decision.make(ReuseDecision::reuseNot,
+                                 "server reply Cache-Control:no-cache has parameters");
         }
 
         // NP: request CC:private is undefined. We ignore.
@@ -383,10 +379,9 @@ HttpStateData::cacheableReply()
 
         // RFC 2616 section 14.9.2 - MUST NOT cache any response with CC:no-store
         if (rep->cache_control && rep->cache_control->hasNoStore() &&
-                !REFRESH_OVERRIDE(ignore_no_store)) {
-            debugs(22, 3, HERE << "NO because server reply Cache-Control:no-store");
-            return 0;
-        }
+                !REFRESH_OVERRIDE(ignore_no_store))
+            return decision.make(ReuseDecision::reuseNot,
+                                 "server reply Cache-Control:no-store");
 
         // RFC 2616 section 14.9.1 - MUST NOT cache any response with CC:private in a shared cache like Squid.
         // CC:private overrides CC:public when both are present in a response.
@@ -399,23 +394,21 @@ HttpStateData::cacheableReply()
              * successfully (ie, must revalidate AND these headers are prohibited on stale replies).
              * That is a bit tricky for squid right now so we avoid caching entirely.
              */
-            debugs(22, 3, HERE << "NO because server reply Cache-Control:private");
-            return 0;
+            return decision.make(ReuseDecision::reuseNot,
+                                 "server reply Cache-Control:private");
         }
     }
 
     // RFC 2068, sec 14.9.4 - MUST NOT cache any response with Authentication UNLESS certain CC controls are present
     // allow HTTP violations to IGNORE those controls (ie re-block caching Auth)
     if (request && (request->flags.auth || request->flags.authSent)) {
-        if (!rep->cache_control) {
-            debugs(22, 3, HERE << "NO because Authenticated and server reply missing Cache-Control");
-            return 0;
-        }
+        if (!rep->cache_control)
+            return decision.make(ReuseDecision::reuseNot,
+                                 "authenticated and server reply missing Cache-Control");
 
-        if (ignoreCacheControl) {
-            debugs(22, 3, HERE << "NO because Authenticated and ignoring Cache-Control");
-            return 0;
-        }
+        if (ignoreCacheControl)
+            return decision.make(ReuseDecision::reuseNot,
+                                 "authenticated and ignoring Cache-Control");
 
         bool mayStore = false;
         // HTTPbis pt6 section 3.2: a response CC:public is present
@@ -444,10 +437,8 @@ HttpStateData::cacheableReply()
             mayStore = true;
         }
 
-        if (!mayStore) {
-            debugs(22, 3, HERE << "NO because Authenticated transaction");
-            return 0;
-        }
+        if (!mayStore)
+            return decision.make(ReuseDecision::reuseNot, "authenticated transaction");
 
         // NP: response CC:no-cache is equivalent to CC:must-revalidate,max-age=0. We MAY cache, and do so.
         // NP: other request CC flags are limiters on HIT/MISS/REFRESH. We don't care about here.
@@ -458,12 +449,26 @@ HttpStateData::cacheableReply()
      * probably should not be cachable
      */
     if ((v = hdr->getStr(Http::HdrType::CONTENT_TYPE)))
-        if (!strncasecmp(v, "multipart/x-mixed-replace", 25)) {
-            debugs(22, 3, HERE << "NO because Content-Type:multipart/x-mixed-replace");
-            return 0;
-        }
+        if (!strncasecmp(v, "multipart/x-mixed-replace", 25))
+            return decision.make(ReuseDecision::reuseNot, "Content-Type:multipart/x-mixed-replace");
+
+    // TODO: if possible, provide more specific message for each status code
+    static const char *shareableError = "shareable error status code";
+    static const char *nonShareableError = "non-shareable error status code";
+    ReuseDecision::Answers statusAnswer = ReuseDecision::reuseNot;
+    const char *statusReason = nonShareableError;
 
     switch (rep->sline.status()) {
+
+    /* There are several situations when a non-cacheable response may be
+     * still shareable (e.g., among collapsed clients). We assume that these
+     * are 3xx and 5xx responses, indicating server problems and some of
+     * 4xx responses, common for all clients with a given cache key (e.g.,
+     * 404 Not Found or 414 URI Too Long). On the other hand, we should not
+     * share non-cacheable client-specific errors, such as 400 Bad Request
+     * or 406 Not Acceptable.
+     */
+
     /* Responses that are cacheable */
 
     case Http::scOkay:
@@ -481,111 +486,87 @@ HttpStateData::cacheableReply()
          * unless we know how to refresh it.
          */
 
-        if (!refreshIsCachable(entry) && !REFRESH_OVERRIDE(store_stale)) {
-            debugs(22, 3, "NO because refreshIsCachable() returned non-cacheable..");
-            return 0;
-        } else {
-            debugs(22, 3, HERE << "YES because HTTP status " << rep->sline.status());
-            return 1;
-        }
-        /* NOTREACHED */
+        if (refreshIsCachable(entry) || REFRESH_OVERRIDE(store_stale))
+            decision.make(ReuseDecision::cachePositively, "refresh check returned cacheable");
+        else
+            decision.make(ReuseDecision::doNotCacheButShare, "refresh check returned non-cacheable");
         break;
 
     /* Responses that only are cacheable if the server says so */
 
     case Http::scFound:
     case Http::scTemporaryRedirect:
-        if (rep->date <= 0) {
-            debugs(22, 3, HERE << "NO because HTTP status " << rep->sline.status() << " and Date missing/invalid");
-            return 0;
-        }
-        if (rep->expires > rep->date) {
-            debugs(22, 3, HERE << "YES because HTTP status " << rep->sline.status() << " and Expires > Date");
-            return 1;
-        } else {
-            debugs(22, 3, HERE << "NO because HTTP status " << rep->sline.status() << " and Expires <= Date");
-            return 0;
-        }
-        /* NOTREACHED */
+        if (rep->date <= 0)
+            decision.make(ReuseDecision::doNotCacheButShare, "Date is missing/invalid");
+        if (rep->expires > rep->date)
+            decision.make(ReuseDecision::cachePositively, "Expires > Date");
+        else
+            decision.make(ReuseDecision::doNotCacheButShare, "Expires <= Date");
         break;
 
-    /* Errors can be negatively cached */
-
+    /* Some responses can be negatively cached */
     case Http::scNoContent:
-
     case Http::scUseProxy:
-
-    case Http::scBadRequest:
-
     case Http::scForbidden:
-
     case Http::scNotFound:
-
     case Http::scMethodNotAllowed:
-
     case Http::scUriTooLong:
-
     case Http::scInternalServerError:
-
     case Http::scNotImplemented:
-
     case Http::scBadGateway:
-
     case Http::scServiceUnavailable:
-
     case Http::scGatewayTimeout:
     case Http::scMisdirectedRequest:
 
-        debugs(22, 3, "MAYBE because HTTP status " << rep->sline.status());
-        return -1;
+        statusAnswer = ReuseDecision::doNotCacheButShare;
+        statusReason = shareableError;
+    case Http::scBadRequest:
 
-        /* NOTREACHED */
+#if USE_HTTP_VIOLATIONS
+        if (Config.negativeTtl > 0)
+            decision.make(ReuseDecision::cacheNegatively, "Config.negativeTtl > 0");
+        else
+#endif
+            decision.make(statusAnswer, statusReason);
         break;
 
-    /* Some responses can never be cached */
-
-    case Http::scPartialContent:    /* Not yet supported */
-
+    /* these responses can never be cached, some
+       of them can be shared though */
     case Http::scSeeOther:
-
     case Http::scNotModified:
-
     case Http::scUnauthorized:
-
     case Http::scProxyAuthenticationRequired:
-
-    case Http::scInvalidHeader: /* Squid header parsing error */
-
-    case Http::scHeaderTooLarge:
-
     case Http::scPaymentRequired:
+    case Http::scInsufficientStorage:
+        // TODO: use more specific reason for non-error status codes
+        decision.make(ReuseDecision::doNotCacheButShare, shareableError);
+        break;
+
+    case Http::scPartialContent: /* Not yet supported. TODO: make shareable for suitable ranges */
     case Http::scNotAcceptable:
-    case Http::scRequestTimeout:
-    case Http::scConflict:
+    case Http::scRequestTimeout: // TODO: is this shareable?
+    case Http::scConflict: // TODO: is this shareable?
     case Http::scLengthRequired:
     case Http::scPreconditionFailed:
     case Http::scPayloadTooLarge:
     case Http::scUnsupportedMediaType:
     case Http::scUnprocessableEntity:
-    case Http::scLocked:
+    case Http::scLocked: // TODO: is this shareable?
     case Http::scFailedDependency:
-    case Http::scInsufficientStorage:
     case Http::scRequestedRangeNotSatisfied:
     case Http::scExpectationFailed:
-
-        debugs(22, 3, HERE << "NO because HTTP status " << rep->sline.status());
-        return 0;
+    case Http::scInvalidHeader: /* Squid header parsing error */
+    case Http::scHeaderTooLarge:
+        decision.make(ReuseDecision::reuseNot, nonShareableError);
+        break;
 
     default:
         /* RFC 2616 section 6.1.1: an unrecognized response MUST NOT be cached. */
-        debugs (11, 3, HERE << "NO because unknown HTTP status code " << rep->sline.status());
-        return 0;
-
-        /* NOTREACHED */
+        decision.make(ReuseDecision::reuseNot, "unknown status code");
         break;
     }
 
-    /* NOTREACHED */
+    return decision.answer;
 }
 
 /// assemble a variant key (vary-mark) from the given Vary header and HTTP request
@@ -921,11 +902,12 @@ HttpStateData::haveParsedReplyHeaders()
 
     Ctx ctx = ctx_enter(entry->mem_obj->urlXXX());
     HttpReply *rep = finalReply();
+    const Http::StatusCode statusCode = rep->sline.status();
 
     entry->timestampsSet();
 
     /* Check if object is cacheable or not based on reply code */
-    debugs(11, 3, "HTTP CODE: " << rep->sline.status());
+    debugs(11, 3, "HTTP CODE: " << statusCode);
 
     if (const StoreEntry *oldEntry = findPreviouslyCachedEntry(entry))
         sawDateGoBack = rep->olderThan(oldEntry->getReply());
@@ -942,7 +924,9 @@ HttpStateData::haveParsedReplyHeaders()
         const SBuf vary(httpMakeVaryMark(request.getRaw(), rep));
 
         if (vary.isEmpty()) {
-            entry->makePrivate();
+            // TODO: check whether such responses are shareable.
+            // Do not share for now.
+            entry->makePrivate(false);
             if (!fwd->reforwardableStatus(rep->sline.status()))
                 EBIT_CLR(entry->flags, ENTRY_FWD_HDR_WAIT);
             varyFailure = true;
@@ -965,30 +949,31 @@ HttpStateData::haveParsedReplyHeaders()
         if (!fwd->reforwardableStatus(rep->sline.status()))
             EBIT_CLR(entry->flags, ENTRY_FWD_HDR_WAIT);
 
-        switch (cacheableReply()) {
+        ReuseDecision decision(entry, statusCode);
 
-        case 1:
-            entry->makePublic();
+        switch (reusableReply(decision)) {
+
+        case ReuseDecision::reuseNot:
+            entry->makePrivate(false);
             break;
 
-        case 0:
-            entry->makePrivate();
+        case ReuseDecision::cachePositively:
+            entry->makePublic();
             break;
 
-        case -1:
+        case ReuseDecision::cacheNegatively:
+            entry->cacheNegatively();
+            break;
 
-#if USE_HTTP_VIOLATIONS
-            if (Config.negativeTtl > 0)
-                entry->cacheNegatively();
-            else
-#endif
-                entry->makePrivate();
+        case ReuseDecision::doNotCacheButShare:
+            entry->makePrivate(true);
             break;
 
         default:
             assert(0);
             break;
         }
+        debugs(11, 3, "decided: " << decision);
     }
 
     if (!ignoreCacheControl) {
@@ -2457,3 +2442,29 @@ HttpStateData::abortAll(const char *reason)
     mustStop(reason);
 }
 
+HttpStateData::ReuseDecision::ReuseDecision(const StoreEntry *e, const Http::StatusCode code)
+    : answer(HttpStateData::ReuseDecision::reuseNot), reason(nullptr), entry(e), statusCode(code) {}
+
+HttpStateData::ReuseDecision::Answers
+HttpStateData::ReuseDecision::make(const HttpStateData::ReuseDecision::Answers ans, const char *why)
+{
+    answer = ans;
+    reason = why;
+    return answer;
+}
+
+std::ostream &operator <<(std::ostream &os, const HttpStateData::ReuseDecision &d)
+{
+    static const char *ReuseMessages[] = {
+        "do not cache and do not share", // reuseNot
+        "cache positively and share", // cachePositively
+        "cache negatively and share", // cacheNegatively
+        "do not cache but share" // doNotCacheButShare
+    };
+
+    assert(d.answer >= HttpStateData::ReuseDecision::reuseNot &&
+            d.answer <= HttpStateData::ReuseDecision::doNotCacheButShare);
+    return os << ReuseMessages[d.answer] << " because " << d.reason <<
+        "; HTTP status " << d.statusCode << " " << *(d.entry);
+}
+
index 5c85b98777469f7fc03c575884178045aaa1cf0e..42e5d443cbe61bfc1e484f4083b43708b3c3048b 100644 (file)
@@ -23,6 +23,23 @@ class HttpStateData : public Client
     CBDATA_CLASS(HttpStateData);
 
 public:
+
+    /// assists in making and relaying entry caching/sharing decision
+    class ReuseDecision
+    {
+    public:
+        enum Answers { reuseNot = 0, cachePositively, cacheNegatively, doNotCacheButShare };
+
+        ReuseDecision(const StoreEntry *e, const Http::StatusCode code);
+        /// stores the corresponding decision
+        Answers make(const Answers ans, const char *why);
+
+        Answers answer; ///< the decision id
+        const char *reason; ///< the decision reason
+        const StoreEntry *entry; ///< entry for debugging
+        const Http::StatusCode statusCode; ///< HTTP status for debugging
+    };
+
     HttpStateData(FwdState *);
     ~HttpStateData();
 
@@ -40,8 +57,8 @@ public:
     void readReply(const CommIoCbParams &io);
     virtual void maybeReadVirginBody(); // read response data from the network
 
-    // Determine whether the response is a cacheable representation
-    int cacheableReply();
+    // Checks whether the response is cacheable/shareable.
+    ReuseDecision::Answers reusableReply(ReuseDecision &decision);
 
     CachePeer *_peer;       /* CachePeer request made to */
     int eof;            /* reached end-of-object? */
@@ -135,6 +152,8 @@ private:
     bool sawDateGoBack;
 };
 
+std::ostream &operator <<(std::ostream &os, const HttpStateData::ReuseDecision &d);
+
 int httpCachable(const HttpRequestMethod&);
 void httpStart(FwdState *);
 SBuf httpMakeVaryMark(HttpRequest * request, HttpReply const * reply);
index 0804d8a972f1292e237c37e03c3b4d38dc626396..9e4190fb95c6b516e10c55207b964c51fa6fc34f 100644 (file)
@@ -147,11 +147,18 @@ StoreEntry::makePublic(const KeyScope scope)
 }
 
 void
-StoreEntry::makePrivate()
+StoreEntry::makePrivate(const bool shareable)
 {
     /* This object should never be cached at all */
     expireNow();
-    releaseRequest(); /* delete object when not used */
+    releaseRequest(shareable); /* delete object when not used */
+}
+
+void
+StoreEntry::clearPrivate()
+{
+    EBIT_CLR(flags, KEY_PRIVATE);
+    shareableWhenPrivate = false;
 }
 
 void
@@ -327,7 +334,8 @@ StoreEntry::StoreEntry() :
     ping_status(PING_NONE),
     store_status(STORE_PENDING),
     swap_status(SWAPOUT_NONE),
-    lock_count(0)
+    lock_count(0),
+    shareableWhenPrivate(false)
 {
     debugs(20, 5, "StoreEntry constructed, this=" << this);
 }
@@ -463,14 +471,14 @@ StoreEntry::setReleaseFlag()
 }
 
 void
-StoreEntry::releaseRequest()
+StoreEntry::releaseRequest(const bool shareable)
 {
     if (EBIT_TEST(flags, RELEASE_REQUEST))
         return;
 
     setReleaseFlag(); // makes validToSend() false, preventing future hits
 
-    setPrivateKey();
+    setPrivateKey(shareable);
 }
 
 int
@@ -582,10 +590,14 @@ getKeyCounter(void)
  * concept'.
  */
 void
-StoreEntry::setPrivateKey()
+StoreEntry::setPrivateKey(const bool shareable)
 {
-    if (key && EBIT_TEST(flags, KEY_PRIVATE))
-        return;                 /* is already private */
+    if (key && EBIT_TEST(flags, KEY_PRIVATE)) {
+        // The entry is already private, but it may be still shareable.
+        if (!shareable)
+            shareableWhenPrivate = false;
+        return;
+    }
 
     if (key) {
         setReleaseFlag(); // will markForUnlink(); all caches/workers will know
@@ -603,6 +615,7 @@ StoreEntry::setPrivateKey()
 
     assert(hash_lookup(store_table, newkey) == NULL);
     EBIT_SET(flags, KEY_PRIVATE);
+    shareableWhenPrivate = shareable;
     hashInsert(newkey);
 }
 
@@ -659,14 +672,17 @@ StoreEntry::forcePublicKey(const cache_key *newkey)
     if (StoreEntry *e2 = (StoreEntry *)hash_lookup(store_table, newkey)) {
         assert(e2 != this);
         debugs(20, 3, "Making old " << *e2 << " private.");
-        e2->setPrivateKey();
-        e2->release();
+
+        // TODO: check whether there is any sense in keeping old entry
+        // shareable here. Leaving it non-shareable for now.
+        e2->setPrivateKey(false);
+        e2->release(false);
     }
 
     if (key)
         hashDelete();
 
-    EBIT_CLR(flags, KEY_PRIVATE);
+    clearPrivate();
 
     hashInsert(newkey);
 
@@ -788,7 +804,7 @@ storeCreateEntry(const char *url, const char *logUrl, const RequestFlags &flags,
     e->lock("storeCreateEntry");
 
     if (neighbors_do_private_keys || !flags.hierarchical)
-        e->setPrivateKey();
+        e->setPrivateKey(false);
     else
         e->setPublicKey();
 
@@ -1232,7 +1248,7 @@ Store::Maintain(void *)
 
 /* release an object from a cache */
 void
-StoreEntry::release()
+StoreEntry::release(const bool shareable)
 {
     PROF_start(storeRelease);
     debugs(20, 3, "releasing " << *this << ' ' << getMD5Text());
@@ -1242,7 +1258,7 @@ StoreEntry::release()
     if (locked()) {
         expireNow();
         debugs(20, 3, "storeRelease: Only setting RELEASE_REQUEST bit");
-        releaseRequest();
+        releaseRequest(shareable);
         PROF_stop(storeRelease);
         return;
     }
@@ -1252,7 +1268,7 @@ StoreEntry::release()
 
         Store::Root().memoryUnlink(*this);
 
-        setPrivateKey();
+        setPrivateKey(shareable);
 
         // lock the entry until rebuilding is done
         lock("storeLateRelease");
@@ -2100,7 +2116,11 @@ std::ostream &operator <<(std::ostream &os, const StoreEntry &e)
         if (EBIT_TEST(e.flags, RELEASE_REQUEST)) os << 'X';
         if (EBIT_TEST(e.flags, REFRESH_REQUEST)) os << 'F';
         if (EBIT_TEST(e.flags, ENTRY_REVALIDATE_STALE)) os << 'E';
-        if (EBIT_TEST(e.flags, ENTRY_DISPATCHED)) os << 'D';
+        if (EBIT_TEST(e.flags, KEY_PRIVATE)) {
+            os << 'I';
+            if (e.shareableWhenPrivate)
+                os << 'H';
+        }
         if (EBIT_TEST(e.flags, KEY_PRIVATE)) os << 'I';
         if (EBIT_TEST(e.flags, ENTRY_FWD_HDR_WAIT)) os << 'W';
         if (EBIT_TEST(e.flags, ENTRY_NEGCACHED)) os << 'N';
index a3adf29859a05ffd09a8d666e47a697ffa138667..5c6c38f128cb955095e78db58281629ef0d752d9 100644 (file)
@@ -38,11 +38,11 @@ bool StoreEntry::mayStartSwapOut() STUB_RETVAL(false)
 void StoreEntry::trimMemory(const bool preserveSwappable) STUB
 void StoreEntry::abort() STUB
 void StoreEntry::makePublic(const KeyScope scope) STUB
-void StoreEntry::makePrivate() STUB
+void StoreEntry::makePrivate(const bool shareable) STUB
 void StoreEntry::setPublicKey(const KeyScope scope) STUB
-void StoreEntry::setPrivateKey() STUB
+void StoreEntry::setPrivateKey(const bool shareable) STUB
 void StoreEntry::expireNow() STUB
-void StoreEntry::releaseRequest() STUB
+void StoreEntry::releaseRequest(const bool shareable) STUB
 void StoreEntry::negativeCache() STUB
 void StoreEntry::cacheNegatively() STUB
 void StoreEntry::purgeMem() STUB
@@ -93,7 +93,7 @@ int64_t StoreEntry::objectLen() const STUB_RETVAL(0)
 int64_t StoreEntry::contentLen() const STUB_RETVAL(0)
 void StoreEntry::lock(const char *) STUB
 void StoreEntry::touch() STUB
-void StoreEntry::release() STUB
+void StoreEntry::release(const bool shareable) STUB
 void StoreEntry::append(char const *, int) STUB
 void StoreEntry::vappendf(const char *, va_list) STUB
 
index fbbada6b26ff8bde49844aa49bb1b8014869758b..116a50d409b55346f5a51c1728df51b94aa68ac5 100644 (file)
@@ -114,7 +114,7 @@ addedEntry(Store::Disk *aStore,
     e->lastModified(squid_curtime);
     e->refcount = 1;
     EBIT_CLR(e->flags, RELEASE_REQUEST);
-    EBIT_CLR(e->flags, KEY_PRIVATE);
+    e->clearPrivate();
     e->ping_status = PING_NONE;
     EBIT_CLR(e->flags, ENTRY_VALIDATED);
     e->hashInsert((const cache_key *)name.termedBuf()); /* do it after we clear KEY_PRIVATE */
index 25ae8e06b1c9185a991508330b31b5b1b4814d8a..96ea8bdad94e698474f7f44bfa8f8b32acd70673 100644 (file)
@@ -92,7 +92,7 @@ addedEntry(Store::Disk *aStore,
     e->lastModified(squid_curtime);
     e->refcount = 1;
     EBIT_CLR(e->flags, RELEASE_REQUEST);
-    EBIT_CLR(e->flags, KEY_PRIVATE);
+    e->clearPrivate();
     e->ping_status = PING_NONE;
     EBIT_CLR(e->flags, ENTRY_VALIDATED);
     e->hashInsert((const cache_key *)name.termedBuf()); /* do it after we clear KEY_PRIVATE */