You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/10/23 09:08:04 UTC

[tomcat] branch master updated: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829

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

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


The following commit(s) were added to refs/heads/master by this push:
     new c41fe3b  Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829
c41fe3b is described below

commit c41fe3b64d0caed36cc3a76c872e272290473a4f
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Oct 23 08:50:11 2019 +0200

    Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829
    
    Improve the check of the Content-Encoding header when looking to see if
    Tomcat is serving pre-compressed content. Ensure that only a full token
    is matched and that the match is case insensitive.
---
 java/org/apache/coyote/CompressionConfig.java      | 28 ++++++++++++++++++----
 java/org/apache/coyote/LocalStrings.properties     |  2 ++
 java/org/apache/coyote/http11/Http11Processor.java | 18 ++------------
 .../apache/tomcat/util/http/parser/TokenList.java  | 24 ++++++++++++++++++-
 webapps/docs/changelog.xml                         |  6 +++++
 5 files changed, 57 insertions(+), 21 deletions(-)

diff --git a/java/org/apache/coyote/CompressionConfig.java b/java/org/apache/coyote/CompressionConfig.java
index fd0652e..520ed2e 100644
--- a/java/org/apache/coyote/CompressionConfig.java
+++ b/java/org/apache/coyote/CompressionConfig.java
@@ -20,17 +20,26 @@ import java.io.IOException;
 import java.io.StringReader;
 import java.util.ArrayList;
 import java.util.Enumeration;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.StringTokenizer;
 import java.util.regex.Pattern;
 
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
 import org.apache.tomcat.util.buf.MessageBytes;
 import org.apache.tomcat.util.http.MimeHeaders;
 import org.apache.tomcat.util.http.ResponseUtil;
 import org.apache.tomcat.util.http.parser.AcceptEncoding;
+import org.apache.tomcat.util.http.parser.TokenList;
+import org.apache.tomcat.util.res.StringManager;
 
 public class CompressionConfig {
 
+    private static final Log log = LogFactory.getLog(CompressionConfig.class);
+    private static final StringManager sm = StringManager.getManager(CompressionConfig.class);
+
     private int compressionLevel = 0;
     private Pattern noCompressionUserAgents = null;
     private String compressibleMimeType = "text/html,text/xml,text/plain,text/css," +
@@ -193,10 +202,21 @@ public class CompressionConfig {
 
         // Check if content is not already compressed
         MessageBytes contentEncodingMB = responseHeaders.getValue("Content-Encoding");
-        if (contentEncodingMB != null &&
-                (contentEncodingMB.indexOf("gzip") != -1 ||
-                        contentEncodingMB.indexOf("br") != -1)) {
-            return false;
+        if (contentEncodingMB != null) {
+            // Content-Encoding values are ordered but order is not important
+            // for this check so use a Set rather than a List
+            Set<String> tokens = new HashSet<>();
+            try {
+                TokenList.parseTokenList(responseHeaders.values("Content-Encoding"), tokens);
+            } catch (IOException e) {
+                // Because we are using StringReader, any exception here is a
+                // Tomcat bug.
+                log.warn(sm.getString("compressionConfig.ContentEncodingParseFail"), e);
+                return false;
+            }
+            if (tokens.contains("gzip") || tokens.contains("br")) {
+                return false;
+            }
         }
 
         // If force mode, the length and MIME type checks are skipped
diff --git a/java/org/apache/coyote/LocalStrings.properties b/java/org/apache/coyote/LocalStrings.properties
index b14fc2c..90159d3 100644
--- a/java/org/apache/coyote/LocalStrings.properties
+++ b/java/org/apache/coyote/LocalStrings.properties
@@ -46,6 +46,8 @@ abstractProtocolHandler.stop=Stopping ProtocolHandler [{0}]
 
 asyncStateMachine.invalidAsyncState=Calling [{0}] is not valid for a request with Async state [{1}]
 
+compressionConfig.ContentEncodingParseFail=Failed to parse Content-Encoding header when chekcing to see if compression was already in use
+
 request.notAsync=It is only valid to switch to non-blocking IO within async processing or HTTP upgrade processing
 request.nullReadListener=The listener passed to setReadListener() may not be null
 request.readListenerSet=The non-blocking read listener has already been set
diff --git a/java/org/apache/coyote/http11/Http11Processor.java b/java/org/apache/coyote/http11/Http11Processor.java
index 6df04cc..7be52d0 100644
--- a/java/org/apache/coyote/http11/Http11Processor.java
+++ b/java/org/apache/coyote/http11/Http11Processor.java
@@ -18,10 +18,7 @@ package org.apache.coyote.http11;
 
 import java.io.IOException;
 import java.io.InterruptedIOException;
-import java.io.StringReader;
 import java.nio.ByteBuffer;
-import java.util.Collection;
-import java.util.Enumeration;
 import java.util.HashSet;
 import java.util.Locale;
 import java.util.Set;
@@ -566,7 +563,7 @@ public class Http11Processor extends AbstractProcessor {
         MessageBytes connectionValueMB = headers.getValue(Constants.CONNECTION);
         if (connectionValueMB != null && !connectionValueMB.isNull()) {
             Set<String> tokens = new HashSet<>();
-            parseConnectionTokens(headers, tokens);
+            TokenList.parseTokenList(headers.values(Constants.CONNECTION), tokens);
             if (tokens.contains(Constants.CLOSE)) {
                 keepAlive = false;
             } else if (tokens.contains(Constants.KEEPALIVE)) {
@@ -960,22 +957,11 @@ public class Http11Processor extends AbstractProcessor {
         }
 
         Set<String> tokens = new HashSet<>();
-        parseConnectionTokens(headers, tokens);
+        TokenList.parseTokenList(headers.values(Constants.CONNECTION), tokens);
         return tokens.contains(token);
     }
 
 
-    private static void parseConnectionTokens(MimeHeaders headers, Collection<String> tokens) throws IOException {
-        Enumeration<String> values = headers.values(Constants.CONNECTION);
-        while (values.hasMoreElements()) {
-            String nextHeaderValue = values.nextElement();
-            if (nextHeaderValue != null) {
-                TokenList.parseTokenList(new StringReader(nextHeaderValue), tokens);
-            }
-        }
-    }
-
-
     private void prepareSendfile(OutputFilter[] outputFilters) {
         String fileName = (String) request.getAttribute(
                 org.apache.coyote.Constants.SENDFILE_FILENAME_ATTR);
diff --git a/java/org/apache/tomcat/util/http/parser/TokenList.java b/java/org/apache/tomcat/util/http/parser/TokenList.java
index ca5e153..db40877 100644
--- a/java/org/apache/tomcat/util/http/parser/TokenList.java
+++ b/java/org/apache/tomcat/util/http/parser/TokenList.java
@@ -18,7 +18,9 @@ package org.apache.tomcat.util.http.parser;
 
 import java.io.IOException;
 import java.io.Reader;
+import java.io.StringReader;
 import java.util.Collection;
+import java.util.Enumeration;
 import java.util.Locale;
 
 public class TokenList {
@@ -29,12 +31,32 @@ public class TokenList {
 
 
     /**
+     * Parses an enumeration of header values of the form 1#token, forcing all
+     * parsed values to lower case.
+     *
+     * @param inputs The headers to parse
+     * @param result The Collection (usually a list of a set) to which the
+     *                   parsed tokens should be added
+     *
+     * @throws IOException If an I/O error occurs reading the header
+     */
+    public static void parseTokenList(Enumeration<String> inputs, Collection<String> result) throws IOException {
+        while (inputs.hasMoreElements()) {
+            String nextHeaderValue = inputs.nextElement();
+            if (nextHeaderValue != null) {
+                TokenList.parseTokenList(new StringReader(nextHeaderValue), result);
+            }
+        }
+    }
+
+
+    /**
      * Parses a header of the form 1#token, forcing all parsed values to lower
      * case. This is typically used when header values are case-insensitive.
      *
      * @param input  The header to parse
      * @param result The Collection (usually a list of a set) to which the
-     *                   parsed token should be added
+     *                   parsed tokens should be added
      *
      * @throws IOException If an I/O error occurs reading the header
      */
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index fec9c7f..e0c6146 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -77,6 +77,12 @@
         <code>Connection</code> HTTP headers looking for a specific token, be
         stricter in ensuring that the exact token is present. (markt)
       </fix>
+      <fix>
+        <bug>63829</bug>: Improve the check of the <code>Content-Encoding</code>
+        header when looking to see if Tomcat is serving pre-compressed content.
+        Ensure that only a full token is matched and that the match is case
+        insensitive. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Other">


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