[PATCH 2 of 2] Cache: send conditional requests only for cached 200 OK responses

[Piotr Sikora]

# HG changeset patch
# User Piotr Sikora <piotr at cloudflare.com>
# Date 1416359233 28800
#      Tue Nov 18 17:07:13 2014 -0800
# Node ID 16f4ca8391ddd98ba99b00a46c0b56390f38e0a2
# Parent  99e65578bc80960b2fdf494e048678dd97bba029
Cache: send conditional requests only for cached 200 OK 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 99e65578bc80 -r 16f4ca8391dd src/http/ngx_http_cache.h
--- a/src/http/ngx_http_cache.h	Tue Nov 18 17:07:13 2014 -0800
+++ b/src/http/ngx_http_cache.h	Tue Nov 18 17:07:13 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       4
+#define NGX_HTTP_CACHE_VERSION       5
 
 
 typedef struct {
@@ -83,6 +83,7 @@ struct ngx_http_cache_s {
 
     ngx_uint_t                       min_uses;
     ngx_uint_t                       error;
+    ngx_uint_t                       status;
 
     ngx_buf_t                       *buf;
 
@@ -114,6 +115,7 @@ typedef struct {
     time_t                           last_modified;
     time_t                           date;
     uint32_t                         crc32;
+    u_short                          status;
     u_short                          header_start;
     u_short                          body_start;
     u_char                           etag_len;
diff -r 99e65578bc80 -r 16f4ca8391dd src/http/ngx_http_file_cache.c
--- a/src/http/ngx_http_file_cache.c	Tue Nov 18 17:07:13 2014 -0800
+++ b/src/http/ngx_http_file_cache.c	Tue Nov 18 17:07:13 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->header_start = h->header_start;
     c->body_start = h->body_start;
     c->etag.len = h->etag_len;
@@ -1117,6 +1118,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->header_start = (u_short) c->header_start;
     h->body_start = (u_short) c->body_start;
 
@@ -1335,6 +1337,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)
     {
@@ -1356,6 +1359,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.header_start = (u_short) c->header_start;
     h.body_start = (u_short) c->body_start;
 
diff -r 99e65578bc80 -r 16f4ca8391dd src/http/ngx_http_upstream.c
--- a/src/http/ngx_http_upstream.c	Tue Nov 18 17:07:13 2014 -0800
+++ b/src/http/ngx_http_upstream.c	Tue Nov 18 17:07:13 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);
@@ -4924,6 +4925,7 @@ ngx_http_upstream_cache_last_modified(ng
     if (r->upstream == NULL
         || !r->upstream->conf->cache_revalidate
         || r->upstream->cache_status != NGX_HTTP_CACHE_EXPIRED
+        || r->cache->status != NGX_HTTP_OK
         || r->cache->last_modified == -1)
     {
         v->not_found = 1;
@@ -4952,6 +4954,7 @@ ngx_http_upstream_cache_etag(ngx_http_re
     if (r->upstream == NULL
         || !r->upstream->conf->cache_revalidate
         || r->upstream->cache_status != NGX_HTTP_CACHE_EXPIRED
+        || r->cache->status != NGX_HTTP_OK
         || r->cache->etag.len == 0)
     {
         v->not_found = 1;