PR 39727: Fixup ETag handling in mod deflate (updated following extensive

discussion on-list).
This is not a full-and-final fix, because we don't ourselves do anything
useful with these ETags.  But at least we're no longer screwing up clients.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@607219 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index d5132c2627ca493c4ff0812c458e077ea2df777d..e07765c1bb0ba24c33c830f4ad1ba03eb600f205 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,9 @@
 Changes with Apache 2.3.0
 [ When backported to 2.2.x, remove entry from this file ]
 
+  *) mod_deflate: Transform ETag when transforming the entity.
+     PR 39727 [Henrik Nordstrom <hno squid-cache.org>, Nick Kew]
+
   *) mod_ldap: Set character set for status page to ISO-8859-1 to avoid
      UTF-7 XSS vulnerabilities of certain browsers. [Joe Orton]
 
@@ -61,10 +64,6 @@ Changes with Apache 2.3.0
   *) mpm winnt: fix null pointer dereference
      PR 42572 [Davi Arnaut]
 
-  *) mod_deflate: Don't leave a strong ETag in place while transforming
-     the entity.
-     PR 39727 [Nick Kew]
-
   *) core: reinstate location walk to fix config for subrequests
      PR 41960 [Jose Kahan <jose w3.org>]
 

diff --git a/modules/filters/mod_deflate.c b/modules/filters/mod_deflate.c
index 0ccc66474796a6f38ac66fdcf281b5c66752b139..de1a57d78a30bdc876de8e43f472bf7905c95651 100644 (file)
--- a/modules/filters/mod_deflate.c
+++ b/modules/filters/mod_deflate.c
@@ -373,25 +373,20 @@ static apr_status_t deflate_ctx_cleanup(void *data)
     return APR_SUCCESS;
 }
 /* PR 39727: we're screwing up our clients if we leave a strong ETag
- * header while transforming content.  A minimal fix that makes us
- * protocol-compliant is to make it a weak ETag.  Whether we can
- * use this ourselves (e.g. in mod_cache) is a different issue.
+ * header while transforming content.  Henrik Nordstrom suggests
+ * appending ";gzip".
  *
- * Henrik Nordstrom suggests instead appending ";gzip", commenting:
- *   "This should allows for easy bidirectional mapping, simplifying most
- *   conditionals as no transformation of the entity body is needed to find
- *   the etag, and the simple format makes it easier to trace should any
- *   misunderstandings occur."
- *
- * We might consider such a strategy in future if we implement support
- * for such a scheme.
+ * Pending a more thorough review of our Etag handling, let's just
+ * implement his suggestion.  It fixes the bug, or at least turns it
+ * from a showstopper to an inefficiency.  And it breaks nothing that
+ * wasn't already broken.
  */
-static void deflate_check_etag(request_rec *r)
+static void deflate_check_etag(request_rec *r, const char *transform)
 {
     const char *etag = apr_table_get(r->headers_out, "ETag");
     if (etag && (((etag[0] != 'W') && (etag[0] !='w')) || (etag[1] != '/'))) {
         apr_table_set(r->headers_out, "ETag",
-                      apr_pstrcat(r->pool, "W/", etag, NULL));
+                      apr_pstrcat(r->pool, etag, "-", transform, NULL));
     }
 }
 static apr_status_t deflate_out_filter(ap_filter_t *f,
@@ -591,7 +586,7 @@ static apr_status_t deflate_out_filter(ap_filter_t *f,
         }
         apr_table_unset(r->headers_out, "Content-Length");
         apr_table_unset(r->headers_out, "Content-MD5");
-        deflate_check_etag(r);
+        deflate_check_etag(r, "gzip");
 
         /* initialize deflate output buffer */
         ctx->stream.next_out = ctx->buffer;
@@ -1084,7 +1079,7 @@ static apr_status_t inflate_out_filter(ap_filter_t *f,
         /* these are unlikely to be set anyway, but ... */
         apr_table_unset(r->headers_out, "Content-Length");
         apr_table_unset(r->headers_out, "Content-MD5");
-        deflate_check_etag(r);
+        deflate_check_etag(r, "gunzip");
 
         /* initialize inflate output buffer */
         ctx->stream.next_out = ctx->buffer;