This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push: new a5deb26913 Fix BZ 69713 - Correctly handle padding when content-length is present a5deb26913 is described below commit a5deb269131be64e4db9dfab7341c40082e8f287 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 | 4 ++ 8 files changed, 74 insertions(+), 17 deletions(-) diff --git a/java/org/apache/coyote/http2/AbstractNonZeroStream.java b/java/org/apache/coyote/http2/AbstractNonZeroStream.java index efaf73bfe3..3285278726 100644 --- a/java/org/apache/coyote/http2/AbstractNonZeroStream.java +++ b/java/org/apache/coyote/http2/AbstractNonZeroStream.java @@ -73,9 +73,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 7c15b3ca97..acd2a23d08 100644 --- a/java/org/apache/coyote/http2/Http2Parser.java +++ b/java/org/apache/coyote/http2/Http2Parser.java @@ -171,7 +171,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 @@ -184,7 +184,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)) { @@ -768,7 +768,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 9d2ec89df8..d54e31ce78 100644 --- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java +++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java @@ -1462,7 +1462,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); @@ -1476,8 +1476,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; @@ -1489,7 +1489,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 b2223e858f..a83abef2bf 100644 --- a/java/org/apache/coyote/http2/Stream.java +++ b/java/org/apache/coyote/http2/Stream.java @@ -670,8 +670,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 606fef11d8..bcee87916a 100644 --- a/test/org/apache/coyote/http2/Http2TestBase.java +++ b/test/org/apache/coyote/http2/Http2TestBase.java @@ -1092,14 +1092,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 451b16b2f1..d1a10217cc 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -193,6 +193,10 @@ Remove NIO2 connector. (remm) </update> <!-- Entries for backport and removal before 12.0.0-M1 below this line --> + <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="Jasper"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org