HTTP/1.1: partial support for no-cache and private= controls with parameters

Since we now support HTTP/1.1 storage and revalidation of
Cache-Control:no-cache it is important that we at least detect the cases
where no-cache= and private= contain parameters.

These are likely still rare occurances due to the historic lack of
support. So for now Squid just detects and exempts these responses from
the caching performed. The basic framework for adding handling of the
header lists is made available but not at this time used.

diff --git a/src/HttpHdrCc.cc b/src/HttpHdrCc.cc
index eb48996da9973a318e9258c97258bc6e34f1efd7..9b2afe5a019dab287ef7b534d95e572b4c7b0a9f 100644 (file)
--- a/src/HttpHdrCc.cc
+++ b/src/HttpHdrCc.cc
@@ -194,15 +194,42 @@ HttpHdrCc::parse(const String & str)
             }
             break;
 
+        case CC_PRIVATE: {
+            String temp;
+            if (!p)  {
+                // Value parameter is optional.
+                private_.clean();
+            }            else if (/* p &&*/ httpHeaderParseQuotedString(p, (ilen-nlen-1), &temp)) {
+                private_.append(temp);
+            }            else {
+                debugs(65, 2, "cc: invalid private= specs near '" << item << "'");
+            }
+            // to be safe we ignore broken parameters, but always remember the 'private' part.
+            setMask(type,true);
+        }
+        break;
+
+        case CC_NO_CACHE: {
+            String temp;
+            if (!p) {
+                // On Requests, missing value parameter is expected syntax.
+                // On Responses, value parameter is optional.
+                setMask(type,true);
+                no_cache.clean();
+            } else if (/* p &&*/ httpHeaderParseQuotedString(p, (ilen-nlen-1), &temp)) {
+                // On Requests, a value parameter is invalid syntax.
+                // XXX: identify when parsing request header and dump err message here.
+                setMask(type,true);
+                no_cache.append(temp);
+            } else {
+                debugs(65, 2, "cc: invalid no-cache= specs near '" << item << "'");
+            }
+        }
+        break;
+
         case CC_PUBLIC:
             Public(true);
             break;
-        case CC_PRIVATE:
-            Private(true);
-            break;
-        case CC_NO_CACHE:
-            noCache(true);
-            break;
         case CC_NO_STORE:
             noStore(true);
             break;

diff --git a/src/HttpHdrCc.h b/src/HttpHdrCc.h
index 366f1524ceb3040bbe5fa03ea4ffaef9cc0bd989..af1cb4a65602a1122b76bde794c1d91aad089c65 100644 (file)
--- a/src/HttpHdrCc.h
+++ b/src/HttpHdrCc.h
@@ -74,15 +74,27 @@ public:
 
     //manipulation for Cache-Control: private header
     bool hasPrivate() const {return isSet(CC_PRIVATE);}
-    bool Private() const {return isSet(CC_PRIVATE);}
-    void Private(bool v) {setMask(CC_PRIVATE,v);}
-    void clearPrivate() {setMask(CC_PRIVATE,false);}
+    const String &Private() const {return private_;}
+    void Private(String &v) {
+        setMask(CC_PRIVATE,true);
+        // uses append for multi-line headers
+        if (private_.defined())
+            private_.append(",");
+        private_.append(v);
+    }
+    void clearPrivate() {setMask(CC_PRIVATE,false); private_.clean();}
 
     //manipulation for Cache-Control: no-cache header
     bool hasNoCache() const {return isSet(CC_NO_CACHE);}
-    bool noCache() const {return isSet(CC_NO_CACHE);}
-    void noCache(bool v) {setMask(CC_NO_CACHE,v);}
-    void clearNoCache() {setMask(CC_NO_CACHE,false);}
+    const String &noCache() const {return no_cache;}
+    void noCache(String &v) {
+        setMask(CC_NO_CACHE,true);
+        // uses append for multi-line headers
+        if (no_cache.defined())
+            no_cache.append(",");
+        no_cache.append(v);
+    }
+    void clearNoCache() {setMask(CC_NO_CACHE,false); no_cache.clean();}
 
     //manipulation for Cache-Control: no-store header
     bool hasNoStore() const {return isSet(CC_NO_STORE);}
@@ -166,6 +178,9 @@ private:
     int32_t max_stale;
     int32_t stale_if_error;
     int32_t min_fresh;
+    String private_; ///< List of headers sent as value for CC:private="...". May be empty/undefined if the value is missing.
+    String no_cache; ///< List of headers sent as value for CC:no-cache="...". May be empty/undefined if the value is missing.
+
     /// low-level part of the public set method, performs no checks
     _SQUID_INLINE_ void setMask(http_hdr_cc_type id, bool newval=true);
     _SQUID_INLINE_ void setValue(int32_t &value, int32_t new_value, http_hdr_cc_type hdr, bool setting=true);

diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index 88d535660a3252bb5bb775b02a168ae9a6f9c913..a4c356463f8959d757595a9a9d430709d2cb36ba 100644 (file)
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -1093,7 +1093,7 @@ clientInterpretRequestHeaders(ClientHttpRequest * http)
 
     if (!request->flags.ignoreCc) {
         if (request->cache_control) {
-            if (request->cache_control->noCache())
+            if (request->cache_control->hasNoCache())
                 no_cache=true;
 
             // RFC 2616: treat Pragma:no-cache as if it was Cache-Control:no-cache when Cache-Control is missing

diff --git a/src/http.cc b/src/http.cc
index 685204132fc7b76aa4fc2ef5b3382ce5177edeff..2ffee8ca341c5e1ad6269edb7c16758af69b625b 100644 (file)
--- a/src/http.cc
+++ b/src/http.cc
@@ -362,6 +362,16 @@ HttpStateData::cacheableReply()
         }
 
         // NP: request CC:no-cache only means cache READ is forbidden. STORE is permitted.
+        if (rep->cache_control && rep->cache_control->hasNoCache() && rep->cache_control->noCache().defined()) {
+            /* TODO: we are allowed to cache when no-cache= has parameters.
+             * Provided we strip away any of the listed headers unless they are revalidated
+             * 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;
+        }
+
         // NP: request CC:private is undefined. We ignore.
         // NP: other request CC flags are limiters on HIT/MISS. We don't care about here.
 
@@ -373,16 +383,21 @@ HttpStateData::cacheableReply()
         }
 
         // 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.
         // TODO: add a shared/private cache configuration possibility.
         if (rep->cache_control &&
-                rep->cache_control->Private() &&
+                rep->cache_control->hasPrivate() &&
                 !REFRESH_OVERRIDE(ignore_private)) {
+            /* TODO: we are allowed to cache when private= has parameters.
+             * Provided we strip away any of the listed headers unless they are revalidated
+             * 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;
         }
-        // NP: being conservative; CC:private overrides CC:public when both are present in a response.
-
     }
+
     // 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) && !REFRESH_OVERRIDE(ignore_auth)) {
@@ -411,8 +426,8 @@ HttpStateData::cacheableReply()
             // NP: given the must-revalidate exception we should also be able to exempt no-cache.
             // HTTPbis WG verdict on this is that it is omitted from the spec due to being 'unexpected' by
             // some. The caching+revalidate is not exactly unsafe though with Squids interpretation of no-cache
-            // as equivalent to must-revalidate in the reply.
-        } else if (rep->cache_control->noCache() && !REFRESH_OVERRIDE(ignore_must_revalidate)) {
+            // (without parameters) as equivalent to must-revalidate in the reply.
+        } else if (rep->cache_control->hasNoCache() && !rep->cache_control->noCache().defined() && !REFRESH_OVERRIDE(ignore_must_revalidate)) {
             debugs(22, 3, HERE << "Authenticated but server reply Cache-Control:no-cache (equivalent to must-revalidate)");
             mayStore = true;
 #endif
@@ -964,10 +979,22 @@ no_cache:
 
     if (!ignoreCacheControl) {
         if (rep->cache_control) {
-            if (rep->cache_control->proxyRevalidate() ||
-                    rep->cache_control->mustRevalidate() ||
-                    rep->cache_control->noCache() ||
-                    rep->cache_control->hasSMaxAge())
+            // We are required to revalidate on many conditions.
+            // For security reasons we do so even if storage was caused by refresh_pattern ignore-* option
+
+            // CC:must-revalidate or CC:proxy-revalidate
+            const bool ccMustRevalidate = (rep->cache_control->proxyRevalidate() || rep->cache_control->mustRevalidate());
+
+            // CC:no-cache (only if there are no parameters)
+            const bool ccNoCacheNoParams = (rep->cache_control->hasNoCache() && rep->cache_control->noCache().undefined());
+
+            // CC:s-maxage=N
+            const bool ccSMaxAge = rep->cache_control->hasSMaxAge();
+
+            // CC:private (yes, these can sometimes be stored)
+            const bool ccPrivate = rep->cache_control->hasPrivate();
+
+            if (ccMustRevalidate || ccNoCacheNoParams || ccSMaxAge || ccPrivate)
                 EBIT_SET(entry->flags, ENTRY_REVALIDATE);
         }
 #if USE_HTTP_VIOLATIONS // response header Pragma::no-cache is undefined in HTTP
@@ -1803,7 +1830,7 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request,
 #endif
 
         /* Add max-age only without no-cache */
-        if (!cc->hasMaxAge() && !cc->noCache()) {
+        if (!cc->hasMaxAge() && !cc->hasNoCache()) {
             const char *url =
                 entry ? entry->url() : urlCanonical(request);
             cc->maxAge(getMaxAge(url));