This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new adb3075e6a Harden HTTP/2 against apps that don't correctly handle
async errors
adb3075e6a is described below
commit adb3075e6a886f13b9de4384156a88cd2bdf52c7
Author: Mark Thomas <[email protected]>
AuthorDate: Mon Sep 28 17:23:23 2026 +0100
Harden HTTP/2 against apps that don't correctly handle async errors
---
java/org/apache/coyote/http2/Stream.java | 10 ++++++++--
java/org/apache/coyote/http2/StreamProcessor.java | 15 ++++++++++++++-
webapps/docs/changelog.xml | 4 ++++
3 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index 4ca1c3d385..44953e1bd4 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -954,10 +954,14 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
*
* - this method is called once the StreamProcessor considers the Stream
closed
*
+ * - the request and response are only added to the pool for re-use if the
StreamProcessor considers the Stream to
+ * have completed cleanly. If the Stream was closed due to an error, an
application thread may still be using the
+ * request and/or response (e.g. blocked in a write when an asynchronous
timeout fired) so they must not be re-used.
+ *
* In theory, the protection against duplicate calls is not required in
this method (the code in StreamProcessor
* should be sufficient) but it is implemented as precaution along with
the WARN level logging.
*/
- final void recycle() {
+ final void recycle(boolean reuseRequestAndResponse) {
if (recycled) {
log.warn(sm.getString("stream.recycle.duplicate",
getConnectionId(), getIdAsString()));
return;
@@ -976,7 +980,9 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
coyoteRequest.setNote(Request.NOTE_BAD_REQUEST, null);
coyoteRequest.recycle();
coyoteResponse.recycle();
- handler.getProtocol().pushRequestAndResponse(coyoteRequest);
+ if (reuseRequestAndResponse) {
+ handler.getProtocol().pushRequestAndResponse(coyoteRequest);
+ }
}
diff --git a/java/org/apache/coyote/http2/StreamProcessor.java
b/java/org/apache/coyote/http2/StreamProcessor.java
index 139b02a2fd..55934fca08 100644
--- a/java/org/apache/coyote/http2/StreamProcessor.java
+++ b/java/org/apache/coyote/http2/StreamProcessor.java
@@ -98,8 +98,20 @@ class StreamProcessor extends AbstractProcessor implements
NonPipeliningProcesso
// HTTP/2 equivalent of AbstractConnectionHandler#process()
without the
// socket <-> processor mapping
SocketState state = SocketState.CLOSED;
+ boolean reuseRequestAndResponse = false;
try {
state = process(socketWrapper, event);
+ /*
+ * The request and response may only be returned to the
pool for re-use if the stream ended cleanly.
+ * If the stream ended with either a timeout or an error
that prevents further I/O (e.g. an
+ * asynchronous timeout or error while an application
thread was blocked in a write), a
+ * non-container thread may still be using the request
and/or response. Re-using them in that case
+ * could result in the remaining data being written to a
different stream, possibly on a different
+ * connection.
+ */
+ if (SocketEvent.TIMEOUT != event) {
+ reuseRequestAndResponse =
getErrorState().isIoAllowed();
+ }
if (state == SocketState.LONG) {
handler.getProtocol().getHttp11Protocol().addWaitingProcessor(this);
@@ -148,6 +160,7 @@ class StreamProcessor extends AbstractProcessor implements
NonPipeliningProcesso
ConnectionException ce = new ConnectionException(msg,
Http2Error.INTERNAL_ERROR, e);
stream.close(ce);
state = SocketState.CLOSED;
+ reuseRequestAndResponse = false;
} finally {
if (state == SocketState.CLOSED) {
/*
@@ -157,7 +170,7 @@ class StreamProcessor extends AbstractProcessor implements
NonPipeliningProcesso
* response are added to the pool to avoid concurrency
issues corrupting the statistics.
*/
recycle();
- stream.recycle();
+ stream.recycle(reuseRequestAndResponse);
}
}
} finally {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 885ad0f83a..8bd00b24d3 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -221,6 +221,10 @@
unexpected <code>IllegalStateExceptions</code> for some error
conditions. (markt)
</fix>
+ <add>
+ Harden HTTP/2 against applications that do not correctly handle
+ asynchronous errors. (markt)
+ </add>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
