From 879debeecb93202c983f25f5f1d765e74d77faa5 Mon Sep 17 00:00:00 2001
From: Remi Tricot-Le Breton <rlebreton@haproxy.com>
Date: Tue, 21 Feb 2023 11:47:17 +0100
Subject: [PATCH] BUG/MINOR: cache: Cache response even if request has
 "no-cache" directive

Since commit cc9bf2e5f "MEDIUM: cache: Change caching conditions"
responses that do not have an explicit expiration time are not cached
anymore. But this mechanism wrongly used the TX_CACHE_IGNORE flag
instead of the TX_CACHEABLE one. The effect this had is that a cacheable
response that corresponded to a request having a "Cache-Control:
no-cache" for instance would not be cached.
Contrary to what was said in the other commit message, the "checkcache"
option should not be impacted by the use of the TX_CACHEABLE flag
instead of the TX_CACHE_IGNORE one. The response is indeed considered as
not cacheable if it has no expiration time, regardless of the presence
of a cookie in the response.

This should fix GitHub issue #2048.
This patch can be backported up to branch 2.4.
---
 include/haproxy/http_ana-t.h      |  2 +-
 reg-tests/cache/caching_rules.vtc | 32 +++++++++++++++++++++++++++++++
 src/cache.c                       |  2 +-
 src/http_ana.c                    |  2 +-
 4 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/include/haproxy/http_ana-t.h b/include/haproxy/http_ana-t.h
index 4db72fd75..3910a4d08 100644
--- a/include/haproxy/http_ana-t.h
+++ b/include/haproxy/http_ana-t.h
@@ -61,7 +61,7 @@
 /* cacheability management, bits values 0x1000 to 0x3000 (0-3 shift 12) */
 #define TX_CACHEABLE	0x00001000	/* at least part of the response is cacheable */
 #define TX_CACHE_COOK	0x00002000	/* a cookie in the response is cacheable */
-#define TX_CACHE_IGNORE 0x00004000	/* do not retrieve object from cache, or avoid caching response */
+#define TX_CACHE_IGNORE 0x00004000	/* do not retrieve object from cache */
 #define TX_CACHE_SHIFT	12		/* bit shift */
 
 #define TX_CON_WANT_TUN 0x00008000	/* Will be a tunnel (CONNECT or 101-Switching-Protocol) */
diff --git a/reg-tests/cache/caching_rules.vtc b/reg-tests/cache/caching_rules.vtc
index b1ea7f979..61274b4b5 100644
--- a/reg-tests/cache/caching_rules.vtc
+++ b/reg-tests/cache/caching_rules.vtc
@@ -67,6 +67,18 @@ server s1 {
     txresp -hdr "Cache-Control: max-age=500" \
         -hdr "Age: 100" -bodylen 140
 
+
+    # "Control-Cache: no-cache" on client request but still stored in cache
+    rxreq
+    expect req.url == "/nocache"
+    txresp -hdr "Cache-Control: max-age=500" \
+        -hdr "Age: 100" -bodylen 140
+
+    rxreq
+    expect req.url == "/nocache"
+    txresp -hdr "Cache-Control: max-age=500" \
+        -hdr "Age: 100" -bodylen 140
+
 } -start
 
 server s2 {
@@ -221,4 +233,24 @@ client c1 -connect ${h1_fe_sock} {
         expect resp.bodylen == 140
         expect resp.http.X-Cache-Hit == 1
 
+        # Cache-Control: no-cache
+        txreq -url "/nocache" -hdr "Cache-Control: no-cache"
+        rxresp
+        expect resp.status == 200
+        expect resp.bodylen == 140
+        expect resp.http.X-Cache-Hit == 0
+
+        txreq -url "/nocache" -hdr "Cache-Control: no-cache"
+        rxresp
+        expect resp.status == 200
+        expect resp.bodylen == 140
+        expect resp.http.X-Cache-Hit == 0
+
+        txreq -url "/nocache"
+        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 2b8a78231..c7a231bbe 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -1101,7 +1101,7 @@ enum act_return http_action_store_cache(struct act_rule *rule, struct proxy *px,
 
 	http_check_response_for_cacheability(s, &s->res);
 
-	if (!(txn->flags & TX_CACHEABLE) || !(txn->flags & TX_CACHE_COOK) || (txn->flags & TX_CACHE_IGNORE))
+	if (!(txn->flags & TX_CACHEABLE) || !(txn->flags & TX_CACHE_COOK))
 		goto out;
 
 	shctx_lock(shctx);
diff --git a/src/http_ana.c b/src/http_ana.c
index cb7cb27b6..ba587df6d 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -3772,7 +3772,7 @@ void http_check_response_for_cacheability(struct stream *s, struct channel *res)
 	/* We won't store an entry that has neither a cache validator nor an
 	 * explicit expiration time, as suggested in RFC 7234#3. */
 	if (!has_freshness_info && !has_validator)
-		txn->flags |= TX_CACHE_IGNORE;
+		txn->flags &= ~TX_CACHEABLE;
 }
 
 /*