From e52559996a41573b028ca11c5ee3a7ad71fe357b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Wed, 24 Sep 2025 15:15:44 +0200 Subject: [PATCH 01/13] Extend response timeout to cover body --- .../jdk/internal/net/http/MultiExchange.java | 8 +- .../httpclient/TimeoutResponseBodyTest.java | 200 +++++++++++++ .../httpclient/TimeoutResponseHeaderTest.java | 107 +++++++ .../TimeoutResponseTestSupport.java | 264 ++++++++++++++++++ 4 files changed, 576 insertions(+), 3 deletions(-) create mode 100644 test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java create mode 100644 test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java create mode 100644 test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java b/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java index ec621f7f95576..e247c54575673 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java @@ -25,7 +25,6 @@ package jdk.internal.net.http; -import java.io.IOError; import java.io.IOException; import java.lang.ref.WeakReference; import java.net.ConnectException; @@ -502,7 +501,6 @@ private CompletableFuture responseAsyncImpl(final boolean applyReqFilt } return completedFuture(response); } else { - cancelTimer(); setNewResponse(currentreq, response, null, exch); if (currentreq.isWebSocket()) { // need to close the connection and open a new one. @@ -520,11 +518,15 @@ private CompletableFuture responseAsyncImpl(final boolean applyReqFilt } }) .handle((response, ex) -> { // 5. handle errors and cancel any timer set - cancelTimer(); if (ex == null) { assert response != null; return completedFuture(response); } + // Cancel the timer only if the response has completed + // exceptionally. That is, don't cancel the timer if there + // are no exceptions, since the response body might still + // get consumed, and it is subject to the response timer. + cancelTimer(); // all exceptions thrown are handled here final RetryContext retryCtx = checkRetryEligible(ex, exch); assert retryCtx != null : "retry context is null"; diff --git a/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java b/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java new file mode 100644 index 0000000000000..4e7ddfe361805 --- /dev/null +++ b/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java @@ -0,0 +1,200 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import jdk.internal.net.http.common.Logger; +import jdk.internal.net.http.common.Utils; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import java.io.IOException; +import java.io.InputStream; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.net.http.HttpTimeoutException; +import java.util.concurrent.CompletableFuture; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; +import static org.junit.jupiter.api.Assertions.fail; + +/* + * @test + * @bug 8208693 + * @summary Verifies `HttpRequest::timeout` is effective for *response body* timeouts + * + * @library /test/lib + * /test/jdk/java/net/httpclient/lib + * @build jdk.httpclient.test.lib.common.HttpServerAdapters + * jdk.test.lib.net.SimpleSSLContext + * TimeoutResponseTestSupport + * + * @comment `-Djdk.httpclient.{disableRetryConnect,auth.retrylimit=0}` are + * added to keep retry mechanisms out of the picture, and solely + * focus on basic request-response round trip. + * @run junit/othervm + * -Djdk.httpclient.disableRetryConnect + * -Djdk.httpclient.auth.retrylimit=0 + * TimeoutResponseBodyTest + */ + +/** + * Verifies {@link HttpRequest#timeout() HttpRequest.timeout()} is effective + * for response body timeouts. + * + * @implNote + * + * Using a response body subscriber (i.e., {@link InputStream}) of type that + * allows gradual consumption of the response body after successfully building + * an {@link HttpResponse} instance to ensure timeouts are propagated even + * after the {@code HttpResponse} construction. + *

+ * Each test is provided a pristine ephemeral client to avoid any unexpected + * effects due to pooling. + * + * @see TimeoutBasic Server connection timeout tests + * @see TimeoutResponseHeaderTest Server response header timeout tests + */ +class TimeoutResponseBodyTest extends TimeoutResponseTestSupport { + + private static final Logger LOGGER = Utils.getDebugLogger( + TimeoutResponseBodyTest.class.getSimpleName()::toString, Utils.DEBUG); + + @ParameterizedTest + @MethodSource("serverRequestPairs") + void testSendOnMissingBody(ServerRequestPair pair) { + + ServerRequestPair.SERVER_HANDLER_BEHAVIOUR = + ServerRequestPair.ServerHandlerBehaviour.BLOCK_BEFORE_BODY_DELIVERY; + + try (HttpClient client = pair.createClient()) { + assertTimeoutPreemptively(TIMEOUT.multipliedBy(2), () -> { + LOGGER.log("Sending the request"); + HttpResponse response = client.send( + pair.request(), HttpResponse.BodyHandlers.ofInputStream()); + LOGGER.log("Consuming the obtained response"); + verifyResponseBodyDoesNotArrive(response); + }); + } + + } + + @ParameterizedTest + @MethodSource("serverRequestPairs") + void testSendAsyncOnMissingBody(ServerRequestPair pair) { + + ServerRequestPair.SERVER_HANDLER_BEHAVIOUR = + ServerRequestPair.ServerHandlerBehaviour.BLOCK_BEFORE_BODY_DELIVERY; + + try (HttpClient client = pair.createClient()) { + assertTimeoutPreemptively(TIMEOUT.multipliedBy(2), () -> { + LOGGER.log("Sending the request asynchronously"); + CompletableFuture> responseFuture = client.sendAsync( + pair.request(), HttpResponse.BodyHandlers.ofInputStream()); + LOGGER.log("Obtaining the response"); + HttpResponse response = responseFuture.get(); + LOGGER.log("Consuming the obtained response"); + verifyResponseBodyDoesNotArrive(response); + }); + } + + } + + private static void verifyResponseBodyDoesNotArrive(HttpResponse response) { + assertEquals(200, response.statusCode()); + IOException exception = assertThrows( + IOException.class, + () -> { + try (InputStream responseBodyStream = response.body()) { + int readByte = responseBodyStream.read(); + fail("Unexpected read byte: " + readByte); + } + }); + assertInstanceOf(HttpTimeoutException.class, exception.getCause()); + } + + @ParameterizedTest + @MethodSource("serverRequestPairs") + void testSendOnSlowBody(ServerRequestPair pair) { + + ServerRequestPair.SERVER_HANDLER_BEHAVIOUR = + ServerRequestPair.ServerHandlerBehaviour.DELIVER_BODY_SLOWLY; + + try (HttpClient client = pair.createClient()) { + assertTimeoutPreemptively(TIMEOUT.multipliedBy(2), () -> { + LOGGER.log("Sending the request"); + HttpResponse response = client.send( + pair.request(), HttpResponse.BodyHandlers.ofInputStream()); + LOGGER.log("Consuming the obtained response"); + verifyResponseBodyArrivesSlow(response); + }); + } + + } + + @ParameterizedTest + @MethodSource("serverRequestPairs") + void testSendAsyncOnSlowBody(ServerRequestPair pair) { + + ServerRequestPair.SERVER_HANDLER_BEHAVIOUR = + ServerRequestPair.ServerHandlerBehaviour.DELIVER_BODY_SLOWLY; + + try (HttpClient client = pair.createClient()) { + assertTimeoutPreemptively(TIMEOUT.multipliedBy(2), () -> { + LOGGER.log("Sending the request asynchronously"); + CompletableFuture> responseFuture = client.sendAsync( + pair.request(), HttpResponse.BodyHandlers.ofInputStream()); + LOGGER.log("Obtaining the response"); + HttpResponse response = responseFuture.get(); + LOGGER.log("Consuming the obtained response"); + verifyResponseBodyArrivesSlow(response); + }); + } + + } + + private static void verifyResponseBodyArrivesSlow(HttpResponse response) { + assertEquals(200, response.statusCode()); + IOException exception = assertThrows( + IOException.class, + () -> { + try (InputStream responseBodyStream = response.body()) { + int i = 0; + int l = ServerRequestPair.CONTENT_LENGTH; + for (; i < l; i++) { + LOGGER.log("Reading byte %s/%s", i, l); + int readByte = responseBodyStream.read(); + if (readByte < 0) { + break; + } + assertEquals(i, readByte); + } + fail("Should not have reached here! (i=%s)".formatted(i)); + } + }); + assertInstanceOf(HttpTimeoutException.class, exception.getCause()); + } + +} diff --git a/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java b/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java new file mode 100644 index 0000000000000..b1d769fafa90a --- /dev/null +++ b/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java @@ -0,0 +1,107 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import jdk.internal.net.http.common.Logger; +import jdk.internal.net.http.common.Utils; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.net.http.HttpTimeoutException; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; + +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; + +/* + * @test + * @bug 8208693 + * @summary Verifies `HttpRequest::timeout` is effective for *response header* timeouts + * + * @library /test/lib + * /test/jdk/java/net/httpclient/lib + * @build jdk.httpclient.test.lib.common.HttpServerAdapters + * jdk.test.lib.net.SimpleSSLContext + * TimeoutResponseTestSupport + * + * @comment `-Djdk.httpclient.{disableRetryConnect,auth.retrylimit=0}` are + * added to keep retry mechanisms out of the picture, and solely + * focus on basic request-response round trip. + * @run junit/othervm + * -Djdk.httpclient.disableRetryConnect + * -Djdk.httpclient.auth.retrylimit=0 + * TimeoutResponseHeaderTest + */ + +/** + * Verifies {@link HttpRequest#timeout() HttpRequest.timeout()} is effective + * for response header timeouts. + * + * @see TimeoutBasic Server connection timeout tests + * @see TimeoutResponseBodyTest Server response body timeout tests + */ +class TimeoutResponseHeaderTest extends TimeoutResponseTestSupport { + + private static final Logger LOGGER = Utils.getDebugLogger( + TimeoutResponseHeaderTest.class.getSimpleName()::toString, Utils.DEBUG); + + static { + ServerRequestPair.SERVER_HANDLER_BEHAVIOUR = + ServerRequestPair.ServerHandlerBehaviour.BLOCK_BEFORE_HEADER_DELIVERY; + } + + @ParameterizedTest + @MethodSource("serverRequestPairs") + void testSend(ServerRequestPair pair) { + try (HttpClient client = pair.createClient()) { + assertTimeoutPreemptively(TIMEOUT.multipliedBy(2), () -> assertThrows( + HttpTimeoutException.class, + () -> { + LOGGER.log("Sending the request"); + client.send(pair.request(), HttpResponse.BodyHandlers.discarding()); + })); + } + } + + @ParameterizedTest + @MethodSource("serverRequestPairs") + void testSendAsync(ServerRequestPair pair) { + try (HttpClient client = pair.createClient()) { + assertTimeoutPreemptively(TIMEOUT.multipliedBy(2), () -> { + LOGGER.log("Sending the request asynchronously"); + CompletableFuture> responseFuture = + client.sendAsync(pair.request(), HttpResponse.BodyHandlers.discarding()); + Exception exception = assertThrows(ExecutionException.class, () -> { + LOGGER.log("Obtaining the response"); + responseFuture.get(); + }); + assertInstanceOf(HttpTimeoutException.class, exception.getCause()); + }); + } + } + +} diff --git a/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java b/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java new file mode 100644 index 0000000000000..811c33f32c33a --- /dev/null +++ b/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java @@ -0,0 +1,264 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import jdk.httpclient.test.lib.common.HttpServerAdapters; +import jdk.internal.net.http.common.Logger; +import jdk.internal.net.http.common.Utils; +import jdk.test.lib.net.SimpleSSLContext; +import org.junit.jupiter.api.AfterAll; + +import javax.net.ssl.SSLContext; +import java.io.IOException; +import java.io.OutputStream; +import java.io.UncheckedIOException; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpClient.Version; +import java.net.http.HttpRequest; +import java.time.Duration; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Stream; + +import static java.net.http.HttpClient.Builder.NO_PROXY; +import static java.net.http.HttpOption.Http3DiscoveryMode.HTTP_3_URI_ONLY; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Utilities for {@code TimeoutResponse*Test}s. + * + * @see TimeoutResponseBodyTest Server response body timeout tests + * @see TimeoutResponseHeaderTest Server response header timeout tests + * @see TimeoutBasic Server connection timeout tests + */ +public class TimeoutResponseTestSupport { + + private static final String CLASS_NAME = TimeoutResponseTestSupport.class.getSimpleName(); + + private static final Logger LOGGER = Utils.getDebugLogger(CLASS_NAME::toString, Utils.DEBUG); + + private static final SSLContext SSL_CONTEXT = createSslContext(); + + /** + * A timeout long enough for all test platforms to ensure that the request reaches to the handler. + */ + protected static final Duration TIMEOUT = Duration.ofSeconds(1); + + protected static final ServerRequestPair + HTTP1 = ServerRequestPair.of(Version.HTTP_1_1, false), + HTTPS1 = ServerRequestPair.of(Version.HTTP_1_1, true), + HTTP2 = ServerRequestPair.of(Version.HTTP_2, false), + HTTPS2 = ServerRequestPair.of(Version.HTTP_2, true), + HTTP3 = ServerRequestPair.of(Version.HTTP_3, true); + + private static SSLContext createSslContext() { + try { + return new SimpleSSLContext().get(); + } catch (IOException exception) { + throw new UncheckedIOException(exception); + } + } + + protected record ServerRequestPair( + HttpServerAdapters.HttpTestServer server, + HttpRequest request, + boolean secure) { + + private static final ExecutorService EXECUTOR = Executors.newVirtualThreadPerTaskExecutor(); + + private static final AtomicInteger SERVER_COUNTER = new AtomicInteger(); + + /** + * An arbitrary content length to cause the client wait for it. + * It just needs to be greater than zero, and big enough to trigger a timeout when delivered slowly. + */ + public static final int CONTENT_LENGTH = 1234; + + public enum ServerHandlerBehaviour { + BLOCK_BEFORE_HEADER_DELIVERY, + BLOCK_BEFORE_BODY_DELIVERY, + DELIVER_BODY_SLOWLY + } + + public static volatile ServerHandlerBehaviour SERVER_HANDLER_BEHAVIOUR; + + private static ServerRequestPair of(Version version, boolean secure) { + + // Create the server and the request URI + SSLContext sslContext = secure ? SSL_CONTEXT : null; + String serverId = "" + SERVER_COUNTER.getAndIncrement(); + HttpServerAdapters.HttpTestServer server = createServer(version, sslContext); + server.getVersion(); + String handlerPath = "/%s/".formatted(CLASS_NAME); + String requestUriScheme = secure ? "https" : "http"; + URI requestUri = URI.create("%s://%s%s-".formatted(requestUriScheme, server.serverAuthority(), handlerPath)); + + // Register the request handler + server.addHandler(createServerHandler(serverId), handlerPath); + + // Create the request + HttpRequest request = HttpRequest.newBuilder(requestUri).version(version).timeout(TIMEOUT).build(); + + // Create the pair + ServerRequestPair pair = new ServerRequestPair(server, request, secure); + pair.server.start(); + LOGGER.log("Server[%s] is started at `%s`", serverId, server.serverAuthority()); + return pair; + + } + + private static HttpServerAdapters.HttpTestServer createServer( + Version version, + SSLContext sslContext) { + try { + return switch (version) { + case HTTP_1_1, HTTP_2 -> HttpServerAdapters.HttpTestServer.create(version, sslContext, EXECUTOR); + case HTTP_3 -> HttpServerAdapters.HttpTestServer.create(HTTP_3_URI_ONLY, sslContext, EXECUTOR); + }; + } catch (IOException exception) { + throw new UncheckedIOException(exception); + } + } + + private static HttpServerAdapters.HttpTestHandler createServerHandler(String serverId) { + return (exchange) -> { + String connectionKey = exchange.getConnectionKey(); + LOGGER.log("Server[%s] has received request (connectionKey=%s)", serverId, connectionKey); + try (exchange) { + switch (SERVER_HANDLER_BEHAVIOUR) { + + case BLOCK_BEFORE_HEADER_DELIVERY: + sleepIndefinitely(serverId, connectionKey); + break; + + case BLOCK_BEFORE_BODY_DELIVERY: + sendResponseHeaders(serverId, exchange, connectionKey); + sleepIndefinitely(serverId, connectionKey); + break; + + case DELIVER_BODY_SLOWLY: + sendResponseHeaders(serverId, exchange, connectionKey); + sendResponseBodySlowly(serverId, exchange, connectionKey); + break; + } + + } catch (Exception exception) { + String message = "Server[%s] has failed! (connectionKey=%s)".formatted(serverId, connectionKey); + LOGGER.log(System.Logger.Level.ERROR, message, exception); + if (exception instanceof InterruptedException) { + // Restore the interrupt + Thread.currentThread().interrupt(); + } + throw new RuntimeException(message, exception); + } + }; + } + + private static void sleepIndefinitely(String serverId, String connectionKey) throws InterruptedException { + LOGGER.log("Server[%s] is sleeping (connectionKey=%s)", serverId, connectionKey); + Thread.sleep(Long.MAX_VALUE); + } + + private static void sendResponseHeaders( + String serverId, + HttpServerAdapters.HttpTestExchange exchange, + String connectionKey) + throws IOException { + LOGGER.log("Server[%s] is sending headers (connectionKey=%s)", serverId, connectionKey); + exchange.sendResponseHeaders(200, CONTENT_LENGTH); + // Force the headers to be flushed + exchange.getResponseBody().flush(); + } + + private static void sendResponseBodySlowly( + String serverId, + HttpServerAdapters.HttpTestExchange exchange, + String connectionKey) + throws Exception { + Duration perBytePauseDuration = Duration.ofMillis(100); + assertTrue( + perBytePauseDuration.multipliedBy(CONTENT_LENGTH).compareTo(TIMEOUT) > 0, + "Per-byte pause duration (%s) must be long enough exceed the timeout (%s) when delivering the content (%s bytes)".formatted( + perBytePauseDuration, TIMEOUT, CONTENT_LENGTH)); + try (OutputStream responseBody = exchange.getResponseBody()) { + for (int i = 0; i < CONTENT_LENGTH; i++) { + LOGGER.log( + "Server[%s] is sending the body %s/%s (connectionKey=%s)", + serverId, i, CONTENT_LENGTH, connectionKey); + responseBody.write(i); + responseBody.flush(); + Thread.sleep(perBytePauseDuration); + } + } + } + + public HttpClient createClient() { + Version version = server.getVersion(); + return HttpServerAdapters + .createClientBuilderFor(version) + .version(version) + .sslContext(SSL_CONTEXT) + .proxy(NO_PROXY) + .build(); + } + + @Override + public String toString() { + Version version = server.getVersion(); + String versionString = version.toString(); + return switch (version) { + case HTTP_1_1, HTTP_2 -> secure ? versionString.replaceFirst("_", "S_") : versionString; + case HTTP_3 -> versionString; + }; + } + + } + + @AfterAll + static void closeServers() { + // Terminate all handlers before shutting down the server, which would block otherwise. + ServerRequestPair.EXECUTOR.shutdownNow(); + Exception[] exceptionRef = {null}; + serverRequestPairs() + .forEach(pair -> { + try { + pair.server.stop(); + } catch (Exception exception) { + if (exceptionRef[0] == null) { + exceptionRef[0] = exception; + } else { + exceptionRef[0].addSuppressed(exception); + } + } + }); + if (exceptionRef[0] != null) { + throw new RuntimeException("failed closing one or more server resources", exceptionRef[0]); + } + } + + protected static Stream serverRequestPairs() { + return Stream.of(HTTP1, HTTPS1, HTTP2, HTTPS2, HTTP3); + } + +} From 582ade7edbfbed76847f4fc903a8b8ea1f840bd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Thu, 25 Sep 2025 10:43:31 +0200 Subject: [PATCH 02/13] Remove `MultiExchange::cancelTimer` --- .../jdk/internal/net/http/MultiExchange.java | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java b/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java index e247c54575673..78fd05b65a554 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java @@ -253,13 +253,6 @@ public Optional remainingConnectTimeout() { .map(ConnectTimeoutTracker::getRemaining); } - private void cancelTimer() { - if (responseTimerEvent != null) { - client.cancelTimer(responseTimerEvent); - responseTimerEvent = null; - } - } - private void requestFilters(HttpRequestImpl r) throws IOException { if (Log.trace()) Log.logTrace("Applying request filters"); for (HeaderFilter filter : filters) { @@ -522,11 +515,17 @@ private CompletableFuture responseAsyncImpl(final boolean applyReqFilt assert response != null; return completedFuture(response); } - // Cancel the timer only if the response has completed - // exceptionally. That is, don't cancel the timer if there - // are no exceptions, since the response body might still - // get consumed, and it is subject to the response timer. - cancelTimer(); + + // Cancel the timer. Note that we only do so if the + // response has completed exceptionally. That is, we don't + // cancel the timer if there are no exceptions, since the + // response body might still get consumed, and it is + // still subject to the response timer. + if (responseTimerEvent != null) { + client.cancelTimer(responseTimerEvent); + responseTimerEvent = null; + } + // all exceptions thrown are handled here final RetryContext retryCtx = checkRetryEligible(ex, exch); assert retryCtx != null : "retry context is null"; From dcb12522aca8e118c546db4e46c3b312e3931546 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Thu, 25 Sep 2025 10:43:49 +0200 Subject: [PATCH 03/13] Start tests with an established connection --- .../httpclient/TimeoutResponseBodyTest.java | 16 ++++++------- .../httpclient/TimeoutResponseHeaderTest.java | 8 +++---- .../TimeoutResponseTestSupport.java | 23 ++++++++++++++++--- 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java b/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java index 4e7ddfe361805..5d00b317905f9 100644 --- a/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java +++ b/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java @@ -84,12 +84,12 @@ class TimeoutResponseBodyTest extends TimeoutResponseTestSupport { @ParameterizedTest @MethodSource("serverRequestPairs") - void testSendOnMissingBody(ServerRequestPair pair) { + void testSendOnMissingBody(ServerRequestPair pair) throws Exception { ServerRequestPair.SERVER_HANDLER_BEHAVIOUR = ServerRequestPair.ServerHandlerBehaviour.BLOCK_BEFORE_BODY_DELIVERY; - try (HttpClient client = pair.createClient()) { + try (HttpClient client = pair.createClientWithEstablishedConnection()) { assertTimeoutPreemptively(TIMEOUT.multipliedBy(2), () -> { LOGGER.log("Sending the request"); HttpResponse response = client.send( @@ -103,12 +103,12 @@ void testSendOnMissingBody(ServerRequestPair pair) { @ParameterizedTest @MethodSource("serverRequestPairs") - void testSendAsyncOnMissingBody(ServerRequestPair pair) { + void testSendAsyncOnMissingBody(ServerRequestPair pair) throws Exception { ServerRequestPair.SERVER_HANDLER_BEHAVIOUR = ServerRequestPair.ServerHandlerBehaviour.BLOCK_BEFORE_BODY_DELIVERY; - try (HttpClient client = pair.createClient()) { + try (HttpClient client = pair.createClientWithEstablishedConnection()) { assertTimeoutPreemptively(TIMEOUT.multipliedBy(2), () -> { LOGGER.log("Sending the request asynchronously"); CompletableFuture> responseFuture = client.sendAsync( @@ -137,12 +137,12 @@ private static void verifyResponseBodyDoesNotArrive(HttpResponse re @ParameterizedTest @MethodSource("serverRequestPairs") - void testSendOnSlowBody(ServerRequestPair pair) { + void testSendOnSlowBody(ServerRequestPair pair) throws Exception { ServerRequestPair.SERVER_HANDLER_BEHAVIOUR = ServerRequestPair.ServerHandlerBehaviour.DELIVER_BODY_SLOWLY; - try (HttpClient client = pair.createClient()) { + try (HttpClient client = pair.createClientWithEstablishedConnection()) { assertTimeoutPreemptively(TIMEOUT.multipliedBy(2), () -> { LOGGER.log("Sending the request"); HttpResponse response = client.send( @@ -156,12 +156,12 @@ void testSendOnSlowBody(ServerRequestPair pair) { @ParameterizedTest @MethodSource("serverRequestPairs") - void testSendAsyncOnSlowBody(ServerRequestPair pair) { + void testSendAsyncOnSlowBody(ServerRequestPair pair) throws Exception { ServerRequestPair.SERVER_HANDLER_BEHAVIOUR = ServerRequestPair.ServerHandlerBehaviour.DELIVER_BODY_SLOWLY; - try (HttpClient client = pair.createClient()) { + try (HttpClient client = pair.createClientWithEstablishedConnection()) { assertTimeoutPreemptively(TIMEOUT.multipliedBy(2), () -> { LOGGER.log("Sending the request asynchronously"); CompletableFuture> responseFuture = client.sendAsync( diff --git a/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java b/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java index b1d769fafa90a..d8326285e6b19 100644 --- a/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java +++ b/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java @@ -76,8 +76,8 @@ class TimeoutResponseHeaderTest extends TimeoutResponseTestSupport { @ParameterizedTest @MethodSource("serverRequestPairs") - void testSend(ServerRequestPair pair) { - try (HttpClient client = pair.createClient()) { + void testSend(ServerRequestPair pair) throws Exception { + try (HttpClient client = pair.createClientWithEstablishedConnection()) { assertTimeoutPreemptively(TIMEOUT.multipliedBy(2), () -> assertThrows( HttpTimeoutException.class, () -> { @@ -89,8 +89,8 @@ void testSend(ServerRequestPair pair) { @ParameterizedTest @MethodSource("serverRequestPairs") - void testSendAsync(ServerRequestPair pair) { - try (HttpClient client = pair.createClient()) { + void testSendAsync(ServerRequestPair pair) throws Exception { + try (HttpClient client = pair.createClientWithEstablishedConnection()) { assertTimeoutPreemptively(TIMEOUT.multipliedBy(2), () -> { LOGGER.log("Sending the request asynchronously"); CompletableFuture> responseFuture = diff --git a/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java b/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java index 811c33f32c33a..b17e7eb3c9501 100644 --- a/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java +++ b/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java @@ -35,6 +35,7 @@ import java.net.http.HttpClient; import java.net.http.HttpClient.Version; import java.net.http.HttpRequest; +import java.net.http.HttpResponse; import java.time.Duration; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -146,6 +147,15 @@ private static HttpServerAdapters.HttpTestHandler createServerHandler(String ser String connectionKey = exchange.getConnectionKey(); LOGGER.log("Server[%s] has received request (connectionKey=%s)", serverId, connectionKey); try (exchange) { + + // Short-circuit on `HEAD` requests. + // They are used for admitting established connections to the pool. + if ("HEAD".equals(exchange.getRequestMethod())) { + LOGGER.log("Server[%s] is responding to the `HEAD` request (connectionKey=%s)", serverId, connectionKey); + exchange.sendResponseHeaders(200, 0); + return; + } + switch (SERVER_HANDLER_BEHAVIOUR) { case BLOCK_BEFORE_HEADER_DELIVERY: @@ -199,7 +209,7 @@ private static void sendResponseBodySlowly( Duration perBytePauseDuration = Duration.ofMillis(100); assertTrue( perBytePauseDuration.multipliedBy(CONTENT_LENGTH).compareTo(TIMEOUT) > 0, - "Per-byte pause duration (%s) must be long enough exceed the timeout (%s) when delivering the content (%s bytes)".formatted( + "Per-byte pause duration (%s) must be long enough to exceed the timeout (%s) when delivering the content (%s bytes)".formatted( perBytePauseDuration, TIMEOUT, CONTENT_LENGTH)); try (OutputStream responseBody = exchange.getResponseBody()) { for (int i = 0; i < CONTENT_LENGTH; i++) { @@ -213,14 +223,21 @@ private static void sendResponseBodySlowly( } } - public HttpClient createClient() { + public HttpClient createClientWithEstablishedConnection() throws IOException, InterruptedException { Version version = server.getVersion(); - return HttpServerAdapters + HttpClient client = HttpServerAdapters .createClientBuilderFor(version) .version(version) .sslContext(SSL_CONTEXT) .proxy(NO_PROXY) .build(); + // Ensure an established connection is admitted to the pool. This + // helps to cross out any possibilities of a timeout before a + // request makes it to the server handler. For instance, consider + // HTTP/1.1 to HTTP/2 upgrades, or long-running TLS handshakes. + HttpRequest headRequest = HttpRequest.newBuilder(this.request.uri()).version(version).HEAD().build(); + client.send(headRequest, HttpResponse.BodyHandlers.discarding()); + return client; } @Override From ebf62363b30f0fb2a8b05d5970d23b116a2f0d4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Tue, 30 Sep 2025 18:20:19 +0200 Subject: [PATCH 04/13] Fix `H3_REQUEST_REJECTED` handling in `Http3ExchangeImpl` --- .../share/classes/jdk/internal/net/http/Http3ExchangeImpl.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Http3ExchangeImpl.java b/src/java.net.http/share/classes/jdk/internal/net/http/Http3ExchangeImpl.java index ff1e024673cb8..1970c0dcdb758 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/Http3ExchangeImpl.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/Http3ExchangeImpl.java @@ -1336,6 +1336,9 @@ void onReaderReset() { Http3Error resetError = Http3Error.fromCode(errorCode) .orElse(Http3Error.H3_REQUEST_CANCELLED); if (!requestSent || !responseReceived) { + if (!responseReceived && resetError == Http3Error.H3_REQUEST_REJECTED) { + exchange.markUnprocessedByPeer(); + } cancelImpl(new IOException("Stream %s reset by peer: %s" .formatted(streamId(), resetReason)), resetError); From 42d21066eb1406f15b57733858f4b03fd8387756 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Tue, 30 Sep 2025 19:04:11 +0200 Subject: [PATCH 05/13] Extend tests to cover client's retry mechanism --- .../httpclient/TimeoutResponseBodyTest.java | 42 ++++-- .../httpclient/TimeoutResponseHeaderTest.java | 42 ++++-- .../TimeoutResponseTestSupport.java | 132 +++++++++++++++--- 3 files changed, 176 insertions(+), 40 deletions(-) diff --git a/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java b/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java index 5d00b317905f9..4e11270fc7252 100644 --- a/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java +++ b/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java @@ -41,22 +41,40 @@ import static org.junit.jupiter.api.Assertions.fail; /* - * @test + * @test id=retriesDisabled * @bug 8208693 - * @summary Verifies `HttpRequest::timeout` is effective for *response body* timeouts + * @summary Verifies `HttpRequest::timeout` is effective for *response body* + * timeouts when all retry mechanisms are disabled. * * @library /test/lib * /test/jdk/java/net/httpclient/lib - * @build jdk.httpclient.test.lib.common.HttpServerAdapters - * jdk.test.lib.net.SimpleSSLContext - * TimeoutResponseTestSupport + * @build TimeoutResponseTestSupport * - * @comment `-Djdk.httpclient.{disableRetryConnect,auth.retrylimit=0}` are - * added to keep retry mechanisms out of the picture, and solely - * focus on basic request-response round trip. * @run junit/othervm + * -Djdk.httpclient.auth.retrylimit=0 * -Djdk.httpclient.disableRetryConnect + * -Djdk.httpclient.redirects.retrylimit=0 + * -Dtest.requestTimeoutMillis=1000 + * TimeoutResponseBodyTest + */ + +/* + * @test id=retriesEnabledForResponseFailure + * @bug 8208693 + * @summary Verifies `HttpRequest::timeout` is effective for *response body* + * timeouts, where some initial responses are intentionally configured + * to fail to trigger retries. + * + * @library /test/lib + * /test/jdk/java/net/httpclient/lib + * @build TimeoutResponseTestSupport + * + * @run junit/othervm * -Djdk.httpclient.auth.retrylimit=0 + * -Djdk.httpclient.disableRetryConnect + * -Djdk.httpclient.redirects.retrylimit=3 + * -Dtest.requestTimeoutMillis=1000 + * -Dtest.responseFailureWaitDurationMillis=600 * TimeoutResponseBodyTest */ @@ -90,7 +108,7 @@ void testSendOnMissingBody(ServerRequestPair pair) throws Exception { ServerRequestPair.ServerHandlerBehaviour.BLOCK_BEFORE_BODY_DELIVERY; try (HttpClient client = pair.createClientWithEstablishedConnection()) { - assertTimeoutPreemptively(TIMEOUT.multipliedBy(2), () -> { + assertTimeoutPreemptively(REQUEST_TIMEOUT.multipliedBy(2), () -> { LOGGER.log("Sending the request"); HttpResponse response = client.send( pair.request(), HttpResponse.BodyHandlers.ofInputStream()); @@ -109,7 +127,7 @@ void testSendAsyncOnMissingBody(ServerRequestPair pair) throws Exception { ServerRequestPair.ServerHandlerBehaviour.BLOCK_BEFORE_BODY_DELIVERY; try (HttpClient client = pair.createClientWithEstablishedConnection()) { - assertTimeoutPreemptively(TIMEOUT.multipliedBy(2), () -> { + assertTimeoutPreemptively(REQUEST_TIMEOUT.multipliedBy(2), () -> { LOGGER.log("Sending the request asynchronously"); CompletableFuture> responseFuture = client.sendAsync( pair.request(), HttpResponse.BodyHandlers.ofInputStream()); @@ -143,7 +161,7 @@ void testSendOnSlowBody(ServerRequestPair pair) throws Exception { ServerRequestPair.ServerHandlerBehaviour.DELIVER_BODY_SLOWLY; try (HttpClient client = pair.createClientWithEstablishedConnection()) { - assertTimeoutPreemptively(TIMEOUT.multipliedBy(2), () -> { + assertTimeoutPreemptively(REQUEST_TIMEOUT.multipliedBy(2), () -> { LOGGER.log("Sending the request"); HttpResponse response = client.send( pair.request(), HttpResponse.BodyHandlers.ofInputStream()); @@ -162,7 +180,7 @@ void testSendAsyncOnSlowBody(ServerRequestPair pair) throws Exception { ServerRequestPair.ServerHandlerBehaviour.DELIVER_BODY_SLOWLY; try (HttpClient client = pair.createClientWithEstablishedConnection()) { - assertTimeoutPreemptively(TIMEOUT.multipliedBy(2), () -> { + assertTimeoutPreemptively(REQUEST_TIMEOUT.multipliedBy(2), () -> { LOGGER.log("Sending the request asynchronously"); CompletableFuture> responseFuture = client.sendAsync( pair.request(), HttpResponse.BodyHandlers.ofInputStream()); diff --git a/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java b/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java index d8326285e6b19..c5911d3b0f667 100644 --- a/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java +++ b/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java @@ -38,22 +38,40 @@ import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; /* - * @test + * @test id=retriesDisabled * @bug 8208693 - * @summary Verifies `HttpRequest::timeout` is effective for *response header* timeouts + * @summary Verifies `HttpRequest::timeout` is effective for *response header* + * timeouts when all retry mechanisms are disabled. * - * @library /test/lib - * /test/jdk/java/net/httpclient/lib - * @build jdk.httpclient.test.lib.common.HttpServerAdapters - * jdk.test.lib.net.SimpleSSLContext - * TimeoutResponseTestSupport + * @library /test/jdk/java/net/httpclient/lib + * /test/lib + * @build TimeoutResponseTestSupport * - * @comment `-Djdk.httpclient.{disableRetryConnect,auth.retrylimit=0}` are - * added to keep retry mechanisms out of the picture, and solely - * focus on basic request-response round trip. * @run junit/othervm + * -Djdk.httpclient.auth.retrylimit=0 * -Djdk.httpclient.disableRetryConnect + * -Djdk.httpclient.redirects.retrylimit=0 + * -Dtest.requestTimeoutMillis=1000 + * TimeoutResponseHeaderTest + */ + +/* + * @test id=retriesEnabledForResponseFailure + * @bug 8208693 + * @summary Verifies `HttpRequest::timeout` is effective for *response header* + * timeouts, where some initial responses are intentionally configured + * to fail to trigger retries. + * + * @library /test/jdk/java/net/httpclient/lib + * /test/lib + * @build TimeoutResponseTestSupport + * + * @run junit/othervm * -Djdk.httpclient.auth.retrylimit=0 + * -Djdk.httpclient.disableRetryConnect + * -Djdk.httpclient.redirects.retrylimit=3 + * -Dtest.requestTimeoutMillis=1000 + * -Dtest.responseFailureWaitDurationMillis=600 * TimeoutResponseHeaderTest */ @@ -78,7 +96,7 @@ class TimeoutResponseHeaderTest extends TimeoutResponseTestSupport { @MethodSource("serverRequestPairs") void testSend(ServerRequestPair pair) throws Exception { try (HttpClient client = pair.createClientWithEstablishedConnection()) { - assertTimeoutPreemptively(TIMEOUT.multipliedBy(2), () -> assertThrows( + assertTimeoutPreemptively(REQUEST_TIMEOUT.multipliedBy(2), () -> assertThrows( HttpTimeoutException.class, () -> { LOGGER.log("Sending the request"); @@ -91,7 +109,7 @@ void testSend(ServerRequestPair pair) throws Exception { @MethodSource("serverRequestPairs") void testSendAsync(ServerRequestPair pair) throws Exception { try (HttpClient client = pair.createClientWithEstablishedConnection()) { - assertTimeoutPreemptively(TIMEOUT.multipliedBy(2), () -> { + assertTimeoutPreemptively(REQUEST_TIMEOUT.multipliedBy(2), () -> { LOGGER.log("Sending the request asynchronously"); CompletableFuture> responseFuture = client.sendAsync(pair.request(), HttpResponse.BodyHandlers.discarding()); diff --git a/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java b/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java index b17e7eb3c9501..8e6e5cdc218c0 100644 --- a/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java +++ b/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java @@ -24,8 +24,12 @@ import jdk.httpclient.test.lib.common.HttpServerAdapters; import jdk.internal.net.http.common.Logger; import jdk.internal.net.http.common.Utils; +import jdk.internal.net.http.frame.ErrorFrame; +import jdk.internal.net.http.http3.Http3Error; import jdk.test.lib.net.SimpleSSLContext; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import javax.net.ssl.SSLContext; import java.io.IOException; @@ -37,6 +41,8 @@ import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.time.Duration; +import java.util.Map; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicInteger; @@ -44,6 +50,7 @@ import static java.net.http.HttpClient.Builder.NO_PROXY; import static java.net.http.HttpOption.Http3DiscoveryMode.HTTP_3_URI_ONLY; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -61,10 +68,50 @@ public class TimeoutResponseTestSupport { private static final SSLContext SSL_CONTEXT = createSslContext(); - /** - * A timeout long enough for all test platforms to ensure that the request reaches to the handler. - */ - protected static final Duration TIMEOUT = Duration.ofSeconds(1); + protected static final Duration REQUEST_TIMEOUT = + Duration.ofMillis(Long.parseLong(System.getProperty("test.requestTimeoutMillis"))); + + static { + assertTrue( + REQUEST_TIMEOUT.isPositive(), + "was expecting `test.requestTimeoutMillis > 0`, found: " + REQUEST_TIMEOUT); + } + + protected static final int RETRY_LIMIT = + Integer.parseInt(System.getProperty("jdk.httpclient.redirects.retrylimit", "0")); + + private static final long RESPONSE_Failure_WAIT_DURATION_MILLIS = + Long.parseLong(System.getProperty("test.responseFailureWaitDurationMillis", "0")); + + static { + if (RETRY_LIMIT > 0) { + + // Verify that response failure wait duration is provided + if (RESPONSE_Failure_WAIT_DURATION_MILLIS <= 0) { + String message = String.format( + "`jdk.httpclient.redirects.retrylimit` (%s) is greater than zero. " + + "`test.responseFailureWaitDurationMillis` (%s) must be greater than zero too.", + RETRY_LIMIT, RESPONSE_Failure_WAIT_DURATION_MILLIS); + throw new AssertionError(message); + } + + // Verify that the total response failure waits exceed the request timeout + Duration totalResponseFailureWaitDuration = Duration + .ofMillis(RESPONSE_Failure_WAIT_DURATION_MILLIS) + .multipliedBy(RETRY_LIMIT); + if (totalResponseFailureWaitDuration.compareTo(REQUEST_TIMEOUT) <= 0) { + String message = ("`test.responseFailureWaitDurationMillis * jdk.httpclient.redirects.retrylimit` (%s * %s = %s) " + + "must be greater than `test.requestTimeoutMillis` (%s)") + .formatted( + RESPONSE_Failure_WAIT_DURATION_MILLIS, + RETRY_LIMIT, + totalResponseFailureWaitDuration, + REQUEST_TIMEOUT); + throw new AssertionError(message); + } + + } + } protected static final ServerRequestPair HTTP1 = ServerRequestPair.of(Version.HTTP_1_1, false), @@ -88,6 +135,8 @@ protected record ServerRequestPair( private static final ExecutorService EXECUTOR = Executors.newVirtualThreadPerTaskExecutor(); + private static final CountDownLatch SHUT_DOWN_LATCH = new CountDownLatch(1); + private static final AtomicInteger SERVER_COUNTER = new AtomicInteger(); /** @@ -104,6 +153,8 @@ public enum ServerHandlerBehaviour { public static volatile ServerHandlerBehaviour SERVER_HANDLER_BEHAVIOUR; + public static volatile int SERVER_HANDLER_PENDING_FAILURE_COUNT = 0; + private static ServerRequestPair of(Version version, boolean secure) { // Create the server and the request URI @@ -119,7 +170,7 @@ private static ServerRequestPair of(Version version, boolean secure) { server.addHandler(createServerHandler(serverId), handlerPath); // Create the request - HttpRequest request = HttpRequest.newBuilder(requestUri).version(version).timeout(TIMEOUT).build(); + HttpRequest request = HttpRequest.newBuilder(requestUri).version(version).timeout(REQUEST_TIMEOUT).build(); // Create the pair ServerRequestPair pair = new ServerRequestPair(server, request, secure); @@ -145,17 +196,43 @@ private static HttpServerAdapters.HttpTestServer createServer( private static HttpServerAdapters.HttpTestHandler createServerHandler(String serverId) { return (exchange) -> { String connectionKey = exchange.getConnectionKey(); - LOGGER.log("Server[%s] has received request (connectionKey=%s)", serverId, connectionKey); + LOGGER.log( + "Server[%s] has received request %s", + serverId, Map.of("connectionKey", connectionKey)); try (exchange) { // Short-circuit on `HEAD` requests. // They are used for admitting established connections to the pool. if ("HEAD".equals(exchange.getRequestMethod())) { - LOGGER.log("Server[%s] is responding to the `HEAD` request (connectionKey=%s)", serverId, connectionKey); + LOGGER.log( + "Server[%s] is responding to the `HEAD` request %s", + serverId, Map.of("connectionKey", connectionKey)); exchange.sendResponseHeaders(200, 0); return; } + // Short-circuit if instructed to fail + synchronized (ServerRequestPair.class) { + if (SERVER_HANDLER_PENDING_FAILURE_COUNT > 0) { + LOGGER.log( + "Server[%s] is prematurely failing as instructed %s", + serverId, + Map.of( + "connectionKey", connectionKey, + "SERVER_HANDLER_PENDING_FAILURE_COUNT", SERVER_HANDLER_PENDING_FAILURE_COUNT)); + // Closing the exchange will trigger an `END_STREAM` without a headers frame. + // This is a protocol violation, hence we must reset the stream first. + // We are doing so using by rejecting the stream, which is known to make the client retry. + if (Version.HTTP_2.equals(exchange.getExchangeVersion())) { + exchange.resetStream(ErrorFrame.REFUSED_STREAM); + } else if (Version.HTTP_3.equals(exchange.getExchangeVersion())) { + exchange.resetStream(Http3Error.H3_REQUEST_REJECTED.code()); + } + SERVER_HANDLER_PENDING_FAILURE_COUNT--; + return; + } + } + switch (SERVER_HANDLER_BEHAVIOUR) { case BLOCK_BEFORE_HEADER_DELIVERY: @@ -174,7 +251,9 @@ private static HttpServerAdapters.HttpTestHandler createServerHandler(String ser } } catch (Exception exception) { - String message = "Server[%s] has failed! (connectionKey=%s)".formatted(serverId, connectionKey); + String message = String.format( + "Server[%s] has failed! %s", + serverId, Map.of("connectionKey", connectionKey)); LOGGER.log(System.Logger.Level.ERROR, message, exception); if (exception instanceof InterruptedException) { // Restore the interrupt @@ -186,8 +265,8 @@ private static HttpServerAdapters.HttpTestHandler createServerHandler(String ser } private static void sleepIndefinitely(String serverId, String connectionKey) throws InterruptedException { - LOGGER.log("Server[%s] is sleeping (connectionKey=%s)", serverId, connectionKey); - Thread.sleep(Long.MAX_VALUE); + LOGGER.log("Server[%s] is sleeping %s", serverId, Map.of("connectionKey", connectionKey)); + SHUT_DOWN_LATCH.await(); } private static void sendResponseHeaders( @@ -195,7 +274,7 @@ private static void sendResponseHeaders( HttpServerAdapters.HttpTestExchange exchange, String connectionKey) throws IOException { - LOGGER.log("Server[%s] is sending headers (connectionKey=%s)", serverId, connectionKey); + LOGGER.log("Server[%s] is sending headers %s", serverId, Map.of("connectionKey", connectionKey)); exchange.sendResponseHeaders(200, CONTENT_LENGTH); // Force the headers to be flushed exchange.getResponseBody().flush(); @@ -208,14 +287,14 @@ private static void sendResponseBodySlowly( throws Exception { Duration perBytePauseDuration = Duration.ofMillis(100); assertTrue( - perBytePauseDuration.multipliedBy(CONTENT_LENGTH).compareTo(TIMEOUT) > 0, + perBytePauseDuration.multipliedBy(CONTENT_LENGTH).compareTo(REQUEST_TIMEOUT) > 0, "Per-byte pause duration (%s) must be long enough to exceed the timeout (%s) when delivering the content (%s bytes)".formatted( - perBytePauseDuration, TIMEOUT, CONTENT_LENGTH)); + perBytePauseDuration, REQUEST_TIMEOUT, CONTENT_LENGTH)); try (OutputStream responseBody = exchange.getResponseBody()) { for (int i = 0; i < CONTENT_LENGTH; i++) { LOGGER.log( - "Server[%s] is sending the body %s/%s (connectionKey=%s)", - serverId, i, CONTENT_LENGTH, connectionKey); + "Server[%s] is sending the body %s/%s %s", + serverId, i, CONTENT_LENGTH, Map.of("connectionKey", connectionKey)); responseBody.write(i); responseBody.flush(); Thread.sleep(perBytePauseDuration); @@ -254,8 +333,12 @@ public String toString() { @AfterAll static void closeServers() { + // Terminate all handlers before shutting down the server, which would block otherwise. - ServerRequestPair.EXECUTOR.shutdownNow(); + ServerRequestPair.SHUT_DOWN_LATCH.countDown(); + ServerRequestPair.EXECUTOR.shutdown(); + + // Shut down servers Exception[] exceptionRef = {null}; serverRequestPairs() .forEach(pair -> { @@ -272,6 +355,23 @@ static void closeServers() { if (exceptionRef[0] != null) { throw new RuntimeException("failed closing one or more server resources", exceptionRef[0]); } + + } + + /** + * Configures how many times the handler should fail. + */ + @BeforeEach + void resetServerHandlerFailureIndex() { + ServerRequestPair.SERVER_HANDLER_PENDING_FAILURE_COUNT = Math.max(0, RETRY_LIMIT - 1); + } + + /** + * Ensures that the handler has failed as many times as instructed. + */ + @AfterEach + void verifyServerHandlerFailureIndex() { + assertEquals(0, ServerRequestPair.SERVER_HANDLER_PENDING_FAILURE_COUNT); } protected static Stream serverRequestPairs() { From 974ac81305b44345c164c7ea76f8b61238d018d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Tue, 30 Sep 2025 19:21:03 +0200 Subject: [PATCH 06/13] Use `HTTP_3_URI_ONLY` for client's HTTP/3 discovery mechanism --- .../net/httpclient/TimeoutResponseTestSupport.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java b/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java index 8e6e5cdc218c0..2f30887d53e82 100644 --- a/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java +++ b/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java @@ -38,6 +38,7 @@ import java.net.URI; import java.net.http.HttpClient; import java.net.http.HttpClient.Version; +import java.net.http.HttpOption; import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.time.Duration; @@ -170,7 +171,7 @@ private static ServerRequestPair of(Version version, boolean secure) { server.addHandler(createServerHandler(serverId), handlerPath); // Create the request - HttpRequest request = HttpRequest.newBuilder(requestUri).version(version).timeout(REQUEST_TIMEOUT).build(); + HttpRequest request = createRequestBuilder(requestUri, version).timeout(REQUEST_TIMEOUT).build(); // Create the pair ServerRequestPair pair = new ServerRequestPair(server, request, secure); @@ -314,11 +315,19 @@ public HttpClient createClientWithEstablishedConnection() throws IOException, In // helps to cross out any possibilities of a timeout before a // request makes it to the server handler. For instance, consider // HTTP/1.1 to HTTP/2 upgrades, or long-running TLS handshakes. - HttpRequest headRequest = HttpRequest.newBuilder(this.request.uri()).version(version).HEAD().build(); + HttpRequest headRequest = createRequestBuilder(request.uri(), version).HEAD().build(); client.send(headRequest, HttpResponse.BodyHandlers.discarding()); return client; } + private static HttpRequest.Builder createRequestBuilder(URI uri, Version version) { + HttpRequest.Builder requestBuilder = HttpRequest.newBuilder(uri).version(version); + if (Version.HTTP_3.equals(version)) { + requestBuilder.setOption(HttpOption.H3_DISCOVERY, HttpOption.Http3DiscoveryMode.HTTP_3_URI_ONLY); + } + return requestBuilder; + } + @Override public String toString() { Version version = server.getVersion(); From 07f237726989273a1733759f85b398442e02953c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Tue, 30 Sep 2025 19:53:28 +0200 Subject: [PATCH 07/13] Remove cross-links between tests, they are not maintainable --- test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java | 3 --- test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java | 3 --- 2 files changed, 6 deletions(-) diff --git a/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java b/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java index 4e11270fc7252..b8be35975c1d0 100644 --- a/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java +++ b/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java @@ -91,9 +91,6 @@ *

* Each test is provided a pristine ephemeral client to avoid any unexpected * effects due to pooling. - * - * @see TimeoutBasic Server connection timeout tests - * @see TimeoutResponseHeaderTest Server response header timeout tests */ class TimeoutResponseBodyTest extends TimeoutResponseTestSupport { diff --git a/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java b/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java index c5911d3b0f667..80099431c0c89 100644 --- a/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java +++ b/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java @@ -78,9 +78,6 @@ /** * Verifies {@link HttpRequest#timeout() HttpRequest.timeout()} is effective * for response header timeouts. - * - * @see TimeoutBasic Server connection timeout tests - * @see TimeoutResponseBodyTest Server response body timeout tests */ class TimeoutResponseHeaderTest extends TimeoutResponseTestSupport { From bb04f87e22c6474d8d1dda700a8fad23281ca150 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Tue, 30 Sep 2025 19:53:42 +0200 Subject: [PATCH 08/13] Document test methods --- .../httpclient/TimeoutResponseBodyTest.java | 20 +++++++++++++++++++ .../httpclient/TimeoutResponseHeaderTest.java | 10 ++++++++++ 2 files changed, 30 insertions(+) diff --git a/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java b/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java index b8be35975c1d0..f4dcdfa71abd7 100644 --- a/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java +++ b/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java @@ -97,6 +97,11 @@ class TimeoutResponseBodyTest extends TimeoutResponseTestSupport { private static final Logger LOGGER = Utils.getDebugLogger( TimeoutResponseBodyTest.class.getSimpleName()::toString, Utils.DEBUG); + /** + * Tests timeouts using + * {@link HttpClient#send(HttpRequest, HttpResponse.BodyHandler) HttpClient::send} + * against a server blocking without delivering the response body. + */ @ParameterizedTest @MethodSource("serverRequestPairs") void testSendOnMissingBody(ServerRequestPair pair) throws Exception { @@ -116,6 +121,11 @@ void testSendOnMissingBody(ServerRequestPair pair) throws Exception { } + /** + * Tests timeouts using + * {@link HttpClient#sendAsync(HttpRequest, HttpResponse.BodyHandler) HttpClient::sendAsync} + * against a server blocking without delivering the response body. + */ @ParameterizedTest @MethodSource("serverRequestPairs") void testSendAsyncOnMissingBody(ServerRequestPair pair) throws Exception { @@ -150,6 +160,11 @@ private static void verifyResponseBodyDoesNotArrive(HttpResponse re assertInstanceOf(HttpTimeoutException.class, exception.getCause()); } + /** + * Tests timeouts using + * {@link HttpClient#send(HttpRequest, HttpResponse.BodyHandler) HttpClient::send} + * against a server delivering the response body very slowly. + */ @ParameterizedTest @MethodSource("serverRequestPairs") void testSendOnSlowBody(ServerRequestPair pair) throws Exception { @@ -169,6 +184,11 @@ void testSendOnSlowBody(ServerRequestPair pair) throws Exception { } + /** + * Tests timeouts using + * {@link HttpClient#sendAsync(HttpRequest, HttpResponse.BodyHandler) HttpClient::sendAsync} + * against a server delivering the response body very slowly. + */ @ParameterizedTest @MethodSource("serverRequestPairs") void testSendAsyncOnSlowBody(ServerRequestPair pair) throws Exception { diff --git a/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java b/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java index 80099431c0c89..27e84420cb263 100644 --- a/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java +++ b/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java @@ -89,6 +89,11 @@ class TimeoutResponseHeaderTest extends TimeoutResponseTestSupport { ServerRequestPair.ServerHandlerBehaviour.BLOCK_BEFORE_HEADER_DELIVERY; } + /** + * Tests timeouts using + * {@link HttpClient#send(HttpRequest, HttpResponse.BodyHandler) HttpClient::send} + * against a server blocking without delivering any response headers. + */ @ParameterizedTest @MethodSource("serverRequestPairs") void testSend(ServerRequestPair pair) throws Exception { @@ -102,6 +107,11 @@ void testSend(ServerRequestPair pair) throws Exception { } } + /** + * Tests timeouts using + * {@link HttpClient#sendAsync(HttpRequest, HttpResponse.BodyHandler) HttpClient::sendAsync} + * against a server blocking without delivering any response headers. + */ @ParameterizedTest @MethodSource("serverRequestPairs") void testSendAsync(ServerRequestPair pair) throws Exception { From defe15db2b3044af71ba7134586824bd53ee468d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Wed, 1 Oct 2025 09:30:43 +0200 Subject: [PATCH 09/13] Suppress warnings in `sendResponseBodySlowly()` --- test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java b/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java index 2f30887d53e82..e15f303ae7202 100644 --- a/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java +++ b/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java @@ -300,6 +300,10 @@ private static void sendResponseBodySlowly( responseBody.flush(); Thread.sleep(perBytePauseDuration); } + throw new AssertionError("Delivery should never have succeeded due to timeout!"); + } catch (IOException _) { + // Client's timeout mechanism is expected to short-circuit and cut the stream. + // Hence, discard I/O failures. } } From 71a5272cce450bfdaefc8b287ee04d46d24488ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Wed, 1 Oct 2025 14:19:24 +0200 Subject: [PATCH 10/13] Fix timer leak (detected by `RedirectTimeoutTest`) --- .../jdk/internal/net/http/MultiExchange.java | 86 +++++++++++++++++-- 1 file changed, 80 insertions(+), 6 deletions(-) diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java b/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java index 78fd05b65a554..13a6e9c5b5fbc 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java @@ -31,6 +31,7 @@ import java.net.http.HttpClient.Version; import java.net.http.HttpConnectTimeoutException; import java.net.http.StreamLimitException; +import java.nio.ByteBuffer; import java.time.Duration; import java.util.List; import java.util.ListIterator; @@ -253,6 +254,13 @@ public Optional remainingConnectTimeout() { .map(ConnectTimeoutTracker::getRemaining); } + private void cancelTimer() { + if (responseTimerEvent != null) { + client.cancelTimer(responseTimerEvent); + responseTimerEvent = null; + } + } + private void requestFilters(HttpRequestImpl r) throws IOException { if (Log.trace()) Log.logTrace("Applying request filters"); for (HeaderFilter filter : filters) { @@ -388,6 +396,7 @@ private HttpResponse setNewResponse(HttpRequest request, Response r, T body, private CompletableFuture> responseAsync0(CompletableFuture start) { + AtomicReference> effectiveResponseHandlerRef = new AtomicReference<>(); return start.thenCompose( _ -> { // this is the first attempt to have the request processed by the server attempts.set(1); @@ -404,11 +413,76 @@ private HttpResponse setNewResponse(HttpRequest request, Response r, T body, } else return handleNoBody(r, exch); } - return exch.readBodyAsync(responseHandler) + HttpResponse.BodyHandler effectiveResponseHandler = effectiveResponseHandlerRef.get(); + if (effectiveResponseHandler == null) { + effectiveResponseHandlerRef.set(effectiveResponseHandler = responseTimerEvent != null + ? new TimerCancellingBodyHandlerWrapper(responseHandler) + : responseHandler); + } + return exch.readBodyAsync(effectiveResponseHandler) .thenApply((T body) -> setNewResponse(r.request, r, body, exch)); }).exceptionallyCompose(this::whenCancelled); } + /** + * Decorates a {@link HttpResponse.BodyHandler} to {@link #cancelTimer()} at completion. + */ + private final class TimerCancellingBodyHandlerWrapper implements HttpResponse.BodyHandler { + + private final HttpResponse.BodyHandler delegate; + + private TimerCancellingBodyHandlerWrapper(HttpResponse.BodyHandler delegate) { + this.delegate = delegate; + } + + @Override + public BodySubscriber apply(HttpResponse.ResponseInfo responseInfo) { + BodySubscriber subscriber = delegate.apply(responseInfo); + return new TimerCancellingBodySubscriberWrapper(subscriber); + } + + } + + /** + * Decorates a {@link HttpResponse.BodySubscriber} to {@link #cancelTimer()} at completion. + */ + private final class TimerCancellingBodySubscriberWrapper implements HttpResponse.BodySubscriber { + + private final HttpResponse.BodySubscriber delegate; + + private TimerCancellingBodySubscriberWrapper(BodySubscriber delegate) { + this.delegate = delegate; + } + + @Override + public CompletionStage getBody() { + return delegate.getBody(); + } + + @Override + public void onSubscribe(Flow.Subscription subscription) { + delegate.onSubscribe(subscription); + } + + @Override + public void onNext(List buffers) { + delegate.onNext(buffers); + } + + @Override + public void onError(Throwable throwable) { + cancelTimer(); + delegate.onError(throwable); + } + + @Override + public void onComplete() { + cancelTimer(); + delegate.onComplete(); + } + + } + // returns a CancellationException that wraps the given cause // if cancel(boolean) was called, the given cause otherwise private Throwable wrapIfCancelled(Throwable cause) { @@ -458,7 +532,10 @@ private CompletableFuture retryRequest() { } private CompletableFuture responseAsyncImpl(final boolean applyReqFilters) { - if (currentreq.timeout().isPresent()) { + if (currentreq.timeout().isPresent() + // Avoid introducing a timer if an active one already exists. + // Consider retried and/or forwarded requests. + && responseTimerEvent == null) { responseTimerEvent = ResponseTimerEvent.of(this); client.registerTimer(responseTimerEvent); } @@ -521,10 +598,7 @@ private CompletableFuture responseAsyncImpl(final boolean applyReqFilt // cancel the timer if there are no exceptions, since the // response body might still get consumed, and it is // still subject to the response timer. - if (responseTimerEvent != null) { - client.cancelTimer(responseTimerEvent); - responseTimerEvent = null; - } + cancelTimer(); // all exceptions thrown are handled here final RetryContext retryCtx = checkRetryEligible(ex, exch); From 2dce1b9cb3bfbbbc3b1d6184c08074973ad95372 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Thu, 2 Oct 2025 10:04:14 +0200 Subject: [PATCH 11/13] Improve `assertInstanceOf` statements --- .../jdk/java/net/httpclient/TimeoutResponseBodyTest.java | 9 ++++++--- .../java/net/httpclient/TimeoutResponseHeaderTest.java | 5 +++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java b/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java index f4dcdfa71abd7..9d2ca892fe268 100644 --- a/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java +++ b/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java @@ -35,7 +35,6 @@ import java.util.concurrent.CompletableFuture; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; import static org.junit.jupiter.api.Assertions.fail; @@ -157,7 +156,9 @@ private static void verifyResponseBodyDoesNotArrive(HttpResponse re fail("Unexpected read byte: " + readByte); } }); - assertInstanceOf(HttpTimeoutException.class, exception.getCause()); + if (!(exception.getCause() instanceof HttpTimeoutException)) { + throw new AssertionError("was expecting a cause of type `HttpTimeoutException`", exception); + } } /** @@ -229,7 +230,9 @@ private static void verifyResponseBodyArrivesSlow(HttpResponse resp fail("Should not have reached here! (i=%s)".formatted(i)); } }); - assertInstanceOf(HttpTimeoutException.class, exception.getCause()); + if (!(exception.getCause() instanceof HttpTimeoutException)) { + throw new AssertionError("was expecting a cause of type `HttpTimeoutException`", exception); + } } } diff --git a/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java b/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java index 27e84420cb263..a0607284bba1d 100644 --- a/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java +++ b/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java @@ -33,7 +33,6 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; @@ -124,7 +123,9 @@ void testSendAsync(ServerRequestPair pair) throws Exception { LOGGER.log("Obtaining the response"); responseFuture.get(); }); - assertInstanceOf(HttpTimeoutException.class, exception.getCause()); + if (!(exception.getCause() instanceof HttpTimeoutException)) { + throw new AssertionError("was expecting a cause of type `HttpTimeoutException`", exception); + } }); } } From 5355782b388ec813c606a45aa0c895207ad6bab6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Thu, 2 Oct 2025 21:33:29 +0200 Subject: [PATCH 12/13] Reset timer on retried/forwarded requests --- .../share/classes/jdk/internal/net/http/MultiExchange.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java b/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java index 13a6e9c5b5fbc..ebda1d30cb38a 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java @@ -532,10 +532,9 @@ private CompletableFuture retryRequest() { } private CompletableFuture responseAsyncImpl(final boolean applyReqFilters) { - if (currentreq.timeout().isPresent() - // Avoid introducing a timer if an active one already exists. - // Consider retried and/or forwarded requests. - && responseTimerEvent == null) { + if (currentreq.timeout().isPresent()) { + // Retried/Forwarded requests should reset the timer, if present + cancelTimer(); responseTimerEvent = ResponseTimerEvent.of(this); client.registerTimer(responseTimerEvent); } From 14e9b87ed9f1429fbcd6920071d716a99170d1c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Fri, 3 Oct 2025 15:36:23 +0200 Subject: [PATCH 13/13] Check the entire causal chain to verify `HttpTimeoutException` --- .../httpclient/TimeoutResponseBodyTest.java | 53 +++++++------------ .../httpclient/TimeoutResponseHeaderTest.java | 14 ++--- .../TimeoutResponseTestSupport.java | 19 +++++++ 3 files changed, 43 insertions(+), 43 deletions(-) diff --git a/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java b/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java index 9d2ca892fe268..91783b613a855 100644 --- a/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java +++ b/test/jdk/java/net/httpclient/TimeoutResponseBodyTest.java @@ -26,16 +26,13 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; -import java.io.IOException; import java.io.InputStream; import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; -import java.net.http.HttpTimeoutException; import java.util.concurrent.CompletableFuture; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; import static org.junit.jupiter.api.Assertions.fail; @@ -148,17 +145,12 @@ void testSendAsyncOnMissingBody(ServerRequestPair pair) throws Exception { private static void verifyResponseBodyDoesNotArrive(HttpResponse response) { assertEquals(200, response.statusCode()); - IOException exception = assertThrows( - IOException.class, - () -> { - try (InputStream responseBodyStream = response.body()) { - int readByte = responseBodyStream.read(); - fail("Unexpected read byte: " + readByte); - } - }); - if (!(exception.getCause() instanceof HttpTimeoutException)) { - throw new AssertionError("was expecting a cause of type `HttpTimeoutException`", exception); - } + assertThrowsHttpTimeoutException(() -> { + try (InputStream responseBodyStream = response.body()) { + int readByte = responseBodyStream.read(); + fail("Unexpected read byte: " + readByte); + } + }); } /** @@ -213,26 +205,21 @@ void testSendAsyncOnSlowBody(ServerRequestPair pair) throws Exception { private static void verifyResponseBodyArrivesSlow(HttpResponse response) { assertEquals(200, response.statusCode()); - IOException exception = assertThrows( - IOException.class, - () -> { - try (InputStream responseBodyStream = response.body()) { - int i = 0; - int l = ServerRequestPair.CONTENT_LENGTH; - for (; i < l; i++) { - LOGGER.log("Reading byte %s/%s", i, l); - int readByte = responseBodyStream.read(); - if (readByte < 0) { - break; - } - assertEquals(i, readByte); - } - fail("Should not have reached here! (i=%s)".formatted(i)); + assertThrowsHttpTimeoutException(() -> { + try (InputStream responseBodyStream = response.body()) { + int i = 0; + int l = ServerRequestPair.CONTENT_LENGTH; + for (; i < l; i++) { + LOGGER.log("Reading byte %s/%s", i, l); + int readByte = responseBodyStream.read(); + if (readByte < 0) { + break; } - }); - if (!(exception.getCause() instanceof HttpTimeoutException)) { - throw new AssertionError("was expecting a cause of type `HttpTimeoutException`", exception); - } + assertEquals(i, readByte); + } + fail("Should not have reached here! (i=%s)".formatted(i)); + } + }); } } diff --git a/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java b/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java index a0607284bba1d..b6db5a492a51c 100644 --- a/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java +++ b/test/jdk/java/net/httpclient/TimeoutResponseHeaderTest.java @@ -29,11 +29,8 @@ import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; -import java.net.http.HttpTimeoutException; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; /* @@ -97,9 +94,9 @@ class TimeoutResponseHeaderTest extends TimeoutResponseTestSupport { @MethodSource("serverRequestPairs") void testSend(ServerRequestPair pair) throws Exception { try (HttpClient client = pair.createClientWithEstablishedConnection()) { - assertTimeoutPreemptively(REQUEST_TIMEOUT.multipliedBy(2), () -> assertThrows( - HttpTimeoutException.class, - () -> { + assertTimeoutPreemptively( + REQUEST_TIMEOUT.multipliedBy(2), + () -> assertThrowsHttpTimeoutException(() -> { LOGGER.log("Sending the request"); client.send(pair.request(), HttpResponse.BodyHandlers.discarding()); })); @@ -119,13 +116,10 @@ void testSendAsync(ServerRequestPair pair) throws Exception { LOGGER.log("Sending the request asynchronously"); CompletableFuture> responseFuture = client.sendAsync(pair.request(), HttpResponse.BodyHandlers.discarding()); - Exception exception = assertThrows(ExecutionException.class, () -> { + assertThrowsHttpTimeoutException(() -> { LOGGER.log("Obtaining the response"); responseFuture.get(); }); - if (!(exception.getCause() instanceof HttpTimeoutException)) { - throw new AssertionError("was expecting a cause of type `HttpTimeoutException`", exception); - } }); } } diff --git a/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java b/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java index e15f303ae7202..de6b83fbe0d09 100644 --- a/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java +++ b/test/jdk/java/net/httpclient/TimeoutResponseTestSupport.java @@ -30,6 +30,7 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.function.Executable; import javax.net.ssl.SSLContext; import java.io.IOException; @@ -41,6 +42,7 @@ import java.net.http.HttpOption; import java.net.http.HttpRequest; import java.net.http.HttpResponse; +import java.net.http.HttpTimeoutException; import java.time.Duration; import java.util.Map; import java.util.concurrent.CountDownLatch; @@ -52,6 +54,7 @@ import static java.net.http.HttpClient.Builder.NO_PROXY; import static java.net.http.HttpOption.Http3DiscoveryMode.HTTP_3_URI_ONLY; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -391,4 +394,20 @@ protected static Stream serverRequestPairs() { return Stream.of(HTTP1, HTTPS1, HTTP2, HTTPS2, HTTP3); } + protected static void assertThrowsHttpTimeoutException(Executable executable) { + Exception rootException = assertThrows(Exception.class, executable); + // Due to intricacies involved in the way exceptions are generated and + // nested, there is no bullet-proof way to determine at which level of + // the causal chain an `HttpTimeoutException` will show up. Hence, we + // scan through the entire causal chain. + Throwable exception = rootException; + while (exception != null) { + if (exception instanceof HttpTimeoutException) { + return; + } + exception = exception.getCause(); + } + throw new AssertionError("was expecting an `HttpTimeoutException` in the causal chain", rootException); + } + }