HTTP Compliance: Reply with an error if required validation failed. RFC 2616 says that proxy MUST not use stale entries that have s-maxage, proxy-revalidate, or must-revalidate cache-directive. Add new fail_on_validation_err request flag to store result from refreshCheck(). It is needed to avoid refreshLimits() recalculation in clientReplyContext::handleIMSReply(). Split LOG_TCP_REFRESH_FAIL into LOG_TCP_REFRESH_FAIL_OLD (stale reply sent) and LOG_TCP_REFRESH_FAIL_ERR (error forwarded). Though both logged as TCP_REFRESH_FAIL for backward-compatibility with external scripts, etc. Co-Advisor test cases: test_case/rfc2616/noSrv-hit-must-reval-s-maxage-resp test_case/rfc2616/noSrv-hit-must-reval-proxy-revalidate-resp test_case/rfc2616/noSrv-hit-must-reval-must-revalidate-resp === modified file 'src/client_side_reply.cc' --- src/client_side_reply.cc 2010-09-13 02:25:09 +0000 +++ src/client_side_reply.cc 2010-09-19 04:19:39 +0000 @@ -332,83 +332,88 @@ clientReplyContext::handleIMSReply(Store { if (deleting) return; debugs(88, 3, "handleIMSReply: " << http->storeEntry()->url() << ", " << (long unsigned) result.length << " bytes" ); if (http->storeEntry() == NULL) return; if (result.flags.error && !EBIT_TEST(http->storeEntry()->flags, ENTRY_ABORTED)) return; /* update size of the request */ reqsize = result.length + reqofs; const http_status status = http->storeEntry()->getReply()->sline.status; // request to origin was aborted if (EBIT_TEST(http->storeEntry()->flags, ENTRY_ABORTED)) { debugs(88, 3, "handleIMSReply: request to origin aborted '" << http->storeEntry()->url() << "', sending old entry to client" ); - http->logType = LOG_TCP_REFRESH_FAIL; + http->logType = LOG_TCP_REFRESH_FAIL_OLD; sendClientOldEntry(); } HttpReply *old_rep = (HttpReply *) old_entry->getReply(); // origin replied 304 // TODO FIXME: old_rep2 was forcibly unshadowed, used to be old_rep. Are we sure // that the right semantics were preserved? if (status == HTTP_NOT_MODIFIED) { http->logType = LOG_TCP_REFRESH_UNMODIFIED; // update headers on existing entry HttpReply *old_rep2 = (HttpReply *) old_entry->getReply(); old_rep2->updateOnNotModified(http->storeEntry()->getReply()); old_entry->timestampsSet(); // if client sent IMS if (http->request->flags.ims && !old_entry->modifiedSince(http->request)) { // forward the 304 from origin debugs(88, 3, "handleIMSReply: origin replied 304, revalidating existing entry and forwarding 304 to client"); sendClientUpstreamResponse(); } else { // send existing entry, it's still valid debugs(88, 3, "handleIMSReply: origin replied 304, revalidating existing entry and sending " << old_rep2->sline.status << " to client"); sendClientOldEntry(); } } // origin replied with a non-error code else if (status > HTTP_STATUS_NONE && status < HTTP_INTERNAL_SERVER_ERROR) { // forward response from origin http->logType = LOG_TCP_REFRESH_MODIFIED; debugs(88, 3, "handleIMSReply: origin replied " << status << ", replacing existing entry and forwarding to client"); sendClientUpstreamResponse(); } // origin replied with an error - else { + else if (http->request->flags.fail_on_validation_err) { + http->logType = LOG_TCP_REFRESH_FAIL_ERR; + debugs(88, 3, "handleIMSReply: origin replied with error " << status << + ", forwarding to client due to fail_on_validation_err"); + sendClientUpstreamResponse(); + } else { // ignore and let client have old entry - http->logType = LOG_TCP_REFRESH_FAIL; + http->logType = LOG_TCP_REFRESH_FAIL_OLD; debugs(88, 3, "handleIMSReply: origin replied with error " << status << ", sending old entry (" << old_rep->sline.status << ") to client"); sendClientOldEntry(); } } extern "C" CSR clientGetMoreData; extern "C" CSD clientReplyDetach; /** * clientReplyContext::cacheHit Should only be called until the HTTP reply headers * have been parsed. Normally this should be a single call, but * it might take more than one. As soon as we have the headers, * we hand off to clientSendMoreData, processExpired, or * processMiss. */ void clientReplyContext::CacheHit(void *data, StoreIOBuffer result) { clientReplyContext *context = (clientReplyContext *)data; === modified file 'src/enums.h' --- src/enums.h 2010-09-01 08:18:47 +0000 +++ src/enums.h 2010-09-19 04:11:16 +0000 @@ -24,41 +24,42 @@ * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. * * You should have received a copy of the GNU General Public License * along with this program; if not, write to the Free Software * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111, USA. * */ #ifndef SQUID_ENUMS_H #define SQUID_ENUMS_H #include "HttpStatusCode.h" typedef enum { LOG_TAG_NONE, LOG_TCP_HIT, LOG_TCP_MISS, LOG_TCP_REFRESH_UNMODIFIED, // refresh from origin revalidated existing entry - LOG_TCP_REFRESH_FAIL, // refresh from origin failed + LOG_TCP_REFRESH_FAIL_OLD, // refresh from origin failed, stale reply sent + LOG_TCP_REFRESH_FAIL_ERR, // refresh from origin failed, error forwarded LOG_TCP_REFRESH_MODIFIED, // refresh from origin replaced existing entry LOG_TCP_CLIENT_REFRESH_MISS, LOG_TCP_IMS_HIT, LOG_TCP_SWAPFAIL_MISS, LOG_TCP_NEGATIVE_HIT, LOG_TCP_MEM_HIT, LOG_TCP_DENIED, LOG_TCP_DENIED_REPLY, LOG_TCP_OFFLINE_HIT, #if LOG_TCP_REDIRECTS LOG_TCP_REDIRECT, #endif LOG_UDP_HIT, LOG_UDP_MISS, LOG_UDP_DENIED, LOG_UDP_INVALID, LOG_UDP_MISS_NOFETCH, LOG_ICP_QUERY, LOG_TYPE_MAX } log_type; === modified file 'src/http.cc' --- src/http.cc 2010-09-14 07:37:38 +0000 +++ src/http.cc 2010-09-19 04:04:53 +0000 @@ -924,43 +924,43 @@ HttpStateData::haveParsedReplyHeaders() case -1: #if USE_HTTP_VIOLATIONS if (Config.negativeTtl > 0) entry->cacheNegatively(); else #endif entry->makePrivate(); break; default: assert(0); break; } no_cache: if (!ignoreCacheControl && rep->cache_control) { - if (EBIT_TEST(rep->cache_control->mask, CC_PROXY_REVALIDATE)) - EBIT_SET(entry->flags, ENTRY_REVALIDATE); - else if (EBIT_TEST(rep->cache_control->mask, CC_MUST_REVALIDATE)) + if (EBIT_TEST(rep->cache_control->mask, CC_PROXY_REVALIDATE) || + EBIT_TEST(rep->cache_control->mask, CC_MUST_REVALIDATE) || + EBIT_TEST(rep->cache_control->mask, CC_S_MAXAGE)) EBIT_SET(entry->flags, ENTRY_REVALIDATE); } #if HEADERS_LOG headersLog(1, 0, request->method, rep); #endif ctx_exit(ctx); } HttpStateData::ConnectionStatus HttpStateData::statusIfComplete() const { const HttpReply *rep = virginReply(); /** \par * If the reply wants to close the connection, it takes precedence */ if (httpHeaderHasConnDir(&rep->header, "close")) return COMPLETE_NONPERSISTENT_MSG; === modified file 'src/log/access_log.cc' --- src/log/access_log.cc 2010-09-01 08:18:47 +0000 +++ src/log/access_log.cc 2010-09-19 03:56:40 +0000 @@ -56,41 +56,42 @@ static void accessLogSquid(AccessLogEntry * al, Logfile * logfile); static void accessLogCommon(AccessLogEntry * al, Logfile * logfile); static void accessLogCustom(AccessLogEntry * al, customlog * log); #if HEADERS_LOG static Logfile *headerslog = NULL; #endif #if MULTICAST_MISS_STREAM static int mcast_miss_fd = -1; static struct sockaddr_in mcast_miss_to; static void mcast_encode(unsigned int *, size_t, const unsigned int *); #endif const char *log_tags[] = { "NONE", "TCP_HIT", "TCP_MISS", "TCP_REFRESH_UNMODIFIED", - "TCP_REFRESH_FAIL", + "TCP_REFRESH_FAIL", // same tag logged for LOG_TCP_REFRESH_FAIL_OLD and + "TCP_REFRESH_FAIL", // LOG_TCP_REFRESH_FAIL_ERR for backward-compatibility "TCP_REFRESH_MODIFIED", "TCP_CLIENT_REFRESH_MISS", "TCP_IMS_HIT", "TCP_SWAPFAIL_MISS", "TCP_NEGATIVE_HIT", "TCP_MEM_HIT", "TCP_DENIED", "TCP_DENIED_REPLY", "TCP_OFFLINE_HIT", #if LOG_TCP_REDIRECTS "TCP_REDIRECT", #endif "UDP_HIT", "UDP_MISS", "UDP_DENIED", "UDP_INVALID", "UDP_MISS_NOFETCH", "ICP_QUERY", "LOG_TYPE_MAX" }; @@ -2482,37 +2483,37 @@ accessLogFreeMemory(AccessLogEntry * aLo HTTPMSGUNLOCK(aLogEntry->reply); HTTPMSGUNLOCK(aLogEntry->request); #if ICAP_CLIENT HTTPMSGUNLOCK(aLogEntry->icap.reply); HTTPMSGUNLOCK(aLogEntry->icap.request); #endif } int logTypeIsATcpHit(log_type code) { /* this should be a bitmap for better optimization */ if (code == LOG_TCP_HIT) return 1; if (code == LOG_TCP_IMS_HIT) return 1; - if (code == LOG_TCP_REFRESH_FAIL) + if (code == LOG_TCP_REFRESH_FAIL_OLD) return 1; if (code == LOG_TCP_REFRESH_UNMODIFIED) return 1; if (code == LOG_TCP_NEGATIVE_HIT) return 1; if (code == LOG_TCP_MEM_HIT) return 1; if (code == LOG_TCP_OFFLINE_HIT) return 1; return 0; } === modified file 'src/refresh.cc' --- src/refresh.cc 2010-05-14 09:47:03 +0000 +++ src/refresh.cc 2010-09-19 04:04:53 +0000 @@ -260,40 +260,41 @@ refreshCheck(const StoreEntry * entry, H debugs(22, 3, "Staleness = " << staleness); debugs(22, 3, "refreshCheck: Matched '" << R->pattern << " " << (int) R->min << " " << (int) (100.0 * R->pct) << "%% " << (int) R->max << "'"); debugs(22, 3, "refreshCheck: age = " << age); debugs(22, 3, "\tcheck_time:\t" << mkrfc1123(check_time)); debugs(22, 3, "\tentry->timestamp:\t" << mkrfc1123(entry->timestamp)); if (EBIT_TEST(entry->flags, ENTRY_REVALIDATE) && staleness > -1 #if USE_HTTP_VIOLATIONS && !R->flags.ignore_must_revalidate #endif ) { debugs(22, 3, "refreshCheck: YES: Must revalidate stale response"); + request->flags.fail_on_validation_err = 1; return STALE_MUST_REVALIDATE; } /* request-specific checks */ if (request && !request->flags.ignore_cc) { HttpHdrCc *cc = request->cache_control; if (request->flags.ims && (R->flags.refresh_ims || Config.onoff.refresh_all_ims)) { /* The clients no-cache header is changed into a IMS query */ debugs(22, 3, "refreshCheck: YES: refresh-ims"); return STALE_FORCED_RELOAD; } #if USE_HTTP_VIOLATIONS if (!request->flags.nocache_hack) { (void) 0; } else if (R->flags.ignore_reload) { /* The clients no-cache header is ignored */ debugs(22, 3, "refreshCheck: MAYBE: ignore-reload"); === modified file 'src/structs.h' --- src/structs.h 2010-09-14 07:37:38 +0000 +++ src/structs.h 2010-09-19 04:04:53 +0000 @@ -991,62 +991,63 @@ struct _netdbEntry { int n_peers; }; struct _iostats { enum { histSize = 16 }; struct { int reads; int reads_deferred; int read_hist[histSize]; int writes; int write_hist[histSize]; } Http, Ftp, Gopher; }; struct request_flags { - request_flags(): range(0),nocache(0),ims(0),auth(0),cachable(0),hierarchical(0),loopdetect(0),proxy_keepalive(0),proxying(0),refresh(0),redirected(0),need_validation(0),accelerated(0),ignore_cc(0),intercepted(0),spoof_client_ip(0),internal(0),internalclient(0),must_keepalive(0),chunked_reply(0),stream_error(0),destinationIPLookedUp_(0) { + request_flags(): range(0),nocache(0),ims(0),auth(0),cachable(0),hierarchical(0),loopdetect(0),proxy_keepalive(0),proxying(0),refresh(0),redirected(0),need_validation(0),fail_on_validation_err(0),accelerated(0),ignore_cc(0),intercepted(0),spoof_client_ip(0),internal(0),internalclient(0),must_keepalive(0),chunked_reply(0),stream_error(0),destinationIPLookedUp_(0) { #if USE_HTTP_VIOLATIONS nocache_hack = 0; #endif #if FOLLOW_X_FORWARDED_FOR done_follow_x_forwarded_for = 0; #endif /* FOLLOW_X_FORWARDED_FOR */ } unsigned int range:1; unsigned int nocache:1; unsigned int ims:1; unsigned int auth:1; unsigned int cachable:1; unsigned int hierarchical:1; unsigned int loopdetect:1; unsigned int proxy_keepalive:1; unsigned int proxying: 1; /* this should be killed, also in httpstateflags */ unsigned int refresh:1; unsigned int redirected:1; unsigned int need_validation:1; + unsigned int fail_on_validation_err:1; ///< whether we should fail if validation fails #if USE_HTTP_VIOLATIONS unsigned int nocache_hack:1; /* for changing/ignoring no-cache requests */ #endif unsigned int accelerated:1; unsigned int ignore_cc:1; unsigned int intercepted:1; /**< transparently intercepted request */ unsigned int spoof_client_ip:1; /**< spoof client ip if possible */ unsigned int internal:1; unsigned int internalclient:1; unsigned int must_keepalive:1; unsigned int connection_auth:1; /** Request wants connection oriented auth */ unsigned int connection_auth_disabled:1; /** Connection oriented auth can not be supported */ unsigned int connection_proxy_auth:1; /** Request wants connection oriented auth */ unsigned int pinned:1; /* Request sent on a pinned connection */ unsigned int auth_sent:1; /* Authentication forwarded */ unsigned int no_direct:1; /* Deny direct forwarding unless overriden by always_direct. Used in accelerator mode */ unsigned int chunked_reply:1; /**< Reply with chunked transfer encoding */ unsigned int stream_error:1; /**< Whether stream error has occured */ // When adding new flags, please update cloneAdaptationImmune() as needed.