From: Amos Jeffries Date: Fri, 19 Jun 2015 07:13:57 +0000 (-0700) Subject: Bug 4269: ignore-must-revalidate broken X-Git-Tag: merge-candidate-3-v1~78 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=064679ea374d2f58ae4660d9a2af213b9be24bba;p=thirdparty%2Fsquid.git 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 2917065764..8f378c8082 100644 --- 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.

Manual squid.conf update may be required on upgrade. + refresh_pattern +

Removed ignore-auth. Its commonly desired behaviour is + performed by default with correct HTTP/1.1 revalidation. +

Removed ignore-must-revalidate. Other more HTTP compliant + directives can be used to prevent objects from caching. + sslcrtd_children

New parameter queue-size= to set the maximum number of queued requests. diff --git a/src/RefreshPattern.h b/src/RefreshPattern.h index 31cc5c739f..e835dd6be9 100644 --- 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 a4bc03682d..399ce2690f 100644 --- 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 e4320d0d3f..1b61f5cf54 100644 --- 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 ffeb0df4ab..44286d77b9 100644 --- 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 cdd015356e..6e222f8957 100644 --- 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;