Possible error on revalidate in ngx_http_upstream

[Jiří Setnička]

Hello!

Thanks for your reply. I didn't realize the implications arising from 
the RFC mentioned.

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

As you mentioned below, I deal with the X-Accel-Expires header. I didn't 
explicitly check ngx_http_upstream_process_expires() and I thought that 
the behavior is similar as X-Accel-Expires, sorry for that.

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

Ok I will look into implementation in 
ngx_http_upstream_process_accel_expires().

Is "revalidate on each request" behaviour intended as the right one, or 
it is considered as a hack because there is no other way to do 
"revalidate on each request"? I did not find it in any documentation, 
only in some email threads and tickets [1].

Would you be interested in the updated patch or should I patch it only 
locally for my own usecase?

Best regards
Jiri Setnicka

[1] https://trac.nginx.org/nginx/ticket/1182