------------------------------------------------------------ revno: 14076 revision-id: squid3@treenet.co.nz-20160907155859-5vp8jh2tchgk8842 parent: squid3@treenet.co.nz-20160817133413-vdmm0d6kvo8bfszk author: Eduard Bagdasaryan committer: Amos Jeffries branch nick: 3.5 timestamp: Thu 2016-09-08 03:58:59 +1200 message: HTTP: MUST always revalidate Cache-Control:no-cache responses. Squid MUST NOT use a CC:no-cache response for satisfying subsequent requests without successful validation with the origin server. Squid violated this MUST because Squid mapped both "no-cache" and "must-revalidate"/etc. directives to the same ENTRY_REVALIDATE flag which was interpreted as "revalidate when the cached response becomes stale" rather than "revalidate on every request". This patch splits ENTRY_REVALIDATE into: - ENTRY_REVALIDATE_ALWAYS, for entries with CC:no-cache and - ENTRY_REVALIDATE_STALE, for entries with other related CC directives like CC:must-revalidate and CC:proxy-revalidate. Reusing ENTRY_CACHABLE_RESERVED_FOR_FUTURE_USE value for the more forgiving ENTRY_REVALIDATE_STALE (instead of the more aggressive ENTRY_REVALIDATE_ALWAYS) fixes the bug for the already cached entries - they will be revalidated and stored with the correct flag on the next request. ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: squid3@treenet.co.nz-20160907155859-5vp8jh2tchgk8842 # target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 # testament_sha1: fda3a2f4decfa341e5b3f8a1f528d4bd89c7fdaa # timestamp: 2016-09-07 16:05:40 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 # base_revision_id: squid3@treenet.co.nz-20160817133413-\ # vdmm0d6kvo8bfszk # # Begin patch === modified file 'src/enums.h' --- src/enums.h 2016-06-18 11:48:03 +0000 +++ src/enums.h 2016-09-07 15:58:59 +0000 @@ -95,11 +95,11 @@ */ enum { ENTRY_SPECIAL, - ENTRY_REVALIDATE, + ENTRY_REVALIDATE_ALWAYS, DELAY_SENDING, RELEASE_REQUEST, REFRESH_REQUEST, - ENTRY_CACHABLE_RESERVED_FOR_FUTURE_USE, + ENTRY_REVALIDATE_STALE, ENTRY_DISPATCHED, KEY_PRIVATE, ENTRY_FWD_HDR_WAIT, === modified file 'src/http.cc' --- src/http.cc 2016-08-17 05:10:37 +0000 +++ src/http.cc 2016-09-07 15:58:59 +0000 @@ -958,8 +958,10 @@ // 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 (ccNoCacheNoParams || ccPrivate) + EBIT_SET(entry->flags, ENTRY_REVALIDATE_ALWAYS); + else if (ccMustRevalidate || ccSMaxAge) + EBIT_SET(entry->flags, ENTRY_REVALIDATE_STALE); } #if USE_HTTP_VIOLATIONS // response header Pragma::no-cache is undefined in HTTP else { @@ -969,7 +971,7 @@ * but servers like "Active Imaging Webcast/2.0" sure do use it */ if (rep->header.has(HDR_PRAGMA) && rep->header.hasListMember(HDR_PRAGMA,"no-cache",',')) - EBIT_SET(entry->flags, ENTRY_REVALIDATE); + EBIT_SET(entry->flags, ENTRY_REVALIDATE_ALWAYS); } #endif } === modified file 'src/refresh.cc' --- src/refresh.cc 2016-01-01 00:14:27 +0000 +++ src/refresh.cc 2016-09-07 15:58:59 +0000 @@ -356,12 +356,15 @@ * Cache-Control: proxy-revalidate * the spec says the response must always be revalidated if stale. */ - if (EBIT_TEST(entry->flags, ENTRY_REVALIDATE) && staleness > -1 + const bool revalidateAlways = EBIT_TEST(entry->flags, ENTRY_REVALIDATE_ALWAYS); + bool revalidateStale = staleness > -1 && EBIT_TEST(entry->flags, ENTRY_REVALIDATE_STALE); #if USE_HTTP_VIOLATIONS - && !R->flags.ignore_must_revalidate + revalidateStale = revalidateStale && !R->flags.ignore_must_revalidate; #endif - ) { - debugs(22, 3, "YES: Must revalidate stale object (origin set must-revalidate or proxy-revalidate)"); + if (revalidateAlways || revalidateStale) { + debugs(22, 3, "YES: Must revalidate stale object (origin set " << + (revalidateAlways ? "no-cache or private" : + "must-revalidate, proxy-revalidate or s-maxage") << ")"); if (request) request->flags.failOnValidationError = true; return STALE_MUST_REVALIDATE; === modified file 'src/stat.cc' --- src/stat.cc 2016-01-01 00:14:27 +0000 +++ src/stat.cc 2016-09-07 15:58:59 +0000 @@ -286,8 +286,8 @@ if (EBIT_TEST(flags, ENTRY_SPECIAL)) strcat(buf, "SPECIAL,"); - if (EBIT_TEST(flags, ENTRY_REVALIDATE)) - strcat(buf, "REVALIDATE,"); + if (EBIT_TEST(flags, ENTRY_REVALIDATE_ALWAYS)) + strcat(buf, "REVALIDATE_ALWAYS,"); if (EBIT_TEST(flags, DELAY_SENDING)) strcat(buf, "DELAY_SENDING,"); @@ -298,6 +298,9 @@ if (EBIT_TEST(flags, REFRESH_REQUEST)) strcat(buf, "REFRESH_REQUEST,"); + if (EBIT_TEST(flags, ENTRY_REVALIDATE_STALE)) + strcat(buf, "REVALIDATE_STALE,"); + if (EBIT_TEST(flags, ENTRY_DISPATCHED)) strcat(buf, "DISPATCHED,"); === modified file 'src/store.cc' --- src/store.cc 2016-04-01 06:15:31 +0000 +++ src/store.cc 2016-09-07 15:58:59 +0000 @@ -2122,10 +2122,11 @@ // print only set flags, using unique letters if (e.flags) { if (EBIT_TEST(e.flags, ENTRY_SPECIAL)) os << 'S'; - if (EBIT_TEST(e.flags, ENTRY_REVALIDATE)) os << 'R'; + if (EBIT_TEST(e.flags, ENTRY_REVALIDATE_ALWAYS)) os << 'R'; if (EBIT_TEST(e.flags, DELAY_SENDING)) os << 'P'; 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 (EBIT_TEST(e.flags, ENTRY_FWD_HDR_WAIT)) os << 'W';