From: Amos Jeffries Date: Sat, 23 Oct 2010 13:29:15 +0000 (-0600) Subject: Author: Alex Rousskov X-Git-Tag: SQUID_3_1_9~26 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=eb8d342f413a2ca8570dde7f97d81ee855e42ff3;p=thirdparty%2Fsquid.git Author: Alex Rousskov HTTP Compliance: Reply with an error if required validation fails. 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). However, both are still logged as TCP_REFRESH_FAIL for backward-compatibility with external scripts and such. We may decide to start logging more detailed codes later. 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 --- diff --git a/src/access_log.cc b/src/access_log.cc index 84e86bd501..34eb8c2dd2 100644 --- a/src/access_log.cc +++ b/src/access_log.cc @@ -67,7 +67,8 @@ const char *log_tags[] = { "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", @@ -2423,7 +2424,7 @@ logTypeIsATcpHit(log_type code) 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) diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc index 522e6faebe..085d5e9226 100644 --- a/src/client_side_reply.cc +++ b/src/client_side_reply.cc @@ -350,7 +350,7 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result) // 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(); } @@ -387,9 +387,14 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result) } // 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(); diff --git a/src/enums.h b/src/enums.h index afa373073e..cb1f5f0b39 100644 --- a/src/enums.h +++ b/src/enums.h @@ -41,7 +41,8 @@ typedef enum { 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, diff --git a/src/http.cc b/src/http.cc index 6a368b66f7..2c03b28834 100644 --- a/src/http.cc +++ b/src/http.cc @@ -899,9 +899,9 @@ HttpStateData::haveParsedReplyHeaders() 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); } diff --git a/src/refresh.cc b/src/refresh.cc index 7891c34ee8..34baae5a18 100644 --- a/src/refresh.cc +++ b/src/refresh.cc @@ -277,6 +277,8 @@ refreshCheck(const StoreEntry * entry, HttpRequest * request, time_t delta) #endif ) { debugs(22, 3, "refreshCheck: YES: Must revalidate stale response"); + if (request) + request->flags.fail_on_validation_err = 1; return STALE_MUST_REVALIDATE; } diff --git a/src/structs.h b/src/structs.h index 493f822e05..eff141ef41 100644 --- a/src/structs.h +++ b/src/structs.h @@ -1010,7 +1010,7 @@ struct _iostats { 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),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),destinationIPLookedUp_(0) { #if HTTP_VIOLATIONS nocache_hack = 0; #endif @@ -1032,6 +1032,7 @@ unsigned int proxying: 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 HTTP_VIOLATIONS unsigned int nocache_hack:1; /* for changing/ignoring no-cache requests */ #endif