This is an automated email from the ASF dual-hosted git repository.
markt-asf 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 1c4d0ef494 Fix BZ 70247 - connection window leak on stream reset
1c4d0ef494 is described below
commit 1c4d0ef49474ab55ef43f8854451af6b67e876df
Author: Mark Thomas <[email protected]>
AuthorDate: Thu Oct 1 13:07:37 2026 +0100
Fix BZ 70247 - connection window leak on stream reset
Based on a patch provided by Guillaume Darmont
---
java/org/apache/coyote/http2/Stream.java | 31 ++--
java/org/apache/coyote/http2/StreamProcessor.java | 7 +-
.../apache/coyote/http2/TestStreamProcessor.java | 171 +++++++++++++++++++++
webapps/docs/changelog.xml | 5 +
4 files changed, 198 insertions(+), 16 deletions(-)
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index b1c9e71ad3..4ba7d456c6 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -876,21 +876,26 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
final void close(Http2Exception http2Exception) {
- if (http2Exception instanceof StreamException) {
+ if (http2Exception instanceof ConnectionException) {
+ handler.closeConnection(http2Exception);
+ } else {
try {
StreamException se = (StreamException) http2Exception;
- if (log.isTraceEnabled()) {
- log.trace(sm.getString("stream.reset.send",
getConnectionId(), getIdAsString(), se.getError()));
- }
+ // se may be null when the clean-up is required without
sending the reset
+ if (se != null) {
+ if (log.isTraceEnabled()) {
+ log.trace(sm.getString("stream.reset.send",
getConnectionId(), getIdAsString(), se.getError()));
+ }
- // Need to update state atomically with the sending of the RST
- // frame else other threads currently working with this stream
- // may see the state change and send a RST frame before the RST
- // frame triggered by this thread. If that happens the client
- // may see out of order RST frames which may hard to follow if
- // the client is unaware the RST frames may be received out of
- // order.
- handler.sendStreamReset(state, se);
+ // Need to update state atomically with the sending of the
RST
+ // frame else other threads currently working with this
stream
+ // may see the state change and send a RST frame before
the RST
+ // frame triggered by this thread. If that happens the
client
+ // may see out of order RST frames which may hard to
follow if
+ // the client is unaware the RST frames may be received
out of
+ // order.
+ handler.sendStreamReset(state, se);
+ }
cancelAllocationRequests();
inputBuffer.swallowUnread();
@@ -900,8 +905,6 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
Http2Error.PROTOCOL_ERROR, ioe);
handler.closeConnection(ce);
}
- } else {
- handler.closeConnection(http2Exception);
}
replace();
}
diff --git a/java/org/apache/coyote/http2/StreamProcessor.java
b/java/org/apache/coyote/http2/StreamProcessor.java
index 3a218a7b60..29d7cd9fe3 100644
--- a/java/org/apache/coyote/http2/StreamProcessor.java
+++ b/java/org/apache/coyote/http2/StreamProcessor.java
@@ -131,8 +131,11 @@ class StreamProcessor extends AbstractProcessor implements
NonPipeliningProcesso
stream.close(se);
} else {
if (!stream.isActive()) {
- // Close calls replace() so need the same call
here
- stream.replace();
+ /*
+ * Still need to call close to perform the
necessary clean-up but no need to send reset
+ * since the stream is not active so pass null.
+ */
+ stream.close(null);
}
}
}
diff --git a/test/org/apache/coyote/http2/TestStreamProcessor.java
b/test/org/apache/coyote/http2/TestStreamProcessor.java
index e65f15feed..47eb991a9a 100644
--- a/test/org/apache/coyote/http2/TestStreamProcessor.java
+++ b/test/org/apache/coyote/http2/TestStreamProcessor.java
@@ -24,6 +24,8 @@ import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
import jakarta.servlet.AsyncContext;
import jakarta.servlet.ServletException;
@@ -45,6 +47,10 @@ import org.apache.tomcat.util.http.Method;
public class TestStreamProcessor extends Http2TestBase {
+ private static final int INCOMPLETE_READ_BODY_SIZE = 8192;
+
+ private static CountDownLatch incompleteReadLatch = null;
+
@Test
public void testAsyncComplete() throws Exception {
enableHttp2();
@@ -796,4 +802,169 @@ public class TestStreamProcessor extends Http2TestBase {
resp.getWriter().write("OK");
}
}
+
+
+ /*
+ * Connection window leak.
+ * <p>
+ * https://bz.apache.org/bugzilla/show_bug.cgi?id=70247
+ * <p>
+ * The client sends the complete request body. The application does not
read it.
+ */
+ @Test
+ public void testWindowLeakWithUnreadRequestBody() throws Exception {
+ incompleteReadLatch = null;
+
+ enableHttp2(200, false, 10000, 10000, 2000, 5000, 5000);
+
+ Tomcat tomcat = getTomcatInstance();
+
+ Context ctxt = getProgrammaticRootContext();
+ Tomcat.addServlet(ctxt, "simple", new SimpleServlet());
+ ctxt.addServletMapping("/simple", "simple");
+ Tomcat.addServlet(ctxt, "reject", new RejectServlet());
+ ctxt.addServletMapping("/reject", "reject");
+
+ tomcat.start();
+
+ openClientConnection();
+ doHttpUpgrade();
+ sendClientPreface();
+ validateHttp2InitialResponse();
+
+ byte[] headersFrameHeader = new byte[9];
+ ByteBuffer headersPayload = ByteBuffer.allocate(128);
+ byte[] dataFrameHeader = new byte[9];
+ ByteBuffer dataPayload =
ByteBuffer.allocate(INCOMPLETE_READ_BODY_SIZE);
+
+ buildPostRequest(headersFrameHeader, headersPayload, false, null, -1,
"/reject", dataFrameHeader,
+ dataPayload, null, false, 3);
+
+ writeFrame(headersFrameHeader, headersPayload);
+ writeFrame(dataFrameHeader, dataPayload);
+
+ // Response headers
+ parser.readFrame();
+ // Empty response body with end of stream
+ parser.readFrame();
+ // connection window update
+ parser.readFrame();
+
+ Assert.assertEquals("3-HeadersStart\n" + "3-Header-[:status]-[403]\n"
+ "3-Header-[content-length]-[0]\n" +
+ "3-Header-[date]-[" + DEFAULT_DATE + "]\n" + "3-HeadersEnd\n"
+ "3-Body-0\n" + "3-EndOfStream\n" +
+ "0-WindowSize-[" + INCOMPLETE_READ_BODY_SIZE + "]\n",
output.getTrace());
+ }
+
+
+ /*
+ * Connection window leak.
+ * <p>
+ * https://bz.apache.org/bugzilla/show_bug.cgi?id=70247
+ * <p>
+ * The client resets the stream after the response has been written but
before the container completes the request.
+ * The request body that the application did not read must still be
returned to the connection window.
+ */
+ @Test
+ public void testResetAfterResponseCommitted() throws Exception {
+ incompleteReadLatch = new CountDownLatch(1);
+
+ enableHttp2(200, false, 10000, 10000, 2000, 5000, 5000);
+
+ Tomcat tomcat = getTomcatInstance();
+
+ Context ctxt = getProgrammaticRootContext();
+ Tomcat.addServlet(ctxt, "simple", new SimpleServlet());
+ ctxt.addServletMapping("/simple", "simple");
+ Tomcat.addServlet(ctxt, "reject", new RejectServlet());
+ ctxt.addServletMapping("/reject", "reject");
+
+ tomcat.start();
+
+ openClientConnection();
+ doHttpUpgrade();
+ sendClientPreface();
+ validateHttp2InitialResponse();
+
+ // No end of stream. The client keeps the stream open and then cancels
it.
+ byte[] headersFrameHeader = new byte[9];
+ ByteBuffer headersPayload = ByteBuffer.allocate(128);
+ byte[] dataFrameHeader = new byte[9];
+ ByteBuffer dataPayload =
ByteBuffer.allocate(INCOMPLETE_READ_BODY_SIZE);
+
+ buildPostRequest(headersFrameHeader, headersPayload, false, null, -1,
"/reject", dataFrameHeader,
+ dataPayload, null, false, 3);
+ // Clear the end of stream flag set by buildPostRequest()
+ dataFrameHeader[4] = 0x00;
+
+ writeFrame(headersFrameHeader, headersPayload);
+ writeFrame(dataFrameHeader, dataPayload);
+
+ // The servlet commits the response and then waits
+ // Response headers
+ parser.readFrame();
+ Assert.assertTrue(output.getTrace(),
output.getTrace().contains("3-Header-[:status]-[403]\n"));
+ output.clearTrace();
+
+ // Cancel the stream. The ping confirms the connection thread
processed the reset before the request
+ // processing thread continues.
+ sendRst(3, Http2Error.CANCEL.getCode());
+ sendPing();
+ parser.readFrame();
+ Assert.assertEquals("0-Ping-Ack-[0,0,0,0,0,0,0,0]\n",
output.getTrace());
+ output.clearTrace();
+
+ // Let the request processing thread complete the request
+ incompleteReadLatch.countDown();
+
+ // The stream is reset, thus only the connection window update remains
+ parser.readFrame();
+ Assert.assertEquals("0-WindowSize-[" + INCOMPLETE_READ_BODY_SIZE +
"]\n", output.getTrace());
+ }
+
+
+ private static class RejectServlet extends SimpleServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected void doPost(HttpServletRequest req, HttpServletResponse
resp) throws ServletException, IOException {
+ // The request body has to be buffered by the container before the
request completes
+ waitForRequestBody(req);
+
+ // Reject the request without reading the request body
+ resp.setStatus(HttpServletResponse.SC_FORBIDDEN);
+
+ CountDownLatch latch = incompleteReadLatch;
+ if (latch != null) {
+ // Commit the response and then wait for the client to reset
the stream
+ resp.flushBuffer();
+ try {
+ Assert.assertTrue(latch.await(10, TimeUnit.SECONDS));
+ } catch (InterruptedException e) {
+ throw new IOException(e);
+ }
+ }
+ }
+
+
+ /*
+ * available() returns a positive value once the complete DATA frame
has been buffered. The parser processes
+ * the end of stream flag of that frame before it makes the payload
available.
+ */
+ private void waitForRequestBody(HttpServletRequest req) throws
IOException {
+ long count = 0;
+ while (req.getInputStream().available() == 0) {
+ // Allow 10s (far more than necessary)
+ if (count > 200) {
+ throw new IOException("Request body did not arrive");
+ }
+ try {
+ Thread.sleep(50);
+ } catch (InterruptedException e) {
+ throw new IOException(e);
+ }
+ count++;
+ }
+ }
+ }
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 6761167eca..98313ae7c6 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -354,6 +354,11 @@
start of the chunk rather than from twice the chunk's start offset
when the chunk does not begin at the start of its buffer. (breken-ai)
</fix>
+ <fix>
+ <bug>70247</bug>: Fix a leak in the HTTP/2 connection flow control
+ window when part of the HTTP request body is received but not read.
+ Based on a patch provided by Guillaume Darmont. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
