[PATCH 2 of 2] Cache: send conditional requests only for cached 200 OK responses

[Piotr Sikora]

Hey Maxim,

> The question is: how many other status codes should be considered?
> I _think_ that 200 and 206 is good enough list, but I'm not sure.

Well, even if we miss a status code or two (and I don't think that we
do), then the worst thing that's going to happen is, to quote your
argument from the other thread, "suboptimal operation, nothing really
bad can happen", which is much, much better than the current
situation.

> Here is the first problem in the upstream server: it returns an
> error with the Last-Modified.  This is what nginx avoids for a
> reason.

As far as I can tell, that's neither wrong nor forbidden by RFC.

> Here you intentionally cache a response which is not
> cacheable as per HTTP specification.

...and yet nginx tries to revalidate it? This is exactly what I'm
trying to prevent with this patch.

Also, quoting your previous response, "that's up to a configuration"
to make it cacheable or not.

And last, but not least, 503 was just an example, it could very well
be 404 Not Found (cacheable by default per RFC), which doesn't change
anything in the described scenario, so it's a moot point.

> Here is another problem in the upstream server: it returns 304
> incorrectly.  Strict Last-Modified matching is here for a reason
> and, among other benefits, allows to mitigate such problems.

Sorry, but you're wrong, there is nothing incorrect about it. Strict
matching is a made up requirement and both RFCs agree on that.

RFC2616 says:

   The If-Modified-Since request-header field is used with a method to
   make it conditional: if the requested variant has not been modified
   since the time specified in this field, an entity will not be
   returned from the server; instead, a 304 (not modified) response will
   be returned without any message-body.

RFC7232 says:

   The "If-Modified-Since" header field makes a GET or HEAD request
   method conditional on the selected representation's modification date
   being more recent than the date provided in the field-value.

It's worth noting that Apache, IIS, Varnish, ATS, Google's, Twitter's
and Akamai's web servers all behave this way (i.e. using "before", not
"exact" logic) and, as far as I can tell, nginx is the only exception
to this rule.

> This is not true.  The upstream server intentionally does at least
> two perfectly stupid things, and additionally you force nginx to
> cache an uncacheable response.  And all of this combined causes
> the problem, not the fact that nginx uses conditional requests.

I disagree. Responses with non-200/206 status codes cannot be
revalidated with 304, so nginx expecting that to work and/or using 304
generated from 200/206 status code to revalidate responses cached with
non-200/206 status is just wrong.

> It's not broken.  Trying to convince me that it's broken will not
> work, for sure.  You may have better luck convincing me that the
> change will help to mitigate known/seen-in-the-wild problems
> without affecting other uses.

I might have better luck doing that, but it doesn't change the fact
that the current behavior is broken.

But yeah, if that's going to make you feel better, we had at least a
few cases where 404 responses were being revalidated by 304 generated
from 200.

Best regards,
Piotr Sikora