[PATCH 2 of 2] Cache: test proxy_no_cache predicates before updating cache

[Maxim Dounin]

Hello!

On Mon, Nov 24, 2014 at 07:27:54PM -0800, Piotr Sikora wrote:

> # HG changeset patch
> # User Piotr Sikora <piotr at cloudflare.com>
> # Date 1416886025 28800
> #      Mon Nov 24 19:27:05 2014 -0800
> # Node ID ab6cff701ca23bee8f24e9efcdbcef2ca938b68f
> # Parent  01f07fc7932b64f261c9e6cb778c87279fabcde2
> Cache: test proxy_no_cache predicates before updating cache.
> 
> Signed-off-by: Piotr Sikora <piotr at cloudflare.com>
> 
> diff -r 01f07fc7932b -r ab6cff701ca2 src/http/ngx_http_upstream.c
> --- a/src/http/ngx_http_upstream.c	Mon Nov 24 19:27:05 2014 -0800
> +++ b/src/http/ngx_http_upstream.c	Mon Nov 24 19:27:05 2014 -0800
> @@ -2008,6 +2008,20 @@ ngx_http_upstream_test_next(ngx_http_req
>          ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
>                         "http upstream not modified");
>  
> +        switch (ngx_http_test_predicates(r, u->conf->no_cache)) {
> +
> +        case NGX_ERROR:
> +            ngx_http_upstream_finalize_request(r, u, NGX_ERROR);
> +            return NGX_OK;
> +
> +        case NGX_DECLINED:
> +            u->cacheable = 0;
> +            break;
> +
> +        default: /* NGX_OK */
> +            break;
> +        }
> +
>          valid = u->cacheable ? r->cache->valid_sec : 0;
>  
>          rc = u->reinit_request(r);

Again, not sure it's a good change.  In particular, suggested code 
checks proxy_no_cache against a 304 response, not against a 
response which will be cached (or, rather, already cached), and 
this looks very questionable.

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