Hello!

On Thu, Oct 07, 2021 at 06:37:11PM +0200, Jiří Setnička wrote:

> I use nginx as a proxy with enabled cache. I encountered strange
> behavior on revalidate.
>
> When upstream does not return any caching headers it is ok - file is
> cached with default cachetime and on revalidate the r->cache->valid_sec
> is updated to now + default cachetime.
>
> Also when upstream consistently returns caching headers it is still ok -
> file is cached according to caching headers and on revalidate the
> r->cache->valid_sec is updated by value from 304 response caching headers.
>
> Problem is when upstream previously returned absolute caching headers on
> 200 response (so the file is cached according to these headers and these
> headers are saved into cache file on disk) but later it changed its
> behavior and on 304 response it does not return any caching headers.
> In such case, I would expect that now + default cachetime would be used
> as the new r->cache->valid_sec, but old absolute time is used instead
> and this yields in revalidate on each request.

Per RFC 2616
(https://datatracker.ietf.org/doc/html/rfc2616#section-10.3.5):

: The response MUST include the following header fields:
:
: ...
:
: - Expires, Cache-Control, and/or Vary, if the field-value might
: differ from that sent in any previous response for the same
: variant

That is, as long as Expires is not present in the 304 response,
per RFC 2616 it is correct to assume that the Expires in the
original response still applies. This is what nginx currently
does: if there are no Expires/Cache-Control in the 304 response,
it uses Expires/Cache-Control from the original response, and
there are none either, tries to use proxy_cache_valid. With this
behaviour it is basically not possible to "remove" cache control
headers, it is only possible to redefine them to something new
explicitly returned.

In RFC 7232 this was changed to
(https://datatracker.ietf.org/doc/html/rfc7232#section-4.1):

: 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.

This implies that the original response Expires and Cache-Control
headers should not be used at all. With this approach, Expires
and Cache-Control headers can be safely removed (in theory).

Switching to RFC 7232 logic might be considered, though this
should be done with care, as it can affect various upstream
servers which follow RFC 2616 and does not generate
Expire/Cache-Control if these are expected to match headers
returned in the original response.

On the other hand, I'm not sure this will help in your case, since
Expires headers in the past are already automatically ignored, see
below.

> In ngx_http_upstream_test_next(...) in revalidate part there is firstly
> cache time from upstream 304 response saved to temporal variable (valid
> = r->cache->valid_sec) and then request is reinited and
> r->cache->valid_sec is set according to headers in the cached file.
> Problem is when value == 0 (no caching info from upstream) and there is
> an absolute time in the cached file headers.
>
> This patch should fix this behavior - time computed from cached file is
> used only when it is in the future otherwise, time calculated by
> ngx_http_file_cache_valid(...) is used.

As long as Expires is in the past, r->cache->valid_sec is not set
and remains 0, see ngx_http_upstream_process_expires(). As such,
suggested patch is a nop as long as standard Expires and
Cache-Control headers are used: nginx will ignore Expires from the
original response automatically, and will use proxy_cache_valid
instead.

Are you trying to address X-Accel-Expires with an absolute time in
the past? Note that it is known to be specifically used to
achieve the "revalidate on each request" behaviour, and the
suggested change will break this. (Also, changing the
X-Accel-Expires behaviour is better to be done in
ngx_http_upstream_process_accel_expires(), rather than indirectly,
in 304 response handling code.)

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel