From 963d33111e5ba3d7239eddb869ca5497462b1963 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 14 Jun 2023 09:57:10 +0200 Subject: [PATCH] Jetty 12 idletimeout (#9905) * IdleTimeout review + pass TimeoutException through all APIs + HttpConnection now passes on TimeoutException to HttpChannel.onFailure * More ServerTests for idletimeout * Recreated a ServerTimeoutsTest for multiple transports * more robust tests Signed-off-by: Simone Bordet * merged work from @sbordet and @gregw * Various improvements to CyclicTimeouts. * Improved reset of the earliest timeout before iteration. * Removed check for getExpireNanoTime() == -1, since it's a valid value. * When onExpired(Expirable) returns false, the Expirable should arrange to move its timeout in the future. * fix keystore to please BoringSSL + use correct temp path Signed-off-by: Ludovic Orban * Fixed ErrorResponseAndCallback succeeded() and failed() to call super.failed() in all cases to complete the wrapped callback. Signed-off-by: Simone Bordet * Revert "Fixed ErrorResponseAndCallback succeeded() and failed() to call super.failed() in all cases to complete the wrapped callback." This reverts commit 5ac57c13e013e62105ef7f0e86a29a420fb8cf2e. * WIP idleTimeout * WIP idleTimeout * Added context wrapper for idle timeout listener * updates from review --------- Signed-off-by: Simone Bordet Signed-off-by: Ludovic Orban Co-authored-by: Simone Bordet Co-authored-by: Ludovic Orban --- .../org/eclipse/jetty/client/Socks4Proxy.java | 4 +- .../org/eclipse/jetty/client/Socks5Proxy.java | 4 +- .../internal/HttpConnectionOverHTTP.java | 4 +- .../client/http/HttpReceiverOverHTTPTest.java | 5 +- .../internal/HttpConnectionOverFCGI.java | 7 +- .../server/internal/HttpStreamOverFCGI.java | 17 +- .../server/internal/ServerFCGIConnection.java | 12 +- .../jetty/fcgi/server/HttpClientTest.java | 4 +- .../internal/ClientHTTP2StreamEndPoint.java | 8 +- .../internal/HttpChannelOverHTTP2.java | 4 +- .../internal/HttpReceiverOverHTTP2.java | 3 +- .../org/eclipse/jetty/http2/HTTP2Channel.java | 5 +- .../eclipse/jetty/http2/HTTP2Connection.java | 3 +- .../org/eclipse/jetty/http2/HTTP2Stream.java | 2 +- .../org/eclipse/jetty/http2/api/Stream.java | 5 +- .../server/HTTP2ServerConnectionFactory.java | 2 +- .../internal/HTTP2ServerConnection.java | 7 +- .../server/internal/HttpStreamOverHTTP2.java | 17 +- .../internal/ServerHTTP2StreamEndPoint.java | 13 +- .../jetty/http2/tests/IdleTimeoutTest.java | 6 +- .../jetty/http2/tests/RawHTTP2ProxyTest.java | 5 +- .../jetty/http3/HTTP3StreamConnection.java | 6 +- .../org/eclipse/jetty/http3/api/Stream.java | 3 +- .../server/HTTP3ServerConnectionFactory.java | 5 +- .../server/internal/HttpStreamOverHTTP3.java | 17 +- .../internal/ServerHTTP3StreamConnection.java | 5 +- .../http3/tests/StreamIdleTimeoutTest.java | 3 +- .../eclipse/jetty/io/AbstractConnection.java | 8 +- .../eclipse/jetty/io/AbstractEndPoint.java | 21 +- .../java/org/eclipse/jetty/io/Connection.java | 3 +- .../eclipse/jetty/io/ssl/SslConnection.java | 5 +- .../jetty/io/ByteArrayEndPointTest.java | 34 +- .../quic/client/ClientQuicConnection.java | 3 +- .../jetty/quic/common/QuicConnection.java | 3 +- .../quic/server/ServerQuicConnection.java | 3 +- .../org/eclipse/jetty/server/HttpChannel.java | 19 +- .../org/eclipse/jetty/server/HttpStream.java | 16 + .../org/eclipse/jetty/server/Request.java | 49 ++- .../jetty/server/handler/ContextHandler.java | 18 + .../jetty/server/handler/ContextRequest.java | 18 +- .../server/internal/HttpChannelState.java | 316 +++++++++++++----- .../jetty/server/internal/HttpConnection.java | 24 ++ .../jetty/server/ConnectorTimeoutTest.java | 12 +- .../eclipse/jetty/server/HttpChannelTest.java | 10 +- .../jetty/server/HttpConnectionTest.java | 24 +- .../eclipse/jetty/server/MockHttpStream.java | 11 + .../org/eclipse/jetty/server/ServerTest.java | 207 +++++++++++- .../jetty/session/TestableRequest.java | 10 +- .../jetty-test-client-transports/pom.xml | 5 + .../transport/HttpClientTimeoutTest.java | 2 +- .../client/transport/ServerTimeoutsTest.java | 198 ++++++++++- .../websocket/core/WebSocketConnection.java | 7 +- .../jetty/ee10/servlet/DefaultServlet.java | 14 - .../jetty/ee10/servlet/ServletChannel.java | 6 - .../ee10/servlet/ServletContextRequest.java | 7 + .../ee10/servlet/ServletRequestState.java | 13 + .../test/client/transport/AbstractTest.java | 42 ++- .../client/transport/ServerTimeoutsTest.java | 11 +- .../src/test/resources/keystore.p12 | Bin 2597 -> 2774 bytes .../eclipse/jetty/ee9/nested/HttpChannel.java | 4 - .../eclipse/jetty/ee9/nested/RequestTest.java | 5 +- .../jetty/ee9/nested/ResponseTest.java | 10 +- 62 files changed, 1016 insertions(+), 298 deletions(-) diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/Socks4Proxy.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/Socks4Proxy.java index e2da92198b2b..855631099ffb 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/Socks4Proxy.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/Socks4Proxy.java @@ -150,9 +150,9 @@ public void failed(Throwable x) } @Override - public boolean onIdleExpired() + public boolean onIdleExpired(TimeoutException timeout) { - failed(new TimeoutException("Idle timeout expired")); + failed(timeout); return false; } diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/Socks5Proxy.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/Socks5Proxy.java index b59b98881834..f4e353ec3b08 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/Socks5Proxy.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/Socks5Proxy.java @@ -185,9 +185,9 @@ private void fail(Throwable x) } @Override - public boolean onIdleExpired() + public boolean onIdleExpired(TimeoutException timeout) { - fail(new TimeoutException("Idle timeout expired")); + fail(timeout); return false; } diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpConnectionOverHTTP.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpConnectionOverHTTP.java index d73db9336f55..6e7517a589a3 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpConnectionOverHTTP.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpConnectionOverHTTP.java @@ -173,12 +173,12 @@ public Object getAttachment() } @Override - public boolean onIdleExpired() + public boolean onIdleExpired(TimeoutException timeout) { long idleTimeout = getEndPoint().getIdleTimeout(); boolean close = onIdleTimeout(idleTimeout); if (close) - close(new TimeoutException("Idle timeout " + idleTimeout + " ms")); + close(timeout); return false; } diff --git a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java index 21fe99bbfcca..45e28c94aba5 100644 --- a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java +++ b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java @@ -45,6 +45,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.sameInstance; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -186,10 +187,12 @@ public void testReceiveResponseContentIdleTimeout(HttpCompliance compliance) thr // ByteArrayEndPoint has an idle timeout of 0 by default, // so to simulate an idle timeout is enough to wait a bit. Thread.sleep(100); - connection.onIdleExpired(); + TimeoutException timeoutException = new TimeoutException(); + connection.onIdleExpired(timeoutException); ExecutionException e = assertThrows(ExecutionException.class, () -> listener.get(5, TimeUnit.SECONDS)); assertThat(e.getCause(), instanceOf(TimeoutException.class)); + assertThat(e.getCause(), sameInstance(timeoutException)); } @ParameterizedTest diff --git a/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/transport/internal/HttpConnectionOverFCGI.java b/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/transport/internal/HttpConnectionOverFCGI.java index fe807fd20cf3..4a28e87880c7 100644 --- a/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/transport/internal/HttpConnectionOverFCGI.java +++ b/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/transport/internal/HttpConnectionOverFCGI.java @@ -217,13 +217,12 @@ private void shutdown() } @Override - public boolean onIdleExpired() + public boolean onIdleExpired(TimeoutException timeoutException) { long idleTimeout = getEndPoint().getIdleTimeout(); - TimeoutException failure = new TimeoutException("Idle timeout " + idleTimeout + " ms"); - boolean close = delegate.onIdleTimeout(idleTimeout, failure); + boolean close = delegate.onIdleTimeout(idleTimeout, timeoutException); if (close) - close(failure); + close(timeoutException); return false; } diff --git a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/HttpStreamOverFCGI.java b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/HttpStreamOverFCGI.java index b5810a9d12db..83ae2a4f321f 100644 --- a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/HttpStreamOverFCGI.java +++ b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/HttpStreamOverFCGI.java @@ -15,6 +15,7 @@ import java.nio.ByteBuffer; import java.util.Locale; +import java.util.concurrent.TimeoutException; import org.eclipse.jetty.fcgi.FCGI; import org.eclipse.jetty.fcgi.generator.Flusher; @@ -301,6 +302,18 @@ private void generateResponseContent(ByteBufferPool.Accumulator accumulator, boo _generator.generateResponseContent(accumulator, _id, buffer, last, _aborted); } + @Override + public long getIdleTimeout() + { + return _connection.getEndPoint().getIdleTimeout(); + } + + @Override + public void setIdleTimeout(long idleTimeoutMs) + { + _connection.getEndPoint().setIdleTimeout(idleTimeoutMs); + } + @Override public boolean isCommitted() { @@ -328,9 +341,9 @@ public void failed(Throwable x) _connection.onCompleted(x); } - public boolean onIdleTimeout(Throwable timeout) + public boolean onIdleTimeout(TimeoutException timeout) { - Runnable task = _httpChannel.onFailure(timeout); + Runnable task = _httpChannel.onIdleTimeout(timeout); if (task != null) execute(task); return false; diff --git a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java index 439ae7352b05..166c536f8141 100644 --- a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java +++ b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java @@ -16,6 +16,7 @@ import java.net.SocketAddress; import java.nio.ByteBuffer; import java.util.Set; +import java.util.concurrent.TimeoutException; import org.eclipse.jetty.fcgi.FCGI; import org.eclipse.jetty.fcgi.generator.Flusher; @@ -291,7 +292,7 @@ private int fillInputBuffer() } @Override - protected boolean onReadTimeout(Throwable timeout) + protected boolean onReadTimeout(TimeoutException timeout) { if (stream != null) return stream.onIdleTimeout(timeout); @@ -323,6 +324,15 @@ void onCompleted(Throwable failure) getFlusher().shutdown(); } + @Override + public boolean onIdleExpired(TimeoutException timeoutException) + { + Runnable task = stream.getHttpChannel().onIdleTimeout(timeoutException); + if (task != null) + getExecutor().execute(task); + return false; + } + private class ServerListener implements ServerParser.Listener { @Override diff --git a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/HttpClientTest.java b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/HttpClientTest.java index d705a00f8307..83c3a75cb550 100644 --- a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/HttpClientTest.java +++ b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/HttpClientTest.java @@ -522,6 +522,8 @@ public void testConnectionIdleTimeout() throws Exception @Override public boolean handle(org.eclipse.jetty.server.Request request, org.eclipse.jetty.server.Response response, Callback callback) throws Exception { + // Handler says it will handle the idletimeout + request.addIdleTimeoutListener(t -> false); TimeUnit.MILLISECONDS.sleep(2 * idleTimeout); callback.succeeded(); return true; @@ -530,7 +532,7 @@ public boolean handle(org.eclipse.jetty.server.Request request, org.eclipse.jett connector.setIdleTimeout(idleTimeout); - // Request does not fail because idle timeouts while dispatched are ignored. + // Request does not fail because handler says it will handle it. ContentResponse response1 = client.newRequest("localhost", connector.getLocalPort()) .scheme(scheme) .idleTimeout(4 * idleTimeout, TimeUnit.MILLISECONDS) diff --git a/jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/ClientHTTP2StreamEndPoint.java b/jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/ClientHTTP2StreamEndPoint.java index c5c973d7bd0b..e5f47edb5d1a 100644 --- a/jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/ClientHTTP2StreamEndPoint.java +++ b/jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/ClientHTTP2StreamEndPoint.java @@ -13,6 +13,8 @@ package org.eclipse.jetty.http2.client.transport.internal; +import java.util.concurrent.TimeoutException; + import org.eclipse.jetty.http2.HTTP2Channel; import org.eclipse.jetty.http2.HTTP2Stream; import org.eclipse.jetty.http2.HTTP2StreamEndPoint; @@ -38,13 +40,13 @@ public void onDataAvailable() } @Override - public void onTimeout(Throwable failure, Promise promise) + public void onTimeout(TimeoutException timeout, Promise promise) { if (LOG.isDebugEnabled()) - LOG.debug("idle timeout on {}: {}", this, failure); + LOG.debug("idle timeout on {}", this, timeout); Connection connection = getConnection(); if (connection != null) - promise.succeeded(connection.onIdleExpired()); + promise.succeeded(connection.onIdleExpired(timeout)); else promise.succeeded(true); } diff --git a/jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/HttpChannelOverHTTP2.java b/jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/HttpChannelOverHTTP2.java index 961429b93e2f..61ea36249e87 100644 --- a/jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/HttpChannelOverHTTP2.java +++ b/jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/HttpChannelOverHTTP2.java @@ -13,6 +13,8 @@ package org.eclipse.jetty.http2.client.transport.internal; +import java.util.concurrent.TimeoutException; + import org.eclipse.jetty.client.Result; import org.eclipse.jetty.client.transport.HttpChannel; import org.eclipse.jetty.client.transport.HttpExchange; @@ -200,7 +202,7 @@ public void onReset(Stream stream, ResetFrame frame, Callback callback) } @Override - public void onIdleTimeout(Stream stream, Throwable x, Promise promise) + public void onIdleTimeout(Stream stream, TimeoutException x, Promise promise) { HTTP2Channel.Client channel = (HTTP2Channel.Client)((HTTP2Stream)stream).getAttachment(); channel.onTimeout(x, promise); diff --git a/jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/HttpReceiverOverHTTP2.java b/jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/HttpReceiverOverHTTP2.java index 6fee831c31d3..153f64d2c897 100644 --- a/jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/HttpReceiverOverHTTP2.java +++ b/jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/HttpReceiverOverHTTP2.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.http2.client.transport.internal; import java.io.IOException; +import java.util.concurrent.TimeoutException; import java.util.function.BiFunction; import org.eclipse.jetty.client.HttpUpgrader; @@ -220,7 +221,7 @@ void onReset(ResetFrame frame) } @Override - public void onTimeout(Throwable failure, Promise promise) + public void onTimeout(TimeoutException failure, Promise promise) { HttpExchange exchange = getHttpExchange(); if (exchange != null) diff --git a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Channel.java b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Channel.java index df5d1b23b672..49c682362af3 100644 --- a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Channel.java +++ b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Channel.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.http2; +import java.util.concurrent.TimeoutException; import java.util.function.BiConsumer; import org.eclipse.jetty.http2.frames.HeadersFrame; @@ -34,7 +35,7 @@ public interface Client { public void onDataAvailable(); - public void onTimeout(Throwable failure, Promise promise); + public void onTimeout(TimeoutException failure, Promise promise); public void onFailure(Throwable failure, Callback callback); } @@ -54,7 +55,7 @@ public interface Server // TODO: review the signature because the serialization done by HttpChannel.onError() // is now failing the callback which fails the HttpStream, which should decide whether // to reset the HTTP/2 stream, so we may not need the boolean return type. - public void onTimeout(Throwable failure, BiConsumer consumer); + public void onTimeout(TimeoutException timeout, BiConsumer consumer); // TODO: can it be simplified? The callback seems to only be succeeded, which // means it can be converted into a Runnable which may just be the return type diff --git a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java index 97a8f1379b18..20e6b36350c1 100644 --- a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java +++ b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java @@ -18,6 +18,7 @@ import java.util.ArrayDeque; import java.util.Queue; import java.util.concurrent.Executor; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicLong; import org.eclipse.jetty.http2.api.Stream; @@ -174,7 +175,7 @@ private int fill(EndPoint endPoint, ByteBuffer buffer) } @Override - public boolean onIdleExpired() + public boolean onIdleExpired(TimeoutException timeoutException) { boolean idle = isFillInterested(); if (idle) diff --git a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java index f992f2fc0de0..75b447c278cb 100644 --- a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java +++ b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java @@ -839,7 +839,7 @@ private void notifyReset(Stream stream, ResetFrame frame, Callback callback) } } - private void notifyIdleTimeout(Stream stream, Throwable failure, Promise promise) + private void notifyIdleTimeout(Stream stream, TimeoutException failure, Promise promise) { Listener listener = this.listener; if (listener != null) diff --git a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java index 8df803d05b0a..dc778c75cb1b 100644 --- a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java +++ b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.http2.api; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeoutException; import org.eclipse.jetty.http2.frames.DataFrame; import org.eclipse.jetty.http2.frames.HeadersFrame; @@ -218,7 +219,7 @@ public default CompletableFuture reset(ResetFrame frame) /** * @param idleTimeout the stream idle timeout * @see #getIdleTimeout() - * @see Stream.Listener#onIdleTimeout(Stream, Throwable, Promise) + * @see Stream.Listener#onIdleTimeout(Stream, TimeoutException, Promise) */ public void setIdleTimeout(long idleTimeout); @@ -369,7 +370,7 @@ public default void onReset(Stream stream, ResetFrame frame, Callback callback) * @param promise the promise to complete * @see #getIdleTimeout() */ - public default void onIdleTimeout(Stream stream, Throwable x, Promise promise) + public default void onIdleTimeout(Stream stream, TimeoutException x, Promise promise) { promise.succeeded(true); } diff --git a/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnectionFactory.java b/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnectionFactory.java index ac99a6f14e47..0bb7517c9ec7 100644 --- a/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnectionFactory.java +++ b/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnectionFactory.java @@ -168,7 +168,7 @@ private void onFailure(Stream stream, Throwable failure, Callback callback) } @Override - public void onIdleTimeout(Stream stream, Throwable x, Promise promise) + public void onIdleTimeout(Stream stream, TimeoutException x, Promise promise) { getConnection().onStreamTimeout(stream, x, promise); } diff --git a/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HTTP2ServerConnection.java b/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HTTP2ServerConnection.java index acbe10e7debe..289bc28a503a 100644 --- a/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HTTP2ServerConnection.java +++ b/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HTTP2ServerConnection.java @@ -19,6 +19,7 @@ import java.util.List; import java.util.Objects; import java.util.Set; +import java.util.concurrent.TimeoutException; import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.HttpField; @@ -155,14 +156,14 @@ public void onTrailers(Stream stream, HeadersFrame frame) } } - public void onStreamTimeout(Stream stream, Throwable failure, Promise promise) + public void onStreamTimeout(Stream stream, TimeoutException timeout, Promise promise) { if (LOG.isDebugEnabled()) - LOG.debug("Idle timeout on {}", stream, failure); + LOG.debug("Idle timeout on {}", stream, timeout); HTTP2Channel.Server channel = (HTTP2Channel.Server)((HTTP2Stream)stream).getAttachment(); if (channel != null) { - channel.onTimeout(failure, (task, timedOut) -> + channel.onTimeout(timeout, (task, timedOut) -> { if (task != null) offerTask(task, true); diff --git a/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HttpStreamOverHTTP2.java b/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HttpStreamOverHTTP2.java index b98d5981d14d..f0b5dbb5f72b 100644 --- a/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HttpStreamOverHTTP2.java +++ b/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HttpStreamOverHTTP2.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.http2.server.internal; import java.nio.ByteBuffer; +import java.util.concurrent.TimeoutException; import java.util.function.BiConsumer; import java.util.function.Supplier; @@ -451,6 +452,18 @@ private boolean isTunnel(MetaData.Request request, MetaData.Response response) return MetaData.isTunnel(request.getMethod(), response.getStatus()); } + @Override + public long getIdleTimeout() + { + return _stream.getIdleTimeout(); + } + + @Override + public void setIdleTimeout(long idleTimeoutMs) + { + _stream.setIdleTimeout(idleTimeoutMs); + } + @Override public void push(MetaData.Request resource) { @@ -558,9 +571,9 @@ public Throwable consumeAvailable() } @Override - public void onTimeout(Throwable failure, BiConsumer consumer) + public void onTimeout(TimeoutException timeout, BiConsumer consumer) { - Runnable task = _httpChannel.onFailure(failure); + Runnable task = _httpChannel.onIdleTimeout(timeout); boolean idle = !_httpChannel.isRequestHandled(); consumer.accept(task, idle); } diff --git a/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/ServerHTTP2StreamEndPoint.java b/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/ServerHTTP2StreamEndPoint.java index bee4bf12a5d5..8f285557f288 100644 --- a/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/ServerHTTP2StreamEndPoint.java +++ b/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/ServerHTTP2StreamEndPoint.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.http2.server.internal; +import java.util.concurrent.TimeoutException; import java.util.function.BiConsumer; import org.eclipse.jetty.http2.HTTP2Channel; @@ -48,19 +49,19 @@ public Runnable onTrailer(HeadersFrame frame) } @Override - public void onTimeout(Throwable failure, BiConsumer consumer) + public void onTimeout(TimeoutException timeout, BiConsumer consumer) { if (LOG.isDebugEnabled()) - LOG.debug("idle timeout on {}: {}", this, failure); + LOG.debug("idle timeout on {}", this, timeout); boolean result = true; Connection connection = getConnection(); if (connection != null) - result = connection.onIdleExpired(); + result = connection.onIdleExpired(timeout); Runnable r = null; if (result) { - processFailure(failure); - r = () -> close(failure); + processFailure(timeout); + r = () -> close(timeout); } consumer.accept(r, result); } @@ -69,7 +70,7 @@ public void onTimeout(Throwable failure, BiConsumer consumer) public Runnable onFailure(Throwable failure, Callback callback) { if (LOG.isDebugEnabled()) - LOG.debug("failure on {}: {}", this, failure); + LOG.debug("failure on {}", this, failure); processFailure(failure); close(failure); return callback::succeeded; diff --git a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/IdleTimeoutTest.java b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/IdleTimeoutTest.java index bd054472e97c..f12aa6e45573 100644 --- a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/IdleTimeoutTest.java +++ b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/IdleTimeoutTest.java @@ -392,7 +392,7 @@ public void onDataAvailable(Stream stream) } @Override - public void onIdleTimeout(Stream stream, Throwable x, Promise promise) + public void onIdleTimeout(Stream stream, TimeoutException x, Promise promise) { assertThat(x, Matchers.instanceOf(TimeoutException.class)); timeoutLatch.countDown(); @@ -429,7 +429,7 @@ public Stream.Listener onNewStream(Stream stream, HeadersFrame frame) return new Stream.Listener() { @Override - public void onIdleTimeout(Stream stream, Throwable x, Promise promise) + public void onIdleTimeout(Stream stream, TimeoutException x, Promise promise) { timeoutLatch.countDown(); promise.succeeded(true); @@ -476,7 +476,7 @@ public Stream.Listener onNewStream(Stream stream, HeadersFrame frame) return new Stream.Listener() { @Override - public void onIdleTimeout(Stream stream, Throwable x, Promise promise) + public void onIdleTimeout(Stream stream, TimeoutException x, Promise promise) { timeoutLatch.countDown(); promise.succeeded(true); diff --git a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/RawHTTP2ProxyTest.java b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/RawHTTP2ProxyTest.java index 80f88d46b41e..e8db27522da8 100644 --- a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/RawHTTP2ProxyTest.java +++ b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/RawHTTP2ProxyTest.java @@ -25,6 +25,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpStatus; @@ -521,7 +522,7 @@ public void onReset(Stream stream, ResetFrame frame, Callback callback) } @Override - public void onIdleTimeout(Stream stream, Throwable x, Promise promise) + public void onIdleTimeout(Stream stream, TimeoutException x, Promise promise) { if (LOGGER.isDebugEnabled()) LOGGER.debug("CPS idle timeout for {}", stream); @@ -684,7 +685,7 @@ public void onReset(Stream stream, ResetFrame frame, Callback callback) } @Override - public void onIdleTimeout(Stream stream, Throwable x, Promise promise) + public void onIdleTimeout(Stream stream, TimeoutException x, Promise promise) { if (LOGGER.isDebugEnabled()) LOGGER.debug("SPC idle timeout for {}", stream); diff --git a/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/HTTP3StreamConnection.java b/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/HTTP3StreamConnection.java index a69ca91e31cd..a2917482d52a 100644 --- a/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/HTTP3StreamConnection.java +++ b/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/HTTP3StreamConnection.java @@ -17,6 +17,7 @@ import java.io.UncheckedIOException; import java.nio.ByteBuffer; import java.util.concurrent.Executor; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicReference; import org.eclipse.jetty.http.HttpStatus; @@ -84,7 +85,7 @@ public void onOpen() } @Override - protected boolean onReadTimeout(Throwable timeout) + protected boolean onReadTimeout(TimeoutException timeout) { // Idle timeouts are handled by HTTP3Stream. return false; @@ -476,7 +477,8 @@ public void onData(long streamId, DataFrame frame) if (LOG.isDebugEnabled()) LOG.debug("received {}#{}", frame, streamId); Runnable delegate = () -> super.onData(streamId, frame); - if (!HTTP3StreamConnection.this.action.compareAndSet(null, () -> processData(frame, delegate))) + Runnable action = () -> processData(frame, delegate); + if (!HTTP3StreamConnection.this.action.compareAndSet(null, action)) throw new IllegalStateException(); } } diff --git a/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/api/Stream.java b/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/api/Stream.java index 2b1a2c8126e0..8a078c62f35e 100644 --- a/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/api/Stream.java +++ b/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/api/Stream.java @@ -16,6 +16,7 @@ import java.nio.ByteBuffer; import java.util.Objects; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeoutException; import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http3.frames.DataFrame; @@ -354,7 +355,7 @@ public default void onTrailer(Stream.Server stream, HeadersFrame frame) * @param promise the promise to complete with true to reset the stream, * false to ignore the idle timeout */ - public default void onIdleTimeout(Server stream, Throwable failure, Promise promise) + public default void onIdleTimeout(Server stream, TimeoutException failure, Promise promise) { promise.succeeded(true); } diff --git a/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/HTTP3ServerConnectionFactory.java b/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/HTTP3ServerConnectionFactory.java index a9ebfe1743d4..42256031e64f 100644 --- a/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/HTTP3ServerConnectionFactory.java +++ b/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/HTTP3ServerConnectionFactory.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.http3.server; import java.util.Objects; +import java.util.concurrent.TimeoutException; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpVersion; @@ -142,10 +143,10 @@ public void onTrailer(Stream.Server stream, HeadersFrame frame) } @Override - public void onIdleTimeout(Stream.Server stream, Throwable failure, Promise promise) + public void onIdleTimeout(Stream.Server stream, TimeoutException timeout, Promise promise) { HTTP3Stream http3Stream = (HTTP3Stream)stream; - getConnection().onIdleTimeout((HTTP3Stream)stream, failure, (task, timedOut) -> + getConnection().onIdleTimeout((HTTP3Stream)stream, timeout, (task, timedOut) -> { if (task != null) { diff --git a/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HttpStreamOverHTTP3.java b/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HttpStreamOverHTTP3.java index 0cbead3f8d66..e392218d844e 100644 --- a/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HttpStreamOverHTTP3.java +++ b/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HttpStreamOverHTTP3.java @@ -16,6 +16,7 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeoutException; import java.util.function.BiConsumer; import java.util.function.Supplier; @@ -471,6 +472,18 @@ private CompletableFuture sendTrailerFrame(HttpFields trailers) return stream.trailer(frame); } + @Override + public long getIdleTimeout() + { + return stream.getIdleTimeout(); + } + + @Override + public void setIdleTimeout(long idleTimeoutMs) + { + stream.setIdleTimeout(idleTimeoutMs); + } + @Override public boolean isCommitted() { @@ -514,9 +527,9 @@ public void failed(Throwable x) stream.reset(HTTP3ErrorCode.REQUEST_CANCELLED_ERROR.code(), x); } - public void onIdleTimeout(Throwable failure, BiConsumer consumer) + public void onIdleTimeout(TimeoutException failure, BiConsumer consumer) { - Runnable runnable = httpChannel.onFailure(failure); + Runnable runnable = httpChannel.onIdleTimeout(failure); boolean idle = !httpChannel.isRequestHandled(); consumer.accept(runnable, idle); } diff --git a/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/ServerHTTP3StreamConnection.java b/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/ServerHTTP3StreamConnection.java index 3ff29e4b2e23..754948318ab1 100644 --- a/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/ServerHTTP3StreamConnection.java +++ b/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/ServerHTTP3StreamConnection.java @@ -16,6 +16,7 @@ import java.net.InetSocketAddress; import java.net.SocketAddress; import java.util.Set; +import java.util.concurrent.TimeoutException; import java.util.function.BiConsumer; import org.eclipse.jetty.http.HttpVersion; @@ -69,10 +70,10 @@ public Runnable onTrailer(HTTP3Stream stream, HeadersFrame frame) return httpStream.onTrailer(frame); } - public void onIdleTimeout(HTTP3Stream stream, Throwable failure, BiConsumer consumer) + public void onIdleTimeout(HTTP3Stream stream, TimeoutException timeout, BiConsumer consumer) { HttpStreamOverHTTP3 httpStream = (HttpStreamOverHTTP3)stream.getAttachment(); - httpStream.onIdleTimeout(failure, consumer); + httpStream.onIdleTimeout(timeout, consumer); } public Runnable onFailure(HTTP3Stream stream, Throwable failure) diff --git a/jetty-core/jetty-http3/jetty-http3-tests/src/test/java/org/eclipse/jetty/http3/tests/StreamIdleTimeoutTest.java b/jetty-core/jetty-http3/jetty-http3-tests/src/test/java/org/eclipse/jetty/http3/tests/StreamIdleTimeoutTest.java index 4b109fe7d158..cf46d35236ce 100644 --- a/jetty-core/jetty-http3/jetty-http3-tests/src/test/java/org/eclipse/jetty/http3/tests/StreamIdleTimeoutTest.java +++ b/jetty-core/jetty-http3/jetty-http3-tests/src/test/java/org/eclipse/jetty/http3/tests/StreamIdleTimeoutTest.java @@ -16,6 +16,7 @@ import java.net.InetSocketAddress; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicReference; import org.eclipse.jetty.http.HttpFields; @@ -157,7 +158,7 @@ public Stream.Server.Listener onRequest(Stream.Server stream, HeadersFrame frame return new Stream.Server.Listener() { @Override - public void onIdleTimeout(Stream.Server stream, Throwable failure, Promise promise) + public void onIdleTimeout(Stream.Server stream, TimeoutException failure, Promise promise) { serverIdleLatch.countDown(); promise.succeeded(true); diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java index dcfb9cb3c88f..3f40f11c3902 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java @@ -174,8 +174,8 @@ protected void onFillInterestedFailed(Throwable cause) if (_endPoint.isOpen()) { boolean close = true; - if (cause instanceof TimeoutException) - close = onReadTimeout(cause); + if (cause instanceof TimeoutException timeout) + close = onReadTimeout(timeout); if (close) { if (_endPoint.isOutputShutdown()) @@ -195,7 +195,7 @@ protected void onFillInterestedFailed(Throwable cause) * @param timeout the cause of the read timeout * @return true to signal that the endpoint must be closed, false to keep the endpoint open */ - protected boolean onReadTimeout(Throwable timeout) + protected boolean onReadTimeout(TimeoutException timeout) { return true; } @@ -268,7 +268,7 @@ public void close() } @Override - public boolean onIdleExpired() + public boolean onIdleExpired(TimeoutException timeoutException) { return true; } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java index 48e726a80ca9..894ccf08344d 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java @@ -392,30 +392,27 @@ public WriteFlusher getWriteFlusher() protected void onIdleExpired(TimeoutException timeout) { Connection connection = _connection; - if (connection != null && !connection.onIdleExpired()) + if (connection != null && !connection.onIdleExpired(timeout)) return; boolean outputShutdown = isOutputShutdown(); boolean inputShutdown = isInputShutdown(); boolean fillFailed = _fillInterest.onFail(timeout); boolean writeFailed = _writeFlusher.onFail(timeout); + boolean isOpen = isOpen(); - // If the endpoint is half closed and there was no fill/write handling, then close here. - // This handles the situation where the connection has completed its close handling - // and the endpoint is half closed, but the other party does not complete the close. - // This perhaps should not check for half closed, however the servlet spec case allows - // for a dispatched servlet or suspended request to extend beyond the connections idle - // time. So if this test would always close an idle endpoint that is not handled, then - // we would need a mode to ignore timeouts for some HTTP states - if (isOpen() && (inputShutdown || outputShutdown) && !(fillFailed || writeFailed)) - close(); - else - LOG.debug("handled idle inputShutdown={} outputShutdown={} fillFailed={} writeFailed={} for {}", + if (LOG.isDebugEnabled()) + LOG.debug("handled idle isOpen={} inputShutdown={} outputShutdown={} fillFailed={} writeFailed={} for {}", + isOpen, inputShutdown, outputShutdown, fillFailed, writeFailed, this); + + // If the endpoint is open and there was no fill/write handling, then close here. + if (isOpen && !(fillFailed || writeFailed)) + close(timeout); } @Override diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Connection.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Connection.java index 52b202009c48..edbd16f20580 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Connection.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Connection.java @@ -16,6 +16,7 @@ import java.io.Closeable; import java.nio.ByteBuffer; import java.util.EventListener; +import java.util.concurrent.TimeoutException; import org.eclipse.jetty.util.component.Container; @@ -82,7 +83,7 @@ public interface Connection extends Closeable * @return true to let the EndPoint handle the idle timeout, * false to tell the EndPoint to halt the handling of the idle timeout. */ - boolean onIdleExpired(); + boolean onIdleExpired(TimeoutException timeoutException); long getMessagesIn(); diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java index ad2d1a8fc160..6f54876226fb 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java @@ -19,6 +19,7 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.Executor; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; import java.util.function.ToIntFunction; @@ -348,9 +349,9 @@ public void close() } @Override - public boolean onIdleExpired() + public boolean onIdleExpired(TimeoutException timeoutException) { - return getSslEndPoint().getConnection().onIdleExpired(); + return getSslEndPoint().getConnection().onIdleExpired(timeoutException); } @Override diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ByteArrayEndPointTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ByteArrayEndPointTest.java index fa6668e13a51..cb4bb5f1a53f 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ByteArrayEndPointTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ByteArrayEndPointTest.java @@ -17,11 +17,9 @@ import java.nio.ByteBuffer; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.FutureCallback; -import org.eclipse.jetty.util.NanoTime; import org.eclipse.jetty.util.thread.Scheduler; import org.eclipse.jetty.util.thread.TimerScheduler; import org.junit.jupiter.api.AfterEach; @@ -30,9 +28,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -270,33 +265,6 @@ public void testIdle() throws Exception assertTrue(endp.isOpen()); Thread.sleep(oneAndHalfIdleTimeout); - // Still open because it has not been oshut or closed explicitly - // and there are no callbacks, so idle timeout is ignored. - assertTrue(endp.isOpen()); - - // Normal read is immediate, since there is data to read. - ByteBuffer buffer = BufferUtil.allocate(1024); - FutureCallback fcb = new FutureCallback(); - endp.fillInterested(fcb); - fcb.get(idleTimeout, TimeUnit.MILLISECONDS); - assertTrue(fcb.isDone()); - assertEquals(4, endp.fill(buffer)); - assertEquals("test", BufferUtil.toString(buffer)); - - // Wait for a read timeout. - long start = NanoTime.now(); - fcb = new FutureCallback(); - endp.fillInterested(fcb); - try - { - fcb.get(); - fail("Expected ExecutionException"); - } - catch (ExecutionException t) - { - assertThat(t.getCause(), instanceOf(TimeoutException.class)); - } - assertThat(NanoTime.millisSince(start), greaterThan(halfIdleTimeout)); - assertThat("Endpoint open", endp.isOpen(), is(true)); + assertFalse(endp.isOpen()); } } diff --git a/jetty-core/jetty-quic/jetty-quic-client/src/main/java/org/eclipse/jetty/quic/client/ClientQuicConnection.java b/jetty-core/jetty-quic/jetty-quic-client/src/main/java/org/eclipse/jetty/quic/client/ClientQuicConnection.java index 3edb7e493d65..bd0e307cb788 100644 --- a/jetty-core/jetty-quic/jetty-quic-client/src/main/java/org/eclipse/jetty/quic/client/ClientQuicConnection.java +++ b/jetty-core/jetty-quic/jetty-quic-client/src/main/java/org/eclipse/jetty/quic/client/ClientQuicConnection.java @@ -23,6 +23,7 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import org.eclipse.jetty.io.ClientConnector; import org.eclipse.jetty.io.DatagramChannelEndPoint; @@ -159,7 +160,7 @@ protected void onFailure(Throwable failure) } @Override - public boolean onIdleExpired() + public boolean onIdleExpired(TimeoutException timeoutException) { boolean idle = isFillInterested(); long idleTimeout = getEndPoint().getIdleTimeout(); diff --git a/jetty-core/jetty-quic/jetty-quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicConnection.java b/jetty-core/jetty-quic/jetty-quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicConnection.java index 37693a98f843..0b324ec8a870 100644 --- a/jetty-core/jetty-quic/jetty-quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicConnection.java +++ b/jetty-core/jetty-quic/jetty-quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicConnection.java @@ -24,6 +24,7 @@ import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Executor; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import org.eclipse.jetty.io.AbstractConnection; @@ -171,7 +172,7 @@ public void fillInterested() } @Override - public abstract boolean onIdleExpired(); + public abstract boolean onIdleExpired(TimeoutException timeoutException); @Override public void close() diff --git a/jetty-core/jetty-quic/jetty-quic-server/src/main/java/org/eclipse/jetty/quic/server/ServerQuicConnection.java b/jetty-core/jetty-quic/jetty-quic-server/src/main/java/org/eclipse/jetty/quic/server/ServerQuicConnection.java index 01a1a8b34edc..988a24053171 100644 --- a/jetty-core/jetty-quic/jetty-quic-server/src/main/java/org/eclipse/jetty/quic/server/ServerQuicConnection.java +++ b/jetty-core/jetty-quic/jetty-quic-server/src/main/java/org/eclipse/jetty/quic/server/ServerQuicConnection.java @@ -18,6 +18,7 @@ import java.net.SocketAddress; import java.nio.ByteBuffer; import java.util.Iterator; +import java.util.concurrent.TimeoutException; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.CyclicTimeouts; @@ -99,7 +100,7 @@ public void schedule(ServerQuicSession session) } @Override - public boolean onIdleExpired() + public boolean onIdleExpired(TimeoutException timeoutException) { // The current server architecture only has one listening // DatagramChannelEndPoint, so we ignore idle timeouts. diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java index 31a9b67af218..3d1d215e21d3 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java @@ -13,6 +13,10 @@ package org.eclipse.jetty.server; +import java.util.concurrent.TimeoutException; +import java.util.function.Consumer; +import java.util.function.Predicate; + import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.server.internal.HttpChannelState; import org.eclipse.jetty.util.thread.Invocable; @@ -70,14 +74,25 @@ public interface HttpChannel extends Invocable */ Runnable onContentAvailable(); + /** + *

Notifies this {@code HttpChannel} that an idle timeout happened.

+ * + * @param idleTimeout the timeout. + * @return a {@code Runnable} that performs the timeout action, or {@code null} + * if no action need be performed by the calling thread + * @see Request#addIdleTimeoutListener(Predicate) + */ + Runnable onIdleTimeout(TimeoutException idleTimeout); + /** *

Notifies this {@code HttpChannel} that an asynchronous failure happened.

- *

Typical failure examples could be idle timeouts, I/O read failures or + *

Typical failure examples could be HTTP/2 resets or * protocol failures (for example, invalid request bytes).

* * @param failure the failure cause. * @return a {@code Runnable} that performs the failure action, or {@code null} - * if no failure action should be performed by the caller thread + * if no failure action need be performed by the calling thread + * @see Request#addFailureListener(Consumer) */ Runnable onFailure(Throwable failure); diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpStream.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpStream.java index ee26a94278eb..99026178623f 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpStream.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpStream.java @@ -99,6 +99,10 @@ default void push(MetaData.Request resource) throw new UnsupportedOperationException(); } + long getIdleTimeout(); + + void setIdleTimeout(long idleTimeoutMs); + boolean isCommitted(); default TunnelSupport getTunnelSupport() @@ -192,6 +196,18 @@ public void push(MetaData.Request resource) getWrapped().push(resource); } + @Override + public long getIdleTimeout() + { + return getWrapped().getIdleTimeout(); + } + + @Override + public void setIdleTimeout(long idleTimeoutMs) + { + getWrapped().setIdleTimeout(idleTimeoutMs); + } + @Override public final boolean isCommitted() { diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index fec4c56d6fc5..cf0fb744492c 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -22,6 +22,8 @@ import java.security.Principal; import java.util.List; import java.util.Locale; +import java.util.concurrent.TimeoutException; +import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -249,18 +251,41 @@ default void push(MetaData.Request resource) } /** - *

Adds a listener for asynchronous errors.

+ *

Adds a listener for idle timeouts.

*

The listener is a predicate function that should return {@code true} to indicate - * that the function will complete (either successfully or with a failure) the callback - * received from {@link org.eclipse.jetty.server.Handler#handle(Request, Response, Callback)}, or - * {@code false} otherwise.

+ * that the idle timeout should be handled by the container as a hard failure + * (see {@link #addFailureListener(Consumer)}); or {@code false} to ignore that specific timeout and for another timeout + * to occur after another idle period.

+ *

Any pending {@link #demand(Runnable)} or {@link Response#write(boolean, ByteBuffer, Callback)} operations + * are not affected by this call. Applications need to be mindful of any such pending operations if attempting + * to make new operations.

*

Listeners are processed in sequence, and the first that returns {@code true} * stops the processing of subsequent listeners, which are therefore not invoked.

* - * @param onError the predicate function - * @return true if the listener completes the callback, false otherwise + * @param onIdleTimeout the predicate function + * @see #addFailureListener(Consumer) */ - boolean addErrorListener(Predicate onError); + void addIdleTimeoutListener(Predicate onIdleTimeout); + + /** + *

Adds a listener for asynchronous hard errors.

+ *

When a listener is called, the effects of the error will already have taken place:

+ *
    + *
  • Pending {@link #demand(Runnable)} will be woken up.
    • + *
  • Calls to {@link #read()} will return the {@code Throwable}.
    • + *
  • Pending and new {@link Response#write(boolean, ByteBuffer, Callback)} calls will be failed by + * calling {@link Callback#failed(Throwable)} on the callback passed to {@code write(...)}.
    • + *
  • Any call to {@link Callback#succeeded()} on the callback passed to + * {@link Handler#handle(Request, Response, Callback)} will effectively be a call to {@link Callback#failed(Throwable)} + * with the notified {@link Throwable}.
    • + *
+ *

Listeners are processed in sequence. When all listeners are invoked then {@link Callback#failed(Throwable)} + * will be called on the callback passed to {@link Handler#handle(Request, Response, Callback)}.

+ * + * @param onFailure the consumer function + * @see #addIdleTimeoutListener(Predicate) + */ + void addFailureListener(Consumer onFailure); TunnelSupport getTunnelSupport(); @@ -657,9 +682,15 @@ public void push(MetaData.Request resource) } @Override - public boolean addErrorListener(Predicate onError) + public void addIdleTimeoutListener(Predicate onIdleTimeout) + { + getWrapped().addIdleTimeoutListener(onIdleTimeout); + } + + @Override + public void addFailureListener(Consumer onFailure) { - return getWrapped().addErrorListener(onError); + getWrapped().addFailureListener(onFailure); } @Override diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index ced33a0e0e2a..26c9ef7ab81b 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -28,6 +28,7 @@ import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; +import java.util.function.Predicate; import java.util.stream.Collectors; import org.eclipse.jetty.http.HttpField; @@ -1202,6 +1203,23 @@ public void call(Invocable.Callable callable, Request request) throws Exception } } + public boolean test(Predicate predicate, T t, Request request) + { + Context lastContext = __context.get(); + if (lastContext == this) + return predicate.test(t); + + ClassLoader lastLoader = enterScope(request); + try + { + return predicate.test(t); + } + finally + { + exitScope(request, lastContext, lastLoader); + } + } + public void accept(Consumer consumer, Throwable t, Request request) { Context lastContext = __context.get(); diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextRequest.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextRequest.java index ba02bfec1539..1a4eb4b30196 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextRequest.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextRequest.java @@ -13,6 +13,8 @@ package org.eclipse.jetty.server.handler; +import java.util.concurrent.TimeoutException; +import java.util.function.Consumer; import java.util.function.Predicate; import org.eclipse.jetty.server.Context; @@ -40,15 +42,15 @@ public void demand(Runnable demandCallback) } @Override - public boolean addErrorListener(Predicate onError) + public void addIdleTimeoutListener(Predicate onIdleTimeout) { - return super.addErrorListener(t -> - { - // TODO: implement the line below - // return _context.apply(onError::test, t, ContextRequest.this); - _context.accept(onError::test, t, ContextRequest.this); - return true; - }); + super.addIdleTimeoutListener(t -> _context.test(onIdleTimeout, t, ContextRequest.this)); + } + + @Override + public void addFailureListener(Consumer onFailure) + { + super.addFailureListener(t -> _context.accept(onFailure, t, ContextRequest.this)); } @Override diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java index 74b9e9f34680..c6e7ab8cf815 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java @@ -19,7 +19,9 @@ import java.util.Objects; import java.util.Set; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.LongAdder; +import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; @@ -110,12 +112,23 @@ private enum StreamSendState private boolean _callbackCompleted = false; private ChannelRequest _request; private ChannelResponse _response; + private long _oldIdleTimeout; private HttpStream _stream; private long _committedContentLength = -1; private Runnable _onContentAvailable; - private Content.Chunk.Error _error; - private Throwable _failure; - private Predicate _onError; + private Predicate _onIdleTimeout; + /** + * Failure passed to {@link #onFailure(Throwable)} + */ + private Content.Chunk.Error _failure; + /** + * Listener for {@link #onFailure(Throwable)} events + */ + private Consumer _onFailure; + /** + * Failure passed to {@link ChannelCallback#failed(Throwable)} + */ + private Throwable _callbackFailure; private Attributes _cache; public HttpChannelState(ConnectionMetaData connectionMetaData) @@ -149,11 +162,11 @@ public void recycle() _handling = null; _handled = false; _callbackCompleted = false; - _failure = null; + _callbackFailure = null; _committedContentLength = -1; _onContentAvailable = null; - _error = null; - _onError = null; + _failure = null; + _onFailure = null; } } @@ -262,13 +275,20 @@ public Runnable onRequest(MetaData.Request request) _response = new ChannelResponse(_request); HttpFields.Mutable responseHeaders = _response.getHeaders(); - if (getHttpConfiguration().getSendServerVersion()) + HttpConfiguration httpConfiguration = getHttpConfiguration(); + if (httpConfiguration.getSendServerVersion()) responseHeaders.add(SERVER_VERSION); - if (getHttpConfiguration().getSendXPoweredBy()) + if (httpConfiguration.getSendXPoweredBy()) responseHeaders.add(POWERED_BY); - if (getHttpConfiguration().getSendDateHeader()) + if (httpConfiguration.getSendDateHeader()) responseHeaders.add(getConnectionMetaData().getConnector().getServer().getDateField()); + long idleTO = httpConfiguration.getIdleTimeout(); + _oldIdleTimeout = _stream.getIdleTimeout(); + if (idleTO >= 0 && _oldIdleTimeout != idleTO) + _stream.setIdleTimeout(idleTO); + + // This is deliberately not serialized to allow a handler to block. return _handlerInvoker; } @@ -326,15 +346,43 @@ public Invocable.InvocationType getInvocationType() return Invocable.getInvocationType(onContent); } - public Runnable onFailure(Throwable x) + @Override + public Runnable onIdleTimeout(TimeoutException t) { - if (LOG.isDebugEnabled()) - LOG.debug("onFailure {}", this, x); + Predicate onIdleTimeout; + try (AutoLock ignored = _lock.lock()) + { + if (LOG.isDebugEnabled()) + LOG.debug("onIdleTimeout {}", this, t); + onIdleTimeout = _onIdleTimeout; + } + + if (onIdleTimeout != null) + { + Runnable onIdle = () -> + { + if (onIdleTimeout.test(t)) + { + Runnable task = onFailure(t); + if (task != null) + task.run(); + } + }; + return _serializedInvoker.offer(onIdle); + } + return onFailure(t); // TODO can we avoid double lock? + } + @Override + public Runnable onFailure(Throwable x) + { HttpStream stream; Runnable task; try (AutoLock ignored = _lock.lock()) { + if (LOG.isDebugEnabled()) + LOG.debug("onFailure {}", this, x); + // If the channel doesn't have a stream, then the error is ignored. if (_stream == null) return null; @@ -349,14 +397,13 @@ public Runnable onFailure(Throwable x) } // Set the error to arrange for any subsequent reads, demands or writes to fail. - if (_error == null) + if (_failure == null) { - _error = Content.Chunk.from(x); + _failure = Content.Chunk.from(x); } - else if (_error.getCause() != x) + else if (ExceptionUtil.areNotAssociated(_failure.getCause(), x) && _failure.getCause().getClass() != x.getClass()) { - _error.getCause().addSuppressed(x); - return null; + _failure.getCause().addSuppressed(x); } // If not handled, then we just fail the request callback @@ -377,18 +424,17 @@ else if (_error.getCause() != x) ChannelRequest request = _request; Runnable invokeListeners = () -> { - Predicate onError; + Consumer onFailure; try (AutoLock ignore = _lock.lock()) { - onError = _onError; + onFailure = _onFailure; } try { if (LOG.isDebugEnabled()) - LOG.debug("invokeListeners {} {}", HttpChannelState.this, onError, x); - if (onError.test(x)) - return; + LOG.debug("invokeListeners {} {}", HttpChannelState.this, onFailure, x); + onFailure.accept(x); } catch (Throwable throwable) { @@ -586,7 +632,7 @@ public void run() stream = _stream; _handling = null; _handled = true; - failure = _failure; + failure = _callbackFailure; callbackCompleted = _callbackCompleted; lastStreamSendComplete = lockedIsLastStreamSendCompleted(); completeStream = callbackCompleted && lastStreamSendComplete; @@ -640,7 +686,7 @@ public void failed(Throwable failure) _streamSendState = StreamSendState.LAST_COMPLETE; completeStream = _handling == null; stream = _stream; - failure = _failure = ExceptionUtil.combine(_failure, failure); + failure = _callbackFailure = ExceptionUtil.combine(_callbackFailure, failure); } if (completeStream) completeStream(stream, failure); @@ -663,6 +709,10 @@ private void completeStream(HttpStream stream, Throwable failure) Parts parts = (Parts)_request.getAttribute(Parts.class.getName()); if (parts != null) parts.close(); + + long idleTO = getHttpConfiguration().getIdleTimeout(); + if (idleTO > 0 && _oldIdleTimeout != idleTO) + stream.setIdleTimeout(_oldIdleTimeout); } finally { @@ -867,7 +917,7 @@ public Content.Chunk read() { HttpChannelState httpChannel = lockedGetHttpChannelState(); - Content.Chunk error = httpChannel._error; + Content.Chunk error = httpChannel._failure; if (error != null) return error; @@ -913,7 +963,7 @@ public void demand(Runnable demandCallback) if (LOG.isDebugEnabled()) LOG.debug("demand {}", httpChannel); - error = httpChannel._error != null; + error = httpChannel._failure != null; if (!error) { if (httpChannel._onContentAvailable != null) @@ -944,30 +994,66 @@ public void push(MetaData.Request resource) } @Override - public boolean addErrorListener(Predicate onError) + public void addIdleTimeoutListener(Predicate onIdleTimeout) { try (AutoLock ignored = _lock.lock()) { HttpChannelState httpChannel = lockedGetHttpChannelState(); - if (httpChannel._error != null) - return false; + if (httpChannel._failure != null) + return; - if (httpChannel._onError == null) + if (httpChannel._onIdleTimeout == null) { - httpChannel._onError = onError; + httpChannel._onIdleTimeout = onIdleTimeout; } else { - Predicate previous = httpChannel._onError; - httpChannel._onError = throwable -> + Predicate previous = httpChannel._onIdleTimeout; + httpChannel._onIdleTimeout = throwable -> { if (!previous.test(throwable)) - return onError.test(throwable); + return onIdleTimeout.test(throwable); return true; }; } - return true; + } + } + + @Override + public void addFailureListener(Consumer onFailure) + { + try (AutoLock ignored = _lock.lock()) + { + HttpChannelState httpChannel = lockedGetHttpChannelState(); + + if (httpChannel._failure != null) + return; + + if (httpChannel._onFailure == null) + { + httpChannel._onFailure = onFailure; + } + else + { + Consumer previous = httpChannel._onFailure; + httpChannel._onFailure = throwable -> + { + try + { + previous.accept(throwable); + } + catch (Throwable t) + { + if (ExceptionUtil.areNotAssociated(throwable, t)) + throwable.addSuppressed(t); + } + finally + { + onFailure.accept(throwable); + } + }; + } } } @@ -1005,33 +1091,26 @@ public String toString() public static class ChannelResponse implements Response, Callback { private final ChannelRequest _request; - private int _status; + private final ResponseHttpFields _httpFields; + protected int _status; private long _contentBytesWritten; private Supplier _trailers; private Callback _writeCallback; - protected boolean _errorMode; private ChannelResponse(ChannelRequest request) { _request = request; + _httpFields = getResponseHttpFields(_request.lockedGetHttpChannelState()); } - private void lockedPrepareErrorResponse() + protected ResponseHttpFields getResponseHttpFields(HttpChannelState httpChannelState) { - // reset the response state, so we can generate an error response, - // remembering any server or date headers (probably a nicer way of doing this). - HttpChannelState httpChannelState = _request.lockedGetHttpChannelState(); - HttpField serverField = httpChannelState._responseHeaders.getField(HttpHeader.SERVER); - HttpField dateField = httpChannelState._responseHeaders.getField(HttpHeader.DATE); - httpChannelState._responseHeaders.reset(); - httpChannelState._committedContentLength = -1; - reset(); - if (serverField != null) - httpChannelState._responseHeaders.put(serverField); - if (dateField != null) - httpChannelState._responseHeaders.put(dateField); - setStatus(HttpStatus.INTERNAL_SERVER_ERROR_500); - _errorMode = true; + return httpChannelState._responseHeaders; + } + + protected ResponseHttpFields getResponseHttpFields() + { + return _httpFields; } private boolean lockedIsWriting() @@ -1075,7 +1154,7 @@ public void setStatus(int code) @Override public HttpFields.Mutable getHeaders() { - return _request.getHttpChannelState()._responseHeaders; + return _httpFields; } @Override @@ -1115,19 +1194,21 @@ public void write(boolean last, ByteBuffer content, Callback callback) if (_writeCallback != null) failure = new IllegalStateException("write pending"); - else if (!_errorMode && httpChannelState._error != null) - failure = httpChannelState._error.getCause(); - else if (contentLength >= 0) + else { - // If the content length were not compatible with what was written, then we need to abort. - String lengthError = (totalWritten > contentLength) ? "written %d > %d content-length" - : (last && totalWritten < contentLength) ? "written %d < %d content-length" : null; - if (lengthError != null) + failure = getFailure(httpChannelState); + if (failure == null && contentLength >= 0) { - String message = lengthError.formatted(totalWritten, contentLength); - if (LOG.isDebugEnabled()) - LOG.debug("fail {} {}", callback, message); - failure = new IOException(message); + // If the content length were not compatible with what was written, then we need to abort. + String lengthError = (totalWritten > contentLength) ? "written %d > %d content-length" + : (last && totalWritten < contentLength) ? "written %d < %d content-length" : null; + if (lengthError != null) + { + String message = lengthError.formatted(totalWritten, contentLength); + if (LOG.isDebugEnabled()) + LOG.debug("fail {} {}", callback, message); + failure = new IOException(message); + } } } @@ -1151,7 +1232,7 @@ else if (failure != null) _writeCallback = callback; _contentBytesWritten = totalWritten; stream = httpChannelState._stream; - if (httpChannelState._responseHeaders.commit()) + if (_httpFields.commit()) responseMetaData = lockedPrepareResponse(httpChannelState, last); } } @@ -1164,6 +1245,12 @@ else if (failure != null) } } + protected Throwable getFailure(HttpChannelState httpChannelState) + { + Content.Chunk.Error failure = httpChannelState._failure; + return failure == null ? null : failure.getCause(); + } + /** * Called when the call to * {@link HttpStream#send(MetaData.Request, MetaData.Response, boolean, ByteBuffer, Callback)} @@ -1227,7 +1314,7 @@ public InvocationType getInvocationType() @Override public boolean isCommitted() { - return _request.getHttpChannelState()._responseHeaders.isCommitted(); + return _httpFields.isCommitted(); } @Override @@ -1238,7 +1325,7 @@ public boolean isCompletedSuccessfully() if (_request._httpChannelState == null) return false; - return _request._httpChannelState._callbackCompleted && _request._httpChannelState._failure == null; + return _request._httpChannelState._callbackCompleted && _request._httpChannelState._callbackFailure == null; } } @@ -1276,7 +1363,7 @@ MetaData.Response lockedPrepareResponse(HttpChannelState httpChannel, boolean la _status = HttpStatus.OK_200; // Can we set the content length? - HttpFields.Mutable mutableHeaders = httpChannel._responseHeaders.getMutableHttpFields(); + HttpFields.Mutable mutableHeaders = _httpFields.getMutableHttpFields(); httpChannel._committedContentLength = mutableHeaders.getLongField(HttpHeader.CONTENT_LENGTH); if (last && httpChannel._committedContentLength < 0L) { @@ -1288,7 +1375,7 @@ MetaData.Response lockedPrepareResponse(HttpChannelState httpChannel, boolean la return new MetaData.Response( _status, null, httpChannel.getConnectionMetaData().getHttpVersion(), - httpChannel._responseHeaders, + _httpFields, httpChannel._committedContentLength, getTrailersSupplier() ); @@ -1326,6 +1413,8 @@ public void succeeded() ChannelResponse response; MetaData.Response responseMetaData = null; boolean completeStream; + ErrorResponse errorResponse = null; + try (AutoLock ignored = _request._lock.lock()) { request = _request; @@ -1343,7 +1432,7 @@ public void succeeded() if (lockedCompleteCallback()) return; - assert httpChannelState._failure == null; + assert httpChannelState._callbackFailure == null; needLastStreamSend = httpChannelState.lockedLastStreamSend(); completeStream = !needLastStreamSend && httpChannelState._handling == null && httpChannelState.lockedIsLastStreamSendCompleted(); @@ -1367,9 +1456,9 @@ public void succeeded() if (failure != null) { - httpChannelState._failure = failure; + httpChannelState._callbackFailure = failure; if (!stream.isCommitted()) - response.lockedPrepareErrorResponse(); + errorResponse = new ErrorResponse(request); else completeStream = true; } @@ -1378,8 +1467,8 @@ public void succeeded() if (LOG.isDebugEnabled()) LOG.debug("succeeded: failure={} needLastStreamSend={} {}", failure, needLastStreamSend, this); - if (failure != null) - Response.writeError(request, response, new ErrorCallback(request, stream, failure), failure); + if (errorResponse != null) + Response.writeError(request, errorResponse, new ErrorCallback(request, errorResponse, stream, failure), failure); else if (needLastStreamSend) stream.send(_request._metaData, responseMetaData, true, null, httpChannelState._handlerInvoker); else if (completeStream) @@ -1398,21 +1487,19 @@ public void failed(Throwable failure) // Called when the request/response cycle is completing with a failure. HttpStream stream; ChannelRequest request; - ChannelResponse response; HttpChannelState httpChannelState; - boolean writeError; + ErrorResponse errorResponse = null; try (AutoLock ignored = _request._lock.lock()) { httpChannelState = _request._httpChannelState; stream = httpChannelState._stream; request = _request; - response = httpChannelState._response; if (lockedCompleteCallback()) return; - assert httpChannelState._failure == null; + assert httpChannelState._callbackFailure == null; - httpChannelState._failure = failure; + httpChannelState._callbackFailure = failure; // Consume any input. Throwable unconsumed = stream.consumeAvailable(); @@ -1422,13 +1509,12 @@ public void failed(Throwable failure) if (LOG.isDebugEnabled()) LOG.debug("failed stream.isCommitted={}, response.isCommitted={} {}", httpChannelState._stream.isCommitted(), httpChannelState._response.isCommitted(), this); - writeError = !stream.isCommitted(); - if (writeError) - response.lockedPrepareErrorResponse(); + if (!stream.isCommitted()) + errorResponse = new ErrorResponse(request); } - if (writeError) - Response.writeError(request, response, new ErrorCallback(request, stream, failure), failure); + if (errorResponse != null) + Response.writeError(request, errorResponse, new ErrorCallback(request, errorResponse, stream, failure), failure); else _request.getHttpChannelState()._handlerInvoker.failed(failure); } @@ -1471,6 +1557,53 @@ public InvocationType getInvocationType() } } + /** + * Used as the {@link Response} when writing the error response + * from {@link HttpChannelState.ChannelCallback#failed(Throwable)}. + */ + private static class ErrorResponse extends ChannelResponse + { + public ErrorResponse(ChannelRequest request) + { + super(request); + _status = HttpStatus.INTERNAL_SERVER_ERROR_500; + } + + @Override + protected Throwable getFailure(HttpChannelState httpChannelState) + { + // we ignore channel failures so we can try to generate an error response. + return null; + } + + @Override + protected ResponseHttpFields getResponseHttpFields(HttpChannelState httpChannelState) + { + httpChannelState._committedContentLength = -1; + HttpFields original = super.getResponseHttpFields(httpChannelState); + ResponseHttpFields httpFields = new ResponseHttpFields(); + + for (HttpField field : original) + { + HttpHeader header = field.getHeader(); + if (header == HttpHeader.SERVER || header == HttpHeader.DATE) + httpFields.add(field); + } + return httpFields; + } + + @Override + MetaData.Response lockedPrepareResponse(HttpChannelState httpChannelState, boolean last) + { + MetaData.Response httpFields = super.lockedPrepareResponse(httpChannelState, last); + httpChannelState._response._status = _status; + HttpFields.Mutable originalResponseFields = httpChannelState._responseHeaders.getMutableHttpFields(); + originalResponseFields.clear(); + originalResponseFields.add(getResponseHttpFields()); + return httpFields; + } + } + /** * Used as the {@link Response} and {@link Callback} when writing the error response * from {@link HttpChannelState.ChannelCallback#failed(Throwable)}. @@ -1478,12 +1611,14 @@ public InvocationType getInvocationType() private static class ErrorCallback implements Callback { private final ChannelRequest _request; + private final ErrorResponse _errorResponse; private final HttpStream _stream; private final Throwable _failure; - public ErrorCallback(ChannelRequest request, HttpStream stream, Throwable failure) + public ErrorCallback(ChannelRequest request, ErrorResponse response, HttpStream stream, Throwable failure) { _request = request; + _errorResponse = response; _stream = stream; _failure = failure; } @@ -1507,8 +1642,8 @@ public void succeeded() // Did the ErrorHandler do the last write? needLastWrite = httpChannelState.lockedLastStreamSend(); - if (needLastWrite && httpChannelState._responseHeaders.commit()) - responseMetaData = httpChannelState._response.lockedPrepareResponse(httpChannelState, true); + if (needLastWrite && _errorResponse.getResponseHttpFields().commit()) + responseMetaData = _errorResponse.lockedPrepareResponse(httpChannelState, true); } if (needLastWrite) @@ -1538,13 +1673,16 @@ public void failed(Throwable x) if (LOG.isDebugEnabled()) LOG.debug("ErrorWrite failed: {}", this, x); Throwable failure; + HttpChannelState httpChannelState; try (AutoLock ignored = _request._lock.lock()) { failure = _failure; + httpChannelState = _request.lockedGetHttpChannelState(); + httpChannelState._response._status = _errorResponse._status; } if (ExceptionUtil.areNotAssociated(failure, x)) failure.addSuppressed(x); - _request.getHttpChannelState()._handlerInvoker.failed(failure); + httpChannelState._handlerInvoker.failed(failure); } @Override @@ -1566,7 +1704,7 @@ protected void onError(Runnable task, Throwable failure) { callbackCompleted = _callbackCompleted; request = _request; - error = _request == null ? null : _error; + error = _request == null ? null : _failure; } if (request == null || callbackCompleted) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java index 56fb22d86fa4..a566d1610911 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java @@ -22,6 +22,7 @@ import java.util.Objects; import java.util.Set; import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; @@ -657,6 +658,17 @@ protected void onFillInterestedFailed(Throwable cause) super.onFillInterestedFailed(cause); } + @Override + public boolean onIdleExpired(TimeoutException timeout) + { + if (_httpChannel.getRequest() == null) + return true; + Runnable task = _httpChannel.onIdleTimeout(timeout); + if (task != null) + getExecutor().execute(task); + return false; // We've handle the exception + } + @Override public void onOpen() { @@ -1448,6 +1460,18 @@ else if (_expects100Continue) _sendCallback.iterate(); } + @Override + public long getIdleTimeout() + { + return getEndPoint().getIdleTimeout(); + } + + @Override + public void setIdleTimeout(long idleTimeoutMs) + { + getEndPoint().setIdleTimeout(idleTimeoutMs); + } + @Override public boolean isCommitted() { diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ConnectorTimeoutTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ConnectorTimeoutTest.java index 82f3e3e396e3..0758b29508cd 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ConnectorTimeoutTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ConnectorTimeoutTest.java @@ -583,15 +583,25 @@ protected static class WaitHandler extends Handler.Abstract @Override public boolean handle(Request request, Response response, Callback callback) throws Exception { + request.addIdleTimeoutListener(t -> false); response.setStatus(200); try { - Thread.sleep(2000); + Thread.sleep(MAX_IDLE_TIME * 3 / 2); } catch (Exception e) { e.printStackTrace(); } + + // TODO what do we do about the failing write? + // should timeout errors be non persistent? + Callback ocb = callback; + callback = Callback.from(ocb::succeeded, t -> + { + t.printStackTrace(); + ocb.failed(t); + }); Content.Sink.write(response, true, "Hello World\r\n", callback); return true; } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelTest.java index 19df0a1fee30..518428b9dcd2 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelTest.java @@ -1175,13 +1175,9 @@ public void testOnError() throws Exception public boolean handle(Request request, Response response, Callback callback) { handling.set(response); - request.addErrorListener(t -> false); - request.addErrorListener(t -> !error.compareAndSet(null, t)); - request.addErrorListener(t -> - { - callback.failed(t); - return true; - }); + request.addFailureListener(t -> error.set(null)); + request.addFailureListener(t -> error.compareAndSet(null, t)); + request.addFailureListener(t -> error.compareAndSet(null, new Throwable("WRONG"))); return true; } }; diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java index 6944d49026bc..5e7b46a85c87 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java @@ -44,7 +44,6 @@ import org.eclipse.jetty.io.Content; import org.eclipse.jetty.logging.StacklessLogging; import org.eclipse.jetty.server.handler.DumpHandler; -import org.eclipse.jetty.server.internal.HttpChannelState; import org.eclipse.jetty.server.internal.HttpConnection; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; @@ -1158,24 +1157,13 @@ public boolean handle(Request request, Response response, Callback callback) }); _server.start(); - String response = null; - try (StacklessLogging stackless = new StacklessLogging(HttpChannelState.class)) - { - LOG.info("Expect IOException: Response header too large..."); - response = _connector.getResponse("GET / HTTP/1.1\r\n" + - "Host: localhost\r\n" + - "\r\n" - ); + String response = _connector.getResponse("GET / HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "\r\n" + ); - checkContains(response, 0, "HTTP/1.1 500"); - assertTrue(checkError.await(1, TimeUnit.SECONDS)); - } - catch (Exception e) - { - if (response != null) - System.err.println(response); - throw e; - } + checkContains(response, 0, "HTTP/1.1 500"); + assertTrue(checkError.await(1, TimeUnit.SECONDS)); } @Test diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/MockHttpStream.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/MockHttpStream.java index bf2c785f7384..cb91cf235823 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/MockHttpStream.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/MockHttpStream.java @@ -218,6 +218,17 @@ public void send(MetaData.Request request, MetaData.Response response, boolean l callback.succeeded(); } + @Override + public long getIdleTimeout() + { + return 0; + } + + @Override + public void setIdleTimeout(long idleTimeoutMs) + { + } + @Override public boolean isCommitted() { diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ServerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ServerTest.java index 5702f13765c2..b2cf089d6d23 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ServerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ServerTest.java @@ -13,8 +13,13 @@ package org.eclipse.jetty.server; +import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Stream; @@ -26,6 +31,7 @@ import org.eclipse.jetty.io.Content; import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.io.QuietException; +import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.internal.HttpChannelState; import org.eclipse.jetty.server.internal.HttpConnection; import org.eclipse.jetty.util.Blocker; @@ -41,12 +47,17 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.sameInstance; +import static org.junit.jupiter.api.Assertions.assertTrue; public class ServerTest { + private static final long IDLE_TIMEOUT = 1000L; private Server _server; + private ContextHandler _context; private LocalConnector _connector; private final AtomicReference _afterHandle = new AtomicReference<>(); @@ -54,6 +65,8 @@ public class ServerTest public void prepare() throws Exception { _server = new Server(); + _context = new ContextHandler("/"); + _server.setHandler(_context); _connector = new LocalConnector(_server, new HttpConnectionFactory() { @Override @@ -95,7 +108,8 @@ public Runnable onRequest(MetaData.Request request) return configure(connection, connector, endPoint); } }); - _connector.setIdleTimeout(60000); + _connector.setIdleTimeout(IDLE_TIMEOUT); + _connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setIdleTimeout(IDLE_TIMEOUT); _server.addConnector(_connector); } @@ -109,10 +123,10 @@ public void dispose() throws Exception @Test public void testSimpleGET() throws Exception { - _server.setHandler(new Handler.Abstract() + _context.setHandler(new Handler.Abstract() { @Override - public boolean handle(Request request, Response response, Callback callback) throws Exception + public boolean handle(Request request, Response response, Callback callback) { response.getHeaders().put(HttpHeader.CONTENT_TYPE, "text/plain"); Content.Sink.write(response, true, "Hello", callback); @@ -154,7 +168,7 @@ public static Stream completionScenarios() @MethodSource("completionScenarios") public void testCompletion(boolean succeeded, boolean handling, boolean written, boolean last) throws Exception { - _server.setHandler(new Handler.Abstract(Invocable.InvocationType.BLOCKING) + _context.setHandler(new Handler.Abstract(Invocable.InvocationType.BLOCKING) { @Override public boolean handle(Request request, Response response, Callback callback) throws Exception @@ -186,8 +200,6 @@ public boolean handle(Request request, Response response, Callback callback) thr \r """; String rawResponse = _connector.getResponse(request); - // System.err.printf("succeeded=%b handling=%b written=%b last=%b%n", succeeded, handling, written, last); - // System.err.println(rawResponse); if (succeeded || written) assertThat(rawResponse, containsString("HTTP/1.1 200 OK")); @@ -212,4 +224,187 @@ public boolean handle(Request request, Response response, Callback callback) thr assertThat(rawResponse, containsString("Content-Length:")); } } + + @Test + public void testIdleTimeoutNoListener() throws Exception + { + // See ServerTimeoutsTest for more complete idle timeout testing. + _context.setHandler(new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + // Handler never completes the callback + return true; + } + }); + _server.start(); + + String request = """ + GET /path HTTP/1.0\r + Host: hostname\r + \r + """; + String rawResponse = _connector.getResponse(request); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(HttpStatus.INTERNAL_SERVER_ERROR_500)); + assertThat(response.getContent(), containsString("HTTP ERROR 500 java.util.concurrent.TimeoutException: Idle timeout expired:")); + } + + @Test + public void testIdleTimeoutNoListenerHttpConfigurationOnly() throws Exception + { + // See ServerTimeoutsTest for more complete idle timeout testing. + + _context.setHandler(new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + // Handler never completes the callback + return true; + } + }); + + _connector.setIdleTimeout(10 * IDLE_TIMEOUT); + _connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setIdleTimeout(IDLE_TIMEOUT); + + _server.start(); + + String request = """ + GET /path HTTP/1.0\r + Host: hostname\r + \r + """; + String rawResponse = _connector.getResponse(request); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(HttpStatus.INTERNAL_SERVER_ERROR_500)); + assertThat(response.getContent(), containsString("HTTP ERROR 500 java.util.concurrent.TimeoutException: Idle timeout expired:")); + } + + @Test + public void testIdleTimeoutFalseListener() throws Exception + { + // See ServerTimeoutsTest for more complete idle timeout testing. + CompletableFuture callbackOnTimeout = new CompletableFuture<>(); + _context.setHandler(new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + request.addIdleTimeoutListener(t -> !callbackOnTimeout.complete(callback)); + return true; + } + }); + _server.start(); + + String request = """ + GET /path HTTP/1.0\r + Host: hostname\r + \r + """; + + try (LocalConnector.LocalEndPoint localEndPoint = _connector.executeRequest(request)) + { + callbackOnTimeout.get(3 * IDLE_TIMEOUT, TimeUnit.MILLISECONDS).succeeded(); + String rawResponse = localEndPoint.getResponse(); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + } + } + + @Test + public void testIdleTimeoutWriteCallback() throws Exception + { + CompletableFuture onTimeout = new CompletableFuture<>(); + CompletableFuture writeFail = new CompletableFuture<>(); + _context.setHandler(new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + Runnable write = new Runnable() + { + final ByteBuffer buffer = ByteBuffer.allocate(128 * 1024 * 1024); + @Override + public void run() + { + response.write(false, buffer, Callback.from(this, + t -> + { + writeFail.complete(t); + callback.failed(t); + })); + } + }; + + request.addIdleTimeoutListener(t -> + { + request.getComponents().getThreadPool().execute(write); + return onTimeout.complete(t); + }); + + return true; + } + }); + _server.start(); + + String request = """ + GET /path HTTP/1.0\r + Host: localhost\r + \r + """; + try (LocalConnector.LocalEndPoint ignored = _connector.executeRequest(request)) + { + Throwable x = onTimeout.get(2 * IDLE_TIMEOUT, TimeUnit.MILLISECONDS); + assertThat(x, instanceOf(TimeoutException.class)); + x = writeFail.get(IDLE_TIMEOUT / 2, TimeUnit.MILLISECONDS); + assertThat(x, instanceOf(TimeoutException.class)); + } + } + + @Test + public void testListenersInContext() throws Exception + { + CountDownLatch latch = new CountDownLatch(3); + _context.setHandler(new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + assertThat(ContextHandler.getCurrentContext(), sameInstance(_context.getContext())); + latch.countDown(); + + request.addIdleTimeoutListener(t -> + { + assertThat(ContextHandler.getCurrentContext(), sameInstance(_context.getContext())); + latch.countDown(); + return true; + }); + + request.addFailureListener(t -> + { + assertThat(ContextHandler.getCurrentContext(), sameInstance(_context.getContext())); + latch.countDown(); + }); + return true; + } + }); + _server.start(); + + String request = """ + GET /path HTTP/1.0\r + Host: hostname\r + \r + """; + + try (LocalConnector.LocalEndPoint localEndPoint = _connector.executeRequest(request)) + { + assertTrue(latch.await(3 * IDLE_TIMEOUT, TimeUnit.MILLISECONDS)); + String rawResponse = localEndPoint.getResponse(); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(HttpStatus.INTERNAL_SERVER_ERROR_500)); + } + } + } diff --git a/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/TestableRequest.java b/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/TestableRequest.java index 0053692953d0..3bda4f7edad1 100644 --- a/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/TestableRequest.java +++ b/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/TestableRequest.java @@ -15,6 +15,8 @@ import java.util.List; import java.util.Set; +import java.util.concurrent.TimeoutException; +import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; @@ -161,9 +163,13 @@ public void fail(Throwable failure) } @Override - public boolean addErrorListener(Predicate onError) + public void addIdleTimeoutListener(Predicate onIdleTimeout) + { + } + + @Override + public void addFailureListener(Consumer onFailure) { - return false; } @Override diff --git a/jetty-core/jetty-tests/jetty-test-client-transports/pom.xml b/jetty-core/jetty-tests/jetty-test-client-transports/pom.xml index 8cf90ab9fc26..46b1f6868cee 100644 --- a/jetty-core/jetty-tests/jetty-test-client-transports/pom.xml +++ b/jetty-core/jetty-tests/jetty-test-client-transports/pom.xml @@ -62,6 +62,11 @@ jetty-http3-client-transport test + + org.eclipse.jetty + jetty-slf4j-impl + test + diff --git a/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/HttpClientTimeoutTest.java b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/HttpClientTimeoutTest.java index 5ce310c2c9f8..7f4130371bf3 100644 --- a/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/HttpClientTimeoutTest.java +++ b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/HttpClientTimeoutTest.java @@ -246,7 +246,7 @@ protected SslConnection newSslConnection(ByteBufferPool bufferPool, Executor exe return new SslConnection(bufferPool, executor, endPoint, engine) { @Override - protected boolean onReadTimeout(Throwable timeout) + protected boolean onReadTimeout(TimeoutException timeout) { sslIdle.set(true); return super.onReadTimeout(timeout); diff --git a/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/ServerTimeoutsTest.java b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/ServerTimeoutsTest.java index c0d2873f48f0..7cd669cb9fb9 100644 --- a/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/ServerTimeoutsTest.java +++ b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/ServerTimeoutsTest.java @@ -13,8 +13,200 @@ package org.eclipse.jetty.test.client.transport; -// TODO: similar to eeX ServerTimeoutsTest but with Handler semantic. -// For example, we may decide to not ignore the timeouts if there is a thread dispatched to the Handler. -public class ServerTimeoutsTest +import java.util.Collection; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Stream; + +import org.eclipse.jetty.client.AsyncRequestContent; +import org.eclipse.jetty.client.ContentResponse; +import org.eclipse.jetty.client.FutureResponseListener; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.server.Handler; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.util.Callback; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsStringIgnoringCase; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class ServerTimeoutsTest extends AbstractTest { + private static final long IDLE_TIMEOUT = 1000L; + + @Override + protected void prepareServer(Transport transport, Handler handler) throws Exception + { + super.prepareServer(transport, handler); + setStreamIdleTimeout(IDLE_TIMEOUT); + } + + public static Stream transportsAndTrueIdleTimeoutListeners() + { + Collection transports = transports(); + return Stream.concat( + transports.stream().map(t -> Arguments.of(t, false)), + transports.stream().map(t -> Arguments.arguments(t, true))); + } + + @ParameterizedTest + @MethodSource("transportsAndTrueIdleTimeoutListeners") + public void testIdleTimeout(Transport transport, boolean listener) throws Exception + { + AtomicBoolean listenerCalled = new AtomicBoolean(); + start(transport, new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + + if (listener) + request.addIdleTimeoutListener(t -> listenerCalled.compareAndSet(false, true)); + + // Do not complete the callback, so it idle times out. + return true; + } + }); + + ContentResponse response = client.newRequest(newURI(transport)) + .timeout(5 * IDLE_TIMEOUT, TimeUnit.MILLISECONDS) + .send(); + + assertThat(response.getStatus(), is(HttpStatus.INTERNAL_SERVER_ERROR_500)); + assertThat(response.getContentAsString(), containsStringIgnoringCase("HTTP ERROR 500 java.util.concurrent.TimeoutException: Idle timeout")); + if (listener) + assertTrue(listenerCalled.get()); + } + + @ParameterizedTest + @MethodSource("transportsAndTrueIdleTimeoutListeners") + public void testIdleTimeoutWithDemand(Transport transport, boolean listener) throws Exception + { + AtomicBoolean listenerCalled = new AtomicBoolean(); + CountDownLatch demanded = new CountDownLatch(1); + AtomicReference requestRef = new AtomicReference<>(); + AtomicReference callbackRef = new AtomicReference<>(); + start(transport, new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + + if (listener) + request.addIdleTimeoutListener(t -> listenerCalled.compareAndSet(false, true)); + requestRef.set(request); + callbackRef.set(callback); + request.demand(demanded::countDown); + return true; + } + }); + + // The response will not be completed, so use a specialized listener. + AsyncRequestContent content = new AsyncRequestContent(); + org.eclipse.jetty.client.Request request = client.newRequest(newURI(transport)) + .timeout(IDLE_TIMEOUT * 5, TimeUnit.MILLISECONDS) + .headers(f -> f.put(HttpHeader.CONTENT_LENGTH, 10)) + .onResponseSuccess(s -> + content.close()) + .body(content); + FutureResponseListener futureResponse = new FutureResponseListener(request); + request.send(futureResponse); + + // Demand is invoked by the idle timeout + assertTrue(demanded.await(2 * IDLE_TIMEOUT, TimeUnit.MILLISECONDS)); + + // Reads should yield the idle timeout. + Content.Chunk chunk = requestRef.get().read(); + assertThat(chunk, instanceOf(Content.Chunk.Error.class)); + Throwable cause = ((Content.Chunk.Error)chunk).getCause(); + assertThat(cause, instanceOf(TimeoutException.class)); + + // Complete the callback as the error listener promised. + callbackRef.get().failed(cause); + + ContentResponse response = futureResponse.get(IDLE_TIMEOUT / 2, TimeUnit.MILLISECONDS); + assertThat(response.getStatus(), is(HttpStatus.INTERNAL_SERVER_ERROR_500)); + assertThat(response.getContentAsString(), containsStringIgnoringCase("HTTP ERROR 500 java.util.concurrent.TimeoutException: Idle timeout")); + if (listener) + assertTrue(listenerCalled.get()); + } + + @ParameterizedTest + @MethodSource("transports") + public void testIdleTimeoutErrorListenerReturnsFalse(Transport transport) throws Exception + { + AtomicReference responseRef = new AtomicReference<>(); + CompletableFuture callbackOnTimeout = new CompletableFuture<>(); + start(transport, new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + responseRef.set(response); + request.addIdleTimeoutListener(t -> + { + callbackOnTimeout.complete(callback); + return false; // ignore timeout + }); + return true; + } + }); + + org.eclipse.jetty.client.Request request = client.newRequest(newURI(transport)) + .timeout(IDLE_TIMEOUT * 5, TimeUnit.MILLISECONDS); + FutureResponseListener futureResponse = new FutureResponseListener(request); + request.send(futureResponse); + + // Get the callback as promised by the error listener. + Callback callback = callbackOnTimeout.get(3 * IDLE_TIMEOUT, TimeUnit.MILLISECONDS); + assertNotNull(callback); + Content.Sink.write(responseRef.get(), true, "OK", callback); + + ContentResponse response = futureResponse.get(IDLE_TIMEOUT / 2, TimeUnit.MILLISECONDS); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + assertThat(response.getContentAsString(), is("OK")); + } + + @ParameterizedTest + @MethodSource("transportsNoFCGI") + public void testIdleTimeoutErrorListenerReturnsFalseThenTrue(Transport transport) throws Exception + { + // TODO fix FCGI for multiple timeouts + AtomicReference error = new AtomicReference<>(); + start(transport, new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + request.addIdleTimeoutListener(t -> error.getAndSet(t) != null); + return true; + } + }); + + ContentResponse response = client.newRequest(newURI(transport)) + .timeout(IDLE_TIMEOUT * 5, TimeUnit.MILLISECONDS) + .send(); + + // The first time the listener returns true, but does not complete the callback, + // so another idle timeout elapses. + // The second time the listener returns false and the implementation produces the response. + assertThat(response.getStatus(), is(HttpStatus.INTERNAL_SERVER_ERROR_500)); + assertThat(response.getContentAsString(), containsStringIgnoringCase("HTTP ERROR 500 java.util.concurrent.TimeoutException: Idle timeout")); + assertThat(error.get(), instanceOf(TimeoutException.class)); + } + + // TODO write side tests } diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketConnection.java b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketConnection.java index 462c68b7ddcf..66d985299278 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketConnection.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketConnection.java @@ -21,6 +21,7 @@ import java.util.Objects; import java.util.Random; import java.util.concurrent.Executor; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.LongAdder; import org.eclipse.jetty.io.AbstractConnection; @@ -218,13 +219,13 @@ public void onClose(Throwable cause) } @Override - public boolean onIdleExpired() + public boolean onIdleExpired(TimeoutException timeoutException) { if (LOG.isDebugEnabled()) LOG.debug("onIdleExpired()"); // treat as a handler error because socket is still open - coreSession.processHandlerError(new WebSocketTimeoutException("Connection Idle Timeout"), Callback.NOOP); + coreSession.processHandlerError(new WebSocketTimeoutException("Connection Idle Timeout", timeoutException), Callback.NOOP); return true; } @@ -234,7 +235,7 @@ public boolean onIdleExpired() * @return true to signal that the endpoint must be closed, false to keep the endpoint open */ @Override - protected boolean onReadTimeout(Throwable timeout) + protected boolean onReadTimeout(TimeoutException timeout) { if (LOG.isDebugEnabled()) LOG.debug("onReadTimeout()"); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java index a6f0f6c0816f..4654ae5db3d8 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java @@ -32,8 +32,6 @@ import java.util.Set; import java.util.StringTokenizer; import java.util.concurrent.CompletableFuture; -import java.util.function.Function; -import java.util.function.Predicate; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -64,7 +62,6 @@ import org.eclipse.jetty.io.ByteBufferInputStream; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.server.Context; -import org.eclipse.jetty.server.HttpStream; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.ResourceService; import org.eclipse.jetty.server.Response; @@ -633,17 +630,6 @@ public boolean isSecure() return _servletRequest.isSecure(); } - @Override - public boolean addErrorListener(Predicate onError) - { - return false; - } - - @Override - public void addHttpStreamWrapper(Function wrapper) - { - } - @Override public Object removeAttribute(String name) { diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java index 475a1ea1551e..ebccf42b098c 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java @@ -83,7 +83,6 @@ public class ServletChannel private final Listener _combinedListener; private volatile ServletContextRequest _servletContextRequest; private volatile boolean _expects100Continue; - private volatile long _oldIdleTimeout; private volatile Callback _callback; // Bytes written after interception (e.g. after compression). private volatile long _written; @@ -380,7 +379,6 @@ private void recycle() _servletContextRequest = null; _callback = null; _written = 0; - _oldIdleTimeout = 0; } /** @@ -838,10 +836,6 @@ public void onCompleted() if (LOG.isDebugEnabled()) LOG.debug("onCompleted for {} written={}", apiRequest.getRequestURI(), getBytesWritten()); - long idleTO = _configuration.getIdleTimeout(); - if (idleTO >= 0 && getIdleTimeout() != _oldIdleTimeout) - setIdleTimeout(_oldIdleTimeout); - if (getServer().getRequestLog() instanceof CustomRequestLog) { CustomRequestLog.LogDetail logDetail = new CustomRequestLog.LogDetail( diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java index 519064b8cd0e..a9144f3f989a 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java @@ -20,6 +20,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.concurrent.TimeoutException; import jakarta.servlet.AsyncListener; import jakarta.servlet.ServletRequest; @@ -108,6 +109,7 @@ protected ServletContextRequest( _matchedPath = matchedResource.getMatchedPath(); _response = newServletContextResponse(response); _sessionManager = sessionManager; + addIdleTimeoutListener(this::onIdleTimeout); } protected ServletApiRequest newServletApiRequest() @@ -131,6 +133,11 @@ protected ServletContextResponse newServletContextResponse(Response response) return new ServletContextResponse(_servletChannel, this, response); } + private boolean onIdleTimeout(TimeoutException timeout) + { + return _servletChannel.getState().onIdleTimeout(timeout); + } + public String getDecodedPathInContext() { return _decodedPathInContext; diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletRequestState.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletRequestState.java index bb251ad9c40f..0be875eb667f 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletRequestState.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletRequestState.java @@ -16,6 +16,7 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import jakarta.servlet.AsyncListener; import jakarta.servlet.ServletContext; @@ -728,6 +729,18 @@ public void asyncError(Throwable failure) } } + public boolean onIdleTimeout(TimeoutException timeout) + { + try (AutoLock ignored = lock()) + { + if (LOG.isDebugEnabled()) + LOG.debug("onIdleTimeout {}", getStatusStringLocked(), timeout); + // TODO this is almost always returning false?!? what about read/write timeouts??? + // return _state == State.IDLE; + return true; + } + } + protected void onError(Throwable th) { final AsyncContextEvent asyncEvent; diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/AbstractTest.java b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/AbstractTest.java index 7eae118d4687..e8f7583bc314 100644 --- a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/AbstractTest.java +++ b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/AbstractTest.java @@ -13,9 +13,11 @@ package org.eclipse.jetty.ee10.test.client.transport; +import java.io.InputStream; import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; +import java.security.KeyStore; import java.util.Collection; import java.util.EnumSet; import java.util.List; @@ -51,17 +53,23 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.SslConnectionFactory; +import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; +import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; import org.eclipse.jetty.unixdomain.server.UnixDomainServerConnector; import org.eclipse.jetty.util.SocketAddressResolver; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.extension.ExtendWith; import static org.junit.jupiter.api.Assertions.assertTrue; +@ExtendWith(WorkDirExtension.class) public class AbstractTest { + public WorkDir workDir; + protected final HttpConfiguration httpConfig = new HttpConfiguration(); protected SslContextFactory.Server sslContextFactoryServer; protected Server server; @@ -140,16 +148,27 @@ protected Server newServer() return new Server(serverThreads); } - protected SslContextFactory.Server newSslContextFactoryServer() + protected SslContextFactory.Server newSslContextFactoryServer() throws Exception { SslContextFactory.Server ssl = new SslContextFactory.Server(); - ssl.setKeyStorePath("src/test/resources/keystore.p12"); - ssl.setKeyStorePassword("storepwd"); - ssl.setUseCipherSuitesOrder(true); - ssl.setCipherComparator(HTTP2Cipher.COMPARATOR); + configureSslContextFactory(ssl); return ssl; } + private void configureSslContextFactory(SslContextFactory sslContextFactory) throws Exception + { + KeyStore keystore = KeyStore.getInstance("PKCS12"); + try (InputStream is = Files.newInputStream(Path.of("src/test/resources/keystore.p12"))) + { + keystore.load(is, "storepwd".toCharArray()); + } + sslContextFactory.setTrustStore(keystore); + sslContextFactory.setKeyStore(keystore); + sslContextFactory.setKeyStorePassword("storepwd"); + sslContextFactory.setUseCipherSuitesOrder(true); + sslContextFactory.setCipherComparator(HTTP2Cipher.COMPARATOR); + } + protected void startClient(Transport transport) throws Exception { QueuedThreadPool clientThreads = new QueuedThreadPool(); @@ -167,7 +186,11 @@ public AbstractConnector newConnector(Transport transport, Server server) case HTTP, HTTPS, H2C, H2, FCGI -> new ServerConnector(server, 1, 1, newServerConnectionFactory(transport)); case H3 -> - new HTTP3ServerConnector(server, sslContextFactoryServer, newServerConnectionFactory(transport)); + { + HTTP3ServerConnector connector = new HTTP3ServerConnector(server, sslContextFactoryServer, newServerConnectionFactory(transport)); + connector.getQuicConfiguration().setPemWorkDirectory(workDir.getEmptyPathDir()); + yield connector; + } case UNIX_DOMAIN -> { UnixDomainServerConnector connector = new UnixDomainServerConnector(server, 1, 1, newServerConnectionFactory(transport)); @@ -215,16 +238,15 @@ protected ConnectionFactory[] newServerConnectionFactory(Transport transport) return list.toArray(ConnectionFactory[]::new); } - protected SslContextFactory.Client newSslContextFactoryClient() + protected SslContextFactory.Client newSslContextFactoryClient() throws Exception { SslContextFactory.Client ssl = new SslContextFactory.Client(); - ssl.setKeyStorePath("src/test/resources/keystore.p12"); - ssl.setKeyStorePassword("storepwd"); + configureSslContextFactory(ssl); ssl.setEndpointIdentificationAlgorithm(null); return ssl; } - protected HttpClientTransport newHttpClientTransport(Transport transport) + protected HttpClientTransport newHttpClientTransport(Transport transport) throws Exception { return switch (transport) { diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/ServerTimeoutsTest.java b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/ServerTimeoutsTest.java index 3f6a9b1525f2..9e78e606dc83 100644 --- a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/ServerTimeoutsTest.java +++ b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/ServerTimeoutsTest.java @@ -60,13 +60,13 @@ // since they may be ignored, so we don't want to remember errors if they are ignored. // However, this behavior is historically so because of Servlets, and we // may decide differently for Handlers. -@Disabled public class ServerTimeoutsTest extends AbstractTest { @ParameterizedTest @MethodSource("transportsNoFCGI") public void testBlockingReadWithDelayedFirstContentWithUndelayedDispatchIdleTimeoutFires(Transport transport) throws Exception { + assumeTrue(transport != Transport.H3 && transport != Transport.H2C && transport != Transport.H2); // TODO Fix testBlockingReadWithDelayedFirstContentIdleTimeoutFires(transport, false); } @@ -74,6 +74,7 @@ public void testBlockingReadWithDelayedFirstContentWithUndelayedDispatchIdleTime @MethodSource("transportsNoFCGI") public void testBlockingReadWithDelayedFirstContentWithDelayedDispatchIdleTimeoutFires(Transport transport) throws Exception { + assumeTrue(transport != Transport.H3 && transport != Transport.H2C && transport != Transport.H2); // TODO Fix testBlockingReadWithDelayedFirstContentIdleTimeoutFires(transport, true); } @@ -369,6 +370,8 @@ protected void service(HttpServletRequest request, HttpServletResponse response) @MethodSource("transportsNoFCGI") public void testBlockingReadWithMinimumDataRateAboveLimit(Transport transport) throws Exception { + assumeTrue(transport != Transport.H3 && transport != Transport.H2C && transport != Transport.H2); // TODO Fix + int bytesPerSecond = 20; httpConfig.setMinRequestDataRate(bytesPerSecond); CountDownLatch handlerLatch = new CountDownLatch(1); @@ -413,6 +416,8 @@ protected void service(HttpServletRequest request, HttpServletResponse response) @MethodSource("transportsNoFCGI") public void testBlockingReadHttpIdleTimeoutOverridesIdleTimeout(Transport transport) throws Exception { + assumeTrue(transport != Transport.H3); // TODO Fix H3 + long httpIdleTimeout = 2500; long idleTimeout = 3 * httpIdleTimeout; httpConfig.setIdleTimeout(httpIdleTimeout); @@ -444,7 +449,7 @@ public void testBlockingReadHttpIdleTimeoutOverridesIdleTimeout(Transport transp @MethodSource("transportsNoFCGI") public void testAsyncReadHttpIdleTimeoutOverridesIdleTimeout(Transport transport) throws Exception { - long httpIdleTimeout = 2500; + long httpIdleTimeout = 2000; long idleTimeout = 3 * httpIdleTimeout; httpConfig.setIdleTimeout(httpIdleTimeout); CountDownLatch handlerLatch = new CountDownLatch(1); @@ -503,6 +508,7 @@ public void onError(Throwable failure) assertTrue(resultLatch.await(5, TimeUnit.SECONDS)); } + @Disabled @ParameterizedTest @MethodSource("transportsNoFCGI") public void testIdleTimeoutBeforeReadIsIgnored(Transport transport) throws Exception @@ -556,6 +562,7 @@ public void onComplete(Result result) assertTrue(latch.await(5, TimeUnit.SECONDS)); } + @Disabled @ParameterizedTest @MethodSource("transportsNoFCGI") public void testBlockingWriteWithMinimumDataRateBelowLimit(Transport transport) throws Exception diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/resources/keystore.p12 b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/resources/keystore.p12 index 8934437fa14490265dfdb5d9a55bfaa88d039dcd..d96d0667bd6619914779294596679f38f320be7e 100644 GIT binary patch literal 2774 zcma);X*d*$8pmhGjLE*tU^HVZ9qJ@AmSjnmWUQI8mT1y&Fj}U@QDGL0B}-!)lq?B3 zB-v^T*%fz;tOsKW84-g*$H?tI_dcEHe!BO=`@GNl`@jG9|w>S>rUc$2GhcpnI)B?|(oqa{W6{QDwE3<5x_ zh>BfgV1Wb?Fz^sq7I5)Idt`U9X~CYmhczbI&%tysSK4*PBMl!g>)i7;P*`kk1oIE^#{lRU|q&5S|+tm6UW{b+`I=m zS0gO*EGmeW)*mr5u$^<~Fcw+C;{JyX&z z>_%g6o?93L!|fH1%n}C`voRqj8T^XasMQIX-@S$=V#v(W(3uORcJ$=lR`(`@mr;o! z>H^V3L+>alBIgvLDy2P`k$$3#a#3^DNEohM8Xqvt;v~4{g-Rnq4A*=_`&W}v^8U)` z(=c{xuZjEb*S8)Mrdokc)+7ZzaaGl@b>$S1x^HNYEIqb>ZaoAJE~2hq4f9eu@%ig` zg4#nymxu+>E?>v-S0E)9+1#Y5#Hc^Crvi_(mnrxAzPUlT?67db*43nQo7NOpmej)` z8%yDyia&MizihVNHopx56y>;^%=--9bDJBKOAYD*W~7IEu%`?sa!GraYDvp33SKXb zX@0K^fl(9hY8h~ZLa=3gcx^Y*S7IE=H_fH6+LN#_!N<&--scYS5C2KzIarGo>-Vj@ z9a`xd#l>l8%u%B_7V&Ffq4dG)XHh}p&%W8^rbE-h2FVF@TKeeL1=<~il{Yr<*J(2% zQU7Rr)@u%VBoNeYjUubLkF_LMLV;;9ZY72tvhpjmJ&G`+U}=~Yplho~*s54$Mh#Z`RdAjbx;b^Xl%;hR&0n)COWF^>*bxr| z4S!I@!#UxcG3RRyrHi&5I#=YfI=r9^8v(xY>GMXB$lbY>y&jZbc;j(lI1i0Fv+eBc zC(Lxa<``F~^W2??;iAWyyMQ~f?%7X*m1ax5<`u-e<=-p&KR7EPbZ_-U-6pWAi(AXS zHvsZ5YY>+Fh?9x$V!H>vEYOflj-kRH-SUHESe|Ui# znGiWVGn#p(X6>V9#*HV^QM5O9&~JF#4L%rLpZ9RgtOMFIsas9BjEHj@JyhfJF2m2q zDP4lPRvy%f-o+7yYX^*R{40Ra!5lV@#yofuaaBc|W{uI8cw@tcUsKU}*48*iOmRiL zwrQeNI*$RhTRTnsPC7?!TIHF1g6;5a_q_~Iik}|U3FqUC7Q(g%oc*rCenZ(Qe^s2a zp{{%oGS$JFCN*Yx79lL5EO*D;wx;J(Vz8-c8phVphuS%DSHgnl$*S7;-mGMxJ*bzD=G-T?`+=^{w31!(UKqHT>hDZhMs50N6kEK?LLUQZcFZps!kpm$|>1$YO`->KA$o4e1VD?w^tAKA;Bss=1x2% zJY~v<6_ETHV~$q-8JA*kjHIeKAOvs=5DXvyZlN{)mJ~2>u)J$v2tgfl^q7IMp0SaE zv9W;xnhvh|xdau>r-KWBpobtJ;74QqtpNX5NayyPe1R(9uTjJ>LUc;SJ8vvu^ZyBH zEr*ue^CNB+MyDeoLTEdQN(ZZh*%g0Aymheq66dvq zHi5C6yBfvsm6KRhT+?hV_35y*_?2r198Vn|Y!`Y!w^WLobvIT&Uuep}i(zp2x3vR-itQ> zjjJubCy0WZuSD0{4yF&8g)u!Zs=i8(*xNfdP%I4i&aCg4(g}(`+38jAMn*8a3td^e zfx9E$(@62oZswW<-pXnWTp}^dm5Ky6dM%=tt95hb?Y1ZqQ|V~>{>c)eU$$iPB1a~| z$hj-gY<~FeQnaKXV?7bP@jAbA&*Og-A)O2wfIR-7%b$1Q2 zT=svLo%(H~P0-meds)NC$hQfnm@SfdI)3MEf%D{a6vG{;NyEl5P#&|+jhsyH8)#GgBV`cM1wp~|&zrC!rm;t&Rz zr%ry6*@uUe^A0!1y5v+9PRF|3dNj~4QvNA|8mvXqd}#AryHxM5kU|*M`HIV8g=log zTD;E@lKA3Z25Ma%&2P8FY9y+8kb4r}0H$gwvBE-MeX+k}o=rc=MqX8FMLI5BC6QNO zq>fGx(3VJ*SG^6--GTvh^%`YqHC8Y`{TlAS@P;}z zpfCd&<28yR{s_wz9%bxw_a?>ar)p#Eu^;rU>=1TsnNY@8fGMS#v}ZD8H>!IJrCl5gBSSSz6C zjJ!Zeo<_;eWFEYsOA5Hol}$=UdFCyu9M)+#=BTZKc0yy&Qa^t^5D)?YtJft>xWfHG z-hKY7$9o^2+FC~UmaiUtJK4YFp9x}b7lPrRba~h>K7~nmwxyu4U$%tQ4cYovMEw&X C)Bn@} delta 2573 zcmY+EX*3iJ8-~po!x-yJM6zVhG9&w1vQ8+A+v(HvnNCdA^b} zH63N|^29N+ODL2t(HbfmO+Q{0`Ww#vh~=4OT3G9XO<;hOW(>1gR?Z5{l?xSG7N=^g zB{0ZzYFEfRQF`lXt6UOz%gAkdtMcKNRlkTmu+;i3-2<@Q19ASTn53b`#s*)%zpK|7 z(y@@f+i9>S5DwWR@(f#2j0hjtD8r(Uq`idI^I}^KH&Iek$CrG0hMfnR*RtZFKT|9@ zlJd%KdnsSDMe>3n1ey>f0tFz3e)O15B`t{Ejb(?K7Yo~C8uKT1uG8zcm5Q&MA&Qdq zm-77L`kJzkuofCWqKeXcgX5l|f1g!K#QTH05I@!#x2s27H%dz!Dn|=F2SAvmwO7mQ zy}yI6Wzp8wW9~gE6FTNN5k-g_UQWppWpLPck#+9Ts`l7+d^zOkk8)fbS$ctJ@D?SO zM)lx~OaMJe?QhiiruS8DrhYr2y*^oB|Q}JW$kx__Rz^t5puKmS61>o6d^A zkA>R3%H4kEa~dS@#cCd8#`4jiu7$fjQDp1z-R>qS{?}qpHZz=O36l?=m1yb>G%?7> z4cL&f`%xQ=Twym+PUAb6g=+AR1%f6c&JfNiuTM`3GaoJ9xH2Q5_evRR5FNumCDJ)r ztQ|nsODX&!$cp4}w;QU<9uZP(ymWQLzq~U-Ku+Q)w!#RyYnp@TFSF=<$qA$!3Q(ao zR;JcQb68aq`mvlB7J6RlLzgg%zz^M88yucw|7{5u?`NnnjuCf8*Rpdh}$-iC3dV zejypvgGoU3V_@Cs<&D?w_V=$ZeWBf+_Y0`qcl=yO=KK_?e4n-BYZxe#3-Ybaims^1 zjrXDw;j7C0#?I;~b^Kh+`YuuWjZrAyMvShkQhl0Y$YZv@uLnyXfq=*N3-Glg0$owtsMQ)DUyptdw-Xr(3f)jo{=;MY;U)(lX0M&g_u+? zsTLYd=Dn>`M%wKO@A8>1@%n!Jva?FEM=kfXrEkXh@p4oaupy=y+|kKEE>*ZaK%jyo zco7Y#JB2C)c5YI7-CH!yKC9|+jCz~v;ClF5V0-pPceAN6GJnRt%r_JZ4d2c|+wPns z)xuO>QnLqN4;Yi*Za_bcUlecxDb|HB?mF6xzRK1Oc;V69x&35ZY1Lvf5#R_Ox%o%* z=Y6+tEc+#^!hFdTTR|d~1CS_Sz&%LP+InyEb240htGRImrDj=)tXdG-EmbJyrGy{m z7kof-ZBI*?K4n8dgK6H}PYRcyh0RlB-N$TABCtNmf!r+=c~G71E^W*x`vRF z#ahDi2G{eCT>zOG49Y0@6$Mo#B}KTJn(CPpE&q`y_{@_AXGj+apgRkVe;UBQ%nAOl zIgtmxYwtEM$d&ZY!2nq;>MK!U$7kkjGml1t=%js&mhzmp64e&69)H+Kz4fI%`?$Xt z^El7*FpNJ|dx5SrG32GjpV@3f;+5{1MlnhG0U^?u&4Un*Rfbo;RcuKlx2P0r#yBu& zQK^VUczs<@j10h`>HVLw7~5Yq2K4sD@C#Ny;gPDFt`1#%3SfgAvj>?d#!rwdZ?c; zN-|R}9G4~XYSvXGipmErsMS}(PkB`GIYyR@ijTKIs1*YfVv`E#*9#n}4lGi`=@i=G zS08p+semAqOKpwdQ%gL3vZlEpAhB$t4IfD0%V!6aiLML~emp6tNMsW1W5NWNzJJqh zRU10aov8m}U%`|wwQ-DJO?UP+o;-%SHper{pqhE1j>RteS+Cs8Hk$~a99cyKwM3eH z@LsAS#^k~9*AEh+W*yM}`tveFjM^l3+j`xC2Y&TV%H6JAr&de_jm+^<2~X{9qRVRi zPj8vD=rsLFYC)2-O>u4MAH+GrD`F=MjH6X<2`L~Ovj#6?C#}N6962Y2LWjdq7#V~3 znnP8;J?#(>msu&@!J*-W&mF2M%da?2;Ud+N+Ea{nEUL!&8&f3qgwQ!8&ozqpJv`EPFrrjR z^rb})LoVUd4#>G^@w+Y-mxqea#Ar_2vjrJvfOdm1pF$)!qD#AQpL1x4^$Goti zt3<-`XLf|Y=GSOp&&X9gxg#^NsBtz>Q#O!Q%CNn~Ew$!hqer{ghC zQ-g>;x}?_d_j7!U=pMeg= 0 && _oldIdleTimeout != idleTO) - setIdleTimeout(idleTO); if (LOG.isDebugEnabled()) { diff --git a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java index 8834bf6b6ef0..f57cf33fd096 100644 --- a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java +++ b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java @@ -37,8 +37,8 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; import java.util.function.Function; -import java.util.function.Predicate; import java.util.stream.Stream; import jakarta.servlet.MultipartConfigElement; @@ -2439,9 +2439,8 @@ public void fail(Throwable failure) } @Override - public boolean addErrorListener(Predicate onError) + public void addFailureListener(Consumer onFailure) { - return false; } @Override diff --git a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ResponseTest.java b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ResponseTest.java index 83d9cedaa412..cefbae90d2da 100644 --- a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ResponseTest.java +++ b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ResponseTest.java @@ -31,6 +31,8 @@ import java.util.Locale; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeoutException; +import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; @@ -2387,9 +2389,13 @@ public void fail(Throwable failure) } @Override - public boolean addErrorListener(Predicate onError) + public void addIdleTimeoutListener(Predicate onIdleTimeout) + { + } + + @Override + public void addFailureListener(Consumer onFailure) { - return false; } @Override