Author: Alex Rousskov <rousskov@measurement-factory.com>

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 555b082390be525abb65211c1a971d53346ed33a..c317b05dfe0868fc40840fee34a8874486c1097e 100644 (file)
--- a/src/HttpHeader.cc
+++ b/src/HttpHeader.cc
@@ -1188,6 +1188,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 7bc7fff2096ccc7838b843f633dc05256af8607a..76a5b3b70f5db8ae43c2200d6b25388231378506 100644 (file)
--- a/src/HttpHeader.h
+++ b/src/HttpHeader.h
@@ -239,6 +239,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 085d5e922633c0af6b79a33872103042add2bdff..8223c103582a1ecf1f3422f142c6ceb7d3b7b48c 100644 (file)
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@ -516,7 +516,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
@@ -1300,6 +1300,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 34baae5a18672c0a4b6e2f8046d7ab7d38956199..dc39714f72a6833c0833b480009ad063b6884ade 100644 (file)
--- a/src/refresh.cc
+++ b/src/refresh.cc
@@ -438,6 +438,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 */
 
@@ -447,7 +461,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 eff141ef410f244cf8212a901768970c7408f448..6e8bbc24e335e5f7b208d12e85db315d540b67bc 100644 (file)
--- a/src/structs.h
+++ b/src/structs.h
@@ -1010,8 +1010,8 @@ 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),destinationIPLookedUp_(0) {
-#if HTTP_VIOLATIONS
+    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),destinationIPLookedUp_(0) 
+{ #if HTTP_VIOLATIONS
         nocache_hack = 0;
 #endif
 #if FOLLOW_X_FORWARDED_FOR
@@ -1033,6 +1033,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 HTTP_VIOLATIONS
     unsigned int nocache_hack:1;       /* for changing/ignoring no-cache requests */
 #endif