From: Alex Rousskov Date: Tue, 28 Sep 2010 15:20:36 +0000 (-0600) Subject: HTTP Compliance: Reply with an error if required validation fails. X-Git-Tag: take1~226 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7948b78453428ba97afe31b86f3cf371456c9713;p=thirdparty%2Fsquid.git 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/client_side_reply.cc b/src/client_side_reply.cc index b76d782b71..f1ee01225d 100644 --- a/src/client_side_reply.cc +++ b/src/client_side_reply.cc @@ -349,7 +349,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(); } @@ -386,9 +386,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 2dc733fd9e..b0809683d7 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 6f613729a9..f8edb45905 100644 --- a/src/http.cc +++ b/src/http.cc @@ -942,9 +942,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/log/access_log.cc b/src/log/access_log.cc index 1bb4e9e607..e21c4c4dad 100644 --- a/src/log/access_log.cc +++ b/src/log/access_log.cc @@ -72,7 +72,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", @@ -2485,7 +2486,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/refresh.cc b/src/refresh.cc index 5f54672515..a91153adee 100644 --- a/src/refresh.cc +++ b/src/refresh.cc @@ -277,6 +277,7 @@ refreshCheck(const StoreEntry * entry, HttpRequest * request, time_t delta) #endif ) { debugs(22, 3, "refreshCheck: YES: Must revalidate stale response"); + request->flags.fail_on_validation_err = 1; return STALE_MUST_REVALIDATE; } diff --git a/src/structs.h b/src/structs.h index 7c08d05636..9351606c79 100644 --- a/src/structs.h +++ b/src/structs.h @@ -1007,7 +1007,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),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 @@ -1029,6 +1029,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 USE_HTTP_VIOLATIONS unsigned int nocache_hack:1; /* for changing/ignoring no-cache requests */ #endif