This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/8.5.x by this push:
     new 8ff903b  Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63932 ETag 
& gzip
8ff903b is described below

commit 8ff903b9a9b64fa30677f44e130d9b0898704a0b
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Nov 28 14:44:20 2019 +0000

    Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63932 ETag & gzip
    
    By default, do not compress content that has a strong ETag. This
    behaviour is configuration for the HTTP/1.1 and HTTP/2 connectors via
    the new Connector attribute noCompressionStrongETag
---
 java/org/apache/coyote/CompressionConfig.java      | 40 ++++++++++++++++++++++
 .../coyote/http11/AbstractHttp11Protocol.java      | 10 ++++++
 java/org/apache/coyote/http2/Http2Protocol.java    | 10 ++++++
 test/org/apache/coyote/TestCompressionConfig.java  | 33 ++++++++++++++----
 webapps/docs/changelog.xml                         |  6 ++++
 webapps/docs/config/http.xml                       |  8 +++++
 webapps/docs/config/http2.xml                      |  8 +++++
 7 files changed, 108 insertions(+), 7 deletions(-)

diff --git a/java/org/apache/coyote/CompressionConfig.java 
b/java/org/apache/coyote/CompressionConfig.java
index 520ed2e..8bb11f9 100644
--- a/java/org/apache/coyote/CompressionConfig.java
+++ b/java/org/apache/coyote/CompressionConfig.java
@@ -46,6 +46,7 @@ public class CompressionConfig {
             
"text/javascript,application/javascript,application/json,application/xml";
     private String[] compressibleMimeTypes = null;
     private int compressionMinSize = 2048;
+    private boolean noCompressionStrongETag = true;
 
 
     /**
@@ -183,6 +184,35 @@ public class CompressionConfig {
 
 
     /**
+     * Determine if compression is disabled if the resource has a strong ETag.
+     *
+     * @return {@code true} if compression is disabled, otherwise {@code false}
+     *
+     * @deprecated Will be removed in Tomcat 10 where it will be hard-coded to
+     *             {@code true}
+     */
+    @Deprecated
+    public boolean getNoCompressionStrongETag() {
+        return noCompressionStrongETag;
+    }
+
+
+    /**
+     * Set whether compression is disabled for resources with a strong ETag.
+     *
+     * @param noCompressionStrongETag {@code true} if compression is disabled,
+     *                                otherwise {@code false}
+     *
+     * @deprecated Will be removed in Tomcat 10 where it will be hard-coded to
+     *             {@code true}
+     */
+    @Deprecated
+    public void setNoCompressionStrongEtag(boolean noCompressionStrongETag) {
+        this.noCompressionStrongETag = noCompressionStrongETag;
+    }
+
+
+    /**
      * Determines if compression should be enabled for the given response and 
if
      * it is, sets any necessary headers to mark it as such.
      *
@@ -235,6 +265,16 @@ public class CompressionConfig {
             }
         }
 
+        // Check if the resource has a strong ETag
+        if (noCompressionStrongETag) {
+            String eTag = responseHeaders.getHeader("ETag");
+            if (eTag != null && !eTag.trim().startsWith("W/")) {
+                // Has an ETag that doesn't start with "W/..." so it must be a
+                // strong ETag
+                return false;
+            }
+        }
+
         // If processing reaches this far, the response might be compressed.
         // Therefore, set the Vary header to keep proxies happy
         ResponseUtil.addVaryFieldName(responseHeaders, "accept-encoding");
diff --git a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java 
b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
index 26aea33..4d65942 100644
--- a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
+++ b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
@@ -265,6 +265,16 @@ public abstract class AbstractHttp11Protocol<S> extends 
AbstractProtocol<S> {
     }
 
 
+    @Deprecated
+    public boolean getNoCompressionStrongETag() {
+        return compressionConfig.getNoCompressionStrongETag();
+    }
+    @Deprecated
+    public void setNoCompressionStrongEtag(boolean noCompressionStrongETag) {
+        compressionConfig.setNoCompressionStrongEtag(noCompressionStrongETag);
+    }
+
+
     public boolean useCompression(Request request, Response response) {
         return compressionConfig.useCompression(request, response);
     }
diff --git a/java/org/apache/coyote/http2/Http2Protocol.java 
b/java/org/apache/coyote/http2/Http2Protocol.java
index 694d424..0afb255 100644
--- a/java/org/apache/coyote/http2/Http2Protocol.java
+++ b/java/org/apache/coyote/http2/Http2Protocol.java
@@ -396,6 +396,16 @@ public class Http2Protocol implements UpgradeProtocol {
     }
 
 
+    @Deprecated
+    public boolean getNoCompressionStrongETag() {
+        return compressionConfig.getNoCompressionStrongETag();
+    }
+    @Deprecated
+    public void setNoCompressionStrongEtag(boolean noCompressionStrongETag) {
+        compressionConfig.setNoCompressionStrongEtag(noCompressionStrongETag);
+    }
+
+
     public boolean useCompression(Request request, Response response) {
         return compressionConfig.useCompression(request, response);
     }
diff --git a/test/org/apache/coyote/TestCompressionConfig.java 
b/test/org/apache/coyote/TestCompressionConfig.java
index 23d9d12..0d8c0a0 100644
--- a/test/org/apache/coyote/TestCompressionConfig.java
+++ b/test/org/apache/coyote/TestCompressionConfig.java
@@ -29,16 +29,24 @@ import org.junit.runners.Parameterized.Parameter;
 @RunWith(Parameterized.class)
 public class TestCompressionConfig {
 
-    @Parameterized.Parameters(name = "{index}: headers[{0}], compress[{1}]")
+    @Parameterized.Parameters(name = "{index}: accept-encoding[{0}], ETag 
[{1}], NoCompressionStrongETag[{2}], compress[{3}]")
     public static Collection<Object[]> parameters() {
         List<Object[]> parameterSets = new ArrayList<>();
 
-        parameterSets.add(new Object[] { new String[] {  }, Boolean.FALSE });
-        parameterSets.add(new Object[] { new String[] { "gzip" }, Boolean.TRUE 
});
-        parameterSets.add(new Object[] { new String[] { "xgzip" }, 
Boolean.FALSE });
-        parameterSets.add(new Object[] { new String[] { "<>gzip" }, 
Boolean.FALSE });
-        parameterSets.add(new Object[] { new String[] { "foo", "gzip" }, 
Boolean.TRUE });
-        parameterSets.add(new Object[] { new String[] { "<>", "gzip" }, 
Boolean.TRUE });
+        parameterSets.add(new Object[] { new String[] {  },              null, 
Boolean.TRUE,  Boolean.FALSE });
+        parameterSets.add(new Object[] { new String[] { "gzip" },        null, 
Boolean.TRUE,  Boolean.TRUE });
+        parameterSets.add(new Object[] { new String[] { "xgzip" },       null, 
Boolean.TRUE,  Boolean.FALSE });
+        parameterSets.add(new Object[] { new String[] { "<>gzip" },      null, 
Boolean.TRUE,  Boolean.FALSE });
+        parameterSets.add(new Object[] { new String[] { "foo", "gzip" }, null, 
Boolean.TRUE,  Boolean.TRUE });
+        parameterSets.add(new Object[] { new String[] { "<>", "gzip" },  null, 
Boolean.TRUE,  Boolean.TRUE });
+
+        parameterSets.add(new Object[] { new String[] { "gzip" },        null, 
Boolean.TRUE,  Boolean.TRUE });
+        parameterSets.add(new Object[] { new String[] { "gzip" },        "W/", 
Boolean.TRUE,  Boolean.TRUE });
+        parameterSets.add(new Object[] { new String[] { "gzip" },        "XX", 
Boolean.TRUE,  Boolean.FALSE });
+
+        parameterSets.add(new Object[] { new String[] { "gzip" },        null, 
Boolean.FALSE, Boolean.TRUE });
+        parameterSets.add(new Object[] { new String[] { "gzip" },        "W/", 
Boolean.FALSE, Boolean.TRUE });
+        parameterSets.add(new Object[] { new String[] { "gzip" },        "XX", 
Boolean.FALSE, Boolean.TRUE });
 
         return parameterSets;
     }
@@ -46,14 +54,20 @@ public class TestCompressionConfig {
     @Parameter(0)
     public String[] headers;
     @Parameter(1)
+    public String eTag;
+    @Parameter(2)
+    public Boolean noCompressionStrongETag;
+    @Parameter(3)
     public Boolean compress;
 
+    @SuppressWarnings("deprecation")
     @Test
     public void testUseCompression() throws Exception {
 
         CompressionConfig compressionConfig = new CompressionConfig();
         // Skip length and MIME type checks
         compressionConfig.setCompression("force");
+        
compressionConfig.setNoCompressionStrongEtag(noCompressionStrongETag.booleanValue());
 
         Request request = new Request();
         Response response = new Response();
@@ -62,6 +76,11 @@ public class TestCompressionConfig {
             
request.getMimeHeaders().addValue("accept-encoding").setString(header);
         }
 
+        if (eTag != null) {
+            response.getMimeHeaders().addValue("ETag").setString(eTag);
+        }
+
+
         Assert.assertEquals(compress, 
Boolean.valueOf(compressionConfig.useCompression(request, response)));
     }
 }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 2ab8af7..7e4bf2b 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -83,6 +83,12 @@
         Add async API to the NIO and APR connector. (remm)
       </add>
       <fix>
+        <bug>63932</bug>: By default, do not compress content that has a strong
+        ETag. This behaviour is configuration for the HTTP/1.1 and HTTP/2
+        connectors via the new Connector attribute
+        <code>noCompressionStrongETag</code>. (markt)
+      </fix>
+      <fix>
         Simplify regular endpoint writes by removing write(Non)BlockingDirect.
         All regular writes will now be buffered for a more predictable
         behavior. (remm)
diff --git a/webapps/docs/config/http.xml b/webapps/docs/config/http.xml
index a89f3fe..1031514 100644
--- a/webapps/docs/config/http.xml
+++ b/webapps/docs/config/http.xml
@@ -537,6 +537,14 @@
       used.</p>
     </attribute>
 
+    <attribute name="noCompressionStrongETag" required="false">
+      <p>This flag configures whether resources with a stong ETag will be
+      considered for compression. If <code>true</code>, resources with a strong
+      ETag will not be compressed. The default value is <code>true</code>.</p>
+      <p>This attribute is deprecated. It will be removed in Tomcat 10 onwards
+      where it will be hard-coded to <code>true</code>.</p>
+    </attribute>
+
     <attribute name="noCompressionUserAgents" required="false">
       <p>The value is a regular expression (using <code>java.util.regex</code>)
       matching the <code>user-agent</code> header of HTTP clients for which
diff --git a/webapps/docs/config/http2.xml b/webapps/docs/config/http2.xml
index 1b2ad1f..d0055dd 100644
--- a/webapps/docs/config/http2.xml
+++ b/webapps/docs/config/http2.xml
@@ -179,6 +179,14 @@
       means no limit. If not specified, a default of 8192 is used.</p>
     </attribute>
 
+    <attribute name="noCompressionStrongETag" required="false">
+      <p>This flag configures whether resources with a stong ETag will be
+      considered for compression. If <code>true</code>, resources with a strong
+      ETag will not be compressed. The default value is <code>true</code>.</p>
+      <p>This attribute is deprecated. It will be removed in Tomcat 10 onwards
+      where it will be hard-coded to <code>true</code>.</p>
+    </attribute>
+
     <attribute name="noCompressionUserAgents" required="false">
       <p>The value is a regular expression (using <code>java.util.regex</code>)
       matching the <code>user-agent</code> header of HTTP clients for which


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to