MINOR: cache: Do not store stale entry

When a response has an Age header (filled in by another cache on the
message's path) that is greater than its defined maximum age (extracted
either from cache-control directives or an expires header), it is
already stale and should not be cached.

diff --git a/reg-tests/cache/caching_rules.vtc b/reg-tests/cache/caching_rules.vtc
index 1abd92439cb99404282003efc4de15f93fa212f2..ccbead7c126df0d2dce4d4143dbc588f71fac48f 100644 (file)
--- a/reg-tests/cache/caching_rules.vtc
+++ b/reg-tests/cache/caching_rules.vtc
@@ -1,5 +1,6 @@
 varnishtest "Caching rules test"
-# A respnse will not be cached unless it has an explicit age (Cache-Control max-age of s-maxage, Expires, Last-Modified headers, or ETag)
+# A response will not be cached unless it has an explicit age (Cache-Control max-age of s-maxage, Expires) or a validator (Last-Modified, or ETag)
+# A response will not be cached either if it has an Age header that is either invalid (should be an integer) or greater than its max age.
 
 #REQUIRE_VERSION=1.9
 
@@ -35,6 +36,37 @@ server s1 {
     expect req.url == "/uncacheable"
     txresp \
         -bodylen 210
+
+    # Age response header checks
+
+    # Invalid age
+    rxreq
+    expect req.url == "/invalid_age"
+    txresp -hdr "Cache-Control: max-age=5" \
+        -hdr "Age: abc" -bodylen 120
+
+    rxreq
+    expect req.url == "/invalid_age"
+    txresp -hdr "Cache-Control: max-age=5" \
+        -hdr "Age: abc" -bodylen 120
+
+    # Old age (greater than max age)
+    rxreq
+    expect req.url == "/old_age"
+    txresp -hdr "Cache-Control: max-age=5" \
+        -hdr "Age: 10" -bodylen 130
+
+    rxreq
+    expect req.url == "/old_age"
+    txresp -hdr "Cache-Control: max-age=5" \
+        -hdr "Age: 10" -bodylen 130
+
+    # Good age
+    rxreq
+    expect req.url == "/good_age"
+    txresp -hdr "Cache-Control: max-age=500" \
+        -hdr "Age: 100" -bodylen 140
+
 } -start
 
 server s2 {
@@ -147,4 +179,41 @@ client c1 -connect ${h1_fe_sock} {
         expect resp.bodylen == 210
         expect resp.http.X-Cache-Hit == 0
 
+        # Age header tests
+        txreq -url "/invalid_age"
+        rxresp
+        expect resp.status == 200
+        expect resp.bodylen == 120
+        expect resp.http.X-Cache-Hit == 0
+
+        txreq -url "/invalid_age"
+        rxresp
+        expect resp.status == 200
+        expect resp.bodylen == 120
+        expect resp.http.X-Cache-Hit == 0
+
+        txreq -url "/old_age"
+        rxresp
+        expect resp.status == 200
+        expect resp.bodylen == 130
+        expect resp.http.X-Cache-Hit == 0
+
+        txreq -url "/old_age"
+        rxresp
+        expect resp.status == 200
+        expect resp.bodylen == 130
+        expect resp.http.X-Cache-Hit == 0
+
+        txreq -url "/good_age"
+        rxresp
+        expect resp.status == 200
+        expect resp.bodylen == 140
+        expect resp.http.X-Cache-Hit == 0
+
+        txreq -url "/good_age"
+        rxresp
+        expect resp.status == 200
+        expect resp.bodylen == 140
+        expect resp.http.X-Cache-Hit == 1
+
 } -run

diff --git a/src/cache.c b/src/cache.c
index 420a009ef3fe6425dd4b31fd3d619dbb2d92ecb5..cc446507e7da5296bd55c4fd1354d7845043e52c 100644 (file)
--- a/src/cache.c
+++ b/src/cache.c
@@ -535,6 +535,10 @@ char *directive_value(const char *sample, int slen, const char *word, int wlen)
 
 /*
  * Return the maxage in seconds of an HTTP response.
+ * The returned value will always take the cache's configuration into account
+ * (cache->maxage) but the actual max age of the response will be set in the
+ * true_maxage parameter. It will be used to determine if a response is already
+ * stale or not.
  * Compute the maxage using either:
  *  - the assigned max-age of the cache
  *  - the s-maxage directive
@@ -543,7 +547,7 @@ char *directive_value(const char *sample, int slen, const char *word, int wlen)
  *  - the default-max-age of the cache
  *
  */
-int http_calc_maxage(struct stream *s, struct cache *cache)
+int http_calc_maxage(struct stream *s, struct cache *cache, int *true_maxage)
 {
        struct htx *htx = htxbuf(&s->res.buf);
        struct http_hdr_ctx ctx = { .blk = NULL };
@@ -599,14 +603,23 @@ int http_calc_maxage(struct stream *s, struct cache *cache)
        }
 
 
-       if (smaxage > 0)
+       if (smaxage > 0) {
+               if (true_maxage)
+                       *true_maxage = smaxage;
                return MIN(smaxage, cache->maxage);
+       }
 
-       if (maxage > 0)
+       if (maxage > 0) {
+               if (true_maxage)
+                       *true_maxage = maxage;
                return MIN(maxage, cache->maxage);
+       }
 
-       if (expires >= 0)
+       if (expires >= 0) {
+               if (true_maxage)
+                       *true_maxage = expires;
                return MIN(expires, cache->maxage);
+       }
 
        return cache->maxage;
 
@@ -698,6 +711,8 @@ enum act_return http_action_store_cache(struct act_rule *rule, struct proxy *px,
                                        struct session *sess, struct stream *s, int flags)
 {
        long long hdr_age;
+       int effective_maxage = 0;
+       int true_maxage = 0;
        struct http_txn *txn = s->txn;
        struct http_msg *msg = &txn->rsp;
        struct filter *filter;
@@ -828,12 +843,21 @@ enum act_return http_action_store_cache(struct act_rule *rule, struct proxy *px,
        first->len = sizeof(struct cache_entry);
        first->last_append = NULL;
 
+       /* Determine the entry's maximum age (taking into account the cache's
+        * configuration) as well as the response's explicit max age (extracted
+        * from cache-control directives or the expires header). */
+       effective_maxage = http_calc_maxage(s, cconf->c.cache, &true_maxage);
+
        ctx.blk = NULL;
        if (http_find_header(htx, ist("Age"), &ctx, 0)) {
                if (!strl2llrc(ctx.value.ptr, ctx.value.len, &hdr_age) && hdr_age > 0) {
                        if (unlikely(hdr_age > CACHE_ENTRY_MAX_AGE))
                                hdr_age = CACHE_ENTRY_MAX_AGE;
+                       /* A response with an Age value greater than its
+                        * announced max age is stale and should not be stored. */
                        object->age = hdr_age;
+                       if (unlikely(object->age > true_maxage))
+                               goto out;
                }
                http_remove_header(htx, &ctx);
        }
@@ -894,8 +918,7 @@ enum act_return http_action_store_cache(struct act_rule *rule, struct proxy *px,
 
                /* store latest value and expiration time */
                object->latest_validation = now.tv_sec;
-               object->expire = now.tv_sec + http_calc_maxage(s, cache);
-
+               object->expire = now.tv_sec + effective_maxage;
                return ACT_RET_CONT;
        }