[MEDIUM] do not add a cache-control: header when on non-cacheable responses

I noticed that haproxy, with "cookie (...) nocache" option, always adds
"Cache-control: private" at the end of a header list received from this
server:

Cache-Control: no-cache
(...)
Set-Cookie: SERVERID=s6; path=/
Cache-control: private

or:

Set-Cookie: ASPSESSIONIDCSRCTSSB=HCCBGGACGBHDHMMKIOILPHNG; path=/
Cache-control: private
Set-Cookie: SERVERID=s5; path=/
Cache-control: private

It may be just redundant (two "Cache-control: private"), but sometimes it
may be quite confused as we may end with two different, more and less
restricted directions (no-cache & private) and even quite conflicting
directions (eg. public & private):

So, I added and rearranged a code, so now haproxy adds a "Cache-control:
private" header only when there is no the same (private) or more
restrictive (no-cache) one. It was done in three steps:

1. Use check_response_for_cacheability to check if response is
not cacheable. I simply moved this call before http_header_add_tail2.

2. Use TX_CACHEABLE (not TX_CACHE_COOK - apache <= 1.3.26) to check if we
need to add a Cache-control header. If we add it, clear TX_CACHEABLE and
TX_CACHE_COOK.

3. Check cacheability not only with PR_O_CHK_CACHE but also with
PR_O_COOK_NOC, so:

-                           unlikely(t->be->options & PR_O_CHK_CACHE))
+                           (t->be->options & (PR_O_CHK_CACHE|PR_O_COOK_NOC)))
                                txn->flags |= TX_CACHEABLE | TX_CACHE_COOK;

I removed this unlikely since I believe that now it is not so unlikely.

The patch is definitely not perfect, proxy should probably also remove
"Cache-control: public". Unfortunately, I do not know the code good enough
to do in myself, yet. ;)

Anyway, I think that even now, it should be very useful.

diff --git a/src/proto_http.c b/src/proto_http.c
index 9fe10392aca8d52964996e82e37da81105117c0d..3cd97fe823750ebe7f725c233af5826b117a77c7 100644 (file)
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -2866,7 +2866,7 @@ int process_srv(struct session *t)
                         *    Cache-Control or Expires header fields."
                         */
                        if (likely(txn->meth != HTTP_METH_POST) &&
-                           unlikely(t->be->options & PR_O_CHK_CACHE))
+                           (t->be->options & (PR_O_CHK_CACHE|PR_O_COOK_NOC)))
                                txn->flags |= TX_CACHEABLE | TX_CACHE_COOK;
                        break;
                default:
@@ -3003,8 +3003,15 @@ int process_srv(struct session *t)
                 */
                manage_server_side_cookies(t, rep);
 
+
                /*
-                * 5: add server cookie in the response if needed
+                * 5: check for cache-control or pragma headers.
+                */
+               check_response_for_cacheability(t, rep);
+
+
+               /*
+                * 6: add server cookie in the response if needed
                 */
                if ((t->srv) && !(t->flags & SN_DIRECT) && (t->be->options & PR_O_COOK_INS) &&
                    (!(t->be->options & PR_O_COOK_POST) || (txn->meth == HTTP_METH_POST))) {
@@ -3029,7 +3036,10 @@ int process_srv(struct session *t)
                         * Some caches understand the correct form: 'no-cache="set-cookie"', but
                         * others don't (eg: apache <= 1.3.26). So we use 'private' instead.
                         */
-                       if (t->be->options & PR_O_COOK_NOC) {
+                       if ((t->be->options & PR_O_COOK_NOC) && (txn->flags & TX_CACHEABLE)) {
+
+                               txn->flags &= ~TX_CACHEABLE & ~TX_CACHE_COOK;
+
                                if (unlikely(http_header_add_tail2(rep, &txn->rsp, &txn->hdr_idx,
                                                                   "Cache-control: private", 22)) < 0)
                                        goto return_bad_resp;
@@ -3037,12 +3047,6 @@ int process_srv(struct session *t)
                }
 
 
-               /*
-                * 6: check for cache-control or pragma headers.
-                */
-               check_response_for_cacheability(t, rep);
-
-
                /*
                 * 7: check if result will be cacheable with a cookie.
                 * We'll block the response if security checks have caught