Inconsistency about cache's valid_sec

[Maxim Dounin]

Hello!

On Wed, Jul 19, 2017 at 01:18:47PM +0800, Peng Fang wrote:

> Hello, Maxim and other developers,
> 
> In the latest official release, ngx_http_file_cache_set_header will be
> called in ngx_http_upstream_send_response to realy set a cached file's
> valid_sec.
> before calling ngx_http_file_cache_set_header, there will be a chance for
> user defined modules to change the valid_sec in their header filter
> hooks,no matter what the value is set by
> Cache-Control/Expires/X-Accel-Expires previously.
> 
> And that's reasonable.
> 
> When a cached file is expired, that logic will work fine too.
> 
> Inconsistency happens, however, when proxy_cache_revalidate
> <http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_cache_revalidate>
> is enabled. when we enable proxy_cache_revalidate
> <http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_cache_revalidate>,
> ngx_http_file_cache_update_header will be called in
> ngx_http_upstream_test_next. And in ngx_http_upstream_test_next, there is
> no chance for user defined modules to change the valid_sec finally in their
> header filter hooks. Because before calling ngx_http_upstream_cache_send in
> ngx_http_upstream_test_next, valid_sec will be cached firstly, and set to a
> cached file after calling ngx_http_upstream_cache_send and
> ngx_http_file_cache_update_header.
> 
> If we get valid_sec after ngx_http_upstream_cache_send in
> ngx_http_upstream_test_next, user defined module's modification of
> valid_sec will take effect.And the consistency will be kept.
> 
> I don't konw if i missed something, so what do you think about this
> situation?

Trying to modify valid_sec from header filters looks like a bug to 
me.  It will expectedly lead to a wrong result in multiple cases, 
including the one you've described.  Don't do that.

And no, the proposed solution won't work.  The goal of the code is 
to use cache validity as calculated based on the 304 response 
returned if it is available.  After ngx_http_upstream_cache_send() 
valid_sec will be based on the original cached response, and can 
be different.

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