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
commit f55293d532765afee27d5cfa7e00095799fb16e0 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 | 8 ++++++-- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java index 4ba7d456c6..d5dd1f0127 100644 --- a/java/org/apache/coyote/http2/Stream.java +++ b/java/org/apache/coyote/http2/Stream.java @@ -943,10 +943,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; @@ -965,7 +969,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 29d7cd9fe3..6174b97d23 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 bdd663fad4..4e3a5e2539 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -399,10 +399,14 @@ Allow for absolute host names in SNI checks. (markt) </fix> <fix> - Correct a gap in asynchronous error handling that could lead to + Correct a gap in asynchronous error handling that could lead to unexpected <code>IllegalStateExceptions</code> for some error - conditions. (markt) + 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]
