From: Remi Tricot-Le Breton Date: Thu, 3 Dec 2020 17:19:29 +0000 (+0100) Subject: MINOR: cache: Do not store stale entry X-Git-Tag: v2.4-dev3~101 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=795e1412b00c76c3a42effd9158b5d8b2f30100d;p=thirdparty%2Fhaproxy.git 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 1abd92439c..ccbead7c12 100644 --- 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 420a009ef3..cc446507e7 100644 --- 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; }