Update the HTTP/2 overhead documentation - particularly code comments

Author: markt-asf

java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java

@@ -131,6 +131,9 @@ void sendStreamReset(StreamStateMachine state, StreamException se) throws IOExce
             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

java/org/apache/coyote/http2/Http2UpgradeHandler.java

@@ -582,6 +582,8 @@ void sendStreamReset(StreamStateMachine state, StreamException se) throws IOExce
                     se.getError(), se.getMessage()));
         }
 
+        increaseOverheadCount(FrameType.RST, getProtocol().getOverheadResetFactor());
+
         // Write a RST frame
         byte[] rstFrame = new byte[13];
         // Length
@@ -1411,39 +1413,59 @@ protected final String getConnectionId() {
 
 
     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);
     }
 
@@ -1652,9 +1674,9 @@ public void headersContinue(int payloadSize, boolean endOfHeaders) {
             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);
                 }
             }
         }

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">

webapps/docs/config/http2.xml

@@ -241,8 +241,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">