HTTP Compliance: add appropriate Warnings if serving a stale hit.

Per RFC 2616, we MUST add "110 Response is stale" Warning if serving a
stale reply for any reason, including configured overrides. We MUST add
"111 Revalidation failed" Warning if serving a stale reply because an
attempt to revalidate the response failed, due to an inability to reach
the server.

The patch adds a new stale_if_hit request flag, which is set in
refreshCheckHTTP() when entry freshness is calculated. refreshCheckHTTP()
is now called in offline mode, to set stale_if_hit properly. We check for
the offline mode before returning from refreshCheckHTTP() to preserve the
original logic.

refreshCheckHTTP() is no longer called for internal requests, to avoid
setting of stale_if_hit flag. It did not do anything important for
internal requests anyway.

Co-Advisor test cases:
    test_case/rfc2616/noSrv-hit-stale-max-age-req
    test_case/rfc2616/ccReqDirMsg-max-stale-warning

diff --git a/src/HttpHeader.cc b/src/HttpHeader.cc
index 4b9bb35b5831167f56e147b70cb187e94b3d2566..4529de663c7aa6da177fb849f37dca4021181ed3 100644 (file)
--- a/src/HttpHeader.cc
+++ b/src/HttpHeader.cc
@@ -1195,6 +1195,14 @@ HttpHeader::putSc(HttpHdrSc *sc)
     mb.clean();
 }
 
+void
+HttpHeader::putWarning(const int code, const char *const text)
+{
+    char buf[512];
+    snprintf(buf, sizeof(buf), "%i %s \"%s\"", code, visible_appname_string, text);
+    putStr(HDR_WARNING, buf);
+}
+
 /* add extension header (these fields are not parsed/analyzed/joined, etc.) */
 void
 HttpHeader::putExt(const char *name, const char *value)

diff --git a/src/HttpHeader.h b/src/HttpHeader.h
index 53bea2e9189e67f68b28dc9f28af44390ce29b80..cb1aa653f22acb4ba074954e111d3ad61e0989f5 100644 (file)
--- a/src/HttpHeader.h
+++ b/src/HttpHeader.h
@@ -238,6 +238,7 @@ public:
     void putContRange(const HttpHdrContRange * cr);
     void putRange(const HttpHdrRange * range);
     void putSc(HttpHdrSc *sc);
+    void putWarning(const int code, const char *const text); ///< add a Warning header
     void putExt(const char *name, const char *value);
     int getInt(http_hdr_type id) const;
     int64_t getInt64(http_hdr_type id) const;

diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc
index f1ee01225ddbc6898cf9a601b75cfe5a28f3292e..4ee5ee6267053d70407d1b0eb89036726a956a57 100644 (file)
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@ -515,7 +515,7 @@ clientReplyContext::cacheHit(StoreIOBuffer result)
        ) {
         http->logType = LOG_TCP_NEGATIVE_HIT;
         sendMoreData(result);
-    } else if (!Config.onoff.offline && refreshCheckHTTP(e, r) && !http->flags.internal) {
+    } else if (!http->flags.internal && refreshCheckHTTP(e, r)) {
         debugs(88, 5, "clientCacheHit: in refreshCheck() block");
         /*
          * We hold a stale copy; it needs to be validated
@@ -1306,6 +1306,13 @@ clientReplyContext::buildReplyHeader()
         }
     }
 
+    // add Warnings required by RFC 2616 if serving a stale hit
+    if (http->request->flags.stale_if_hit && logTypeIsATcpHit(http->logType)) {
+        hdr->putWarning(110, "Response is stale");
+        if (http->request->flags.need_validation)
+            hdr->putWarning(111, "Revalidation failed");
+    }
+
     /* Filter unproxyable authentication types */
 
     if (http->logType != LOG_TCP_DENIED &&

diff --git a/src/refresh.cc b/src/refresh.cc
index a91153adeecfd36250c2fcce87718ad06dd8cc24..36885035e35fc55eb318f173527605155bfc7e52 100644 (file)
--- a/src/refresh.cc
+++ b/src/refresh.cc
@@ -437,6 +437,20 @@ refreshIsCachable(const StoreEntry * entry)
     return 1;
 }
 
+/// whether reply is stale if it is a hit
+static bool
+refreshIsStaleIfHit(const int reason)
+{
+    switch (reason) {
+    case FRESH_MIN_RULE:
+    case FRESH_LMFACTOR_RULE:
+    case FRESH_EXPIRES:
+        return false;
+    default:
+        return true;
+    }
+}
+
 /* refreshCheck... functions below are protocol-specific wrappers around
  * refreshCheck() function above */
 
@@ -446,7 +460,8 @@ refreshCheckHTTP(const StoreEntry * entry, HttpRequest * request)
     int reason = refreshCheck(entry, request, 0);
     refreshCounts[rcHTTP].total++;
     refreshCounts[rcHTTP].status[reason]++;
-    return (reason < 200) ? 0 : 1;
+    request->flags.stale_if_hit = refreshIsStaleIfHit(reason);
+    return (Config.onoff.offline || reason < 200) ? 0 : 1;
 }
 
 int

diff --git a/src/structs.h b/src/structs.h
index 9351606c79b565235a78c5c54fadd57344225ce2..f8aa8c5b76db1c92de6801fa806afa935fd1ea02 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),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) {
+    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),stale_if_hit(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
@@ -1030,6 +1030,7 @@ unsigned int proxying:
     unsigned int redirected:1;
     unsigned int need_validation:1;
     unsigned int fail_on_validation_err:1; ///< whether we should fail if validation fails
+    unsigned int stale_if_hit:1; ///< reply is stale if it is a hit
 #if USE_HTTP_VIOLATIONS
     unsigned int nocache_hack:1;       /* for changing/ignoring no-cache requests */
 #endif