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? Regards Rüdiger
