This is an automated email from the ASF dual-hosted git repository.
markt 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 382ae1f1dc Fix BZ 69068 - trigger read timeouts for async reads on
HTTP/2
382ae1f1dc is described below
commit 382ae1f1dc25f30d1991ebd1adf2cfc9b71ba570
Author: Mark Thomas <[email protected]>
AuthorDate: Sun Jun 2 15:22:54 2024 +0100
Fix BZ 69068 - trigger read timeouts for async reads on HTTP/2
Test case provided by hypnoce
---
.../apache/coyote/http2/LocalStrings.properties | 1 +
java/org/apache/coyote/http2/Stream.java | 23 ++++
java/org/apache/coyote/http2/StreamProcessor.java | 20 +++
test/org/apache/coyote/http2/Http2TestBase.java | 2 +-
.../apache/coyote/http2/TestAsyncReadListener.java | 145 +++++++++++++++++++++
webapps/docs/changelog.xml | 4 +
6 files changed, 194 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/coyote/http2/LocalStrings.properties
b/java/org/apache/coyote/http2/LocalStrings.properties
index 137e3346db..891ba7ad81 100644
--- a/java/org/apache/coyote/http2/LocalStrings.properties
+++ b/java/org/apache/coyote/http2/LocalStrings.properties
@@ -121,6 +121,7 @@ streamProcessor.error.connection=Connection [{0}], Stream
[{1}], An error occurr
streamProcessor.error.stream=Connection [{0}], Stream [{1}], An error occurred
during processing that was fatal to the stream
streamProcessor.flushBufferedWrite.entry=Connection [{0}], Stream [{1}],
Flushing buffered writes
streamProcessor.service.error=Error during request processing
+streamProcessor.streamReadTimeout=Stream read timeout
streamStateMachine.debug.change=Connection [{0}], Stream [{1}], State changed
from [{2}] to [{3}]
streamStateMachine.invalidFrame=Connection [{0}], Stream [{1}], State [{2}],
Frame type [{3}]
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index 096cc52c30..062221d9a4 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -1162,6 +1162,8 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
abstract boolean isRequestBodyFullyRead();
abstract void insertReplayedBody(ByteChunk body);
+
+ protected abstract boolean timeoutRead(long now);
}
@@ -1188,6 +1190,8 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
// 'write mode'.
private volatile ByteBuffer inBuffer;
private volatile boolean readInterest;
+ // 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;
@@ -1287,6 +1291,12 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
if (!isRequestBodyFullyRead()) {
readInterest = true;
+ long readTimeout =
handler.getProtocol().getStreamReadTimeout();
+ if (readTimeout > 0) {
+ readTimeoutExpiry = System.currentTimeMillis() +
handler.getProtocol().getStreamReadTimeout();
+ } else {
+ readTimeoutExpiry = Long.MAX_VALUE;
+ }
}
return false;
@@ -1438,6 +1448,12 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
}
}
}
+
+
+ @Override
+ protected boolean timeoutRead(long now) {
+ return readInterest && now > readTimeoutExpiry;
+ }
}
@@ -1499,5 +1515,12 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
void insertReplayedBody(ByteChunk body) {
// NO-OP
}
+
+
+ @Override
+ protected boolean timeoutRead(long now) {
+ // Reading from a saved request. Will never time out.
+ return false;
+ }
}
}
diff --git a/java/org/apache/coyote/http2/StreamProcessor.java
b/java/org/apache/coyote/http2/StreamProcessor.java
index 43865c1ea4..4e5645736e 100644
--- a/java/org/apache/coyote/http2/StreamProcessor.java
+++ b/java/org/apache/coyote/http2/StreamProcessor.java
@@ -18,6 +18,7 @@ package org.apache.coyote.http2;
import java.io.File;
import java.io.IOException;
+import java.net.SocketTimeoutException;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.Iterator;
@@ -25,6 +26,7 @@ import java.util.Set;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
+import jakarta.servlet.RequestDispatcher;
import jakarta.servlet.ServletConnection;
import jakarta.servlet.http.HttpServletResponse;
@@ -553,4 +555,22 @@ class StreamProcessor extends AbstractProcessor {
protected final SocketState dispatchEndRequest() throws IOException {
return SocketState.CLOSED;
}
+
+
+ /**
+ * {@inheritDoc}
+ * <p>
+ * First checks for a stream read timeout and processes it if detected. If
no stream read timeout is detected then
+ * the superclass is called to check for an asynchronous processing
timeout.
+ */
+ @Override
+ public void timeoutAsync(long now) {
+ if (stream.getInputBuffer().timeoutRead(now)) {
+
stream.getCoyoteRequest().setAttribute(RequestDispatcher.ERROR_EXCEPTION,
+ new
SocketTimeoutException(sm.getString("streamProcessor.streamReadTimeout")));
+ processSocketEvent(SocketEvent.ERROR, true);
+ } else {
+ super.timeoutAsync(now);
+ }
+ }
}
diff --git a/test/org/apache/coyote/http2/Http2TestBase.java
b/test/org/apache/coyote/http2/Http2TestBase.java
index 7df1b02a4f..da7dc94ab1 100644
--- a/test/org/apache/coyote/http2/Http2TestBase.java
+++ b/test/org/apache/coyote/http2/Http2TestBase.java
@@ -113,7 +113,7 @@ public abstract class Http2TestBase extends TomcatBaseTest {
protected static final String TRAILER_HEADER_VALUE = "test";
// Client
- private Socket s;
+ protected Socket s;
protected HpackEncoder hpackEncoder;
protected Input input;
protected TestOutput output;
diff --git a/test/org/apache/coyote/http2/TestAsyncReadListener.java
b/test/org/apache/coyote/http2/TestAsyncReadListener.java
new file mode 100644
index 0000000000..8b9a9bc9b8
--- /dev/null
+++ b/test/org/apache/coyote/http2/TestAsyncReadListener.java
@@ -0,0 +1,145 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.coyote.http2;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+
+import jakarta.servlet.AsyncContext;
+import jakarta.servlet.AsyncEvent;
+import jakarta.servlet.AsyncListener;
+import jakarta.servlet.ReadListener;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServlet;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.LifecycleException;
+import org.apache.catalina.Wrapper;
+import org.apache.catalina.startup.Tomcat;
+
+public class TestAsyncReadListener extends Http2TestBase {
+
+ private AsyncServlet asyncServlet;
+
+ @Before
+ public void before() {
+ asyncServlet = new AsyncServlet();
+ }
+
+ @Test
+ public void testEmptyWindow() throws Exception {
+ http2Connect();
+
+ byte[] headersFrameHeader = new byte[9];
+ ByteBuffer headersPayload = ByteBuffer.allocate(128);
+ byte[] dataFrameHeader = new byte[9];
+ ByteBuffer dataPayload = ByteBuffer.allocate(256);
+ byte[] trailerFrameHeader = new byte[9];
+ ByteBuffer trailerPayload = ByteBuffer.allocate(256);
+
+
+ buildPostRequest(headersFrameHeader, headersPayload, false, null, -1,
"/async", dataFrameHeader, dataPayload,
+ null, trailerFrameHeader, trailerPayload, 3);
+
+ synchronized (asyncServlet) {
+ // Write the headers
+ writeFrame(headersFrameHeader, headersPayload);
+ asyncServlet.wait(4000);
+ s.close();
+ asyncServlet.wait(4000);
+ }
+ Assert.assertNotNull(asyncServlet.t);
+ }
+
+ @Override
+ protected void configureAndStartWebApplication() throws LifecycleException
{
+ Tomcat tomcat = getTomcatInstance();
+
+ Context ctxt = getProgrammaticRootContext();
+ Tomcat.addServlet(ctxt, "simple", new SimpleServlet());
+ ctxt.addServletMappingDecoded("/simple", "simple");
+ Wrapper w = Tomcat.addServlet(ctxt, "async", asyncServlet);
+ w.setAsyncSupported(true);
+ ctxt.addServletMappingDecoded("/async", "async");
+ tomcat.start();
+ }
+
+ public static class AsyncServlet extends HttpServlet {
+ private static final long serialVersionUID = 1L;
+ private volatile Throwable t;
+ @Override
+ protected void doPost(HttpServletRequest req, HttpServletResponse
resp) throws ServletException, IOException {
+ final AsyncContext asyncContext = req.startAsync();
+ asyncContext.getRequest().getInputStream().setReadListener(new
ReadListener() {
+ @Override
+ public void onDataAvailable() throws IOException {
+ System.out.println("RL-onDataAvailable");
+ synchronized (AsyncServlet.this) {
+ AsyncServlet.this.notifyAll();
+ }
+ }
+
+ @Override
+ public void onAllDataRead() throws IOException {
+ System.out.println("RL-onAllDataRead");
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ System.out.println("RL-onError ");
+ System.out.println(throwable);
+ synchronized (AsyncServlet.this) {
+ t = throwable;
+ AsyncServlet.this.notifyAll();
+ asyncContext.complete();
+ }
+ }
+ });
+ System.out.println("setReadListener");
+ asyncContext.addListener(new AsyncListener() {
+
+ @Override
+ public void onComplete(AsyncEvent event) throws IOException {
+ System.out.println("AL-onComplete");
+ }
+
+ @Override
+ public void onTimeout(AsyncEvent event) throws IOException {
+ System.out.println("AL-onTimeout");
+ }
+
+ @Override
+ public void onError(AsyncEvent event) throws IOException {
+ System.out.println("AL-onError");
+ }
+
+ @Override
+ public void onStartAsync(AsyncEvent event) throws IOException {
+ System.out.println("AL-onStartAsync");
+ }
+ });
+ System.out.println("setAsyncListener");
+ }
+ }
+}
\ No newline at end of file
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 4204687f55..51c3d00807 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -130,6 +130,10 @@
Fix a crash on Windows setting CA certificate on null path.
(remm)
</fix>
+ <fix>
+ <bug>69068</bug>: Ensure read timouts are triggered for asynchronous,
+ non-blocking reads when using HTTP/2. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Other">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
