You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2020/02/03 20:53:50 UTC

[tomcat] branch master updated: Share more configuration between HTTP/1.1 and nested HTTP/2

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

remm 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 2aa5f6b  Share more configuration between HTTP/1.1 and nested HTTP/2
2aa5f6b is described below

commit 2aa5f6bbb4a88b5d806b4289430ce9025ca5c75e
Author: remm <re...@apache.org>
AuthorDate: Mon Feb 3 21:53:29 2020 +0100

    Share more configuration between HTTP/1.1 and nested HTTP/2
    
    maxHeaderCount and maxTrailerCount attributes remain on the HTTP/2
    UpgradeProtocol element as the functionality is not present in HTTP/1.1.
    Maybe this could be added at some point.
---
 TOMCAT-NEXT.txt                                    |   2 -
 .../coyote/http11/AbstractHttp11Protocol.java      |   3 +
 java/org/apache/coyote/http2/Http2Protocol.java    | 110 ++-------------------
 test/org/apache/coyote/http2/Http2TestBase.java    |   2 +
 .../org/apache/coyote/http2/TestAbortedUpload.java |   3 +-
 test/org/apache/coyote/http2/TestHttp2Limits.java  |  12 +--
 .../apache/coyote/http2/TestHttp2Section_8_1.java  |   4 +-
 webapps/docs/changelog.xml                         |   5 +
 webapps/docs/config/http2.xml                      |  90 ++---------------
 9 files changed, 36 insertions(+), 195 deletions(-)

diff --git a/TOMCAT-NEXT.txt b/TOMCAT-NEXT.txt
index 332bc9b..95d6376 100644
--- a/TOMCAT-NEXT.txt
+++ b/TOMCAT-NEXT.txt
@@ -49,8 +49,6 @@ New items for 10.0.0.x onwards:
 
  8. Consider disabling the AJP connector by default.
 
- 9. Share configuration between HTTP/1.1 and nested HTTP/2 rather than duplicating.
-
 
 Deferred until 10.0.x:
 
diff --git a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
index 58b05a3..fd3ab74 100644
--- a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
+++ b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
@@ -414,6 +414,9 @@ public abstract class AbstractHttp11Protocol<S> extends AbstractProtocol<S> {
     protected Set<String> getAllowedTrailerHeadersInternal() {
         return allowedTrailerHeaders;
     }
+    public boolean isTrailerHeaderAllowed(String headerName) {
+        return allowedTrailerHeaders.contains(headerName);
+    }
     public String getAllowedTrailerHeaders() {
         // Chances of a size change between these lines are small enough that a
         // sync is unnecessary.
diff --git a/java/org/apache/coyote/http2/Http2Protocol.java b/java/org/apache/coyote/http2/Http2Protocol.java
index aab980c..56cb8e1 100644
--- a/java/org/apache/coyote/http2/Http2Protocol.java
+++ b/java/org/apache/coyote/http2/Http2Protocol.java
@@ -17,27 +17,18 @@
 package org.apache.coyote.http2;
 
 import java.nio.charset.StandardCharsets;
-import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Enumeration;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Locale;
-import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.regex.Pattern;
 
 import org.apache.coyote.AbstractProtocol;
 import org.apache.coyote.Adapter;
-import org.apache.coyote.CompressionConfig;
 import org.apache.coyote.Processor;
 import org.apache.coyote.Request;
 import org.apache.coyote.Response;
 import org.apache.coyote.UpgradeProtocol;
 import org.apache.coyote.UpgradeToken;
+import org.apache.coyote.http11.AbstractHttp11Protocol;
 import org.apache.coyote.http11.upgrade.InternalHttpUpgradeHandler;
 import org.apache.coyote.http11.upgrade.UpgradeProcessorInternal;
-import org.apache.tomcat.util.buf.StringUtils;
 import org.apache.tomcat.util.net.SocketWrapperBase;
 
 public class Http2Protocol implements UpgradeProtocol {
@@ -77,12 +68,8 @@ public class Http2Protocol implements UpgradeProtocol {
     // change the default defined in ConnectionSettingsBase.
     private int initialWindowSize = ConnectionSettingsBase.DEFAULT_INITIAL_WINDOW_SIZE;
     // Limits
-    private Set<String> allowedTrailerHeaders =
-            Collections.newSetFromMap(new ConcurrentHashMap<String, Boolean>());
     private int maxHeaderCount = Constants.DEFAULT_MAX_HEADER_COUNT;
-    private int maxHeaderSize = Constants.DEFAULT_MAX_HEADER_SIZE;
     private int maxTrailerCount = Constants.DEFAULT_MAX_TRAILER_COUNT;
-    private int maxTrailerSize = Constants.DEFAULT_MAX_TRAILER_SIZE;
     private int overheadCountFactor = DEFAULT_OVERHEAD_COUNT_FACTOR;
     private int overheadContinuationThreshold = DEFAULT_OVERHEAD_CONTINUATION_THRESHOLD;
     private int overheadDataThreshold = DEFAULT_OVERHEAD_DATA_THRESHOLD;
@@ -90,10 +77,8 @@ public class Http2Protocol implements UpgradeProtocol {
 
     private boolean initiatePingDisabled = false;
     private boolean useSendfile = true;
-    // Compression
-    private final CompressionConfig compressionConfig = new CompressionConfig();
     // Reference to HTTP/1.1 protocol that this instance is configured under
-    private AbstractProtocol<?> http11Protocol = null;
+    private AbstractHttp11Protocol<?> http11Protocol = null;
 
     @Override
     public String getHttpUpgradeName(boolean isSSLEnabled) {
@@ -243,37 +228,8 @@ public class Http2Protocol implements UpgradeProtocol {
     }
 
 
-    public void setAllowedTrailerHeaders(String commaSeparatedHeaders) {
-        // Jump through some hoops so we don't end up with an empty set while
-        // doing updates.
-        Set<String> toRemove = new HashSet<>();
-        toRemove.addAll(allowedTrailerHeaders);
-        if (commaSeparatedHeaders != null) {
-            String[] headers = commaSeparatedHeaders.split(",");
-            for (String header : headers) {
-                String trimmedHeader = header.trim().toLowerCase(Locale.ENGLISH);
-                if (toRemove.contains(trimmedHeader)) {
-                    toRemove.remove(trimmedHeader);
-                } else {
-                    allowedTrailerHeaders.add(trimmedHeader);
-                }
-            }
-            allowedTrailerHeaders.removeAll(toRemove);
-        }
-    }
-
-
-    public String getAllowedTrailerHeaders() {
-        // Chances of a size change between these lines are small enough that a
-        // sync is unnecessary.
-        List<String> copy = new ArrayList<>(allowedTrailerHeaders.size());
-        copy.addAll(allowedTrailerHeaders);
-        return StringUtils.join(copy);
-    }
-
-
     boolean isTrailerHeaderAllowed(String headerName) {
-        return allowedTrailerHeaders.contains(headerName);
+        return http11Protocol.isTrailerHeaderAllowed(headerName);
     }
 
 
@@ -287,13 +243,8 @@ public class Http2Protocol implements UpgradeProtocol {
     }
 
 
-    public void setMaxHeaderSize(int maxHeaderSize) {
-        this.maxHeaderSize = maxHeaderSize;
-    }
-
-
     public int getMaxHeaderSize() {
-        return maxHeaderSize;
+        return http11Protocol.getMaxHttpHeaderSize();
     }
 
 
@@ -307,13 +258,8 @@ public class Http2Protocol implements UpgradeProtocol {
     }
 
 
-    public void setMaxTrailerSize(int maxTrailerSize) {
-        this.maxTrailerSize = maxTrailerSize;
-    }
-
-
     public int getMaxTrailerSize() {
-        return maxTrailerSize;
+        return http11Protocol.getMaxTrailerSize();
     }
 
 
@@ -367,57 +313,17 @@ public class Http2Protocol implements UpgradeProtocol {
     }
 
 
-    public void setCompression(String compression) {
-        compressionConfig.setCompression(compression);
-    }
-    public String getCompression() {
-        return compressionConfig.getCompression();
-    }
-    protected int getCompressionLevel() {
-        return compressionConfig.getCompressionLevel();
-    }
-
-
-    public String getNoCompressionUserAgents() {
-        return compressionConfig.getNoCompressionUserAgents();
-    }
-    protected Pattern getNoCompressionUserAgentsPattern() {
-        return compressionConfig.getNoCompressionUserAgentsPattern();
-    }
-    public void setNoCompressionUserAgents(String noCompressionUserAgents) {
-        compressionConfig.setNoCompressionUserAgents(noCompressionUserAgents);
-    }
-
-
-    public String getCompressibleMimeType() {
-        return compressionConfig.getCompressibleMimeType();
-    }
-    public void setCompressibleMimeType(String valueS) {
-        compressionConfig.setCompressibleMimeType(valueS);
-    }
-    public String[] getCompressibleMimeTypes() {
-        return compressionConfig.getCompressibleMimeTypes();
-    }
-
-
-    public int getCompressionMinSize() {
-        return compressionConfig.getCompressionMinSize();
-    }
-    public void setCompressionMinSize(int compressionMinSize) {
-        compressionConfig.setCompressionMinSize(compressionMinSize);
-    }
-
-
     public boolean useCompression(Request request, Response response) {
-        return compressionConfig.useCompression(request, response);
+        return http11Protocol.useCompression(request, response);
     }
 
 
     public AbstractProtocol<?> getHttp11Protocol() {
         return this.http11Protocol;
     }
+
     @Override
     public void setHttp11Protocol(AbstractProtocol<?> http11Protocol) {
-        this.http11Protocol = http11Protocol;
+        this.http11Protocol = (AbstractHttp11Protocol<?>) http11Protocol;
     }
 }
diff --git a/test/org/apache/coyote/http2/Http2TestBase.java b/test/org/apache/coyote/http2/Http2TestBase.java
index 7bd341c..2afeb92 100644
--- a/test/org/apache/coyote/http2/Http2TestBase.java
+++ b/test/org/apache/coyote/http2/Http2TestBase.java
@@ -47,6 +47,7 @@ import org.apache.catalina.connector.Connector;
 import org.apache.catalina.startup.Tomcat;
 import org.apache.catalina.startup.TomcatBaseTest;
 import org.apache.catalina.util.IOTools;
+import org.apache.coyote.http11.Http11NioProtocol;
 import org.apache.coyote.http2.HpackDecoder.HeaderEmitter;
 import org.apache.coyote.http2.Http2Parser.Input;
 import org.apache.coyote.http2.Http2Parser.Output;
@@ -555,6 +556,7 @@ public abstract class Http2TestBase extends TomcatBaseTest {
         http2Protocol.setStreamReadTimeout(3000);
         http2Protocol.setStreamWriteTimeout(3000);
         http2Protocol.setMaxConcurrentStreams(maxConcurrentStreams);
+        http2Protocol.setHttp11Protocol(new Http11NioProtocol());
         connector.addUpgradeProtocol(http2Protocol);
     }
 
diff --git a/test/org/apache/coyote/http2/TestAbortedUpload.java b/test/org/apache/coyote/http2/TestAbortedUpload.java
index da130c3..46083b3 100644
--- a/test/org/apache/coyote/http2/TestAbortedUpload.java
+++ b/test/org/apache/coyote/http2/TestAbortedUpload.java
@@ -30,6 +30,7 @@ import org.junit.Test;
 import org.apache.catalina.Context;
 import org.apache.catalina.LifecycleException;
 import org.apache.catalina.startup.Tomcat;
+import org.apache.coyote.http11.AbstractHttp11Protocol;
 
 public class TestAbortedUpload extends Http2TestBase {
 
@@ -37,7 +38,7 @@ public class TestAbortedUpload extends Http2TestBase {
     public void testAbortedRequest() throws Exception {
         http2Connect();
 
-        http2Protocol.setAllowedTrailerHeaders(TRAILER_HEADER_NAME);
+        ((AbstractHttp11Protocol<?>) http2Protocol.getHttp11Protocol()).setAllowedTrailerHeaders(TRAILER_HEADER_NAME);
 
         int bodySize = 8192;
         int bodyCount = 20;
diff --git a/test/org/apache/coyote/http2/TestHttp2Limits.java b/test/org/apache/coyote/http2/TestHttp2Limits.java
index d5f109f..d3ad8c8 100644
--- a/test/org/apache/coyote/http2/TestHttp2Limits.java
+++ b/test/org/apache/coyote/http2/TestHttp2Limits.java
@@ -29,6 +29,7 @@ import org.junit.Assert;
 import org.junit.Test;
 
 import org.apache.catalina.connector.Connector;
+import org.apache.coyote.http11.AbstractHttp11Protocol;
 import org.apache.coyote.http2.HpackEncoder.State;
 import org.apache.tomcat.util.http.MimeHeaders;
 import org.apache.tomcat.util.res.StringManager;
@@ -109,7 +110,6 @@ public class TestHttp2Limits extends Http2TestBase {
         // Bug 60232
         doTestHeaderLimits(1, 12*1024, 1024, FailureMode.STREAM_RESET);
 
-
         output.clearTrace();
         sendSimpleGetRequest(5);
         parser.readFrame(true);
@@ -195,11 +195,11 @@ public class TestHttp2Limits extends Http2TestBase {
         }
 
         enableHttp2();
+        configureAndStartWebApplication();
 
         http2Protocol.setMaxHeaderCount(maxHeaderCount);
-        http2Protocol.setMaxHeaderSize(maxHeaderSize);
+        ((AbstractHttp11Protocol<?>) http2Protocol.getHttp11Protocol()).setMaxHttpHeaderSize(maxHeaderSize);
 
-        configureAndStartWebApplication();
         openClientConnection();
         doHttpUpgrade();
         sendClientPreface();
@@ -437,12 +437,12 @@ public class TestHttp2Limits extends Http2TestBase {
     private void doTestPostWithTrailerHeaders(int maxTrailerCount, int maxTrailerSize,
             FailureMode failMode) throws Exception {
         enableHttp2();
+        configureAndStartWebApplication();
 
-        http2Protocol.setAllowedTrailerHeaders(TRAILER_HEADER_NAME);
+        ((AbstractHttp11Protocol<?>) http2Protocol.getHttp11Protocol()).setAllowedTrailerHeaders(TRAILER_HEADER_NAME);
         http2Protocol.setMaxTrailerCount(maxTrailerCount);
-        http2Protocol.setMaxTrailerSize(maxTrailerSize);
+        ((AbstractHttp11Protocol<?>) http2Protocol.getHttp11Protocol()).setMaxTrailerSize(maxTrailerSize);
 
-        configureAndStartWebApplication();
         openClientConnection();
         doHttpUpgrade();
         sendClientPreface();
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_8_1.java b/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
index cd44280..b14fc90 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
@@ -23,6 +23,8 @@ import java.util.List;
 import org.junit.Assert;
 import org.junit.Test;
 
+import org.apache.coyote.http11.AbstractHttp11Protocol;
+
 /**
  * Unit tests for Section 8.1 of
  * <a href="https://tools.ietf.org/html/rfc7540">RFC 7540</a>.
@@ -47,7 +49,7 @@ public class TestHttp2Section_8_1 extends Http2TestBase {
     private void doTestPostWithTrailerHeaders(boolean allowTrailerHeader) throws Exception{
         http2Connect();
         if (allowTrailerHeader) {
-            http2Protocol.setAllowedTrailerHeaders(TRAILER_HEADER_NAME);
+            ((AbstractHttp11Protocol<?>) http2Protocol.getHttp11Protocol()).setAllowedTrailerHeaders(TRAILER_HEADER_NAME);
         }
 
         byte[] headersFrameHeader = new byte[9];
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index b9fb1c3..fb86ab4 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -93,6 +93,11 @@
         When reporting / logging invalid HTTP headers encode any non-printing
         characters using the 0xNN form. (markt)
       </add>
+      <update>
+        Remove duplication of HTTP/1.1 configuration on the HTTP/2
+        UpgradeProtocol element. Configuration from the main Connector element
+        will now be used. (remm)
+      </update>
     </changelog>
   </subsection>
   <subsection name="Other">
diff --git a/webapps/docs/config/http2.xml b/webapps/docs/config/http2.xml
index 00f75cb..98fd49b 100644
--- a/webapps/docs/config/http2.xml
+++ b/webapps/docs/config/http2.xml
@@ -71,52 +71,6 @@
 
   <attributes>
 
-    <attribute name="allowedTrailerHeaders" required="false">
-      <p>By default Tomcat will ignore all trailer headers when processing
-      HTTP/2 connections. For a header to be processed, it must be added to this
-      comma-separated list of header names.</p>
-    </attribute>
-
-    <attribute name="compressibleMimeType" required="false">
-      <p>The value is a comma separated list of MIME types for which HTTP
-      compression may be used.
-      The default value is
-      <code>
-      text/html,text/xml,text/plain,text/css,text/javascript,application/javascript,application/json,application/xml
-      </code>.
-      </p>
-    </attribute>
-
-    <attribute name="compression" required="false">
-      <p>The HTTP/2 protocol may use compression in an attempt to save server
-      bandwidth. The acceptable values for the parameter is "off" (disable
-      compression), "on" (allow compression, which causes text data to be
-      compressed), "force" (forces compression in all cases), or a numerical
-      integer value (which is equivalent to "on", but specifies the minimum
-      amount of data before the output is compressed). If the content-length is
-      not known and compression is set to "on" or more aggressive, the output
-      will also be compressed. If not specified, this attribute is set to
-      "off".</p>
-      <p><em>Note</em>: There is a tradeoff between using compression (saving
-      your bandwidth) and using the sendfile feature (saving your CPU cycles).
-      If the connector supports the sendfile feature, e.g. the NIO2 connector,
-      using sendfile will take precedence over compression. The symptoms will
-      be that static files greater that 48 Kb will be sent uncompressed.
-      You can turn off sendfile by setting <code>useSendfile</code> attribute
-      of the protocol, as documented below, or change the sendfile usage
-      threshold in the configuration of the
-      <a href="../default-servlet.html">DefaultServlet</a> in the default
-      <code>conf/web.xml</code> or in the <code>web.xml</code> of your web
-      application.
-      </p>
-    </attribute>
-
-    <attribute name="compressionMinSize" required="false">
-      <p>If <strong>compression</strong> is set to "on" then this attribute
-      may be used to specify the minimum amount of data before the output is
-      compressed. If not specified, this attribute is defaults to "2048".</p>
-    </attribute>
-
     <attribute name="initialWindowSize" required="false">
       <p>Controls the initial size of the flow control window for streams that
       Tomcat advertises to clients. If not specified, the default value of
@@ -152,16 +106,6 @@
       If not specified, a default of 100 is used.</p>
     </attribute>
 
-    <attribute name="maxHeaderSize" required="false">
-      <p>The maximum total size for all headers in a request that is allowed by
-      the container. Total size for a header is calculated as the uncompressed
-      size of the header name in bytes, plus the uncompressed size of the header
-      value in bytes plus an HTTP/2 overhead of 3 bytes per header. A request
-      that contains a set of headers that requires more than the specified limit
-      will be rejected. A value of less than 0 means no limit. If not specified,
-      a default of 8192 is used.</p>
-    </attribute>
-
     <attribute name="maxTrailerCount" required="false">
       <p>The maximum number of trailer headers in a request that is allowed by
       the container. A request that contains more trailer headers than the
@@ -169,33 +113,6 @@
       If not specified, a default of 100 is used.</p>
     </attribute>
 
-    <attribute name="maxTrailerSize" required="false">
-      <p>The maximum total size for all trailer headers in a request that is
-      allowed by the container. Total size for a header is calculated as the
-      uncompressed size of the header name in bytes, plus the uncompressed size
-      of the header value in bytes plus an HTTP/2 overhead of 3 bytes per
-      header. A request that contains a set of trailer headers that requires
-      more than the specified limit will be rejected. A value of less than 0
-      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
-      compression should not be used,
-      because these clients, although they do advertise support for the
-      feature, have a broken implementation.
-      The default value is an empty String (regexp matching disabled).</p>
-    </attribute>
-
     <attribute name="overheadContinuationThreshold" required="false">
       <p>The threshold below which the payload size of a non-final
       <code>CONTINUATION</code> frame will trigger an increase in the overhead
@@ -285,10 +202,17 @@
   <a href="http.html">HTTP Connector</a> it is nested with:</p>
 
   <ul>
+    <li>allowedTrailerHeaders</li>
+    <li>compressibleMimeType</li>
+    <li>compression</li>
+    <li>compressionMinSize</li>
     <li>maxCookieCount</li>
+    <li>maxHeaderSize</li>
     <li>maxParameterCount</li>
     <li>maxPostSize</li>
     <li>maxSavePostSize</li>
+    <li>maxTrailerSize</li>
+    <li>noCompressionUserAgents</li>
   </ul>
 
   </subsection>


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