httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <>
Subject Re: [PATCH] mod_cache 304 on HEAD (bug 41230)
Date Mon, 16 Apr 2007 20:58:22 GMT

On 04/13/2007 05:31 PM, Niklas Edmundsson wrote:
> On Wed, 11 Apr 2007, Niklas Edmundsson wrote:
>> Looking a bit further, I think that something like this would actually
>> be enough:
> <snip, included as an attachment>
> I have now tested this patch, and it seems to solve the problem. This is
> on httpd-2.2.4 + patch for PR41475 + our mod_disk_cache patches.
> Without the patch a HEAD on a cached expired object that isn't modified
> will unconditionally return 304 and furthermore cause the cached object
> to be deleted. We believe that this is the explanation to why it has
> been so hard to track down this bug - it only bites one user and that
> user usually has no clue on what happens, and even if we try to
> reproduce it immediately afterwards it won't trigger.
> With the patch stuff works like expected:
> - A HEAD on a cached expired object that isn't modified will update
>   the cache header and return the proper return code, it follows the
>   same code path as other requests on expired unmodified objects.
> - A HEAD on a cached expired object that IS modified will remove the
>   object from cache and then decline the opportunity to cache the
>   object.

Are you really sure that it gets deleted? cache->provider->remove_entity does
not really remove the object from the cache. Only cache->provider->remove_url
does this.
I consider the CACHE_SAVE filter already as hard to read (not your fault by any means),
but from my point of view your patch does increase this (specificly I think about
the rv = !OK line. I know that a similar trick is done some lines above, but I don't
like that one either).
Looking at the problem I noticed a related problem already mentioned in a FIXME comment:
It can happen that the headers of a 304 response from the backend cause the response to be
uncacheable (e.g. Cache-Control: no-store).
Currently this leads to a wrong response (304) if the original request was unconditional
(just like in your HEAD case and your HEAD case will also fail here, even after your patch).
My first question in this situation is: What is the correct thing to do here?
Generate the response from the cache (of course with the updated headers from the 304
backend response) and delete the cache entry afterwards?



View raw message