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

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

commit 76bb4bfbfeae827dce896f650655bbf6e251ed49
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Oct 5 20:54:14 2023 +0100

    Improvements to HTTP/2 overhead protection.
---
 java/org/apache/coyote/http2/Http2AsyncParser.java | 51 ++++++++++++----------
 java/org/apache/coyote/http2/Http2Protocol.java    | 19 +++++++-
 .../apache/coyote/http2/Http2UpgradeHandler.java   |  9 +++-
 webapps/docs/changelog.xml                         |  3 ++
 webapps/docs/config/http2.xml                      |  9 +++-
 5 files changed, 64 insertions(+), 27 deletions(-)

diff --git a/java/org/apache/coyote/http2/Http2AsyncParser.java 
b/java/org/apache/coyote/http2/Http2AsyncParser.java
index 1965123bc4..e1c8170765 100644
--- a/java/org/apache/coyote/http2/Http2AsyncParser.java
+++ b/java/org/apache/coyote/http2/Http2AsyncParser.java
@@ -284,36 +284,39 @@ class Http2AsyncParser extends Http2Parser {
                                     readUnknownFrame(streamId, frameTypeId, 
flags, payloadSize, payload);
                             }
                         }
-                        // See if there is a new 9 byte header and continue 
parsing if possible
-                        if (payload.remaining() >= 9) {
-                            int position = payload.position();
-                            payloadSize = ByteUtil.getThreeBytes(payload, 
position);
-                            frameTypeId = ByteUtil.getOneByte(payload, 
position + 3);
-                            frameType = FrameType.valueOf(frameTypeId);
-                            flags = ByteUtil.getOneByte(payload, position + 4);
-                            streamId = ByteUtil.get31Bits(payload, position + 
5);
-                            streamException = false;
-                            if (payload.remaining() - 9 >= payloadSize) {
-                                continueParsing = true;
-                                // Now go over frame header
-                                payload.position(payload.position() + 9);
-                                try {
-                                    validateFrame(null, frameType, streamId, 
flags, payloadSize);
-                                } catch (StreamException e) {
-                                    error = e;
-                                    streamException = true;
-                                } catch (Http2Exception e) {
-                                    error = e;
-                                    continueParsing = false;
+                        if (!upgradeHandler.isOverheadLimitExceeded()) {
+                            // See if there is a new 9 byte header and 
continue parsing if possible
+                            if (payload.remaining() >= 9) {
+                                int position = payload.position();
+                                payloadSize = ByteUtil.getThreeBytes(payload, 
position);
+                                frameTypeId = ByteUtil.getOneByte(payload, 
position + 3);
+                                frameType = FrameType.valueOf(frameTypeId);
+                                flags = ByteUtil.getOneByte(payload, position 
+ 4);
+                                streamId = ByteUtil.get31Bits(payload, 
position + 5);
+                                streamException = false;
+                                if (payload.remaining() - 9 >= payloadSize) {
+                                    continueParsing = true;
+                                    // Now go over frame header
+                                    payload.position(payload.position() + 9);
+                                    try {
+                                        validateFrame(null, frameType, 
streamId, flags, payloadSize);
+                                    } catch (StreamException e) {
+                                        error = e;
+                                        streamException = true;
+                                    } catch (Http2Exception e) {
+                                        error = e;
+                                        continueParsing = false;
+                                    }
                                 }
                             }
                         }
                     } while (continueParsing);
                 } catch (RuntimeException | IOException | Http2Exception e) {
                     error = e;
-                }
-                if (payload.hasRemaining()) {
-                    socketWrapper.unRead(payload);
+                } finally {
+                    if (payload.hasRemaining()) {
+                        socketWrapper.unRead(payload);
+                    }
                 }
             }
             if (state == CompletionState.DONE) {
diff --git a/java/org/apache/coyote/http2/Http2Protocol.java 
b/java/org/apache/coyote/http2/Http2Protocol.java
index 484d5d200a..8287af0e69 100644
--- a/java/org/apache/coyote/http2/Http2Protocol.java
+++ b/java/org/apache/coyote/http2/Http2Protocol.java
@@ -53,8 +53,10 @@ public class Http2Protocol implements UpgradeProtocol {
     // Maximum amount of streams which can be concurrently executed over
     // a single connection
     static final int DEFAULT_MAX_CONCURRENT_STREAM_EXECUTION = 20;
-
+    // Default factor used when adjusting overhead count for overhead frames
     static final int DEFAULT_OVERHEAD_COUNT_FACTOR = 10;
+    // Default factor used when adjusting overhead count for reset frames
+    static final int DEFAULT_OVERHEAD_RESET_FACTOR = 50;
     // Not currently configurable. This makes the practical limit for
     // overheadCountFactor to be ~20. The exact limit will vary with traffic
     // patterns.
@@ -85,6 +87,7 @@ public class Http2Protocol implements UpgradeProtocol {
     private int maxHeaderCount = Constants.DEFAULT_MAX_HEADER_COUNT;
     private int maxTrailerCount = Constants.DEFAULT_MAX_TRAILER_COUNT;
     private int overheadCountFactor = DEFAULT_OVERHEAD_COUNT_FACTOR;
+    private int overheadResetFactor = DEFAULT_OVERHEAD_RESET_FACTOR;
     private int overheadContinuationThreshold = 
DEFAULT_OVERHEAD_CONTINUATION_THRESHOLD;
     private int overheadDataThreshold = DEFAULT_OVERHEAD_DATA_THRESHOLD;
     private int overheadWindowUpdateThreshold = 
DEFAULT_OVERHEAD_WINDOW_UPDATE_THRESHOLD;
@@ -290,6 +293,20 @@ public class Http2Protocol implements UpgradeProtocol {
     }
 
 
+    public int getOverheadResetFactor() {
+        return overheadResetFactor;
+    }
+
+
+    public void setOverheadResetFactor(int overheadResetFactor) {
+        if (overheadResetFactor < 0) {
+            this.overheadResetFactor = 0;
+        } else {
+            this.overheadResetFactor = overheadResetFactor;
+        }
+    }
+
+
     public int getOverheadContinuationThreshold() {
         return overheadContinuationThreshold;
     }
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java 
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 53b75b5457..cfba750566 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -378,7 +378,7 @@ class Http2UpgradeHandler extends AbstractStream implements 
InternalHttpUpgradeH
                                     stream.close(se);
                                 }
                             } finally {
-                                if (overheadCount.get() > 0) {
+                                if (isOverheadLimitExceeded()) {
                                     throw new ConnectionException(
                                             
sm.getString("upgradeHandler.tooMuchOverhead", connectionId),
                                             Http2Error.ENHANCE_YOUR_CALM);
@@ -1428,6 +1428,11 @@ class Http2UpgradeHandler extends AbstractStream 
implements InternalHttpUpgradeH
     }
 
 
+    boolean isOverheadLimitExceeded() {
+        return overheadCount.get() > 0;
+    }
+
+
     // ----------------------------------------------- Http2Parser.Input 
methods
 
     @Override
@@ -1707,6 +1712,7 @@ class Http2UpgradeHandler extends AbstractStream 
implements InternalHttpUpgradeH
             log.debug(sm.getString("upgradeHandler.reset.receive", 
getConnectionId(), Integer.toString(streamId),
                     Long.toString(errorCode)));
         }
+        increaseOverheadCount(FrameType.RST, 
getProtocol().getOverheadResetFactor());
         AbstractNonZeroStream abstractNonZeroStream = 
getAbstractNonZeroStream(streamId, true);
         abstractNonZeroStream.checkState(FrameType.RST);
         if (abstractNonZeroStream instanceof Stream) {
@@ -1840,6 +1846,7 @@ class Http2UpgradeHandler extends AbstractStream 
implements InternalHttpUpgradeH
 
     @Override
     public void priorityUpdate(int prioritizedStreamID, Priority p) throws 
Http2Exception {
+        increaseOverheadCount(FrameType.PRIORITY_UPDATE);
         AbstractNonZeroStream abstractNonZeroStream = 
getAbstractNonZeroStream(prioritizedStreamID, true);
         if (abstractNonZeroStream instanceof Stream) {
             Stream stream = (Stream) abstractNonZeroStream;
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 20ef9f669a..37b7774513 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -167,6 +167,9 @@
       <fix>
         Align validation of HTTP trailer fields with standard fields. (markt)
       </fix>
+      <fix>
+        Improvements to HTTP/2 overhead protection. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">
diff --git a/webapps/docs/config/http2.xml b/webapps/docs/config/http2.xml
index 629b135cc2..f78aa03538 100644
--- a/webapps/docs/config/http2.xml
+++ b/webapps/docs/config/http2.xml
@@ -139,7 +139,7 @@
       count starts at <code>-10 * overheadCountFactor</code>. The count is
       decreased by 20 for each data frame sent or received and each headers 
frame
       received. The count is increased by the <code>overheadCountFactor</code>
-      for each setting received, priority frame received and ping received. If
+      for each setting, priority, priority update and ping frame received. If
       the overhead count exceeds zero, the connection is closed. A value of 
less
       than <code>1</code> disables this protection. In normal usage a value of
       approximately <code>20</code> or higher will close the connection before
@@ -147,6 +147,13 @@
       <code>10</code> will be used.</p>
     </attribute>
 
+    <attribute name="overheadResetFactor" required="false">
+      <p>The amount by which the overhead count (see
+      <strong>overheadCountFactor</strong>) will be increased for each reset
+      frame received. If not specified, a default value of <code>50</code> will
+      be used. A value of less than zero will be treated as zero.</p>
+    </attribute>
+
     <attribute name="overheadDataThreshold" required="false">
       <p>The threshold below which the average payload size of the current and
       previous non-final <code>DATA</code> frames will trigger an increase in


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

Reply via email to