Opened 9 years ago

Last modified 7 years ago

#772 assigned defect

No Vary header on 304 Response.

Reported by: uudruid74.startssl.com Owned by: Maxim Dounin
Priority: minor Milestone:
Component: nginx-core Version: 1.9.x
Keywords: Vary Cc:
uname -a: Linux navi 2.6.32-042stab108.2 #1 SMP Fri Jun 19 19:47:59 2015 x86_64 Intel(R) Xeon(R) CPU X5670 @ 2.93GHz GenuineIntel GNU/Linux
nginx -V: Tengine version: Tengine/2.1.0 (nginx/1.6.2)
TLS SNI support enabled

Description

Yes, I know its Tengine, but I'm betting this will be in Nginx 1.6.2 as well, as its better to fix it "upstream" so that everyone gets it fixed. Tested this via RedBot.org.

Here, everything is working on regular 200 Response

HTTP/1.1 200 OK
Server: Tengine
Date: Sun, 05 Jul 2015 06:45:18 GMT
Content-Type: text/html; charset=UTF-8
Last-Modified: Sun, 05 Jul 2015 03:15:13 GMT
Transfer-Encoding: chunked
Connection: keep-alive
Vary: Accept-Encoding
Expires: Sun, 12 Jul 2015 06:45:18 GMT
Cache-Control: max-age=604800
Strict-Transport-Security: max-age=63072000; includeSubdomains; preload
X-Content-Type-Options: nosniff
Content-Encoding: gzip

Now, the Vary header is gone (and I'm pretty sure it should be there for this response)

HTTP/1.1 304 Not Modified

Server: Tengine
Date: Sun, 05 Jul 2015 06:45:18 GMT
Last-Modified: Sun, 05 Jul 2015 03:15:13 GMT
Connection: keep-alive
ETag: "5598a141-5eb"
Expires: Sun, 12 Jul 2015 06:45:18 GMT
Cache-Control: max-age=604800
Strict-Transport-Security: max-age=63072000; includeSubdomains; preload
X-Content-Type-Options: nosniff

Change History (3)

comment:1 by Maxim Dounin, 9 years ago

While presence of the Vary header is not required by RFC2616, looks like it's not mandatory with RFC7232. The following patch should fix this:

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1436116179 -10800
#      Sun Jul 05 20:09:39 2015 +0300
# Node ID 137a5031414793ce42e5b0741d2815372ef12a73
# Parent  114d1f8cdcabe5c7552b518c4d7ac0a7e98930c1
Gzip filter: 304 (Not Modified) handling (ticket #772).

With this change, Vary is returned for 304 (Not Modified) responses
much like for normal ones (if gzip_vary is switched on).  Additionally,
entity tags are downgraded to weak ones if needed to exactly match
corresponding normal responses.

diff --git a/src/http/modules/ngx_http_gzip_filter_module.c b/src/http/modules/ngx_http_gzip_filter_module.c
--- a/src/http/modules/ngx_http_gzip_filter_module.c
+++ b/src/http/modules/ngx_http_gzip_filter_module.c
@@ -245,6 +245,7 @@ ngx_http_gzip_header_filter(ngx_http_req
 
     if (!conf->enable
         || (r->headers_out.status != NGX_HTTP_OK
+            && r->headers_out.status != NGX_HTTP_NOT_MODIFIED
             && r->headers_out.status != NGX_HTTP_FORBIDDEN
             && r->headers_out.status != NGX_HTTP_NOT_FOUND)
         || (r->headers_out.content_encoding
@@ -280,6 +281,18 @@ ngx_http_gzip_header_filter(ngx_http_req
         return ngx_http_next_header_filter(r);
     }
 
+    if (r->headers_out.status == NGX_HTTP_NOT_MODIFIED) {
+
+        /*
+         * We don't do anything with 304 responses, but ensure
+         * they have the same headers as normal ones.
+         */
+
+        ngx_http_weak_etag(r);
+
+        return ngx_http_next_header_filter(r);
+    }
+
     ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_gzip_ctx_t));
     if (ctx == NULL) {
         return NGX_ERROR;
diff --git a/src/http/modules/ngx_http_not_modified_filter_module.c b/src/http/modules/ngx_http_not_modified_filter_module.c
--- a/src/http/modules/ngx_http_not_modified_filter_module.c
+++ b/src/http/modules/ngx_http_not_modified_filter_module.c
@@ -93,7 +93,6 @@ ngx_http_not_modified_header_filter(ngx_
 
         r->headers_out.status = NGX_HTTP_NOT_MODIFIED;
         r->headers_out.status_line.len = 0;
-        r->headers_out.content_type.len = 0;
         ngx_http_clear_content_length(r);
         ngx_http_clear_accept_ranges(r);
 
diff --git a/src/http/ngx_http_header_filter_module.c b/src/http/ngx_http_header_filter_module.c
--- a/src/http/ngx_http_header_filter_module.c
+++ b/src/http/ngx_http_header_filter_module.c
@@ -237,6 +237,7 @@ ngx_http_header_filter(ngx_http_request_
 
             if (status == NGX_HTTP_NOT_MODIFIED) {
                 r->header_only = 1;
+                r->headers_out.content_type.len = 0;
             }
 
             status = status - NGX_HTTP_MOVED_PERMANENTLY + NGX_HTTP_OFF_3XX;

comment:2 by Maxim Dounin, 8 years ago

Owner: set to Maxim Dounin
Status: newassigned

comment:3 by hthetiot@…, 7 years ago

Testing nginx static from https://redbot.org result:

HTTP requires 304 Not Modified responses to have certain headers, if they are also present in a normal (e.g., 200 OK response).

The 304 response is missing the following headers: vary.

This can affect cache operation; because the headers are missing, caches might remove them from their cached copies.
Note: See TracTickets for help on using tickets.