Don't add redundant Cache-Control

From: Krzysztof Oledzki <ole#ans.pl>
Date: Sun, 23 Sep 2007 17:24:05 +0200 (CEST)

On Sat, 22 Sep 2007, Willy Tarreau wrote:

> Hi Krzysztof,
>
> On Sat, Sep 22, 2007 at 03:09:45AM +0200, Krzysztof Oledzki wrote:
>> 2. haproxy-cache-control-once.patch.gz
>>
>> This one is little more complicated.
>>
>> 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
>> Content-Length: 45102
>> Content-Type: text/html
>> Content-Location: http://www.newsweek.pl/index.htm
>> Last-Modified: Sat, 22 Sep 2007 00:15:08 GMT
>> Accept-Ranges: bytes
>> ETag: "63a4aa2adfcc71:dde"
>> Server: Microsoft-IIS/6.0
>> X-Powered-By: ASP.NET
>> Date: Sat, 22 Sep 2007 00:18:34 GMT
>> Connection: close
>> Set-Cookie: SERVERID=s6; path=/
>> Cache-control: private
>
> That's strange for this one, I had in mind that it would only add it if
> there was not already a "Cache-Control: no-cache". Or maybe I'm confusing
> with the "check_cache" option. I don't remember.
>
>> or:
>>
>> Connection: close
>> Date: Sat, 22 Sep 2007 01:03:24 GMT
>> Server: Microsoft-IIS/6.0
>> X-Powered-By: ASP.NET
>> Content-Length: 7458
>> Content-Type: text/html
>> Set-Cookie: ASPSESSIONIDCSRCTSSB=HCCBGGACGBHDHMMKIOILPHNG; path=/
>> Cache-control: private
>> Set-Cookie: SERVERID=s5; path=/
>> Cache-control: private
>
> Oh yes, seen like thatn it's plain stupid :-)
>
>> 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.
>
> OK, thanks very much. I'll analyze it all to ensure that it does not change
> any known corner case I may have in mind, but your description seems right.

OK, attached a new version with two additional fixes:

• s/Cache-control/Cache-Control/ (proper case)
• accpet no-cache as ~TX_CACHEABLE & ~TX_CACHE_COOK, oryginal haproxy code only accepted 'no-cache="set-cookie' as ~TX_CACHE_COOK.

So now:

Cache-Control: no-cache

Content-Length: 44972
Content-Type: text/html
Content-Location: (...)

Last-Modified: Sun, 23 Sep 2007 15:20:16 GMT Accept-Ranges: bytes
ETag: "6086cd3ef5fdc71:dde"
Server: Microsoft-IIS/6.0
X-Powered-By: ASP.NET
Date: Sun, 23 Sep 2007 15:21:48 GMT
Connection: close
Set-Cookie: SERVERID=s6; path=/

Best regards,

                                 Krzysztof Olêdzki

Received on 2007/09/23 17:24