On Sun, Mar 10, 2013 at 1:55 AM, Graham Leggett <minf...@sharp.fm> wrote: > On 04 Mar 2013, at 8:22 PM, ylavic dev <ylavic....@gmail.com> wrote: > >> For what I understand, mod_cache is allowed to serve its cached entity >> (though without the specified header(s)). > > I read this through again, this time having slept properly.
Thank you for your enlightened consideration. > The way I read the spec, "the specified field-name(s) MUST NOT be sent in the > response to a subsequent request without successful revalidation with the > origin server". What this means is that if the specified field names are > found to be present in the cached response, then the origin server needs to > be given the opportunity to update these fields through a conditional > request. In the current cache code, we return 0 meaning "this is stale, > revalidate", and a conditional request is sent to the origin. We hope the > origin sends "304 Not Modified", with updated headers corresponding to the > fields. Ok, I see your point, and this is surely the right reading of the rfc, but there is necessarily a difference between no-cache and no-cache="<header(s)>", particularly with the handling of that (stale) header(s). For what I understand now, if the origin does not send (one of) the header(s) in its 304 response, the stale header(s) "MUST NOT" be served. So mod_cache should never send these stale headers to the client, and either do not cache them at all, or strip them before overlaping the 304's ones. The actual code does not comply with this requirement since it overlaps the stale headers with the origin ones, hence the no-cache headers will still be there if there are not specified in the 304 response. > If we were to follow this patch, it means that the first time we hit the URL, > the client sees the private/no-cache fields, but every cached response after > will be treated as fresh with the field missing. This breaks caching. Indeed this patch is broken, but I can modify it to comply with my comment above, meaning (at first glance) that the treatment should be in cache_accept_headers(). Should I propose the new patch or my understanding is definitively broken ? Regards, Yann.
