From 9eb9e7d38e21363dc78cfb8c70c74167c7d29594 Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 18 Sep 2024 10:13:33 +1000 Subject: [PATCH] WIP updates from review --- .../server/internal/HttpChannelState.java | 7 ++-- .../jetty/server/internal/HttpConnection.java | 32 ++++++++++++------- .../jetty/ee10/servlet/ServletTest.java | 2 -- .../jetty/ee11/servlet/ServletTest.java | 2 -- .../jetty/ee9/servlet/ServletTest.java | 2 -- 5 files changed, 24 insertions(+), 21 deletions(-) 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 f567ded55e7..8cd29e8f62d 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 @@ -363,8 +363,8 @@ public class HttpChannelState implements HttpChannel, Components if (LOG.isDebugEnabled()) LOG.debug("onIdleTimeout {}", this, t); - // Too late? - if (_request == null || _response == null) + // too late? + if (_stream == null) return null; Runnable invokeOnContentAvailable = null; @@ -445,7 +445,8 @@ public class HttpChannelState implements HttpChannel, Components // If not handled, then we just fail the request callback if (!_handled && _handling == null) { - task = () -> _request._callback.failed(x); + Callback callback = _request._callback; + task = () -> callback.failed(x); } else { 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 060f845cd25..88831e1a2ec 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 @@ -583,10 +583,16 @@ public class HttpConnection extends AbstractMetaDataConnection implements Runnab return true; Runnable task; - if (_delayedForContent && _onRequest != null) + + if (_delayedForContent || _onRequest == null) + { + task = _httpChannel.onIdleTimeout(timeout); + } + else { Runnable onRequest = _onRequest; _onRequest = null; + task = () -> { try @@ -596,19 +602,12 @@ public class HttpConnection extends AbstractMetaDataConnection implements Runnab finally { _handling.set(false); - Runnable next = _httpChannel.onIdleTimeout(timeout); - if (next != null) - getExecutor().execute(next); } }; } - else - { - task = _httpChannel.onIdleTimeout(timeout); - } if (task != null) getExecutor().execute(task); - return false; // We've handle the exception + return false; // We've handle (or ignored) the timeout } @Override @@ -967,11 +966,16 @@ public class HttpConnection extends AbstractMetaDataConnection implements Runnab _onRequest = stream.headerComplete(); // Should we delay dispatch until we have some content? - _delayedForContent = getHttpConfiguration().isDelayDispatchUntilContent() && + if (getHttpConfiguration().isDelayDispatchUntilContent() && + getEndPoint().getIdleTimeout() > 0 && (_parser.getContentLength() > 0 || _parser.isChunking()) && !stream._expects100Continue && !stream.isCommitted() && - _requestBuffer != null && _requestBuffer.isEmpty(); + _requestBuffer != null && _requestBuffer.isEmpty()) + { + getEndPoint().setIdleTimeout(getEndPoint().getIdleTimeout() / 2); + _delayedForContent = true; + } return !_delayedForContent; } @@ -988,7 +992,11 @@ public class HttpConnection extends AbstractMetaDataConnection implements Runnab _requestBuffer.retain(); stream._chunk = Content.Chunk.asChunk(buffer, false, _requestBuffer); - _delayedForContent = false; + if (_delayedForContent) + { + _delayedForContent = false; + getEndPoint().setIdleTimeout(getEndPoint().getIdleTimeout() * 2); + } return true; } diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ServletTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ServletTest.java index 866ca69dc16..2c6088e93d6 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ServletTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ServletTest.java @@ -28,7 +28,6 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; -import org.eclipse.jetty.server.HttpConnectionFactory; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; @@ -139,7 +138,6 @@ public class ServletTest }, "/post"); _connector.setIdleTimeout(idleTimeout); - _connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setDelayDispatchUntilContent(false); _server.start(); try (LocalConnector.LocalEndPoint endPoint = _connector.connect()) diff --git a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/ServletTest.java b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/ServletTest.java index d28aec0ff5c..aa687f40b51 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/ServletTest.java +++ b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/ServletTest.java @@ -28,7 +28,6 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; -import org.eclipse.jetty.server.HttpConnectionFactory; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; @@ -142,7 +141,6 @@ public class ServletTest }, "/post"); _connector.setIdleTimeout(idleTimeout); - _connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setDelayDispatchUntilContent(false); _server.start(); try (LocalConnector.LocalEndPoint endPoint = _connector.connect()) diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ServletTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ServletTest.java index 12a52276f70..88fa68a1bd1 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ServletTest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/ServletTest.java @@ -24,7 +24,6 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; -import org.eclipse.jetty.server.HttpConnectionFactory; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.util.IO; @@ -134,7 +133,6 @@ public class ServletTest }), "/post"); _connector.setIdleTimeout(idleTimeout); - _connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setDelayDispatchUntilContent(false); _server.start(); try (LocalConnector.LocalEndPoint endPoint = _connector.connect())