Re: Add Support for Variant ETags

2013-12-13 Thread Maxim Dounin
Hello!

On Thu, Dec 12, 2013 at 04:30:07PM -0800, Aaron Peschel wrote:

 Hello,
 
 Weak ETAGs proved to be insufficient to meet my requirements.
 
 Apache has solved their issues with ETAGs by appending the type of the variant
 to the existing ETAG. For example, the ETAG for a gzip variant would have 
 -gzip
 appended (EG abc123 becomes abc123-gzip). This seems like a reasonable
 solution to me, and it should work with Nginx as well.

This is incorrect solution, as gzipping may result in many 
different byte representations of a resource.  Strict entity tags 
aren't allowed as a result.

See also this ticket for some related discussion:

http://trac.nginx.org/nginx/ticket/377

-- 
Maxim Dounin
http://nginx.org/

___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel


Re: Add Support for Variant ETags

2013-12-13 Thread Jeff Kaufman
On Fri, Dec 13, 2013 at 6:12 AM, Maxim Dounin mdou...@mdounin.ru wrote:
 gzipping may result in many different byte representations of
 a resource.  Strict entity tags aren't allowed as a result.


Is gzip deterministic, where you only get different byte
representations with different gzip settings?  If so we could include
the gzip settings (or a hash of them) in the etag.

___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel


Re: Add Support for Variant ETags

2013-12-13 Thread Maxim Dounin
Hello!

On Fri, Dec 13, 2013 at 09:59:16AM -0500, Jeff Kaufman wrote:

 On Fri, Dec 13, 2013 at 6:12 AM, Maxim Dounin mdou...@mdounin.ru wrote:
  gzipping may result in many different byte representations of
  a resource.  Strict entity tags aren't allowed as a result.
 
 
 Is gzip deterministic, where you only get different byte
 representations with different gzip settings?  If so we could include
 the gzip settings (or a hash of them) in the etag.

No, it's not, at least if flushes are done.

As already suggested, I belive the correct solution is to 
downgrade entity tags to weak ones.

-- 
Maxim Dounin
http://nginx.org/

___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel


Add Support for Variant ETags

2013-12-12 Thread Aaron Peschel
Hello,

Weak ETAGs proved to be insufficient to meet my requirements.

Apache has solved their issues with ETAGs by appending the type of the variant
to the existing ETAG. For example, the ETAG for a gzip variant would have -gzip
appended (EG abc123 becomes abc123-gzip). This seems like a reasonable
solution to me, and it should work with Nginx as well.

Here is a link to the discussion:

https://issues.apache.org/bugzilla/show_bug.cgi?id=39727

I have created a patch to provide support for variant ETAGs in Nginx. Feedback
would be appreciated! I would like to see this merged into the main line, and
I'm willing to put in any additional work required.

Currently I have only switched the gzip module to using variant ETAGs, but
others could be switched as well. I have not tested this patch very thoroughly
yet either.

Thank you,

--Aaron Peschel



# HG changeset patch
# User Aaron Peschel aaron.pesc...@gmail.com
# Date 1386735423 28800
#  Tue Dec 10 20:17:03 2013 -0800
# Node ID 1d645b958ca1d086f4ef1d4c0dd63dff6cb2f2af
# Parent  1bdf906bba99b4c03d306dd8f2756d4d8ffa4a13
Add Support for Variant ETags

Rather than stripping strict ETAGs, we can generate a variant of ETAG on the
fly that will act as a strict ETAG for the variant. The solution used by Apache
is to simply append the type of the variant to the existing ETAG. This seems
like a reasonable solution, and it is what this commit implments.

diff -r 1bdf906bba99 -r 1d645b958ca1
src/http/modules/ngx_http_gzip_filter_module.c
--- a/src/http/modules/ngx_http_gzip_filter_module.c Fri Dec 06
19:58:27 2013 +0400
+++ b/src/http/modules/ngx_http_gzip_filter_module.c Tue Dec 10
20:17:03 2013 -0800
@@ -306,7 +306,10 @@

 ngx_http_clear_content_length(r);
 ngx_http_clear_accept_ranges(r);
-ngx_http_clear_etag(r);
+
+if (ngx_http_set_etag_variant(r, gzip) != NGX_OK) {
+return NGX_ERROR;
+}

 return ngx_http_next_header_filter(r);
 }
diff -r 1bdf906bba99 -r 1d645b958ca1 src/http/ngx_http_core_module.c
--- a/src/http/ngx_http_core_module.c Fri Dec 06 19:58:27 2013 +0400
+++ b/src/http/ngx_http_core_module.c Tue Dec 10 20:17:03 2013 -0800
@@ -1855,6 +1855,42 @@


 ngx_int_t
+ngx_http_set_etag_variant(ngx_http_request_t *r, const char *append_str)
+{
+ngx_str_t  *etag;
+u_char *variant_str;
+ngx_int_t   append_str_len;
+ngx_int_t   variant_str_len;
+ngx_int_t   closing_quote;
+
+etag = r-headers_out.etag-value;
+append_str_len = ngx_strlen(append_str);
+closing_quote = etag-len - 1;
+
+if (etag-len = 0) {
+return NGX_OK;
+}
+
+// Add one byte for the dash between the etag and the append string.
+variant_str = ngx_pnalloc(r-pool, etag-len + append_str_len + 1);
+if (variant_str == NULL) {
+return NGX_ERROR;
+}
+
+ngx_cpystrn(variant_str, etag-data, closing_quote);
+variant_str_len = ngx_sprintf(variant_str + closing_quote,
+  -%s\,
+  append_str)
+  - variant_str;
+
+etag-len = variant_str_len;
+etag-data = variant_str;
+
+return NGX_OK;
+}
+
+
+ngx_int_t
 ngx_http_send_response(ngx_http_request_t *r, ngx_uint_t status,
 ngx_str_t *ct, ngx_http_complex_value_t *cv)
 {
diff -r 1bdf906bba99 -r 1d645b958ca1 src/http/ngx_http_core_module.h
--- a/src/http/ngx_http_core_module.h Fri Dec 06 19:58:27 2013 +0400
+++ b/src/http/ngx_http_core_module.h Tue Dec 10 20:17:03 2013 -0800
@@ -499,6 +499,7 @@
 ngx_int_t ngx_http_set_content_type(ngx_http_request_t *r);
 void ngx_http_set_exten(ngx_http_request_t *r);
 ngx_int_t ngx_http_set_etag(ngx_http_request_t *r);
+ngx_int_t ngx_http_set_etag_variant(ngx_http_request_t *r, const char
*append_str);
 ngx_int_t ngx_http_send_response(ngx_http_request_t *r, ngx_uint_t status,
 ngx_str_t *ct, ngx_http_complex_value_t *cv);
 u_char *ngx_http_map_uri_to_path(ngx_http_request_t *r, ngx_str_t *name,


etags.patch
Description: Binary data
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel