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 b76d782b713144cf63857f3cfdb9c39c751f864b..f1ee01225ddbc6898cf9a601b75cfe5a28f3292e 100644 (file)
--- 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 2dc733fd9ed39e85fd1141f0e41f9600e88db842..b0809683d701966d6706f1e2033cc14ffca216fc 100644 (file)
--- 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 6f613729a9163108ea21af471223020a4f350747..f8edb4590520791cce9a8dd6b6caa16d38bfa8c8 100644 (file)
--- 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 1bb4e9e6079bbd5405c439a36bfaaa004ed383d5..e21c4c4dad7415ec79edea67975957d6ccf37cdf 100644 (file)
--- 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 5f5467251510489ea9a2d9c92ca48e233ca2f63b..a91153adeecfd36250c2fcce87718ad06dd8cc24 100644 (file)
--- 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 7c08d0563671f3ec35b4fc3a30268dbda3983136..9351606c79b565235a78c5c54fadd57344225ce2 100644 (file)
--- 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