[PATCH] Upstream: Cache "immediately stale" responses if revalidate is on.

[Maxim Dounin]

Hello!

On Mon, Dec 14, 2015 at 06:17:21PM +0100, Thorvaldur Thorvaldsson wrote:

> # HG changeset patch
> # User Thorvaldur Thorvaldsson <thorvaldur.thorvaldsson at gmail.com>
> # Date 1450109082 -3600
> #      Mon Dec 14 17:04:42 2015 +0100
> # Node ID e017db1282d4b8c4d541416e42fe7df8abc73302
> # Parent  def9c9c9ae05cfa7467b0ec96e76afa180c23dfb
> Upstream: Cache "immediately stale" responses if revalidate is on.
> 
> Previously, the proxy cache would never store responses if max-age=0,
> even when "proxy_cache_revalidate" was "on" and the response included an
> ETag. This came as a surprise.
> 
> Now, a header like "Cache-Control: max-age=0, must-revalidate" can be
> used to make nginx cache responses that always require revalidation,
> like, when authorization is required (and cheap).

[...]

> diff -r def9c9c9ae05 -r e017db1282d4 src/http/ngx_http_upstream.c
> --- a/src/http/ngx_http_upstream.c Sat Dec 12 10:32:58 2015 +0300
> +++ b/src/http/ngx_http_upstream.c Mon Dec 14 17:04:42 2015 +0100
> @@ -2819,7 +2819,7 @@
>              }
>          }
> 
> -        if (valid) {
> +        if (valid || r->upstream->conf->cache_revalidate) {
>              r->cache->date = now;
>              r->cache->body_start = (u_short) (u->buffer.pos -
> u->buffer.start);
> 

At least this part looks wrong, as it will enable caching of all 
responses, even ones without Cache-Control/Expires/X-Accel-Expires 
and proxy_cache_valid configured.

Note well that revalidation only make sense if there is a 
validator, i.e., Last-Modified or ETag.  It's probably wrong to 
assume they are always present.

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