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 6bc4ba6e1e Fix BZ 69302 Client reset should trigger
ReadListener.onError ...
6bc4ba6e1e is described below
commit 6bc4ba6e1ef6985d0f30dc883d40a82475f0ef27
Author: Mark Thomas <[email protected]>
AuthorDate: Tue Sep 3 15:08:34 2024 +0100
Fix BZ 69302 Client reset should trigger ReadListener.onError ...
... if the request body has not been fully read.
https://bz.apache.org/bugzilla/show_bug.cgi?id=69302
---
java/org/apache/coyote/AbstractProcessor.java | 4 +++
java/org/apache/coyote/ActionCode.java | 6 +++++
.../apache/coyote/http2/LocalStrings.properties | 1 +
java/org/apache/coyote/http2/Stream.java | 30 +++++++++++++++++++++-
java/org/apache/tomcat/util/net/DispatchType.java | 3 ++-
.../apache/coyote/http2/TestAsyncReadListener.java | 19 +++++++++++++-
.../apache/coyote/http2/TestHttp2Section_5_1.java | 6 ++---
webapps/docs/changelog.xml | 5 ++++
8 files changed, 68 insertions(+), 6 deletions(-)
diff --git a/java/org/apache/coyote/AbstractProcessor.java
b/java/org/apache/coyote/AbstractProcessor.java
index e4789cd80e..51ca269d11 100644
--- a/java/org/apache/coyote/AbstractProcessor.java
+++ b/java/org/apache/coyote/AbstractProcessor.java
@@ -601,6 +601,10 @@ public abstract class AbstractProcessor extends
AbstractProcessorLight implement
addDispatch(DispatchType.NON_BLOCKING_WRITE);
break;
}
+ case DISPATCH_ERROR: {
+ addDispatch(DispatchType.NON_BLOCKING_ERROR);
+ break;
+ }
case DISPATCH_EXECUTE: {
executeDispatches();
break;
diff --git a/java/org/apache/coyote/ActionCode.java
b/java/org/apache/coyote/ActionCode.java
index 84d0eda0ea..6cd304d08e 100644
--- a/java/org/apache/coyote/ActionCode.java
+++ b/java/org/apache/coyote/ActionCode.java
@@ -237,6 +237,12 @@ public enum ActionCode {
*/
DISPATCH_WRITE,
+ /**
+ * Indicates that the container needs to trigger a call to onError() for
the registered non-blocking write and/or
+ * read listener(s).
+ */
+ DISPATCH_ERROR,
+
/**
* Execute any non-blocking dispatches that have been registered via
{@link #DISPATCH_READ} or
* {@link #DISPATCH_WRITE}. Typically required when the non-blocking
listeners are configured on a thread where the
diff --git a/java/org/apache/coyote/http2/LocalStrings.properties
b/java/org/apache/coyote/http2/LocalStrings.properties
index c9bcb28a01..6bdb05696a 100644
--- a/java/org/apache/coyote/http2/LocalStrings.properties
+++ b/java/org/apache/coyote/http2/LocalStrings.properties
@@ -85,6 +85,7 @@ http2Protocol.jmxRegistration.fail=JMX registration for the
HTTP/2 protocol fail
pingManager.roundTripTime=Connection [{0}] Round trip time measured as [{1}]ns
+stream.clientResetRequest=Client reset the stream before the request was fully
read
stream.closed=Connection [{0}], Stream [{1}], Unable to write to stream once
it has been closed
stream.header.case=Connection [{0}], Stream [{1}], HTTP header name [{2}] must
be in lower case
stream.header.connection=Connection [{0}], Stream [{1}], HTTP header [{2}] is
not permitted in an HTTP/2 request
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index 1b2f244f2d..4ac19a3fb5 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -33,6 +33,8 @@ import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Supplier;
+import jakarta.servlet.RequestDispatcher;
+
import org.apache.coyote.ActionCode;
import org.apache.coyote.CloseNowException;
import org.apache.coyote.InputBuffer;
@@ -1236,7 +1238,7 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
// If readInterest is true, data must be available to read no later
than this time.
private volatile long readTimeoutExpiry;
private volatile boolean closed;
- private boolean resetReceived;
+ private volatile boolean resetReceived;
@SuppressWarnings("deprecation")
@Override
@@ -1332,6 +1334,16 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
return true;
}
+ if (resetReceived) {
+ // Trigger ReadListener.onError()
+
getCoyoteRequest().setAttribute(RequestDispatcher.ERROR_EXCEPTION,
+ new
IOException(sm.getString("stream.clientResetRequest")));
+ coyoteRequest.action(ActionCode.DISPATCH_ERROR, null);
+ coyoteRequest.action(ActionCode.DISPATCH_EXECUTE, null);
+
+ return false;
+ }
+
if (!isRequestBodyFullyRead()) {
readInterest = true;
long readTimeout =
handler.getProtocol().getStreamReadTimeout();
@@ -1453,6 +1465,22 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
inBuffer.notifyAll();
}
}
+
+ // If a read is in progress, cancel it.
+ readStateLock.lock();
+ try {
+ if (readInterest) {
+ readInterest = false;
+ }
+ } finally {
+ readStateLock.unlock();
+ }
+
+ // Trigger ReadListener.onError()
+ getCoyoteRequest().setAttribute(RequestDispatcher.ERROR_EXCEPTION,
+ new
IOException(sm.getString("stream.clientResetRequest")));
+ coyoteRequest.action(ActionCode.DISPATCH_ERROR, null);
+ coyoteRequest.action(ActionCode.DISPATCH_EXECUTE, null);
}
@Override
diff --git a/java/org/apache/tomcat/util/net/DispatchType.java
b/java/org/apache/tomcat/util/net/DispatchType.java
index 5914c87044..ee04a0555e 100644
--- a/java/org/apache/tomcat/util/net/DispatchType.java
+++ b/java/org/apache/tomcat/util/net/DispatchType.java
@@ -24,7 +24,8 @@ package org.apache.tomcat.util.net;
public enum DispatchType {
NON_BLOCKING_READ(SocketEvent.OPEN_READ),
- NON_BLOCKING_WRITE(SocketEvent.OPEN_WRITE);
+ NON_BLOCKING_WRITE(SocketEvent.OPEN_WRITE),
+ NON_BLOCKING_ERROR(SocketEvent.ERROR);
private final SocketEvent status;
diff --git a/test/org/apache/coyote/http2/TestAsyncReadListener.java
b/test/org/apache/coyote/http2/TestAsyncReadListener.java
index 28a32f825e..a6f77f8e7e 100644
--- a/test/org/apache/coyote/http2/TestAsyncReadListener.java
+++ b/test/org/apache/coyote/http2/TestAsyncReadListener.java
@@ -47,10 +47,27 @@ public class TestAsyncReadListener extends Http2TestBase {
asyncServlet = new AsyncServlet();
}
+
+ @Test
+ public void testEmptyWindowDefaultReadTimeout() throws Exception {
+ doTestEmptyWindowMaximumTimeout(false);
+ }
+
+
@Test
- public void testEmptyWindow() throws Exception {
+ public void testEmptyWindowMaximumReadTimeout() throws Exception {
+ doTestEmptyWindowMaximumTimeout(true);
+ }
+
+
+ public void doTestEmptyWindowMaximumTimeout(boolean useMaxReadTimeout)
throws Exception {
http2Connect();
+ if (useMaxReadTimeout) {
+ http2Protocol.setStreamReadTimeout(Integer.MAX_VALUE);
+ http2Protocol.setReadTimeout(Integer.MAX_VALUE);
+ }
+
byte[] headersFrameHeader = new byte[9];
ByteBuffer headersPayload = ByteBuffer.allocate(128);
byte[] dataFrameHeader = new byte[9];
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
b/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
index 391ce865b1..312d5c97be 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
@@ -296,10 +296,10 @@ public class TestHttp2Section_5_1 extends Http2TestBase {
// Reset stream 3 (client cancel)
sendRst(3, Http2Error.NO_ERROR.getCode());
- // Client reset triggers a write error which in turn triggers a server
- // reset
+ // Client reset triggers both a read error and a write error which in
turn trigger two server resets
parser.readFrame();
- Assert.assertEquals("3-RST-[5]\n", output.getTrace());
+ parser.readFrame();
+ Assert.assertEquals("3-RST-[5]\n3-RST-[5]\n", output.getTrace());
output.clearTrace();
// Open up the connection window.
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 9734336b4f..b915804c6a 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -138,6 +138,11 @@
writing response headers to the access log. Based on a patch and test
case provided by hypnoce. (markt)
</fix>
+ <fix>
+ <bug>69302</bug>: If an HTTP/2 client resets a stream before the
request
+ body is fully written, ensure that any <code>ReadListener</code> is
+ notified via a call to <code>ReadListener.onErrror()</code>. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
