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

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel