[PATCH] Upstream: fix the cache duration calculation

[Maxim Dounin]

Hello!

On Thu, Nov 14, 2013 at 06:44:00PM +0100, Florent Le Coz wrote:

[...]

> Where would you then process these values, if not after every
> headers have been processed? At the moment, nginx processes them one
> by one, in the order there are found in upstream’s response. Do you
> suggest that in process_cache_control I add some "if 'expires' has
> already been processed" condition, and something equivalent in
> 'process_cache_control'? That doesn’t look ideal to me, hence why I
> chose to set the valid_sec value once every header has been
> processed.
> 
> Do you have any suggestion?

It would probably be enough to just look at the 
u->headers_in.x_accel_expires, like this (untested):

--- a/src/http/ngx_http_upstream.c
+++ b/src/http/ngx_http_upstream.c
@@ -3599,7 +3599,7 @@ ngx_http_upstream_process_cache_control(
         return NGX_OK;
     }
 
-    if (r->cache->valid_sec != 0) {
+    if (r->cache->valid_sec != 0 && u->headers_in.x_accel_expires != NULL) {
         return NGX_OK;
     }
 
This way the r->cache->valid_sec will be only checked while 
parsing Cache-Control if an X-Accel-Expires header was in the 
response, and thus Cache-Control will be preferred over Expires.

This is not perfect solution - e.g., if X-Accel-Expires header was 
present but ignored or contained an invalid data, the code will do 
wrong things.  On the other hand, it's probably good enough for 
practical things and certainly much better then what we currently 
have.

Perfectly correct solution would be to store a bit (likely in 
u->headers_in) to indicate that valid_sec was set based on 
X-Accel-Expires and shouldn't be overwritten.  I'm not sure it 
worth the effort though.

-- 
Maxim Dounin
http://nginx.org/en/donation.html