[PATCH] Cache: send conditional requests only for cached 200/206 responses

[Piotr Sikora]

# HG changeset patch
# User Piotr Sikora <piotr at cloudflare.com>
# Date 1416965642 28800
#      Tue Nov 25 17:34:02 2014 -0800
# Node ID a39048b405998c977426613668861efae545f70f
# Parent  2c10db908b8c4a9c0532c58830275d5ad84ae686
Cache: send conditional requests only for cached 200/206 responses.

RFC7232 says:

   The 304 (Not Modified) status code indicates that a conditional GET
   or HEAD request has been received and would have resulted in a 200
   (OK) response if it were not for the fact that the condition
   evaluated to false.

which means that there is no reason to send requests with "If-None-Match"
and/or "If-Modified-Since" headers for responses cached with other status
codes.

Also, sending conditional requests for responses cached with other status
codes could result in a strange behavior, e.g. upstream server returning
304 Not Modified for cached 404 Not Found responses, etc.

Signed-off-by: Piotr Sikora <piotr at cloudflare.com>

diff -r 2c10db908b8c -r a39048b40599 src/http/ngx_http_cache.h
--- a/src/http/ngx_http_cache.h	Fri Nov 21 22:51:49 2014 +0300
+++ b/src/http/ngx_http_cache.h	Tue Nov 25 17:34:02 2014 -0800
@@ -27,7 +27,7 @@
 #define NGX_HTTP_CACHE_ETAG_LEN      42
 #define NGX_HTTP_CACHE_VARY_LEN      42
 
-#define NGX_HTTP_CACHE_VERSION       3
+#define NGX_HTTP_CACHE_VERSION       4
 
 
 typedef struct {
@@ -84,6 +84,7 @@ struct ngx_http_cache_s {
 
     ngx_uint_t                       min_uses;
     ngx_uint_t                       error;
+    ngx_uint_t                       status;
     ngx_uint_t                       valid_msec;
 
     ngx_buf_t                       *buf;
@@ -116,6 +117,7 @@ typedef struct {
     time_t                           last_modified;
     time_t                           date;
     uint32_t                         crc32;
+    u_short                          status;
     u_short                          valid_msec;
     u_short                          header_start;
     u_short                          body_start;
diff -r 2c10db908b8c -r a39048b40599 src/http/ngx_http_file_cache.c
--- a/src/http/ngx_http_file_cache.c	Fri Nov 21 22:51:49 2014 +0300
+++ b/src/http/ngx_http_file_cache.c	Tue Nov 25 17:34:02 2014 -0800
@@ -561,6 +561,7 @@ ngx_http_file_cache_read(ngx_http_reques
     c->valid_sec = h->valid_sec;
     c->last_modified = h->last_modified;
     c->date = h->date;
+    c->status = h->status;
     c->valid_msec = h->valid_msec;
     c->header_start = h->header_start;
     c->body_start = h->body_start;
@@ -1119,6 +1120,7 @@ ngx_http_file_cache_set_header(ngx_http_
     h->last_modified = c->last_modified;
     h->date = c->date;
     h->crc32 = c->crc32;
+    h->status = (u_short) c->status;
     h->valid_msec = (u_short) c->valid_msec;
     h->header_start = (u_short) c->header_start;
     h->body_start = (u_short) c->body_start;
@@ -1338,6 +1340,7 @@ ngx_http_file_cache_update_header(ngx_ht
     if (h.version != NGX_HTTP_CACHE_VERSION
         || h.last_modified != c->last_modified
         || h.crc32 != c->crc32
+        || h.status != c->status
         || h.header_start != c->header_start
         || h.body_start != c->body_start)
     {
@@ -1359,6 +1362,7 @@ ngx_http_file_cache_update_header(ngx_ht
     h.last_modified = c->last_modified;
     h.date = c->date;
     h.crc32 = c->crc32;
+    h.status = (u_short) c->status;
     h.valid_msec = (u_short) c->valid_msec;
     h.header_start = (u_short) c->header_start;
     h.body_start = (u_short) c->body_start;
diff -r 2c10db908b8c -r a39048b40599 src/http/ngx_http_upstream.c
--- a/src/http/ngx_http_upstream.c	Fri Nov 21 22:51:49 2014 +0300
+++ b/src/http/ngx_http_upstream.c	Tue Nov 25 17:34:02 2014 -0800
@@ -2560,6 +2560,7 @@ ngx_http_upstream_send_response(ngx_http
         }
 
         if (valid) {
+            r->cache->status = u->headers_in.status_n;
             r->cache->last_modified = u->headers_in.last_modified_time;
             r->cache->date = now;
             r->cache->body_start = (u_short) (u->buffer.pos - u->buffer.start);
@@ -4936,6 +4937,13 @@ ngx_http_upstream_cache_last_modified(ng
         return NGX_OK;
     }
 
+    if (r->cache->status != NGX_HTTP_OK
+        && r->cache->status != NGX_HTTP_PARTIAL_CONTENT)
+    {
+        v->not_found = 1;
+        return NGX_OK;
+    }
+
     p = ngx_pnalloc(r->pool, sizeof("Mon, 28 Sep 1970 06:00:00 GMT") - 1);
     if (p == NULL) {
         return NGX_ERROR;
@@ -4964,6 +4972,13 @@ ngx_http_upstream_cache_etag(ngx_http_re
         return NGX_OK;
     }
 
+    if (r->cache->status != NGX_HTTP_OK
+        && r->cache->status != NGX_HTTP_PARTIAL_CONTENT)
+    {
+        v->not_found = 1;
+        return NGX_OK;
+    }
+
     v->valid = 1;
     v->no_cacheable = 0;
     v->not_found = 0;