Add Support for Weak ETags

[Aaron Peschel]

Hello,

I reduced the scope of the changes to just the gzip and gunzip modules.

-Aaron

# HG changeset patch
# User Aaron Peschel <aaron.peschel at gmail.com>
# Date 1383613159 28800
#      Mon Nov 04 16:59:19 2013 -0800
# Node ID c0a50e6aac95feac6393dd6bff0b30bd1a05ef9e
# Parent  e6a1623f87bc96d5ec62b6d77356aa47dbc60756
Add Support for Weak ETags

This is a response to rev 4746 which removed ETags. 4746 removes the ETag field
from the header in all instances where content is modified by the web server
prior to being sent to the requesting client. This is far more stringent than
required by the HTTP spec.

The HTTP spec requires that strict ETags be dependent on the variant that is
returned by the server. While removing all ETags from these variants meets the
spec, it is a bit extreme.

This commit modifies the gzip and gunzip modules to check if the ETag is marked
as a weak ETag. IFF that case, the ETag is retained, and not dropped.

diff -r e6a1623f87bc -r c0a50e6aac95
src/http/modules/ngx_http_gunzip_filter_module.c
--- a/src/http/modules/ngx_http_gunzip_filter_module.c Mon Oct 21
18:20:32 2013 +0800
+++ b/src/http/modules/ngx_http_gunzip_filter_module.c Mon Nov 04
16:59:19 2013 -0800
@@ -165,7 +165,9 @@

     ngx_http_clear_content_length(r);
     ngx_http_clear_accept_ranges(r);
-    ngx_http_clear_etag(r);
+    if (!ngx_http_has_weak_etag(r)) {
+        ngx_http_clear_etag(r);
+    }

     return ngx_http_next_header_filter(r);
 }
diff -r e6a1623f87bc -r c0a50e6aac95
src/http/modules/ngx_http_gzip_filter_module.c
--- a/src/http/modules/ngx_http_gzip_filter_module.c Mon Oct 21
18:20:32 2013 +0800
+++ b/src/http/modules/ngx_http_gzip_filter_module.c Mon Nov 04
16:59:19 2013 -0800
@@ -306,7 +306,9 @@

     ngx_http_clear_content_length(r);
     ngx_http_clear_accept_ranges(r);
-    ngx_http_clear_etag(r);
+    if (!ngx_http_has_weak_etag(r)) {
+        ngx_http_clear_etag(r);
+    }

     return ngx_http_next_header_filter(r);
 }
diff -r e6a1623f87bc -r c0a50e6aac95 src/http/ngx_http_core_module.h
--- a/src/http/ngx_http_core_module.h Mon Oct 21 18:20:32 2013 +0800
+++ b/src/http/ngx_http_core_module.h Mon Nov 04 16:59:19 2013 -0800
@@ -572,6 +572,11 @@
         r->headers_out.location = NULL;                                       \
     }

+#define ngx_http_has_weak_etag(r)                                             \
+                                                                              \
+    ((r->headers_out.etag) &&                                                 \
+    (ngx_strncmp(r->headers_out.etag->value.data, "W/", 2)))
+
 #define ngx_http_clear_etag(r)                                                \
                                                                               \
     if (r->headers_out.etag) {                                                \

On Tue, Oct 29, 2013 at 3:04 PM, Aaron Peschel <aaron.peschel at gmail.com> wrote:
> On Fri, Oct 25, 2013 at 2:54 PM, Piotr Sikora <piotr at cloudflare.com> wrote:
>> Hi Aaron,
>> I disagree with your patch... While retaining weak ETags in case of
>> gzip/gunzip modules is correct, other modules are modifying the
>> content and weak ETags should be removed from responses processed by
>> them.
>>
>> Best regards,
>> Piotr Sikora
>>
>> _______________________________________________
>> nginx-devel mailing list
>> nginx-devel at nginx.org
>> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
> What are your thoughts on the correct way to proceed from here? Should there be
> two macros, ngx_http_clear_strict_etag and ngx_http_clear_etag and then have
> the gunzip and gzip only clear the strict etag?
>
> EG
>
> #define ngx_http_clear_etag(r)                                                \
>                                                                               \
>     if (r->headers_out.etag) {                                                \
>         r->headers_out.etag->hash = 0;                                        \
>         r->headers_out.etag = NULL;                                           \
>     }
>
> #define ngx_http_clear_strict_etag(r)                                         \
>                                                                               \
>     if (r->headers_out.etag) {                                                \
>         if (! ngx_strncmp(r->headers_out.etag->value.data, "W/", 2)) {        \
>             r->headers_out.etag->hash = 0;                                    \
>             r->headers_out.etag = NULL;                                       \
>         }                                                                     \
>     }
>
> The gzip module is the module I am most interested in providing weak ETag
> support for. Please let me know what the suggested path is here, and I will put
> in the work for it.
>
> Thank you,
>
> -Aaron