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

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


The following commit(s) were added to refs/heads/9.0.x by this push:
     new 1ebba3817e Fix BZ 69713 - Correctly handle padding when content-length 
is present
1ebba3817e is described below

commit 1ebba3817e0b7ab58cba970821058217946c9ce3
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Jun 17 17:02:40 2025 +0100

    Fix BZ 69713 - Correctly handle padding when content-length is present
    
    https://bz.apache.org/bugzilla/show_bug.cgi?id=69713
---
 .../apache/coyote/http2/AbstractNonZeroStream.java |  4 +-
 java/org/apache/coyote/http2/Http2Parser.java      |  6 +--
 .../apache/coyote/http2/Http2UpgradeHandler.java   |  8 ++--
 java/org/apache/coyote/http2/RecycledStream.java   |  4 +-
 java/org/apache/coyote/http2/Stream.java           |  4 +-
 test/org/apache/coyote/http2/Http2TestBase.java    |  8 ++--
 .../apache/coyote/http2/TestHttp2Section_6_1.java  | 53 ++++++++++++++++++++++
 webapps/docs/changelog.xml                         |  8 ++++
 8 files changed, 78 insertions(+), 17 deletions(-)

diff --git a/java/org/apache/coyote/http2/AbstractNonZeroStream.java 
b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
index 6dc7c19077..d4fbcf526d 100644
--- a/java/org/apache/coyote/http2/AbstractNonZeroStream.java
+++ b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
@@ -85,9 +85,9 @@ abstract class AbstractNonZeroStream extends AbstractStream {
     /**
      * Notify that some data has been received.
      *
-     * @param payloadSize the byte count
+     * @param dataLength the byte count
      *
      * @throws Http2Exception if an error is detected
      */
-    abstract void receivedData(int payloadSize) throws Http2Exception;
+    abstract void receivedData(int dataLength) throws Http2Exception;
 }
diff --git a/java/org/apache/coyote/http2/Http2Parser.java 
b/java/org/apache/coyote/http2/Http2Parser.java
index 90c1142616..74b770a69d 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -188,7 +188,7 @@ class Http2Parser {
                     Integer.toString(dataLength), padding));
         }
 
-        ByteBuffer dest = output.startRequestBodyFrame(streamId, payloadSize, 
endOfStream);
+        ByteBuffer dest = output.startRequestBodyFrame(streamId, dataLength, 
endOfStream);
         if (dest == null) {
             swallowPayload(streamId, FrameType.DATA.getId(), dataLength, 
false, buffer);
             // Process padding before sending any notifications in case padding
@@ -201,7 +201,7 @@ class Http2Parser {
             }
         } else {
             synchronized (dest) {
-                if (dest.remaining() < payloadSize) {
+                if (dest.remaining() < dataLength) {
                     // Client has sent more data than permitted by Window size
                     swallowPayload(streamId, FrameType.DATA.getId(), 
dataLength, false, buffer);
                     if (Flags.hasPadding(flags)) {
@@ -785,7 +785,7 @@ class Http2Parser {
         HpackDecoder getHpackDecoder();
 
         // Data frames
-        ByteBuffer startRequestBodyFrame(int streamId, int payloadSize, 
boolean endOfStream) throws Http2Exception;
+        ByteBuffer startRequestBodyFrame(int streamId, int dataLength, boolean 
endOfStream) throws Http2Exception;
 
         void endRequestBodyFrame(int streamId, int dataLength) throws 
Http2Exception, IOException;
 
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java 
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index d489a28783..51bf389238 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -1520,7 +1520,7 @@ class Http2UpgradeHandler extends AbstractStream 
implements InternalHttpUpgradeH
 
 
     @Override
-    public ByteBuffer startRequestBodyFrame(int streamId, int payloadSize, 
boolean endOfStream) throws Http2Exception {
+    public ByteBuffer startRequestBodyFrame(int streamId, int dataLength, 
boolean endOfStream) throws Http2Exception {
         // DATA frames reduce the overhead count ...
         reduceOverheadCount(FrameType.DATA);
 
@@ -1534,8 +1534,8 @@ class Http2UpgradeHandler extends AbstractStream 
implements InternalHttpUpgradeH
         // average over two frames to avoid false positives.
         if (!endOfStream) {
             int overheadThreshold = protocol.getOverheadDataThreshold();
-            int average = (lastNonFinalDataPayload >> 1) + (payloadSize >> 1);
-            lastNonFinalDataPayload = payloadSize;
+            int average = (lastNonFinalDataPayload >> 1) + (dataLength >> 1);
+            lastNonFinalDataPayload = dataLength;
             // Avoid division by zero
             if (average == 0) {
                 average = 1;
@@ -1547,7 +1547,7 @@ class Http2UpgradeHandler extends AbstractStream 
implements InternalHttpUpgradeH
 
         AbstractNonZeroStream abstractNonZeroStream = 
getAbstractNonZeroStream(streamId, true);
         abstractNonZeroStream.checkState(FrameType.DATA);
-        abstractNonZeroStream.receivedData(payloadSize);
+        abstractNonZeroStream.receivedData(dataLength);
         ByteBuffer result = abstractNonZeroStream.getInputByteBuffer(true);
 
         if (log.isTraceEnabled()) {
diff --git a/java/org/apache/coyote/http2/RecycledStream.java 
b/java/org/apache/coyote/http2/RecycledStream.java
index 4d0da6c34f..04924f289a 100644
--- a/java/org/apache/coyote/http2/RecycledStream.java
+++ b/java/org/apache/coyote/http2/RecycledStream.java
@@ -47,8 +47,8 @@ class RecycledStream extends AbstractNonZeroStream {
 
 
     @Override
-    void receivedData(int payloadSize) throws ConnectionException {
-        remainingFlowControlWindow -= payloadSize;
+    void receivedData(int dataLength) throws ConnectionException {
+        remainingFlowControlWindow -= dataLength;
     }
 
 
diff --git a/java/org/apache/coyote/http2/Stream.java 
b/java/org/apache/coyote/http2/Stream.java
index cac39350e3..012b1efebf 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -676,8 +676,8 @@ class Stream extends AbstractNonZeroStream implements 
HeaderEmitter {
 
 
     @Override
-    final void receivedData(int payloadSize) throws Http2Exception {
-        contentLengthReceived += payloadSize;
+    final void receivedData(int dataLength) throws Http2Exception {
+        contentLengthReceived += dataLength;
         long contentLengthHeader = coyoteRequest.getContentLengthLong();
         if (contentLengthHeader > -1 && contentLengthReceived > 
contentLengthHeader) {
             throw new ConnectionException(
diff --git a/test/org/apache/coyote/http2/Http2TestBase.java 
b/test/org/apache/coyote/http2/Http2TestBase.java
index a87540dca9..999f9057e0 100644
--- a/test/org/apache/coyote/http2/Http2TestBase.java
+++ b/test/org/apache/coyote/http2/Http2TestBase.java
@@ -1089,14 +1089,14 @@ public abstract class Http2TestBase extends 
TomcatBaseTest {
 
 
         @Override
-        public ByteBuffer startRequestBodyFrame(int streamId, int payloadSize, 
boolean endOfStream) {
+        public ByteBuffer startRequestBodyFrame(int streamId, int dataLength, 
boolean endOfStream) {
             lastStreamId = Integer.toString(streamId);
-            bytesRead += payloadSize;
+            bytesRead += dataLength;
             if (traceBody) {
-                bodyBuffer = ByteBuffer.allocate(payloadSize);
+                bodyBuffer = ByteBuffer.allocate(dataLength);
                 return bodyBuffer;
             } else {
-                trace.append(lastStreamId + "-Body-" + payloadSize + "\n");
+                trace.append(lastStreamId + "-Body-" + dataLength + "\n");
                 return null;
             }
         }
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_6_1.java 
b/test/org/apache/coyote/http2/TestHttp2Section_6_1.java
index 27c576fb4b..353e71108d 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_6_1.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_6_1.java
@@ -16,6 +16,7 @@
  */
 package org.apache.coyote.http2;
 
+import java.nio.ByteBuffer;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -88,6 +89,58 @@ public class TestHttp2Section_6_1 extends Http2TestBase {
     }
 
 
+    @Test
+    public void testDataFrameWithPaddingAndContentLength() throws Exception {
+        Logger.getLogger("org.apache.coyote").setLevel(Level.ALL);
+        Logger.getLogger("org.apache.tomcat.util.net").setLevel(Level.ALL);
+        try {
+            http2Connect();
+
+            // Disable overhead protection for window update as it breaks the
+            // test
+            http2Protocol.setOverheadWindowUpdateThreshold(0);
+
+            byte[] padding = new byte[8];
+
+            byte[] headersFrameHeader = new byte[9];
+            ByteBuffer headersPayload = ByteBuffer.allocate(128);
+            byte[] dataFrameHeader = new byte[9];
+            ByteBuffer dataPayload = ByteBuffer.allocate(128);
+
+            // 128 byte payload, less 8 bytes padding, less 1 padding byte 
gives 119 bytes
+            buildPostRequest(headersFrameHeader, headersPayload, false, null, 
119, "/simple", dataFrameHeader,
+                    dataPayload, padding, false, 3);
+            writeFrame(headersFrameHeader, headersPayload);
+            writeFrame(dataFrameHeader, dataPayload);
+
+            readSimplePostResponse(true);
+
+            // The window updates for padding could occur anywhere since they
+            // happen on a different thread to the response.
+            // The connection window update is always present if there is
+            // padding.
+            String trace = output.getTrace();
+            String paddingWindowUpdate = "0-WindowSize-[9]\n";
+            Assert.assertTrue(trace, trace.contains(paddingWindowUpdate));
+            trace = trace.replace(paddingWindowUpdate, "");
+
+            // The stream window update may or may not be present depending on
+            // timing. Remove it if present.
+            if (trace.contains("3-WindowSize-[9]\n")) {
+                trace = trace.replace("3-WindowSize-[9]\n", "");
+            }
+
+            Assert.assertEquals("0-WindowSize-[119]\n" + 
"3-WindowSize-[119]\n" + "3-HeadersStart\n" +
+                    "3-Header-[:status]-[200]\n" + 
"3-Header-[content-length]-[119]\n" +
+                    "3-Header-[date]-[Wed, 11 Nov 2015 19:18:42 GMT]\n" + 
"3-HeadersEnd\n" + "3-Body-119\n" +
+                    "3-EndOfStream\n", trace);
+        } finally {
+            Logger.getLogger("org.apache.coyote").setLevel(Level.INFO);
+            
Logger.getLogger("org.apache.tomcat.util.net").setLevel(Level.INFO);
+        }
+    }
+
+
     @Test
     public void testDataFrameWithNonZeroPadding() throws Exception {
         http2Connect();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index a4f8ed08f9..2f6c4c7c18 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -118,6 +118,14 @@
       </fix>
     </changelog>
   </subsection>
+  <subsection name="Coyote">
+    <changelog>
+      <fix>
+        <bug>69713</bug>: Correctly handle an HTTP/2 data frame that includes
+        padding when the headers include a content-length. (remm/markt)
+      </fix>
+    </changelog>
+  </subsection>
   <subsection name="Web applications">
     <changelog>
       <add>


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

Reply via email to