Bug 4269: ignore-must-revalidate broken

ignore-must-revalidate appears to prevent revalidation by disabling
storage of objects with must-revalidate/proxy-revalidate header.

However it was also preventing revalidation of objects cached due to
ignore-private, or the presence of no-cache, s-maxage, and use of auth
credentials.

Remove the violation option entirely.

Also cleanup the documentation of ignore-auth which was removed earlier.

diff --git a/doc/release-notes/release-4.sgml b/doc/release-notes/release-4.sgml
index 29170657642ef42010da6f08d33306a29acb61a1..8f378c8082561e9d25991a0e1034fd654116beab 100644 (file)
--- a/doc/release-notes/release-4.sgml
+++ b/doc/release-notes/release-4.sgml
@@ -145,6 +145,12 @@ This section gives a thorough account of those changes in three categories:
           parameter file name.
        <p>Manual squid.conf update may be required on upgrade.
 
+       <tag>refresh_pattern</tag>
+       <p>Removed <em>ignore-auth</em>. Its commonly desired behaviour is
+          performed by default with correct HTTP/1.1 revalidation.
+       <p>Removed <em>ignore-must-revalidate</em>. Other more HTTP compliant
+          directives can be used to prevent objects from caching.
+
        <tag>sslcrtd_children</tag>
        <p>New parameter <em>queue-size=</em> to set the maximum number
           of queued requests.

diff --git a/src/RefreshPattern.h b/src/RefreshPattern.h
index 31cc5c739f50d614c3c12973913c43dd302c5ab4..e835dd6be9d55dfd71a89d9a0c1438935a731f34 100644 (file)
--- a/src/RefreshPattern.h
+++ b/src/RefreshPattern.h
@@ -32,7 +32,6 @@ public:
         bool reload_into_ims;
         bool ignore_reload;
         bool ignore_no_store;
-        bool ignore_must_revalidate;
         bool ignore_private;
 #endif
     } flags;

diff --git a/src/cache_cf.cc b/src/cache_cf.cc
index a4bc03682dc29b842e75eaa6ddbad2be2d0a0b7d..399ce2690fc1ff0658c99fd1c0616476befff0e0 100644 (file)
--- a/src/cache_cf.cc
+++ b/src/cache_cf.cc
@@ -784,13 +784,6 @@ configDoConfigure(void)
             break;
         }
 
-        for (R = Config.Refresh; R; R = R->next) {
-            if (!R->flags.ignore_must_revalidate)
-                continue;
-            debugs(22, DBG_IMPORTANT, "WARNING: use of 'ignore-must-revalidate' in 'refresh_pattern' violates HTTP");
-            break;
-        }
-
         for (R = Config.Refresh; R; R = R->next) {
             if (!R->flags.ignore_private)
                 continue;
@@ -2592,9 +2585,6 @@ dump_refreshpattern(StoreEntry * entry, const char *name, RefreshPattern * head)
         if (head->flags.ignore_no_store)
             storeAppendPrintf(entry, " ignore-no-store");
 
-        if (head->flags.ignore_must_revalidate)
-            storeAppendPrintf(entry, " ignore-must-revalidate");
-
         if (head->flags.ignore_private)
             storeAppendPrintf(entry, " ignore-private");
 #endif
@@ -2624,7 +2614,6 @@ parse_refreshpattern(RefreshPattern ** head)
     int reload_into_ims = 0;
     int ignore_reload = 0;
     int ignore_no_store = 0;
-    int ignore_must_revalidate = 0;
     int ignore_private = 0;
 #endif
 
@@ -2694,6 +2683,7 @@ parse_refreshpattern(RefreshPattern ** head)
             store_stale = 1;
         } else if (!strncmp(token, "max-stale=", 10)) {
             max_stale = xatoi(token + 10);
+
 #if USE_HTTP_VIOLATIONS
 
         } else if (!strcmp(token, "override-expire"))
@@ -2702,12 +2692,8 @@ parse_refreshpattern(RefreshPattern ** head)
             override_lastmod = 1;
         else if (!strcmp(token, "ignore-no-store"))
             ignore_no_store = 1;
-        else if (!strcmp(token, "ignore-must-revalidate"))
-            ignore_must_revalidate = 1;
         else if (!strcmp(token, "ignore-private"))
             ignore_private = 1;
-        else if (!strcmp(token, "ignore-auth"))
-            debugs(22, DBG_PARSE_NOTE(2), "UPGRADE: refresh_pattern option 'ignore-auth' is obsolete. Remove it.");
         else if (!strcmp(token, "reload-into-ims")) {
             reload_into_ims = 1;
             refresh_nocache_hack = 1;
@@ -2718,8 +2704,11 @@ parse_refreshpattern(RefreshPattern ** head)
             /* tell client_side.c that this is used */
 #endif
 
-        } else if (!strcmp(token, "ignore-no-cache")) {
-            debugs(22, DBG_PARSE_NOTE(2), "UPGRADE: refresh_pattern option 'ignore-no-cache' is obsolete. Remove it.");
+        } else if (!strcmp(token, "ignore-no-cache") ||
+                   !strcmp(token, "ignore-must-revalidate") ||
+                   !strcmp(token, "ignore-auth")
+                ) {
+            debugs(22, DBG_PARSE_NOTE(2), "UPGRADE: refresh_pattern option '" << token << "' is obsolete. Remove it.");
         } else
             debugs(22, DBG_CRITICAL, "refreshAddToList: Unknown option '" << pattern << "': " << token);
     }
@@ -2770,9 +2759,6 @@ parse_refreshpattern(RefreshPattern ** head)
     if (ignore_no_store)
         t->flags.ignore_no_store = true;
 
-    if (ignore_must_revalidate)
-        t->flags.ignore_must_revalidate = true;
-
     if (ignore_private)
         t->flags.ignore_private = true;
 #endif

diff --git a/src/cf.data.pre b/src/cf.data.pre
index e4320d0d3fa178d0b29f730e5d442815f7d2b734..1b61f5cf54458120cec4aef47c791586e61e7ba7 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -5451,9 +5451,7 @@ DOC_START
                 reload-into-ims
                 ignore-reload
                 ignore-no-store
-                ignore-must-revalidate
                 ignore-private
-                ignore-auth
                 max-stale=NN
                 refresh-ims
                 store-stale
@@ -5489,22 +5487,11 @@ DOC_START
                the HTTP standard. Enabling this feature could make you
                liable for problems which it causes.
 
-               ignore-must-revalidate ignores any ``Cache-Control: must-revalidate``
-               headers received from a server. Doing this VIOLATES
-               the HTTP standard. Enabling this feature could make you
-               liable for problems which it causes.
-
                ignore-private ignores any ``Cache-control: private''
                headers received from a server. Doing this VIOLATES
                the HTTP standard. Enabling this feature could make you
                liable for problems which it causes.
 
-               ignore-auth caches responses to requests with authorization,
-               as if the originserver had sent ``Cache-control: public''
-               in the response header. Doing this VIOLATES the HTTP standard.
-               Enabling this feature could make you liable for problems which
-               it causes.
-
                refresh-ims causes squid to contact the origin server
                when a client issues an If-Modified-Since request. This
                ensures that the client will receive an updated version

diff --git a/src/http.cc b/src/http.cc
index ffeb0df4ab4c8cb2662f27475fe004ec4e311b5a..44286d77b9b78b89c929ec8ca5ceeaf14865d87b 100644 (file)
--- a/src/http.cc
+++ b/src/http.cc
@@ -406,7 +406,7 @@ HttpStateData::cacheableReply()
             mayStore = true;
 
             // HTTPbis pt6 section 3.2: a response CC:must-revalidate is present
-        } else if (rep->cache_control->mustRevalidate() && !REFRESH_OVERRIDE(ignore_must_revalidate)) {
+        } else if (rep->cache_control->mustRevalidate()) {
             debugs(22, 3, HERE << "Authenticated but server reply Cache-Control:must-revalidate");
             mayStore = true;
 
@@ -415,7 +415,7 @@ HttpStateData::cacheableReply()
             // 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
             // (without parameters) as equivalent to must-revalidate in the reply.
-        } else if (rep->cache_control->hasNoCache() && rep->cache_control->noCache().size() == 0 && !REFRESH_OVERRIDE(ignore_must_revalidate)) {
+        } else if (rep->cache_control->hasNoCache() && rep->cache_control->noCache().size() == 0) {
             debugs(22, 3, HERE << "Authenticated but server reply Cache-Control:no-cache (equivalent to must-revalidate)");
             mayStore = true;
 #endif

diff --git a/src/refresh.cc b/src/refresh.cc
index cdd015356e14b44b46cf575ab6c609e840711622..6e222f89571243b023df745b7314e1970eca0034 100644 (file)
--- a/src/refresh.cc
+++ b/src/refresh.cc
@@ -360,12 +360,8 @@ refreshCheck(const StoreEntry * entry, HttpRequest * request, time_t delta)
      *   Cache-Control: proxy-revalidate
      * the spec says the response must always be revalidated if stale.
      */
-    if (EBIT_TEST(entry->flags, ENTRY_REVALIDATE) && staleness > -1
-#if USE_HTTP_VIOLATIONS
-            && !R->flags.ignore_must_revalidate
-#endif
-       ) {
-        debugs(22, 3, "YES: Must revalidate stale object (origin set must-revalidate or proxy-revalidate)");
+    if (EBIT_TEST(entry->flags, ENTRY_REVALIDATE) && staleness > -1) {
+        debugs(22, 3, "YES: Must revalidate stale object (origin set must-revalidate, proxy-revalidate, no-cache, s-maxage, or private)");
         if (request)
             request->flags.failOnValidationError = true;
         return STALE_MUST_REVALIDATE;