BUG/MINOR: cache: do not remove the Age header from the forwarded response

When a response carrying an Age header is stored in the cache, the header
is removed from the response itself, so the client which triggered the
storage receives the object without its Age header, as if it had just
been produced by the origin server, while it may have been sitting in an
upstream cache for a long time. This client may then keep it fresh for
longer than allowed. Worse, when the storage finally fails after this
point (headers too large, unknown content-encoding with Vary, no room in
the cache...), the header is lost although nothing was stored. Copies
delivered from the cache are not affected, they carry a freshly computed
Age header.

The Age header was removed with http_remove_header() from the live
response, just so that the headers copied into the cache entry do not
contain it, since the applet adds its own when delivering the object.
Instead of altering the response, the Age headers are now simply skipped
when the headers are copied into the entry. The ETag offset is computed
from the copied data so it remains correct. This also removes all the Age
headers from the stored copy and not only the first one, so a copy is now
always delivered with a single Age header.

Thi patch fix may be backported to all supported versions. In versions
before 3.5, the removal is done unconditionally (no hints-only mode) but
the fix is the same.

diff --git a/src/cache.c b/src/cache.c
index 9000eb98ddec8f1aedf188d61afa7659a1114aa4..7248fa1f66752dd097d5271988493a53be6fedf5 100644 (file)
--- a/src/cache.c
+++ b/src/cache.c
@@ -1825,12 +1825,6 @@ enum act_return http_action_store_cache(struct act_rule *rule, struct proxy *px,
                }
                else
                        goto out;
-               /* The cache serves its copies with a freshly computed Age, so
-                * the stored response's Age is stripped. In hints-only mode the
-                * response is only passed through, never served, so leave it.
-                */
-               if (!(cache->flags & CACHE_CF_EARLY_HINTS_ONLY))
-                       http_remove_header(htx, &ctx);
        }
 
        /* Build a last-modified time that will be stored in the cache_entry and
@@ -1847,6 +1841,17 @@ enum act_return http_action_store_cache(struct act_rule *rule, struct proxy *px,
                        struct htx_blk *blk = htx_get_blk(htx, pos);
                        enum htx_blk_type type = htx_get_blk_type(blk);
                        uint32_t sz = htx_get_blksz(blk);
+                       struct ist header_name = IST_NULL;
+
+                       /* The cache serves its copies with a freshly computed
+                        * Age, so the Age headers are not stored. They are left
+                        * in the response which is forwarded as-is.
+                        */
+                       if (type == HTX_BLK_HDR) {
+                               header_name = htx_get_blk_name(htx, blk);
+                               if (isteq(header_name, ist("age")))
+                                       continue;
+                       }
 
                        hdrs_len += sizeof(*blk) + sz;
                        chunk_memcat(&trash, (char *)&blk->info, sizeof(blk->info));
@@ -1857,7 +1862,6 @@ enum act_return http_action_store_cache(struct act_rule *rule, struct proxy *px,
                         * future conditional requests to be able to perform ETag
                         * comparisons. */
                        if (type == HTX_BLK_HDR) {
-                               struct ist header_name = htx_get_blk_name(htx, blk);
                                if (isteq(header_name, ist("etag"))) {
                                        object->etag_length = sz - istlen(header_name);
                                        object->etag_offset = sizeof(struct cache_entry) +