[PATCH 1 of 2] Cache: don't update cache if revalidated response is not cacheable

[Maxim Dounin]

Hello!

On Sat, Dec 20, 2014 at 04:05:13PM -0800, Piotr Sikora wrote:

> Hey Maxim,
> 
> > The cacheability of the response was already signalled previously.
> > And a 304 response can't really change it.
> 
> Fine, if you want to get technical, then yes, cacheability of the
> response was signaled previously, however the 304 response can still
> signal that the validity shouldn't be extended (i.e. that the cache
> shouldn't be updated), which is what we're really talking about here.
> 
> > Consider, for example,
> > that there were no validation request at all - and it happens some
> > time later.  So the only thing that really changes is validity.
> 
> I don't follow your logic here, what exactly are you trying to say?

I'm trying to explain the current behaviour - which, as you said, 
"is strange".

> > That's in line with the RFC2616: it allows 304 to supply new
> > Cache-Control and Expires headers if they are different from what
> > was previously sent.  So we use validity from the 304 response,
> > then from the cached response, then from proxy_cache_valid.
> >
> > What is probably missing here is not using valid_sec if
> > u->cacheable is set.
> >
> > (...)
> >
> > I agree, see above.
> 
> Which is basically what my patch adds, isn't it?

Maybe, but the result looks wrong to me.

> > The problem is consistency of the behaviour in equivalent cases.
> >
> > E.g., consider the following:
> >
> > - A response with Set-Cookie cached with
> >   "proxy_ignore_headers Set-Cookie" in the configuration.  The
> >   response contains "Cache-Control: max-age=100".
> >
> > - Now proxy_ignore_headers is removed from the configuration.  We
> >   do a request with If-Modified-Since, 304 is returned:
> >
> >   (a) If the 304 contains "Cache-Control: max-age=100", the
> >       response will be cached for 100 more seconds.
> >
> >   (b) If the 304 does not contain "Cache-Control: max-age=100",
> >       the response won't be updated.
> >
> > On the other hand, (a) and (b) are equivalent per RFC2616.
> 
> However, according to RFC7232, (a) and (b) are different:
> 
>    The server generating a 304 response MUST generate any of the
>    following header fields that would have been sent in a 200 (OK)
>    response to the same request: Cache-Control, Content-Location, Date,
>    ETag, Expires, and Vary.

As you can see, RFC7232 doesn't really say they are different - 
instead, it requires servers to always add relevant headers.  On 
the other hand, if you'll look at the RFC7234 4.3.4 as linked from the 
paragraph you quoted, you'll find that after merging headers these 
two cases are still equivalent.

But even if you assume they are not equivalent (that is, (b) final 
response shouldn't have "Cache-Control: max-age=100") in case of (b) 
validity should be extended as if there are no Cache-Control 
headers in the response, i.e., per proxy_cache_valid.  This is not what 
happens with your patch, so the problem is here anyway.

-- 
Maxim Dounin
http://nginx.org/