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


The following commit(s) were added to refs/heads/10.1.x by this push:
     new 73c04a1039 Update the HTTP/2 overhead documentation - particularly 
code comments
73c04a1039 is described below

commit 73c04a10395774bda71a0b37802cf983662ce255
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Jul 31 14:53:16 2025 +0100

    Update the HTTP/2 overhead documentation - particularly code comments
---
 .../coyote/http2/Http2AsyncUpgradeHandler.java     |  3 +
 .../apache/coyote/http2/Http2UpgradeHandler.java   | 72 ++++++++++++++--------
 webapps/docs/changelog.xml                         |  6 ++
 webapps/docs/config/http2.xml                      |  5 +-
 4 files changed, 59 insertions(+), 27 deletions(-)

diff --git a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java 
b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
index fefedbdb77..80f9205b8f 100644
--- a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
@@ -132,6 +132,9 @@ public class Http2AsyncUpgradeHandler extends 
Http2UpgradeHandler {
             log.trace(sm.getString("upgradeHandler.rst.debug", connectionId, 
Integer.toString(se.getStreamId()),
                     se.getError(), se.getMessage()));
         }
+
+        increaseOverheadCount(FrameType.RST, 
getProtocol().getOverheadResetFactor());
+
         // Write a RST frame
         byte[] rstFrame = new byte[13];
         // Length
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java 
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 388d91151d..e006baf80c 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -585,6 +585,8 @@ class Http2UpgradeHandler extends AbstractStream implements 
InternalHttpUpgradeH
                     se.getError(), se.getMessage()));
         }
 
+        increaseOverheadCount(FrameType.RST, 
getProtocol().getOverheadResetFactor());
+
         // Write a RST frame
         byte[] rstFrame = new byte[13];
         // Length
@@ -1414,39 +1416,59 @@ class Http2UpgradeHandler extends AbstractStream 
implements InternalHttpUpgradeH
 
 
     void reduceOverheadCount(FrameType frameType) {
-        // A non-overhead frame reduces the overhead count by
-        // Http2Protocol.DEFAULT_OVERHEAD_REDUCTION_FACTOR. A simple browser
-        // request is likely to have one non-overhead frame (HEADERS) and one
-        // overhead frame (REPRIORITISE). With the default settings the 
overhead
-        // count will reduce by 10 for each simple request.
-        // Requests and responses with bodies will create additional
-        // non-overhead frames, further reducing the overhead count.
+        /*
+         * A non-overhead frame reduces the overhead count by {@code 
Http2Protocol.DEFAULT_OVERHEAD_REDUCTION_FACTOR}.
+         *
+         * A simple browser request is likely to have one non-overhead frame 
(HEADERS) that results in a response with
+         * one further non-overhead frame (DATA). With the default settings, 
the overhead count will reduce by 40 for
+         * each simple request.
+         *
+         * Requests and responses with bodies will create additional 
non-overhead frames, further reducing the overhead
+         * count.
+         */
         updateOverheadCount(frameType, 
Http2Protocol.DEFAULT_OVERHEAD_REDUCTION_FACTOR);
     }
 
 
     @Override
     public void increaseOverheadCount(FrameType frameType) {
-        // An overhead frame increases the overhead count by
-        // overheadCountFactor. By default, this means an overhead frame
-        // increases the overhead count by 10. A simple browser request is
-        // likely to have one non-overhead frame (HEADERS) and one overhead
-        // frame (REPRIORITISE). With the default settings the overhead count
-        // will reduce by 10 for each simple request.
+        /*
+         * An overhead frame (SETTINGS, PRIORITY, PING) increases the overhead 
count by overheadCountFactor. By default,
+         * this means an overhead frame increases the overhead count by 10.
+         *
+         * If the client ignores maxConcurrentStreams then any HEADERS frame 
received will also increase the overhead
+         * count by overheadCountFactor.
+         *
+         * A simple browser request should not trigger any overhead frames.
+         */
         updateOverheadCount(frameType, getProtocol().getOverheadCountFactor());
     }
 
 
-    private void increaseOverheadCount(FrameType frameType, int increment) {
-        // Overhead frames that indicate inefficient (and potentially 
malicious)
-        // use of small frames trigger an increase that is inversely
-        // proportional to size. The default threshold for all three potential
-        // areas for abuse (HEADERS, DATA, WINDOW_UPDATE) is 1024 bytes. Frames
-        // with sizes smaller than this will trigger an increase of
-        // threshold/size.
-        // DATA and WINDOW_UPDATE take an average over the last two non-final
-        // frames to allow for client buffering schemes that can result in some
-        // small DATA payloads.
+    /**
+     * Used to increase the overhead for frames that don't use the {@code 
overheadCountFactor} ({@code CONTINUATION},
+     * {@code DATA}, {@code WINDOW_UPDATE} and {@code RESET}).
+     *
+     * @param frameType The frame type triggering the overhead increase
+     * @param increment The amount by which the overhead is increased
+     */
+    protected void increaseOverheadCount(FrameType frameType, int increment) {
+        /*
+         * Three types of frame are susceptible to inefficient (and 
potentially malicious) use of small frames. These
+         * trigger an increase in overhead that is inversely proportional to 
size. The default threshold for all three
+         * potential areas for abuse (CONTINUATION, DATA, WINDOW_UPDATE) is 
1024 bytes. Frames with sizes smaller than
+         * this will trigger an increase of threshold/size.
+         *
+         * The check for DATA and WINDOW_UPDATE frames takes an average over 
the last two frames to allow for client
+         * buffering schemes that can result in some small DATA payloads.
+         *
+         * The CONTINUATION and DATA frames checks are skipped for end of 
headers (CONTINUATION) and end of stream
+         * (DATA) as those frames may be small for legitimate reasons.
+         *
+         * RESET frames (received or sent) trigger an increase of 
overheadResetFactor.
+         *
+         * In all cases, the calling method determines the extent to which the 
overhead count is increased.
+         */
         updateOverheadCount(frameType, increment);
     }
 
@@ -1655,9 +1677,9 @@ class Http2UpgradeHandler extends AbstractStream 
implements InternalHttpUpgradeH
             if (payloadSize < overheadThreshold) {
                 if (payloadSize == 0) {
                     // Avoid division by zero
-                    increaseOverheadCount(FrameType.HEADERS, 
overheadThreshold);
+                    increaseOverheadCount(FrameType.CONTINUATION, 
overheadThreshold);
                 } else {
-                    increaseOverheadCount(FrameType.HEADERS, overheadThreshold 
/ payloadSize);
+                    increaseOverheadCount(FrameType.CONTINUATION, 
overheadThreshold / payloadSize);
                 }
             }
         }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 4803b62d1f..e12b9d57d9 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -125,6 +125,12 @@
         integers. Note that the maximum permitted value of an HPACK decoded
         integer is <code>Integer.MAX_VALUE</code>. (markt)
       </fix>
+      <fix>
+        Update the HTTP/2 overhead documentation - particularly the code
+        comments - to reflect the deprecation of the <code>PRIORITY</code> 
frame
+        and clarify that a stream reset always triggers an overhead increase.
+        (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Cluster">
diff --git a/webapps/docs/config/http2.xml b/webapps/docs/config/http2.xml
index 702f31149c..4f288c9598 100644
--- a/webapps/docs/config/http2.xml
+++ b/webapps/docs/config/http2.xml
@@ -158,8 +158,9 @@
     <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>
+      frame received or sent. 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">


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

Reply via email to