[PATCH] Upstream: Cache stale responses if they may be revalidated.

[Thorvaldur Thorvaldsson]

Hello!

I should improve two things here:

  1) Check for a valid Last-Modified header instead of just existence.
  2) After removing the fall-through case in the switch statement, I should
     rewrite it as an if/else statement.

I'll send an updated patch in a moment.

I've also written some test cases to accompany this patch. I'll post them
separately.

Best regards,
Thorvaldur

On Tue, Dec 15, 2015 at 3:45 PM, Thorvaldur Thorvaldsson
<thorvaldur.thorvaldsson at gmail.com> wrote:
> # HG changeset patch
> # User Thorvaldur Thorvaldsson <thorvaldur.thorvaldsson at gmail.com>
> # Date 1450190015 -3600
> #      Tue Dec 15 15:33:35 2015 +0100
> # Node ID 4a1914481e2bd3eecdc5d23e1386c01e4fb08414
> # Parent  def9c9c9ae05cfa7467b0ec96e76afa180c23dfb
> Upstream: Cache stale responses if they may be revalidated.
>
> Previously, the proxy cache would never store stale responses, e.g.,
> when the "Cache-Control" header contained "max-age=0", even if the
> "proxy_cache_revalidate" directive was "on" and the response included
> both an "ETag" and a "Last-Modified" header. 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,
> e.g., when authorization is required (and cheap).
>
> diff -r def9c9c9ae05 -r 4a1914481e2b src/http/ngx_http_file_cache.c
> --- a/src/http/ngx_http_file_cache.c Sat Dec 12 10:32:58 2015 +0300
> +++ b/src/http/ngx_http_file_cache.c Tue Dec 15 15:33:35 2015 +0100
> @@ -628,7 +628,7 @@
>
>      now = ngx_time();
>
> -    if (c->valid_sec < now) {
> +    if (c->valid_sec <= now) {
>
>          ngx_shmtx_lock(&cache->shpool->mutex);
>
> @@ -831,7 +831,7 @@
>
>          if (fcn->error) {
>
> -            if (fcn->valid_sec < ngx_time()) {
> +            if (fcn->valid_sec <= ngx_time()) {
>                  goto renew;
>              }
>
> diff -r def9c9c9ae05 -r 4a1914481e2b 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 Tue Dec 15 15:33:35 2015 +0100
> @@ -2815,11 +2815,15 @@
>              valid = ngx_http_file_cache_valid(u->conf->cache_valid,
>                                                u->headers_in.status_n);
>              if (valid) {
> -                r->cache->valid_sec = now + valid;
> +                valid += now;
> +                r->cache->valid_sec = valid;
>              }
>          }
>
> -        if (valid) {
> +        if (valid > now
> +            || (r->upstream->conf->cache_revalidate
> +                && (u->headers_in.etag ||
> u->headers_in.last_modified_time)))
> +        {
>              r->cache->date = now;
>              r->cache->body_start = (u_short) (u->buffer.pos -
> u->buffer.start);
>
> @@ -4272,11 +4276,6 @@
>          return NGX_OK;
>      }
>
> -    if (n == 0) {
> -        u->cacheable = 0;
> -        return NGX_OK;
> -    }
> -
>      r->cache->valid_sec = ngx_time() + n;
>      }
>  #endif
> @@ -4312,7 +4311,7 @@
>
>      expires = ngx_parse_http_time(h->value.data, h->value.len);
>
> -    if (expires == NGX_ERROR || expires < ngx_time()) {
> +    if (expires == NGX_ERROR) {
>          u->cacheable = 0;
>          return NGX_OK;
>      }
> @@ -4355,10 +4354,6 @@
>          n = ngx_atoi(p, len);
>
>          switch (n) {
> -        case 0:
> -            u->cacheable = 0;
> -            /* fall through */
> -
>          case NGX_ERROR:
>              return NGX_OK;
>
>