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