Re: Don't add redundant Cache-Control

From: Krzysztof Oledzki <ole#ans.pl>
Date: Sat, 29 Sep 2007 17:35:23 +0200 (CEST)

On Sat, 29 Sep 2007, Willy Tarreau wrote:

> On Sun, Sep 23, 2007 at 05:24:05PM +0200, Krzysztof Oledzki wrote:

>>
>>
>> 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)
>
> There is no "proper case" in HTTP, as all headers are case insensitive.
> I did the same mistake in the past (and used "memcmp" to check for cache).

Yes, I know but in all examples it is 'Cache-Control' so since it does not cost anything I think it is better to send it this way.

>> - accpet no-cache as ~TX_CACHEABLE & ~TX_CACHE_COOK, oryginal haproxy
>> code only accepted 'no-cache="set-cookie' as ~TX_CACHE_COOK.

>
> To be honnest, I have to check the original code an intended behaviour.
> The reason was to block responses with a cacheable cookie, and return a
> "502 Bad Gateway" instead.

I did not touch 'no-cache="set-cookie', only added no-cache as AFAIK it means "do not cache at all". Please correct me if I am wrong.

>> 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=/

>
> Looks clean at first glance. I should merge it once I understand all
> the implications on the cookie cacheability checks.

OK. Thank you.

Best regards,

                                         Krzysztof Olędzki Received on 2007/09/29 17:35

This archive was generated by hypermail 2.2.0 : 2007/11/04 19:21 CET