From fc910b7849d7a18c910a4b529c9b059754a85004 Mon Sep 17 00:00:00 2001 From: Pavel Ptashyts <49400901+pavel-ptashyts@users.noreply.github.com> Date: Mon, 17 Aug 2026 21:37:12 +0200 Subject: [PATCH] Bound an exchange by one request timeout TimeoutsHolder anchors the request deadline on its own construction, and a redirect, a retry and an auth replay each build a new one for the same future. Every hop therefore starts the budget again: with maxRedirects=5 a chain can legitimately run for six times the configured requestTimeout. The javadoc claims requestTimeout is the maximum time until the response is completed, which is not what happens. Add isUseAbsoluteRequestDeadline(), off by default, which anchors the deadline on when the exchange was submitted instead, so a later hop gets whatever is left of the budget rather than a fresh one. Off by default because turning it on shortens exchanges that rely on the per-attempt behaviour; the getRequestTimeout() javadoc now describes what actually happens and points at the flag either way. Settable per request as well as per client, following the followRedirect pattern: a nullable Boolean on Request that overrides the config value. Resolved once, in newNettyResponseFuture, and kept on the NettyResponseFuture rather than read from the request per hop. The first attempt put it on Request alone, and the two override tests failed in opposite directions because Redirect30xInterceptor rebuilds the request for the next hop from a hand-picked set of fields: the override was dropped mid-exchange and the config value took over. Anything carried only on the request has that problem, so the flag lives on the exchange, which is also what it describes. A redirect target cannot change it, which is right - the budget belongs to the caller. DefaultRequest keeps its existing public constructor, delegating to a new one that takes the flag as a trailing argument. Inserting the parameter beside followRedirect instead was a binary-incompatible change to a public constructor, which revapi correctly rejected. Claude Code on behalf of Pavel Ptashyts Co-Authored-By: Claude Opus 5 --- .../AsyncHttpClientConfig.java | 24 +++ .../DefaultAsyncHttpClientConfig.java | 23 +++ .../org/asynchttpclient/DefaultRequest.java | 46 +++++ .../java/org/asynchttpclient/Request.java | 11 ++ .../asynchttpclient/RequestBuilderBase.java | 17 +- .../config/AsyncHttpClientConfigDefaults.java | 5 + .../netty/NettyResponseFuture.java | 18 ++ .../netty/request/NettyRequestSender.java | 3 + .../netty/timeout/TimeoutsHolder.java | 12 +- .../org/asynchttpclient/util/HttpUtils.java | 5 + .../config/ahc-default.properties | 1 + .../AbsoluteRequestDeadlineTest.java | 176 ++++++++++++++++++ 12 files changed, 338 insertions(+), 3 deletions(-) create mode 100644 client/src/test/java/org/asynchttpclient/AbsoluteRequestDeadlineTest.java diff --git a/client/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java b/client/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java index 7304626083..05f9444ed2 100644 --- a/client/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java +++ b/client/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java @@ -109,11 +109,35 @@ public interface AsyncHttpClientConfig { /** * Return the maximum time an {@link AsyncHttpClient} waits until the response is completed. + *

+ * By default this bounds each attempt within an exchange rather than the exchange as a whole: a redirect, a + * retry and an auth replay each start it again, so a chain of n hops may run for n times this value. Set + * {@link #isUseAbsoluteRequestDeadline()} to bound the exchange instead. * * @return the maximum time an {@link AsyncHttpClient} waits until the response is completed. */ Duration getRequestTimeout(); + /** + * Whether {@link #getRequestTimeout()} is a deadline for the whole exchange rather than for each attempt + * within it. + *

+ * A redirect, a retry and an auth replay all continue the same exchange on the same response future, but + * each builds its own timeout state. Anchoring the deadline on that state gives every hop a fresh budget, + * which is why a five-redirect chain can legitimately take six times the configured timeout today. Enabling + * this anchors it on when the exchange was submitted instead, so a later hop gets whatever is left and the + * caller's total wait is bounded by the one value. + *

+ * Off by default because turning it on shortens exchanges that rely on the per-attempt behaviour. A caller + * working to an end-to-end budget wants it on; {@link Request#getUseAbsoluteRequestDeadline()} sets it for a + * single request. + * + * @return {@code true} to treat the request timeout as a deadline for the whole exchange + */ + default boolean isUseAbsoluteRequestDeadline() { + return false; + } + /** * Is HTTP redirect enabled * diff --git a/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClientConfig.java b/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClientConfig.java index 75aa1bd16a..f2c44b1cf1 100644 --- a/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClientConfig.java +++ b/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClientConfig.java @@ -63,6 +63,7 @@ import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultEnabledProtocols; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultExpiredCookieEvictionDelay; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultFailedIpCooldownEnabled; +import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultUseAbsoluteRequestDeadline; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultFailedIpCooldownPeriod; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultFilterInsecureCipherSuites; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultFollowRedirect; @@ -138,6 +139,7 @@ public class DefaultAsyncHttpClientConfig implements AsyncHttpClientConfig { private final int maxRequestRetry; private final LoadBalance loadBalance; private final boolean failedIpCooldownEnabled; + private final boolean useAbsoluteRequestDeadline; private final Duration failedIpCooldownPeriod; private final boolean disableUrlEncodingForBoundRequests; private final boolean useLaxCookieEncoder; @@ -243,6 +245,7 @@ private DefaultAsyncHttpClientConfig(// http int maxRequestRetry, LoadBalance loadBalance, boolean failedIpCooldownEnabled, + boolean useAbsoluteRequestDeadline, Duration failedIpCooldownPeriod, boolean disableUrlEncodingForBoundRequests, boolean useLaxCookieEncoder, @@ -348,6 +351,7 @@ private DefaultAsyncHttpClientConfig(// http this.maxRequestRetry = maxRequestRetry; this.loadBalance = loadBalance; this.failedIpCooldownEnabled = failedIpCooldownEnabled; + this.useAbsoluteRequestDeadline = useAbsoluteRequestDeadline; this.failedIpCooldownPeriod = failedIpCooldownPeriod; this.disableUrlEncodingForBoundRequests = disableUrlEncodingForBoundRequests; this.useLaxCookieEncoder = useLaxCookieEncoder; @@ -518,6 +522,11 @@ public boolean isFailedIpCooldownEnabled() { return failedIpCooldownEnabled; } + @Override + public boolean isUseAbsoluteRequestDeadline() { + return useAbsoluteRequestDeadline; + } + @Override public Duration getFailedIpCooldownPeriod() { return failedIpCooldownPeriod; @@ -937,6 +946,7 @@ public static class Builder { private int maxRequestRetry = defaultMaxRequestRetry(); private LoadBalance loadBalance = defaultLoadBalance(); private boolean failedIpCooldownEnabled = defaultFailedIpCooldownEnabled(); + private boolean useAbsoluteRequestDeadline = defaultUseAbsoluteRequestDeadline(); private Duration failedIpCooldownPeriod = defaultFailedIpCooldownPeriod(); private boolean disableUrlEncodingForBoundRequests = defaultDisableUrlEncodingForBoundRequests(); private boolean useLaxCookieEncoder = defaultUseLaxCookieEncoder(); @@ -1045,6 +1055,7 @@ public Builder(AsyncHttpClientConfig config) { maxRequestRetry = config.getMaxRequestRetry(); loadBalance = config.getLoadBalance(); failedIpCooldownEnabled = config.isFailedIpCooldownEnabled(); + useAbsoluteRequestDeadline = config.isUseAbsoluteRequestDeadline(); failedIpCooldownPeriod = config.getFailedIpCooldownPeriod(); disableUrlEncodingForBoundRequests = config.isDisableUrlEncodingForBoundRequests(); useLaxCookieEncoder = config.isUseLaxCookieEncoder(); @@ -1244,6 +1255,17 @@ public Builder setFailedIpCooldownEnabled(boolean failedIpCooldownEnabled) { return this; } + /** + * @param useAbsoluteRequestDeadline whether the request timeout is a deadline for the whole exchange + * rather than for each attempt within it; see + * {@link AsyncHttpClientConfig#isUseAbsoluteRequestDeadline()} + * @return this + */ + public Builder setUseAbsoluteRequestDeadline(boolean useAbsoluteRequestDeadline) { + this.useAbsoluteRequestDeadline = useAbsoluteRequestDeadline; + return this; + } + /** * @param failedIpCooldownPeriod how long a failed IP is deprioritized before it is re-probed; * {@code null} resets to the default. Must not be negative; use @@ -1751,6 +1773,7 @@ public DefaultAsyncHttpClientConfig build() { maxRequestRetry, loadBalance, failedIpCooldownEnabled, + useAbsoluteRequestDeadline, failedIpCooldownPeriod, disableUrlEncodingForBoundRequests, useLaxCookieEncoder, diff --git a/client/src/main/java/org/asynchttpclient/DefaultRequest.java b/client/src/main/java/org/asynchttpclient/DefaultRequest.java index c8e44e338f..269ed9b546 100644 --- a/client/src/main/java/org/asynchttpclient/DefaultRequest.java +++ b/client/src/main/java/org/asynchttpclient/DefaultRequest.java @@ -63,6 +63,7 @@ public class DefaultRequest implements Request { private final @Nullable Realm realm; private final @Nullable File file; private final @Nullable Boolean followRedirect; + private final @Nullable Boolean useAbsoluteRequestDeadline; private final Duration requestTimeout; private final Duration readTimeout; private final long rangeOffset; @@ -99,6 +100,45 @@ public DefaultRequest(String method, @Nullable Charset charset, ChannelPoolPartitioning channelPoolPartitioning, NameResolver nameResolver) { + this(method, uri, address, localAddress, headers, cookies, byteData, compositeByteData, stringData, + byteBufferData, byteBufData, streamData, bodyGenerator, formParams, bodyParts, virtualHost, + proxyServer, realm, file, followRedirect, requestTimeout, readTimeout, rangeOffset, charset, + channelPoolPartitioning, nameResolver, null); + } + + /** + * @param useAbsoluteRequestDeadline whether {@code requestTimeout} bounds the whole exchange rather than + * each attempt within it, or null to defer to the client config. Trailing + * rather than beside {@code followRedirect} so the original signature + * stays intact for callers that build a request without the builder. + */ + public DefaultRequest(String method, + Uri uri, + @Nullable InetAddress address, + @Nullable InetAddress localAddress, + HttpHeaders headers, + List cookies, + byte @Nullable [] byteData, + @Nullable List compositeByteData, + @Nullable String stringData, + @Nullable ByteBuffer byteBufferData, + @Nullable ByteBuf byteBufData, + @Nullable InputStream streamData, + @Nullable BodyGenerator bodyGenerator, + List formParams, + List bodyParts, + @Nullable String virtualHost, + @Nullable ProxyServer proxyServer, + @Nullable Realm realm, + @Nullable File file, + @Nullable Boolean followRedirect, + @Nullable Duration requestTimeout, + @Nullable Duration readTimeout, + long rangeOffset, + @Nullable Charset charset, + ChannelPoolPartitioning channelPoolPartitioning, + NameResolver nameResolver, + @Nullable Boolean useAbsoluteRequestDeadline) { this.method = method; this.uri = uri; this.address = address; @@ -119,6 +159,7 @@ public DefaultRequest(String method, this.realm = realm; this.file = file; this.followRedirect = followRedirect; + this.useAbsoluteRequestDeadline = useAbsoluteRequestDeadline; this.requestTimeout = requestTimeout == null ? Duration.ZERO : requestTimeout; this.readTimeout = readTimeout == null ? Duration.ZERO : readTimeout; this.rangeOffset = rangeOffset; @@ -232,6 +273,11 @@ public List getBodyParts() { return followRedirect; } + @Override + public @Nullable Boolean getUseAbsoluteRequestDeadline() { + return useAbsoluteRequestDeadline; + } + @Override public Duration getRequestTimeout() { return requestTimeout; diff --git a/client/src/main/java/org/asynchttpclient/Request.java b/client/src/main/java/org/asynchttpclient/Request.java index 1d95016b36..1ec85fd62b 100644 --- a/client/src/main/java/org/asynchttpclient/Request.java +++ b/client/src/main/java/org/asynchttpclient/Request.java @@ -172,6 +172,17 @@ public interface Request { @Nullable Boolean getFollowRedirect(); + /** + * Whether {@link #getRequestTimeout()} is a deadline for the whole exchange rather than for each attempt + * within it. See {@link AsyncHttpClientConfig#isUseAbsoluteRequestDeadline()}. + * + * @return the override, or null to use the config value + */ + @Nullable + default Boolean getUseAbsoluteRequestDeadline() { + return null; + } + /** * @return the request timeout. Non zero values means "override config value". */ diff --git a/client/src/main/java/org/asynchttpclient/RequestBuilderBase.java b/client/src/main/java/org/asynchttpclient/RequestBuilderBase.java index b185cbdc90..4ed6904fbc 100644 --- a/client/src/main/java/org/asynchttpclient/RequestBuilderBase.java +++ b/client/src/main/java/org/asynchttpclient/RequestBuilderBase.java @@ -87,6 +87,7 @@ public abstract class RequestBuilderBase> { protected @Nullable Realm realm; protected @Nullable File file; protected @Nullable Boolean followRedirect; + protected @Nullable Boolean useAbsoluteRequestDeadline; protected @Nullable Duration requestTimeout; protected @Nullable Duration readTimeout; protected long rangeOffset; @@ -165,6 +166,7 @@ protected RequestBuilderBase(Request prototype, boolean disableUrlEncoding, bool realm = prototype.getRealm(); file = prototype.getFile(); followRedirect = prototype.getFollowRedirect(); + useAbsoluteRequestDeadline = prototype.getUseAbsoluteRequestDeadline(); requestTimeout = prototype.getRequestTimeout(); readTimeout = prototype.getReadTimeout(); rangeOffset = prototype.getRangeOffset(); @@ -598,6 +600,17 @@ public T setRealm(Realm realm) { return asDerivedType(); } + /** + * @param useAbsoluteRequestDeadline whether this request's timeout is a deadline for the whole exchange + * rather than for each attempt within it, overriding + * {@link AsyncHttpClientConfig#isUseAbsoluteRequestDeadline()} + * @return {@code this} + */ + public T setUseAbsoluteRequestDeadline(boolean useAbsoluteRequestDeadline) { + this.useAbsoluteRequestDeadline = useAbsoluteRequestDeadline; + return asDerivedType(); + } + public T setFollowRedirect(boolean followRedirect) { this.followRedirect = followRedirect; return asDerivedType(); @@ -685,6 +698,7 @@ private RequestBuilderBase executeSignatureCalculator() { rb.realm = realm; rb.file = file; rb.followRedirect = followRedirect; + rb.useAbsoluteRequestDeadline = useAbsoluteRequestDeadline; rb.requestTimeout = requestTimeout; rb.rangeOffset = rangeOffset; rb.charset = charset; @@ -755,6 +769,7 @@ public Request build() { rb.rangeOffset, rb.charset, rb.channelPoolPartitioning, - rb.nameResolver); + rb.nameResolver, + rb.useAbsoluteRequestDeadline); } } diff --git a/client/src/main/java/org/asynchttpclient/config/AsyncHttpClientConfigDefaults.java b/client/src/main/java/org/asynchttpclient/config/AsyncHttpClientConfigDefaults.java index a31fdf2855..71508600f6 100644 --- a/client/src/main/java/org/asynchttpclient/config/AsyncHttpClientConfigDefaults.java +++ b/client/src/main/java/org/asynchttpclient/config/AsyncHttpClientConfigDefaults.java @@ -62,6 +62,7 @@ public final class AsyncHttpClientConfigDefaults { public static final String MAX_REQUEST_RETRY_CONFIG = "maxRequestRetry"; public static final String LOAD_BALANCE_CONFIG = "loadBalance"; public static final String FAILED_IP_COOLDOWN_ENABLED_CONFIG = "failedIpCooldownEnabled"; + public static final String USE_ABSOLUTE_REQUEST_DEADLINE_CONFIG = "useAbsoluteRequestDeadline"; public static final String FAILED_IP_COOLDOWN_PERIOD_CONFIG = "failedIpCooldownPeriod"; public static final String DISABLE_URL_ENCODING_FOR_BOUND_REQUESTS_CONFIG = "disableUrlEncodingForBoundRequests"; public static final String USE_LAX_COOKIE_ENCODER_CONFIG = "useLaxCookieEncoder"; @@ -183,6 +184,10 @@ public static boolean defaultFailedIpCooldownEnabled() { return AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getBoolean(ASYNC_CLIENT_CONFIG_ROOT + FAILED_IP_COOLDOWN_ENABLED_CONFIG); } + public static boolean defaultUseAbsoluteRequestDeadline() { + return AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getBoolean(ASYNC_CLIENT_CONFIG_ROOT + USE_ABSOLUTE_REQUEST_DEADLINE_CONFIG); + } + public static Duration defaultFailedIpCooldownPeriod() { return AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getDuration(ASYNC_CLIENT_CONFIG_ROOT + FAILED_IP_COOLDOWN_PERIOD_CONFIG); } diff --git a/client/src/main/java/org/asynchttpclient/netty/NettyResponseFuture.java b/client/src/main/java/org/asynchttpclient/netty/NettyResponseFuture.java index 86d312617e..e1ac837eac 100755 --- a/client/src/main/java/org/asynchttpclient/netty/NettyResponseFuture.java +++ b/client/src/main/java/org/asynchttpclient/netty/NettyResponseFuture.java @@ -154,6 +154,9 @@ public final class NettyResponseFuture implements ListenableFuture { // future no longer takes, which is how a connection through a proxy comes to be offered as a direct one. // Volatile: the mutators run on the redirect and replay paths while reads happen on other threads. private volatile Object basePartitionKeyCache; + // Read when a TimeoutsHolder is built, which happens on the caller thread, an event loop or the timer + // thread depending on the path, so it is published rather than plain. + private volatile boolean useAbsoluteRequestDeadline; public NettyResponseFuture(Request originalRequest, AsyncHandler asyncHandler, @@ -726,6 +729,21 @@ public void acquirePartitionLockLazily(boolean nonBlocking) throws IOException { } } + /** + * Whether this exchange's request timeout is a deadline for the exchange as a whole. Resolved once, from the + * request the caller submitted and the client config, and then kept here rather than re-read per hop: a + * redirect rebuilds the request from a hand-picked set of fields, so anything carried only on the request + * would silently revert to the config value partway through the exchange, which is exactly the case this + * setting exists for. + */ + public boolean isUseAbsoluteRequestDeadline() { + return useAbsoluteRequestDeadline; + } + + public void setUseAbsoluteRequestDeadline(boolean useAbsoluteRequestDeadline) { + this.useAbsoluteRequestDeadline = useAbsoluteRequestDeadline; + } + public Realm getRealm() { return realm; } diff --git a/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java b/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java index af3610164d..e5ce08c63f 100755 --- a/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java +++ b/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java @@ -110,6 +110,7 @@ import static org.asynchttpclient.util.HttpUtils.GZIP_DEFLATE; import static org.asynchttpclient.util.HttpUtils.GZIP_DEFLATE_HPACK; import static org.asynchttpclient.util.HttpUtils.hostHeader; +import static org.asynchttpclient.util.HttpUtils.useAbsoluteRequestDeadline; import static org.asynchttpclient.util.MiscUtils.getCause; import static org.asynchttpclient.util.ProxyUtils.getProxyServer; @@ -629,6 +630,8 @@ private NettyResponseFuture newNettyResponseFuture(Request request, Async connectionSemaphore, proxyServer); + future.setUseAbsoluteRequestDeadline(useAbsoluteRequestDeadline(config, request)); + String expectHeader = request.getHeaders().get(EXPECT); if (HttpHeaderValues.CONTINUE.contentEqualsIgnoreCase(expectHeader)) { future.setDontWriteBodyBecauseExpectContinue(true); diff --git a/client/src/main/java/org/asynchttpclient/netty/timeout/TimeoutsHolder.java b/client/src/main/java/org/asynchttpclient/netty/timeout/TimeoutsHolder.java index 93f6b26a26..7b4efc1fc8 100755 --- a/client/src/main/java/org/asynchttpclient/netty/timeout/TimeoutsHolder.java +++ b/client/src/main/java/org/asynchttpclient/netty/timeout/TimeoutsHolder.java @@ -59,8 +59,16 @@ public TimeoutsHolder(Timer nettyTimer, NettyResponseFuture nettyResponseFutu } if (requestTimeoutInMs > -1) { - requestTimeoutMillisTime = unpreciseMillisTime() + requestTimeoutInMs; - requestTimeout = newTimeout(new RequestTimeoutTimerTask(nettyResponseFuture, requestSender, this, requestTimeoutInMs), requestTimeoutInMs); + // A redirect, a retry or an auth replay builds a new holder for the same future. Anchoring the + // deadline here hands each of those hops a fresh budget, so a chain of n hops runs for n times the + // configured timeout; anchoring it on the future bounds the exchange as a whole instead. Which one + // applies is the caller's choice, per request or per client. + requestTimeoutMillisTime = (nettyResponseFuture.isUseAbsoluteRequestDeadline() + ? nettyResponseFuture.getStart() : unpreciseMillisTime()) + requestTimeoutInMs; + // A deadline already behind us is scheduled at zero rather than negative, so the task still runs and + // still cancels its read-timeout sibling, which is bookkeeping only it does. + requestTimeout = newTimeout(new RequestTimeoutTimerTask(nettyResponseFuture, requestSender, this, requestTimeoutInMs), + Math.max(requestTimeoutMillisTime - unpreciseMillisTime(), 0L)); } else { requestTimeoutMillisTime = -1L; requestTimeout = null; diff --git a/client/src/main/java/org/asynchttpclient/util/HttpUtils.java b/client/src/main/java/org/asynchttpclient/util/HttpUtils.java index 4e8d802575..2b970595be 100644 --- a/client/src/main/java/org/asynchttpclient/util/HttpUtils.java +++ b/client/src/main/java/org/asynchttpclient/util/HttpUtils.java @@ -142,6 +142,11 @@ public static boolean followRedirect(AsyncHttpClientConfig config, Request reque return request.getFollowRedirect() != null ? request.getFollowRedirect() : config.isFollowRedirect(); } + public static boolean useAbsoluteRequestDeadline(AsyncHttpClientConfig config, Request request) { + Boolean override = request.getUseAbsoluteRequestDeadline(); + return override != null ? override : config.isUseAbsoluteRequestDeadline(); + } + public static ByteBuffer urlEncodeFormParams(List params, Charset charset) { return StringUtils.charSequence2ByteBuffer(urlEncodeFormParams0(params, charset), US_ASCII); } diff --git a/client/src/main/resources/org/asynchttpclient/config/ahc-default.properties b/client/src/main/resources/org/asynchttpclient/config/ahc-default.properties index 6bf4e0f7b2..b7bf74a5b1 100644 --- a/client/src/main/resources/org/asynchttpclient/config/ahc-default.properties +++ b/client/src/main/resources/org/asynchttpclient/config/ahc-default.properties @@ -26,6 +26,7 @@ org.asynchttpclient.keepAlive=true org.asynchttpclient.maxRequestRetry=5 org.asynchttpclient.loadBalance=DEFAULT org.asynchttpclient.failedIpCooldownEnabled=true +org.asynchttpclient.useAbsoluteRequestDeadline=false org.asynchttpclient.failedIpCooldownPeriod=PT10S org.asynchttpclient.disableUrlEncodingForBoundRequests=false org.asynchttpclient.useLaxCookieEncoder=false diff --git a/client/src/test/java/org/asynchttpclient/AbsoluteRequestDeadlineTest.java b/client/src/test/java/org/asynchttpclient/AbsoluteRequestDeadlineTest.java new file mode 100644 index 0000000000..89eeb47609 --- /dev/null +++ b/client/src/test/java/org/asynchttpclient/AbsoluteRequestDeadlineTest.java @@ -0,0 +1,176 @@ +/* + * Copyright (c) 2026 AsyncHttpClient Project. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.asynchttpclient; + +import io.github.artsok.RepeatedIfExceptionsTest; +import io.netty.handler.codec.http.HttpHeaderNames; +import org.asynchttpclient.testserver.HttpServer; +import org.asynchttpclient.testserver.HttpTest; +import org.jetbrains.annotations.Nullable; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; + +import java.io.IOException; +import java.time.Duration; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicReference; + +import static org.asynchttpclient.Dsl.config; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * {@link AsyncHttpClientConfig#isUseAbsoluteRequestDeadline()} decides whether the request timeout bounds the + * whole exchange or each attempt within it. A redirect builds a fresh {@code TimeoutsHolder} for the same + * future, so with the deadline anchored on the holder each hop gets a budget of its own, and with it anchored + * on the future a later hop gets only what is left. + *

+ * Timing-based, so repeated: the margins are wide (a 600 ms budget against hops of 400 ms) but a loaded CI box + * can still miss one. + */ +public class AbsoluteRequestDeadlineTest extends HttpTest { + + private static final Duration BUDGET = Duration.ofMillis(600); + private static final long HOP_DELAY_MS = 400; + + private HttpServer server; + + @BeforeEach + public void start() throws Throwable { + server = new HttpServer(); + server.start(); + } + + @AfterEach + public void stop() throws Throwable { + server.close(); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void byDefaultEachHopGetsItsOwnBudget() throws Throwable { + // Two hops of 400 ms against a 600 ms budget. Each hop on its own fits, the pair does not, so with a + // per-attempt timeout the exchange completes. + enqueueTwoDelayedHops(); + + Throwable cause = runAndAwait(baseConfig(), null); + + assertNull(cause, "per-attempt timeouts should let both hops run, got " + cause); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void withAnAbsoluteDeadlineTheChainCannotOutrunTheBudget() throws Throwable { + enqueueTwoDelayedHops(); + + Throwable cause = runAndAwait(baseConfig().setUseAbsoluteRequestDeadline(true), null); + + assertNotNull(cause, "the exchange should have run out of budget across the two hops"); + assertEquals(TimeoutException.class, cause.getClass(), "expected a request timeout, got " + cause); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void aRequestCanAskForAnAbsoluteDeadlineOnAPerAttemptClient() throws Throwable { + enqueueTwoDelayedHops(); + + Throwable cause = runAndAwait(baseConfig(), Boolean.TRUE); + + assertNotNull(cause, "the request-level override should have bounded the exchange"); + assertEquals(TimeoutException.class, cause.getClass(), "expected a request timeout, got " + cause); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void aRequestCanOptOutOfAnAbsoluteDeadlineClient() throws Throwable { + enqueueTwoDelayedHops(); + + Throwable cause = runAndAwait(baseConfig().setUseAbsoluteRequestDeadline(true), Boolean.FALSE); + + assertNull(cause, "the request-level override should have restored per-attempt timeouts, got " + cause); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void aSingleHopStillGetsTheWholeBudget() throws Throwable { + // Guards the other direction: with a deadline, the first hop must not be handed a shortened budget. + enqueueDelayed(HOP_DELAY_MS, 200, null); + + Throwable cause = runAndAwait(baseConfig().setUseAbsoluteRequestDeadline(true), null); + + assertNull(cause, "a single hop well inside the budget should not time out, got " + cause); + } + + private DefaultAsyncHttpClientConfig.Builder baseConfig() { + return config().setRequestTimeout(BUDGET).setFollowRedirect(true).setMaxRedirects(5); + } + + private void enqueueTwoDelayedHops() { + enqueueDelayed(HOP_DELAY_MS, 302, "/foo/bar2"); + enqueueDelayed(HOP_DELAY_MS, 200, null); + } + + /** + * Answers after {@code delayMs}, so the hop consumes a known slice of the budget before the client sees a + * status at all. + */ + private void enqueueDelayed(long delayMs, int status, @Nullable String location) { + server.enqueueResponse(response -> { + try { + Thread.sleep(delayMs); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IOException(e); + } + response.setStatus(status); + if (location != null) { + response.setHeader(HttpHeaderNames.LOCATION.toString(), location); + } + }); + } + + /** + * @return the throwable the exchange was aborted with, or null when it completed + */ + private Throwable runAndAwait(DefaultAsyncHttpClientConfig.Builder builder, + @Nullable Boolean perRequestOverride) throws Throwable { + AtomicReference cause = new AtomicReference<>(); + CountDownLatch settled = new CountDownLatch(1); + + withClient(builder).run(client -> withServer(server).run(server -> { + BoundRequestBuilder request = client.prepareGet(server.getHttpUrl() + "/foo/bar"); + if (perRequestOverride != null) { + request.setUseAbsoluteRequestDeadline(perRequestOverride); + } + request.execute(new AsyncCompletionHandler() { + @Override + public Void onCompleted(Response response) { + settled.countDown(); + return null; + } + + @Override + public void onThrowable(Throwable t) { + cause.set(t); + settled.countDown(); + } + }); + + assertTrue(settled.await(30, TimeUnit.SECONDS), "the exchange neither completed nor failed"); + })); + + return cause.get(); + } +}