BUG/MINOR: cache: A 'max-age=0' cache-control directive can be overriden by a s-maxage
When a s-maxage cache-control directive is present, it overrides any
other max-age or expires value (see section 5.2.2.9 of RFC7234). So if
we have a max-age=0 alongside a strictly positive s-maxage, the response
should be cached.
This bug was raised in GitHub issue #2203.
The fix can be backported to all stable branches.
(cherry picked from commit ca4fd73938f3eeb7bb81bc85d49b3076e2e26178)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 02f47244f3e474ba5a1baf8afffa9ad0b5921235)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 7340985c554b6fd791a5de6c540ffbe08d3207ea)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 49cbae91937f5f4fd478c6859ba5d271afcc698c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/reg-tests/cache/caching_rules.vtc b/reg-tests/cache/caching_rules.vtc
index 0e10d83..10840e1 100644
--- a/reg-tests/cache/caching_rules.vtc
+++ b/reg-tests/cache/caching_rules.vtc
@@ -79,6 +79,30 @@
txresp -hdr "Cache-Control: max-age=500" \
-hdr "Age: 100" -bodylen 140
+
+ # max-age=0
+ rxreq
+ expect req.url == "/maxage_zero"
+ txresp -hdr "Cache-Control: max-age=0" \
+ -bodylen 150
+
+ rxreq
+ expect req.url == "/maxage_zero"
+ txresp -hdr "Cache-Control: max-age=0" \
+ -bodylen 150
+
+ # Overridden null max-age
+ rxreq
+ expect req.url == "/overridden"
+ txresp -hdr "Cache-Control: max-age=1, s-maxage=5" \
+ -bodylen 160
+
+ rxreq
+ expect req.url == "/overridden_null_maxage"
+ txresp -hdr "Cache-Control: max-age=0, s-maxage=5" \
+ -bodylen 190
+
+
} -start
server s2 {
@@ -253,5 +277,45 @@
expect resp.bodylen == 140
expect resp.http.X-Cache-Hit == 1
+ # max-age=0 (control test for the overridden null max-age test below)
+ txreq -url "/maxage_zero"
+ rxresp
+ expect resp.status == 200
+ expect resp.bodylen == 150
+ expect resp.http.X-Cache-Hit == 0
+
+ txreq -url "/maxage_zero"
+ rxresp
+ expect resp.status == 200
+ expect resp.bodylen == 150
+ expect resp.http.X-Cache-Hit == 0
+
+ # Overridden max-age directive
+ txreq -url "/overridden"
+ rxresp
+ expect resp.status == 200
+ expect resp.bodylen == 160
+ expect resp.http.X-Cache-Hit == 0
+
+ txreq -url "/overridden"
+ rxresp
+ expect resp.status == 200
+ expect resp.bodylen == 160
+ expect resp.http.X-Cache-Hit == 1
+
+ txreq -url "/overridden_null_maxage"
+ rxresp
+ expect resp.status == 200
+ expect resp.bodylen == 190
+ expect resp.http.X-Cache-Hit == 0
+
+ # The previous response should have been cached even if it had
+ # a max-age=0 since it also had a positive s-maxage
+ txreq -url "/overridden_null_maxage"
+ rxresp
+ expect resp.status == 200
+ expect resp.bodylen == 190
+ expect resp.http.X-Cache-Hit == 1
+
} -run
diff --git a/src/http_ana.c b/src/http_ana.c
index f629492..f265e93 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -3869,6 +3869,7 @@
struct htx *htx;
int has_freshness_info = 0;
int has_validator = 0;
+ int has_null_maxage = 0;
if (txn->status < 200) {
/* do not try to cache interim responses! */
@@ -3893,10 +3894,16 @@
txn->flags |= TX_CACHEABLE | TX_CACHE_COOK;
continue;
}
+ /* This max-age might be overridden by a s-maxage directive, do
+ * not unset the TX_CACHEABLE yet. */
+ if (isteqi(ctx.value, ist("max-age=0"))) {
+ has_null_maxage = 1;
+ continue;
+ }
+
if (isteqi(ctx.value, ist("private")) ||
isteqi(ctx.value, ist("no-cache")) ||
isteqi(ctx.value, ist("no-store")) ||
- isteqi(ctx.value, ist("max-age=0")) ||
isteqi(ctx.value, ist("s-maxage=0"))) {
txn->flags &= ~TX_CACHEABLE & ~TX_CACHE_COOK;
continue;
@@ -3907,11 +3914,21 @@
continue;
}
- if (istmatchi(ctx.value, ist("s-maxage")) ||
- istmatchi(ctx.value, ist("max-age"))) {
+ if (istmatchi(ctx.value, ist("s-maxage"))) {
has_freshness_info = 1;
+ has_null_maxage = 0; /* The null max-age is overridden, ignore it */
continue;
}
+ if (istmatchi(ctx.value, ist("max-age"))) {
+ has_freshness_info = 1;
+ continue;
+ }
+ }
+
+ /* We had a 'max-age=0' directive but no extra s-maxage, do not cache
+ * the response. */
+ if (has_null_maxage) {
+ txn->flags &= ~TX_CACHEABLE & ~TX_CACHE_COOK;
}
/* If no freshness information could be found in Cache-Control values,