From dd4ae693183a7b2df2a35e1a5db982ccab17a7a1 Mon Sep 17 00:00:00 2001 From: Aayush Atharva Date: Sat, 19 Sep 2026 10:41:40 +0000 Subject: [PATCH 1/4] Add opt-in refusals for risky redirects --- .../AsyncHttpClientConfig.java | 42 ++ .../DefaultAsyncHttpClientConfig.java | 34 ++ .../org/asynchttpclient/DefaultRequest.java | 24 +- .../java/org/asynchttpclient/Request.java | 32 ++ .../asynchttpclient/RequestBuilderBase.java | 38 +- .../config/AsyncHttpClientConfigDefaults.java | 10 + .../handler/RedirectRefusedException.java | 79 +++ .../intercept/Redirect30xInterceptor.java | 137 ++++- .../config/ahc-default.properties | 2 + .../AsyncHttpClientDefaultsTest.java | 12 + .../org/asynchttpclient/BasicHttp2Test.java | 41 ++ .../RedirectRefusalConfigTest.java | 58 ++ .../asynchttpclient/RedirectRefusalTest.java | 528 ++++++++++++++++++ .../asynchttpclient/RequestBuilderTest.java | 36 ++ .../intercept/Redirect30xInterceptorTest.java | 67 +++ .../ws/WebSocketRedirectRefusalTest.java | 146 +++++ 16 files changed, 1273 insertions(+), 13 deletions(-) create mode 100644 client/src/main/java/org/asynchttpclient/handler/RedirectRefusedException.java create mode 100644 client/src/test/java/org/asynchttpclient/RedirectRefusalConfigTest.java create mode 100644 client/src/test/java/org/asynchttpclient/RedirectRefusalTest.java create mode 100644 client/src/test/java/org/asynchttpclient/ws/WebSocketRedirectRefusalTest.java diff --git a/client/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java b/client/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java index 78a8a086b..573539971 100644 --- a/client/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java +++ b/client/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java @@ -184,6 +184,48 @@ default boolean isUseAbsoluteRequestDeadline() { */ int getMaxRedirects(); + /** + * Whether to refuse a redirect that leaves an {@code https} or {@code wss} exchange for a target whose + * scheme is not secured, instead of following it. The redirected request is never sent and the exchange + * fails with a {@link org.asynchttpclient.handler.RedirectRefusedException}. + *

+ * Applies to every hop, bodies or not, and only once {@link #isFollowRedirect()} and + * {@link #getMaxRedirects()} have already let the hop through. It says nothing about an exchange that + * began in cleartext: there is no downgrade to refuse there. + *

+ * {@link org.asynchttpclient.Request#getRefuseSchemeDowngradeOnRedirect()} can refuse a hop this allows, + * but cannot permit one this refuses. + * + * @return true to refuse such a hop, false to follow it + */ + default boolean isRefuseSchemeDowngradeOnRedirect() { + return false; + } + + /** + * Whether to refuse a redirect that would resend this request's content to a different origin, instead of + * following it. It applies only to a hop that keeps the body and only when the request actually carries + * content, so an ordinary bodyless redirect across origins is untouched, as is a same-origin hop. A + * {@code GET} that does carry content is not untouched: it will be refused. + *

+ * Moving the same host onto TLS is exempt, so {@code http://host/x} to {@code https://host/x} is followed + * even though RFC 6454 counts it as a different origin. The exemption needs the host to match and the port + * to be unchanged or both at their scheme's default, so {@code :8080} to {@code :9999} is still refused. + * The hop is still not same-base, so credentials are stripped from it as they always were. + *

+ * Refusing rather than dropping the body is deliberate: a {@code PUT} that arrived empty would overwrite + * the target with nothing. Weigh that against what it blocks: an upload answered with a redirect to + * another host, which some storage services use to route a request to its home region, stops working. + *

+ * {@link org.asynchttpclient.Request#getRefuseCrossOriginBodyOnRedirect()} can refuse a hop this allows, + * but cannot permit one this refuses. + * + * @return true to refuse such a hop, false to follow it + */ + default boolean isRefuseCrossOriginBodyOnRedirect() { + return false; + } + /** * Is the {@link ChannelPool} support enabled. * diff --git a/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClientConfig.java b/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClientConfig.java index a35f5505a..8fc3426b7 100644 --- a/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClientConfig.java +++ b/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClientConfig.java @@ -95,6 +95,8 @@ import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultSoSndBuf; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultSslSessionCacheSize; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultSslSessionTimeout; +import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultRefuseCrossOriginBodyOnRedirect; +import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultRefuseSchemeDowngradeOnRedirect; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultStrict302Handling; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultTcpNoDelay; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultThreadPoolName; @@ -131,6 +133,8 @@ public class DefaultAsyncHttpClientConfig implements AsyncHttpClientConfig { private final boolean followRedirect; private final int maxRedirects; private final boolean strict302Handling; + private final boolean refuseSchemeDowngradeOnRedirect; + private final boolean refuseCrossOriginBodyOnRedirect; private final boolean compressionEnforced; private final boolean enableAutomaticDecompression; @@ -239,6 +243,8 @@ private DefaultAsyncHttpClientConfig(// http boolean followRedirect, int maxRedirects, boolean strict302Handling, + boolean refuseSchemeDowngradeOnRedirect, + boolean refuseCrossOriginBodyOnRedirect, boolean compressionEnforced, boolean enableAutomaticDecompression, long maxDecompressedResponseSize, @@ -346,6 +352,8 @@ private DefaultAsyncHttpClientConfig(// http this.followRedirect = followRedirect; this.maxRedirects = maxRedirects; this.strict302Handling = strict302Handling; + this.refuseSchemeDowngradeOnRedirect = refuseSchemeDowngradeOnRedirect; + this.refuseCrossOriginBodyOnRedirect = refuseCrossOriginBodyOnRedirect; this.compressionEnforced = compressionEnforced; this.enableAutomaticDecompression = enableAutomaticDecompression; this.maxDecompressedResponseSize = maxDecompressedResponseSize; @@ -499,6 +507,16 @@ public boolean isStrict302Handling() { return strict302Handling; } + @Override + public boolean isRefuseSchemeDowngradeOnRedirect() { + return refuseSchemeDowngradeOnRedirect; + } + + @Override + public boolean isRefuseCrossOriginBodyOnRedirect() { + return refuseCrossOriginBodyOnRedirect; + } + @Override public boolean isCompressionEnforced() { return compressionEnforced; @@ -960,6 +978,8 @@ public static class Builder { private boolean followRedirect = defaultFollowRedirect(); private int maxRedirects = defaultMaxRedirects(); private boolean strict302Handling = defaultStrict302Handling(); + private boolean refuseSchemeDowngradeOnRedirect = defaultRefuseSchemeDowngradeOnRedirect(); + private boolean refuseCrossOriginBodyOnRedirect = defaultRefuseCrossOriginBodyOnRedirect(); private boolean compressionEnforced = defaultCompressionEnforced(); private boolean enableAutomaticDecompression = defaultEnableAutomaticDecompression(); private long maxDecompressedResponseSize = defaultMaxDecompressedResponseSize(); @@ -1070,6 +1090,8 @@ public Builder(AsyncHttpClientConfig config) { followRedirect = config.isFollowRedirect(); maxRedirects = config.getMaxRedirects(); strict302Handling = config.isStrict302Handling(); + refuseSchemeDowngradeOnRedirect = config.isRefuseSchemeDowngradeOnRedirect(); + refuseCrossOriginBodyOnRedirect = config.isRefuseCrossOriginBodyOnRedirect(); compressionEnforced = config.isCompressionEnforced(); enableAutomaticDecompression = config.isEnableAutomaticDecompression(); maxDecompressedResponseSize = config.getMaxDecompressedResponseSize(); @@ -1193,6 +1215,16 @@ public Builder setStrict302Handling(final boolean strict302Handling) { return this; } + public Builder setRefuseSchemeDowngradeOnRedirect(final boolean refuseSchemeDowngradeOnRedirect) { + this.refuseSchemeDowngradeOnRedirect = refuseSchemeDowngradeOnRedirect; + return this; + } + + public Builder setRefuseCrossOriginBodyOnRedirect(final boolean refuseCrossOriginBodyOnRedirect) { + this.refuseCrossOriginBodyOnRedirect = refuseCrossOriginBodyOnRedirect; + return this; + } + /** * If true, AHC will add Accept-Encoding HTTP header to each request *

@@ -1800,6 +1832,8 @@ public DefaultAsyncHttpClientConfig build() { followRedirect, maxRedirects, strict302Handling, + refuseSchemeDowngradeOnRedirect, + refuseCrossOriginBodyOnRedirect, compressionEnforced, enableAutomaticDecompression, maxDecompressedResponseSize, diff --git a/client/src/main/java/org/asynchttpclient/DefaultRequest.java b/client/src/main/java/org/asynchttpclient/DefaultRequest.java index 3a4885601..7df31fb95 100644 --- a/client/src/main/java/org/asynchttpclient/DefaultRequest.java +++ b/client/src/main/java/org/asynchttpclient/DefaultRequest.java @@ -64,6 +64,8 @@ public class DefaultRequest implements Request { private final @Nullable File file; private final @Nullable Boolean followRedirect; private final @Nullable Boolean useAbsoluteRequestDeadline; + private final @Nullable Boolean refuseSchemeDowngradeOnRedirect; + private final @Nullable Boolean refuseCrossOriginBodyOnRedirect; private final Duration requestTimeout; private final Duration readTimeout; private final long rangeOffset; @@ -103,7 +105,7 @@ public DefaultRequest(String method, 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); + channelPoolPartitioning, nameResolver, null, null, null); } /** @@ -114,6 +116,10 @@ public DefaultRequest(String method, * * @param useAbsoluteRequestDeadline whether {@code requestTimeout} bounds the whole exchange rather than * each attempt within it, or null to defer to the client config + * @param refuseSchemeDowngradeOnRedirect whether to refuse a redirect to an unsecured scheme, or null to + * defer to the client config + * @param refuseCrossOriginBodyOnRedirect whether to refuse a redirect that resends the content to another + * origin, or null to defer to the client config */ DefaultRequest(String method, Uri uri, @@ -141,7 +147,9 @@ public DefaultRequest(String method, @Nullable Charset charset, ChannelPoolPartitioning channelPoolPartitioning, NameResolver nameResolver, - @Nullable Boolean useAbsoluteRequestDeadline) { + @Nullable Boolean useAbsoluteRequestDeadline, + @Nullable Boolean refuseSchemeDowngradeOnRedirect, + @Nullable Boolean refuseCrossOriginBodyOnRedirect) { this.method = method; this.uri = uri; this.address = address; @@ -163,6 +171,8 @@ public DefaultRequest(String method, this.file = file; this.followRedirect = followRedirect; this.useAbsoluteRequestDeadline = useAbsoluteRequestDeadline; + this.refuseSchemeDowngradeOnRedirect = refuseSchemeDowngradeOnRedirect; + this.refuseCrossOriginBodyOnRedirect = refuseCrossOriginBodyOnRedirect; this.requestTimeout = requestTimeout == null ? Duration.ZERO : requestTimeout; this.readTimeout = readTimeout == null ? Duration.ZERO : readTimeout; this.rangeOffset = rangeOffset; @@ -281,6 +291,16 @@ public List getBodyParts() { return useAbsoluteRequestDeadline; } + @Override + public @Nullable Boolean getRefuseSchemeDowngradeOnRedirect() { + return refuseSchemeDowngradeOnRedirect; + } + + @Override + public @Nullable Boolean getRefuseCrossOriginBodyOnRedirect() { + return refuseCrossOriginBodyOnRedirect; + } + @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 1ec85fd62..50569afab 100644 --- a/client/src/main/java/org/asynchttpclient/Request.java +++ b/client/src/main/java/org/asynchttpclient/Request.java @@ -183,6 +183,38 @@ default Boolean getUseAbsoluteRequestDeadline() { return null; } + /** + * Refuses a scheme downgrade on this request and the hops it leads to, even when + * {@link AsyncHttpClientConfig#isRefuseSchemeDowngradeOnRedirect()} is off. Tightening only, so + * {@code false} behaves as null and cannot re-enable a hop the client configuration refuses. + *

+ * Like any per-request setting, a filter that builds a fresh {@link Request} rather than deriving one from + * {@link #toBuilder()} replaces this along with everything else; the client-wide option is what holds a + * posture across replays. + * + * @return true to refuse, or null or false to use the config value + */ + @Nullable + default Boolean getRefuseSchemeDowngradeOnRedirect() { + return null; + } + + /** + * Refuses a cross-origin body replay on this request and the hops it leads to, even when + * {@link AsyncHttpClientConfig#isRefuseCrossOriginBodyOnRedirect()} is off. Tightening only, so + * {@code false} behaves as null and cannot re-enable a hop the client configuration refuses. + *

+ * Like any per-request setting, a filter that builds a fresh {@link Request} rather than deriving one from + * {@link #toBuilder()} replaces this along with everything else; the client-wide option is what holds a + * posture across replays. + * + * @return true to refuse, or null or false to use the config value + */ + @Nullable + default Boolean getRefuseCrossOriginBodyOnRedirect() { + 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 42a5b28b2..b6d32008e 100644 --- a/client/src/main/java/org/asynchttpclient/RequestBuilderBase.java +++ b/client/src/main/java/org/asynchttpclient/RequestBuilderBase.java @@ -88,6 +88,8 @@ public abstract class RequestBuilderBase> { protected @Nullable File file; protected @Nullable Boolean followRedirect; protected @Nullable Boolean useAbsoluteRequestDeadline; + protected @Nullable Boolean refuseSchemeDowngradeOnRedirect; + protected @Nullable Boolean refuseCrossOriginBodyOnRedirect; protected @Nullable Duration requestTimeout; protected @Nullable Duration readTimeout; protected long rangeOffset; @@ -167,6 +169,8 @@ protected RequestBuilderBase(Request prototype, boolean disableUrlEncoding, bool file = prototype.getFile(); followRedirect = prototype.getFollowRedirect(); useAbsoluteRequestDeadline = prototype.getUseAbsoluteRequestDeadline(); + refuseSchemeDowngradeOnRedirect = prototype.getRefuseSchemeDowngradeOnRedirect(); + refuseCrossOriginBodyOnRedirect = prototype.getRefuseCrossOriginBodyOnRedirect(); requestTimeout = prototype.getRequestTimeout(); readTimeout = prototype.getReadTimeout(); rangeOffset = prototype.getRangeOffset(); @@ -611,6 +615,34 @@ public T setUseAbsoluteRequestDeadline(boolean useAbsoluteRequestDeadline) { return asDerivedType(); } + /** + * Refuses a scheme downgrade on this request even when the client allows it. + * + * @param refuseSchemeDowngradeOnRedirect true to refuse a redirect off this request that leaves a secured + * scheme for one that is not; false leaves + * {@link AsyncHttpClientConfig#isRefuseSchemeDowngradeOnRedirect()} + * in charge and cannot relax it + * @return {@code this} + */ + public T setRefuseSchemeDowngradeOnRedirect(boolean refuseSchemeDowngradeOnRedirect) { + this.refuseSchemeDowngradeOnRedirect = refuseSchemeDowngradeOnRedirect; + return asDerivedType(); + } + + /** + * Refuses a cross-origin body replay on this request even when the client allows it. + * + * @param refuseCrossOriginBodyOnRedirect true to refuse a redirect off this request that would resend its + * content to another origin; false leaves + * {@link AsyncHttpClientConfig#isRefuseCrossOriginBodyOnRedirect()} + * in charge and cannot relax it + * @return {@code this} + */ + public T setRefuseCrossOriginBodyOnRedirect(boolean refuseCrossOriginBodyOnRedirect) { + this.refuseCrossOriginBodyOnRedirect = refuseCrossOriginBodyOnRedirect; + return asDerivedType(); + } + public T setFollowRedirect(boolean followRedirect) { this.followRedirect = followRedirect; return asDerivedType(); @@ -699,6 +731,8 @@ private RequestBuilderBase executeSignatureCalculator() { rb.file = file; rb.followRedirect = followRedirect; rb.useAbsoluteRequestDeadline = useAbsoluteRequestDeadline; + rb.refuseSchemeDowngradeOnRedirect = refuseSchemeDowngradeOnRedirect; + rb.refuseCrossOriginBodyOnRedirect = refuseCrossOriginBodyOnRedirect; rb.requestTimeout = requestTimeout; rb.readTimeout = readTimeout; rb.rangeOffset = rangeOffset; @@ -771,6 +805,8 @@ public Request build() { rb.charset, rb.channelPoolPartitioning, rb.nameResolver, - rb.useAbsoluteRequestDeadline); + rb.useAbsoluteRequestDeadline, + rb.refuseSchemeDowngradeOnRedirect, + rb.refuseCrossOriginBodyOnRedirect); } } diff --git a/client/src/main/java/org/asynchttpclient/config/AsyncHttpClientConfigDefaults.java b/client/src/main/java/org/asynchttpclient/config/AsyncHttpClientConfigDefaults.java index 25778dee9..7154d2480 100644 --- a/client/src/main/java/org/asynchttpclient/config/AsyncHttpClientConfigDefaults.java +++ b/client/src/main/java/org/asynchttpclient/config/AsyncHttpClientConfigDefaults.java @@ -62,6 +62,8 @@ public final class AsyncHttpClientConfigDefaults { public static final String KEEP_ALIVE_CONFIG = "keepAlive"; public static final String MAX_REQUEST_RETRY_CONFIG = "maxRequestRetry"; public static final String LOAD_BALANCE_CONFIG = "loadBalance"; + public static final String REFUSE_SCHEME_DOWNGRADE_ON_REDIRECT_CONFIG = "refuseSchemeDowngradeOnRedirect"; + public static final String REFUSE_CROSS_ORIGIN_BODY_ON_REDIRECT_CONFIG = "refuseCrossOriginBodyOnRedirect"; 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"; @@ -408,6 +410,14 @@ public static boolean defaultHttp2CleartextEnabled() { return AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getBoolean(ASYNC_CLIENT_CONFIG_ROOT + HTTP2_CLEARTEXT_ENABLED_CONFIG); } + public static boolean defaultRefuseSchemeDowngradeOnRedirect() { + return AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getBoolean(ASYNC_CLIENT_CONFIG_ROOT + REFUSE_SCHEME_DOWNGRADE_ON_REDIRECT_CONFIG); + } + + public static boolean defaultRefuseCrossOriginBodyOnRedirect() { + return AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getBoolean(ASYNC_CLIENT_CONFIG_ROOT + REFUSE_CROSS_ORIGIN_BODY_ON_REDIRECT_CONFIG); + } + public static LoadBalance defaultLoadBalance() { String value = AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getString(ASYNC_CLIENT_CONFIG_ROOT + LOAD_BALANCE_CONFIG); if (value == null || value.trim().isEmpty()) { diff --git a/client/src/main/java/org/asynchttpclient/handler/RedirectRefusedException.java b/client/src/main/java/org/asynchttpclient/handler/RedirectRefusedException.java new file mode 100644 index 000000000..4f61de0fd --- /dev/null +++ b/client/src/main/java/org/asynchttpclient/handler/RedirectRefusedException.java @@ -0,0 +1,79 @@ +/* + * 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.handler; + +import org.asynchttpclient.AsyncHttpClientConfig; +import org.asynchttpclient.uri.Uri; +import org.jetbrains.annotations.Nullable; + +/** + * Thrown when a redirect was refused rather than followed, by + * {@link AsyncHttpClientConfig#isRefuseSchemeDowngradeOnRedirect()} or + * {@link AsyncHttpClientConfig#isRefuseCrossOriginBodyOnRedirect()}. The redirected request was never sent. + * + *

When the origin answers the 3xx before the request content has finished writing, the failed write can + * surface first and the exchange fails with that {@link java.io.IOException} instead. The content still never + * reaches the new origin, but a caller branching on this type alone will miss that case. + * + *

Deliberately not an {@link java.io.IOException}: both handlers gate their {@code IOExceptionFilter} + * replay on that type, so a refusal that was one would come back as several more attempts to send the content + * the caller asked to send once. {@link MaxRedirectException} avoids it for the same reason. + * + *

Whether re-sending by hand is safe depends on {@link #getStatusCode()}. Every refusal except a 303, and + * except a 301 or non-strict 302 on a {@code POST}, preserved both the method and the content, so the same + * request can be aimed somewhere else unchanged. A 303 is defined as an indirect response to the original + * request, typically the output of a {@code POST} that has already run, so the origin has very likely applied + * it (RFC 9110 section 15.4.4). No status proves it did not, though, so a non-idempotent method still + * needs the caller's judgement (section 9.2.2). + */ +public class RedirectRefusedException extends Exception { + + private static final long serialVersionUID = 1L; + + private final int statusCode; + // Uri is not Serializable. getMessage() and getStatusCode() carry the same facts and do survive. + private final transient @Nullable Uri targetUri; + + /** + * @param message the refusal, naming both origins by base URL only + * @param statusCode the status code of the redirect response that was refused + * @param targetUri the redirect target that was refused + */ + public RedirectRefusedException(String message, int statusCode, Uri targetUri) { + // No stack trace: the frames would all be Netty's event loop, and the caller's ExecutionException + // already carries theirs. + super(message, null, true, false); + this.statusCode = statusCode; + this.targetUri = targetUri; + } + + /** + * @return the status code of the redirect response that was refused + */ + public int getStatusCode() { + return statusCode; + } + + /** + * The refused target. Its path, query and userinfo are server-chosen, so prefer {@link Uri#getBaseUrl()} + * when logging it, as {@link #getMessage()} already does. + * + * @return the refused redirect target, or null on an instance that was deserialized + */ + public @Nullable Uri getTargetUri() { + return targetUri; + } +} diff --git a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java index 24ff03407..e106d5be8 100644 --- a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java +++ b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java @@ -22,6 +22,7 @@ import io.netty.handler.codec.http.HttpUtil; import io.netty.handler.codec.http.cookie.Cookie; import io.netty.handler.codec.http2.Http2StreamChannel; +import io.netty.util.AsciiString; import org.asynchttpclient.AsyncHttpClientConfig; import org.asynchttpclient.Realm; import org.asynchttpclient.Realm.AuthScheme; @@ -29,6 +30,7 @@ import org.asynchttpclient.RequestBuilder; import org.asynchttpclient.cookie.CookieStore; import org.asynchttpclient.handler.MaxRedirectException; +import org.asynchttpclient.handler.RedirectRefusedException; import org.asynchttpclient.netty.NettyResponseFuture; import org.asynchttpclient.netty.channel.ChannelManager; import org.asynchttpclient.netty.channel.PrincipalScopedPartitionKey; @@ -54,6 +56,10 @@ import static io.netty.handler.codec.http.HttpHeaderNames.HOST; import static io.netty.handler.codec.http.HttpHeaderNames.LOCATION; import static io.netty.handler.codec.http.HttpHeaderNames.PROXY_AUTHORIZATION; +import static org.asynchttpclient.uri.Uri.HTTP; +import static org.asynchttpclient.uri.Uri.HTTPS; +import static org.asynchttpclient.uri.Uri.WS; +import static org.asynchttpclient.uri.Uri.WSS; import static org.asynchttpclient.util.HttpConstants.Methods.GET; import static org.asynchttpclient.util.HttpConstants.Methods.HEAD; import static org.asynchttpclient.util.HttpConstants.Methods.OPTIONS; @@ -94,12 +100,16 @@ static boolean isRedirect(int statusCode) { private final NettyRequestSender requestSender; private final MaxRedirectException maxRedirectException; private final boolean stripAuthorizationOnRedirect; + private final boolean refuseSchemeDowngradeOnRedirect; + private final boolean refuseCrossOriginBodyOnRedirect; Redirect30xInterceptor(ChannelManager channelManager, AsyncHttpClientConfig config, NettyRequestSender requestSender) { this.channelManager = channelManager; this.config = config; this.requestSender = requestSender; stripAuthorizationOnRedirect = config.isStripAuthorizationOnRedirect(); // New flag + refuseSchemeDowngradeOnRedirect = config.isRefuseSchemeDowngradeOnRedirect(); + refuseCrossOriginBodyOnRedirect = config.isRefuseCrossOriginBodyOnRedirect(); maxRedirectException = unknownStackTrace(new MaxRedirectException("Maximum redirect reached: " + config.getMaxRedirects()), Redirect30xInterceptor.class, "exitAfterHandlingRedirect"); } @@ -132,22 +142,45 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture HttpHeaders responseHeaders = response.headers(); String location = responseHeaders.get(LOCATION); - Uri newUri = Uri.create(future.getUri(), location); - LOGGER.debug("Redirecting to {}", newUri); + // The gates below must judge the same URI this hop resolves against, so both come from the + // request actually sent on this leg (RFC 9110 section 15.4 item 1). A 401 or 407 retry + // rebuilds that request without touching the future's own, so the two agree by each + // interceptor's care rather than by construction. + Uri currentUri = request.getUri(); + Uri newUri = Uri.create(currentUri, location); + + boolean sameBase = currentUri.isSameBase(newUri); + boolean schemeDowngrade = currentUri.isSecured() && !newUri.isSecured(); + + // Refuse here, before the next request is built and before ensureBodyReplayable can throw an + // IOException that an IOExceptionFilter would replay. + if (schemeDowngrade && refuseSchemeDowngrade(request)) { + throw refused(statusCode, currentUri, newUri, "the target scheme is not secured"); + } + BodyRepresentation bodyRepresentation = + keepBody ? selectedBodyRepresentation(request) : BodyRepresentation.NONE; + if (keepBody && refuseCrossOriginBody(request) + && !sameOrigin(currentUri, newUri) && !secureUpgrade(currentUri, newUri) + && bodyRepresentation != BodyRepresentation.NONE) { + throw refused(statusCode, currentUri, newUri, + "it would resend the request content to another origin"); + } + + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("Redirecting to {}", newUri.toUrlWithoutUserInfo()); + } - boolean sameBase = request.getUri().isSameBase(newUri); - boolean schemeDowngrade = request.getUri().isSecured() && !newUri.isSecured(); boolean stripAuth = !sameBase || schemeDowngrade || stripAuthorizationOnRedirect; - if (stripAuth && (request.getRealm() != null + if (LOGGER.isDebugEnabled() && stripAuth && (request.getRealm() != null || request.getHeaders().contains(AUTHORIZATION) || request.getHeaders().contains(COOKIE))) { - LOGGER.debug("Stripping credentials on redirect to {}", newUri); + LOGGER.debug("Stripping credentials on redirect to {}", newUri.toUrlWithoutUserInfo()); } final RequestBuilder requestBuilder; if (keepBody) { - ensureBodyReplayable(request, future.isStreamConsumed()); + ensureBodyReplayable(request, bodyRepresentation, future.isStreamConsumed()); requestBuilder = request.toBuilder(); if (!sameBase) { // An explicitly resolved address and virtual host belong to the previous target. @@ -179,6 +212,17 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture requestBuilder.setUseAbsoluteRequestDeadline(useAbsoluteRequestDeadline); } + // The gate reads these off the request each hop, and the !keepBody branch starts from an + // empty builder, so without this the caller's choice lapses from the second hop on. + Boolean refuseSchemeDowngrade = request.getRefuseSchemeDowngradeOnRedirect(); + if (refuseSchemeDowngrade != null) { + requestBuilder.setRefuseSchemeDowngradeOnRedirect(refuseSchemeDowngrade); + } + Boolean refuseCrossOriginBody = request.getRefuseCrossOriginBodyOnRedirect(); + if (refuseCrossOriginBody != null) { + requestBuilder.setRefuseCrossOriginBodyOnRedirect(refuseCrossOriginBody); + } + if (stripAuth) { future.setRealm(null); future.setProxyRealm(null); @@ -211,7 +255,9 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture final Request nextRequest = requestBuilder.setUri(newUri).build(); future.setTargetRequest(nextRequest); - LOGGER.debug("Sending redirect to {}", newUri); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("Sending redirect to {}", newUri.toUrlWithoutUserInfo()); + } if (channel instanceof Http2StreamChannel) { // HTTP/2 stream channels are single-use and close immediately after the response. @@ -240,8 +286,79 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture return false; } - private static void ensureBodyReplayable(Request request, boolean streamConsumed) throws IOException { - BodyRepresentation bodyRepresentation = selectedBodyRepresentation(request); + // Tightening only, unlike followRedirect: these are security switches, and a framework layer that builds + // the Request should not be able to void a posture the operator set on the client. + private boolean refuseSchemeDowngrade(Request request) { + return refuseSchemeDowngradeOnRedirect || Boolean.TRUE.equals(request.getRefuseSchemeDowngradeOnRedirect()); + } + + private boolean refuseCrossOriginBody(Request request) { + return refuseCrossOriginBodyOnRedirect || Boolean.TRUE.equals(request.getRefuseCrossOriginBodyOnRedirect()); + } + + private static RedirectRefusedException refused(int statusCode, Uri from, Uri to, String reason) { + // getBaseUrl() on both sides: toBaseUrl() keeps the path and toString() keeps userinfo, neither of + // which belongs in a message headed for a log. + return new RedirectRefusedException("Refusing to follow the " + statusCode + " redirect from " + + from.getBaseUrl() + " to " + to.getBaseUrl() + ": " + reason, statusCode, to); + } + + /** + * Same scheme, host and effective port, per RFC 6454 section 4. + *

+ * Not {@link Uri#isSameBase(Uri)}, which compares hosts with {@link String#equals}: a host differing only + * in case is still the same origin, and refusing there would break a legitimate upload. The fold is + * ASCII-only because that is the {@code i;ascii-casemap} collation section 4 step 5 asks for; + * {@link String#equalsIgnoreCase} folds Unicode and would call {@code i.example} equal to a host starting + * with {@code \\u0130}, which punycodes to a different host. Nothing here does IDNA + * either, so a Unicode host and its A-label spelling read as different origins, erring towards refusal. + * + * @param from the URI this hop is leaving + * @param to the redirect target + * @return whether the two are the same origin + */ + static boolean sameOrigin(Uri from, Uri to) { + return from.getScheme().equals(to.getScheme()) + && AsciiString.contentEqualsIgnoreCase(from.getHost(), to.getHost()) + && from.getExplicitPort() == to.getExplicitPort(); + } + + /** + * Whether this hop only swaps the same host onto TLS. This is a deliberate exemption from the origin + * rule, not something RFC 6454 allows: section 4 puts the scheme and port in the triple, so an upgrade is + * a different origin. It is our own policy, because refusing a move onto TLS would cost confidentiality + * rather than protect it. + *

+ * The host is compared, not trusted: a {@code Location} naming anything else stays cross-origin. The port + * rule is modelled on the upgrade mapping in RFC 6797 section 8.3, which is not binding here (that + * section is scoped to a Known HSTS Host, and we keep no HSTS state): the same port, or both at their + * scheme's default. So {@code :8080} to {@code :9999} is not an upgrade, while an explicit {@code :80} to + * {@code :80} is, which section 8.3 would have mapped to {@code :443}. That last case keeps the very + * endpoint the redirect was served from, so it reaches no party the request was not already aimed at. + *

+ * Only the content arm consults this. The hop is still not {@code isSameBase}, so {@code Authorization}, + * {@code Proxy-Authorization} and {@code Cookie} are stripped and the realm is dropped, as on any + * cross-base hop; cookies are then re-derived from the {@link org.asynchttpclient.cookie.CookieStore} + * against the new URI, which on an upgraded hop includes the host's {@code Secure} ones. + * + * @param from the URI this hop is leaving + * @param to the redirect target + * @return whether this is the same host moving to its secured scheme + */ + static boolean secureUpgrade(Uri from, Uri to) { + // Named pairs, not isSecured(), which would also admit http to wss. Schemes compare exactly because + // the Uri constructor lowercases them on every construction path. + boolean upgraded = HTTP.equals(from.getScheme()) && HTTPS.equals(to.getScheme()) + || WS.equals(from.getScheme()) && WSS.equals(to.getScheme()); + return upgraded + && AsciiString.contentEqualsIgnoreCase(from.getHost(), to.getHost()) + && (from.getExplicitPort() == to.getExplicitPort() + || from.getExplicitPort() == from.getSchemeDefaultPort() + && to.getExplicitPort() == to.getSchemeDefaultPort()); + } + + private static void ensureBodyReplayable(Request request, BodyRepresentation bodyRepresentation, + boolean streamConsumed) throws IOException { if (bodyRepresentation == BodyRepresentation.BODY_PARTS) { for (Part part : request.getBodyParts()) { if (part instanceof InputStreamPart) { 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 1179088ca..55d14bd2a 100644 --- a/client/src/main/resources/org/asynchttpclient/config/ahc-default.properties +++ b/client/src/main/resources/org/asynchttpclient/config/ahc-default.properties @@ -23,6 +23,8 @@ org.asynchttpclient.useProxyProperties=false org.asynchttpclient.validateResponseHeaders=true org.asynchttpclient.aggregateWebSocketFrameFragments=true org.asynchttpclient.strict302Handling=false +org.asynchttpclient.refuseSchemeDowngradeOnRedirect=false +org.asynchttpclient.refuseCrossOriginBodyOnRedirect=false org.asynchttpclient.keepAlive=true org.asynchttpclient.maxRequestRetry=5 org.asynchttpclient.loadBalance=DEFAULT diff --git a/client/src/test/java/org/asynchttpclient/AsyncHttpClientDefaultsTest.java b/client/src/test/java/org/asynchttpclient/AsyncHttpClientDefaultsTest.java index 39c7bab8f..34056a818 100644 --- a/client/src/test/java/org/asynchttpclient/AsyncHttpClientDefaultsTest.java +++ b/client/src/test/java/org/asynchttpclient/AsyncHttpClientDefaultsTest.java @@ -132,6 +132,18 @@ public void testDefaultStrict302Handling() { testBooleanSystemProperty("strict302Handling", "defaultStrict302Handling", "true"); } + @RepeatedIfExceptionsTest(repeats = 5) + public void testDefaultRefuseSchemeDowngradeOnRedirect() { + assertFalse(AsyncHttpClientConfigDefaults.defaultRefuseSchemeDowngradeOnRedirect()); + testBooleanSystemProperty("refuseSchemeDowngradeOnRedirect", "defaultRefuseSchemeDowngradeOnRedirect", "true"); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testDefaultRefuseCrossOriginBodyOnRedirect() { + assertFalse(AsyncHttpClientConfigDefaults.defaultRefuseCrossOriginBodyOnRedirect()); + testBooleanSystemProperty("refuseCrossOriginBodyOnRedirect", "defaultRefuseCrossOriginBodyOnRedirect", "true"); + } + @RepeatedIfExceptionsTest(repeats = 5) public void testDefaultAllowPoolingConnection() { assertTrue(AsyncHttpClientConfigDefaults.defaultKeepAlive()); diff --git a/client/src/test/java/org/asynchttpclient/BasicHttp2Test.java b/client/src/test/java/org/asynchttpclient/BasicHttp2Test.java index bcb8336f6..b4d35b2d4 100644 --- a/client/src/test/java/org/asynchttpclient/BasicHttp2Test.java +++ b/client/src/test/java/org/asynchttpclient/BasicHttp2Test.java @@ -50,6 +50,7 @@ import io.netty.pkitesting.CertificateBuilder; import io.netty.pkitesting.X509Bundle; import io.netty.util.concurrent.GlobalEventExecutor; +import org.asynchttpclient.handler.RedirectRefusedException; import org.asynchttpclient.proxy.ProxyServer; import org.asynchttpclient.proxy.ProxyType; import org.asynchttpclient.test.EventCollectingHandler; @@ -217,6 +218,18 @@ private void routeRequest(ChannelHandlerContext ctx, ByteBuf body) { } ctx.write(new DefaultHttp2HeadersFrame(responseHeaders, true)); ctx.flush(); + } else if (routePath.equals("/downgrade")) { + ReferenceCountUtil.safeRelease(body); + Http2Headers responseHeaders = new DefaultHttp2Headers().status("302"); + responseHeaders.add("location", "http://127.0.0.1:1/target"); + ctx.write(new DefaultHttp2HeadersFrame(responseHeaders, true)); + ctx.flush(); + } else if (routePath.equals("/cross-origin")) { + ReferenceCountUtil.safeRelease(body); + Http2Headers responseHeaders = new DefaultHttp2Headers().status("307"); + responseHeaders.add("location", "https://127.0.0.1:1/target"); + ctx.write(new DefaultHttp2HeadersFrame(responseHeaders, true)); + ctx.flush(); } else if (routePath.equals("/head")) { ReferenceCountUtil.safeRelease(body); Http2Headers responseHeaders = new DefaultHttp2Headers() @@ -1573,4 +1586,32 @@ public void http2SequentialRequestsWithMaxConnectionsPerHostOne() throws Excepti } } } + + /** + * The refusal has to terminate an HTTP/2 stream as cleanly as an HTTP/1.1 connection, and Http2Handler + * gates its own IOExceptionFilter replay on the exception type, so both arms are worth pinning here. + */ + @Test + public void schemeDowngradeRefusalAppliesOverHttp2() throws Exception { + try (AsyncHttpClient client = http2ClientWithConfig(builder -> builder + .setFollowRedirect(true) + .setRefuseSchemeDowngradeOnRedirect(true))) { + ExecutionException failure = assertThrows(ExecutionException.class, + () -> client.prepareGet(httpsUrl("/downgrade")).execute().get(10, TimeUnit.SECONDS)); + assertInstanceOf(RedirectRefusedException.class, failure.getCause()); + } + } + + @Test + public void crossOriginBodyRefusalAppliesOverHttp2() throws Exception { + try (AsyncHttpClient client = http2ClientWithConfig(builder -> builder + .setFollowRedirect(true) + .setRefuseCrossOriginBodyOnRedirect(true))) { + ExecutionException failure = assertThrows(ExecutionException.class, + () -> client.preparePut(httpsUrl("/cross-origin")).setBody("payload") + .execute().get(10, TimeUnit.SECONDS)); + assertInstanceOf(RedirectRefusedException.class, failure.getCause()); + } + } + } diff --git a/client/src/test/java/org/asynchttpclient/RedirectRefusalConfigTest.java b/client/src/test/java/org/asynchttpclient/RedirectRefusalConfigTest.java new file mode 100644 index 000000000..a18afded8 --- /dev/null +++ b/client/src/test/java/org/asynchttpclient/RedirectRefusalConfigTest.java @@ -0,0 +1,58 @@ +/* + * 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 org.junit.jupiter.api.Test; + +import static org.asynchttpclient.Dsl.config; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class RedirectRefusalConfigTest { + + @Test + public void bothArmsAreOffByDefault() { + AsyncHttpClientConfig built = config().build(); + assertFalse(built.isRefuseSchemeDowngradeOnRedirect()); + assertFalse(built.isRefuseCrossOriginBodyOnRedirect()); + } + + @Test + public void theBuilderSetsEachArmOnItsOwn() { + AsyncHttpClientConfig downgradeOnly = config().setRefuseSchemeDowngradeOnRedirect(true).build(); + assertTrue(downgradeOnly.isRefuseSchemeDowngradeOnRedirect()); + assertFalse(downgradeOnly.isRefuseCrossOriginBodyOnRedirect()); + + AsyncHttpClientConfig bodyOnly = config().setRefuseCrossOriginBodyOnRedirect(true).build(); + assertFalse(bodyOnly.isRefuseSchemeDowngradeOnRedirect()); + assertTrue(bodyOnly.isRefuseCrossOriginBodyOnRedirect()); + } + + /** + * The copy constructor is how a framework layer usually derives a client. + */ + @Test + public void theCopyConstructorKeepsBoth() { + AsyncHttpClientConfig original = config() + .setRefuseSchemeDowngradeOnRedirect(true) + .setRefuseCrossOriginBodyOnRedirect(true) + .build(); + + AsyncHttpClientConfig copy = new DefaultAsyncHttpClientConfig.Builder(original).build(); + assertTrue(copy.isRefuseSchemeDowngradeOnRedirect()); + assertTrue(copy.isRefuseCrossOriginBodyOnRedirect()); + } +} diff --git a/client/src/test/java/org/asynchttpclient/RedirectRefusalTest.java b/client/src/test/java/org/asynchttpclient/RedirectRefusalTest.java new file mode 100644 index 000000000..6d5de0077 --- /dev/null +++ b/client/src/test/java/org/asynchttpclient/RedirectRefusalTest.java @@ -0,0 +1,528 @@ +/* + * 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.netty.buffer.Unpooled; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.asynchttpclient.filter.FilterContext; +import org.asynchttpclient.filter.IOExceptionFilter; +import org.asynchttpclient.handler.MaxRedirectException; +import org.asynchttpclient.handler.RedirectRefusedException; +import org.asynchttpclient.request.body.generator.InputStreamBodyGenerator; +import org.asynchttpclient.request.body.multipart.StringPart; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.server.handler.AbstractHandler; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.FilterInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.function.Consumer; +import java.util.stream.Stream; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +import static java.util.Collections.singletonList; +import static org.asynchttpclient.Dsl.asyncHttpClient; +import static org.asynchttpclient.Dsl.config; +import static org.asynchttpclient.test.TestUtils.addHttpConnector; +import static org.asynchttpclient.test.TestUtils.addHttpsConnector; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * The redirect refusal gate in {@code Redirect30xInterceptor}. + *

+ * {@code setUpGlobal()} stays on {@link BeforeAll} so one server serves the class and the inherited + * {@code @AfterAll} stops it. + */ +public class RedirectRefusalTest extends AbstractBasicTest { + + private final AtomicBoolean targetHit = new AtomicBoolean(); + private final AtomicReference methodOnTarget = new AtomicReference<>(); + private final AtomicReference bodyOnTarget = new AtomicReference<>(); + + @Override + @BeforeAll + public void setUpGlobal() throws Exception { + server = new Server(); + ServerConnector plain = addHttpConnector(server); + ServerConnector secure = addHttpsConnector(server); + server.setHandler(new RedirectHandler()); + server.start(); + port1 = plain.getLocalPort(); + port2 = secure.getLocalPort(); + } + + @BeforeEach + public void resetCaptures() { + targetHit.set(false); + methodOnTarget.set(null); + bodyOnTarget.set(null); + } + + // ---------------------------------------------------------------- scheme downgrade + + @Test + public void downgradeIsFollowedByDefault() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(followingConfig())) { + Response response = client.prepareGet(secure("/downgrade")).execute().get(TIMEOUT, TimeUnit.SECONDS); + assertEquals(200, response.getStatusCode()); + assertTrue(targetHit.get()); + } + } + + @Test + public void downgradeOnBodylessGetIsRefused() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(followingConfig().setRefuseSchemeDowngradeOnRedirect(true))) { + RedirectRefusedException refusal = expectRefusal(client.prepareGet(secure("/downgrade"))); + assertEquals(307, refusal.getStatusCode()); + assertFalse(targetHit.get(), "the refused target must not be reached"); + } + } + + @Test + public void downgradeOnPutWithBodyIsRefused() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(followingConfig().setRefuseSchemeDowngradeOnRedirect(true))) { + expectRefusal(client.preparePut(secure("/downgrade")).setBody("payload")); + assertFalse(targetHit.get()); + } + } + + @Test + public void theDowngradeArmNeverRefusesAnUpgrade() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(followingConfig().setRefuseSchemeDowngradeOnRedirect(true))) { + Response response = client.preparePut(plain("/upgrade")).setBody("payload") + .execute().get(TIMEOUT, TimeUnit.SECONDS); + assertEquals(200, response.getStatusCode()); + assertEquals("payload", bodyOnTarget.get()); + } + } + + /** + * The exempted upgrade keeps its port, so the hop leaves the gate and the client goes on to attempt TLS + * against the plaintext connector, which fails. That failure is the point: it can only be reached by not + * refusing. Nothing here proves the content arrives, which would need a listener on the same port. + */ + @Test + public void anUpgradeThatKeepsItsPortIsNotRefused() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(followingConfig().setRefuseCrossOriginBodyOnRedirect(true))) { + ExecutionException failure = assertThrows(ExecutionException.class, + () -> client.preparePut(plain("/upgrade-same-port")).setBody("payload") + .execute().get(TIMEOUT, TimeUnit.SECONDS)); + assertInstanceOf(IOException.class, failure.getCause(), + "expected the TLS attempt against the plaintext connector to fail, got: " + failure.getCause()); + assertFalse(failure.getCause() instanceof RedirectRefusedException, + "the same-host upgrade should have been exempt, but was refused: " + failure.getCause()); + } + } + + /** + * The exemption wants the same port or both scheme defaults, and the two test connectors are on unrelated + * ephemeral ports, so this hop is a port change rather than an upgrade and stays refused. + */ + @Test + public void anUpgradeThatAlsoChangesThePortIsStillRefused() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(followingConfig().setRefuseCrossOriginBodyOnRedirect(true))) { + expectRefusal(client.preparePut(plain("/upgrade")).setBody("payload")); + assertFalse(targetHit.get()); + } + } + + @Test + public void refusalDeliversNoResponseToTheHandler() throws Exception { + AtomicReference completed = new AtomicReference<>(); + AtomicReference failed = new AtomicReference<>(); + try (AsyncHttpClient client = asyncHttpClient(followingConfig().setRefuseSchemeDowngradeOnRedirect(true))) { + assertThrows(ExecutionException.class, () -> client.prepareGet(secure("/downgrade")) + .execute(new AsyncCompletionHandler() { + @Override + public Response onCompleted(Response response) { + completed.set(response); + return response; + } + + @Override + public void onThrowable(Throwable t) { + failed.set(t); + } + }).get(TIMEOUT, TimeUnit.SECONDS)); + + assertNull(completed.get(), "a refused hop must not hand the 3xx back as the result"); + assertInstanceOf(RedirectRefusedException.class, failed.get()); + } + } + + // ---------------------------------------------------------------- cross-origin body + + @Test + public void crossOriginBodyReplayIsRefused() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(followingConfig().setRefuseCrossOriginBodyOnRedirect(true))) { + RedirectRefusedException refusal = expectRefusal(client.preparePut(plain("/cross-origin")).setBody("payload")); + assertEquals(307, refusal.getStatusCode()); + assertFalse(targetHit.get(), "the content must not reach the other origin"); + } + } + + /** + * The arm has to hold for every status that keeps method and content, not just the 307 the other tests + * use. 301 and 302 keep both here because the legacy rewrite is limited to POST. + */ + @ParameterizedTest + @ValueSource(ints = {301, 302, 308}) + public void crossOriginBodyReplayIsRefusedOnEveryBodyPreservingStatus(int statusCode) throws Exception { + try (AsyncHttpClient client = asyncHttpClient(followingConfig().setRefuseCrossOriginBodyOnRedirect(true))) { + RedirectRefusedException refusal = + expectRefusal(client.preparePut(plain("/cross-origin-" + statusCode)).setBody("payload")); + assertEquals(statusCode, refusal.getStatusCode()); + assertFalse(targetHit.get()); + } + } + + /** + * hasContent reads the request rather than the wire, so every representation the factory can write has to + * be recognised. A representation added there without a row here fails open. + */ + @ParameterizedTest(name = "{0}") + @MethodSource("bodyRepresentations") + public void crossOriginBodyReplayIsRefusedForEveryRepresentation(String name, Consumer body) + throws Exception { + try (AsyncHttpClient client = asyncHttpClient(followingConfig().setRefuseCrossOriginBodyOnRedirect(true))) { + BoundRequestBuilder request = client.preparePut(plain("/cross-origin")); + body.accept(request); + expectRefusal(request); + assertFalse(targetHit.get(), name + " reached the other origin"); + } + } + + /** + * The gate sits above ensureBodyReplayable, which would throw an IOException for a stream that cannot be + * replayed. Getting the refusal rather than that IOException is what proves the order, and the filter + * count proves the IOException path was never entered. + */ + @Test + public void aConsumedStreamIsRefusedRatherThanFailingReplayable() throws Exception { + AtomicInteger filterCalls = new AtomicInteger(); + DefaultAsyncHttpClientConfig.Builder builder = followingConfig() + .setRefuseCrossOriginBodyOnRedirect(true) + .addIOExceptionFilter(new IOExceptionFilter() { + @Override + public FilterContext filter(FilterContext ctx) { + filterCalls.incrementAndGet(); + return ctx; + } + }); + + try (AsyncHttpClient client = asyncHttpClient(builder)) { + expectRefusal(client.preparePut(plain("/cross-origin")).setBody(nonResettable("payload"))); + assertEquals(0, filterCalls.get()); + assertFalse(targetHit.get()); + } + } + + private static Stream bodyRepresentations() throws IOException { + byte[] payload = "payload".getBytes(StandardCharsets.UTF_8); + File file = File.createTempFile("ahc-redirect-refusal", ".txt"); + file.deleteOnExit(); + Files.write(file.toPath(), payload); + + return Stream.of( + Arguments.of("byteData", (Consumer) rb -> rb.setBody(payload)), + Arguments.of("stringData", (Consumer) rb -> rb.setBody("payload")), + Arguments.of("byteBufferData", (Consumer) rb -> rb.setBody(ByteBuffer.wrap(payload))), + Arguments.of("byteBufData", (Consumer) rb -> rb.setBody(Unpooled.wrappedBuffer(payload))), + Arguments.of("streamData", (Consumer) rb -> rb.setBody(new ByteArrayInputStream(payload))), + Arguments.of("file", (Consumer) rb -> rb.setBody(file)), + Arguments.of("formParams", (Consumer) rb -> rb.setFormParams(singletonList(new Param("k", "v")))), + Arguments.of("bodyParts", (Consumer) rb -> rb.addBodyPart(new StringPart("k", "v"))), + Arguments.of("bodyGenerator", (Consumer) rb -> + rb.setBody(new InputStreamBodyGenerator(new ByteArrayInputStream(payload))))); + } + + private static InputStream nonResettable(String content) { + return new FilterInputStream(new ByteArrayInputStream(content.getBytes(StandardCharsets.UTF_8))) { + @Override + public boolean markSupported() { + return false; + } + }; + } + + /** + * keepBody is true for every GET on a 301/302/307/308, so a gate that keyed on it alone would refuse most + * of the redirects on the web. + */ + @Test + public void crossOriginBodylessGetIsFollowed() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(followingConfig().setRefuseCrossOriginBodyOnRedirect(true))) { + Response response = client.prepareGet(plain("/cross-origin")).execute().get(TIMEOUT, TimeUnit.SECONDS); + assertEquals(200, response.getStatusCode()); + assertTrue(targetHit.get()); + } + } + + @Test + public void crossOriginPostRewrittenToGetIsFollowed() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(strictConfig())) { + Response response = client.preparePost(plain("/see-other")).setBody("payload") + .execute().get(TIMEOUT, TimeUnit.SECONDS); + assertEquals(200, response.getStatusCode()); + assertEquals("GET", methodOnTarget.get()); + assertEquals("", bodyOnTarget.get()); + } + } + + @Test + public void sameOriginBodyHopIsFollowed() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(strictConfig())) { + Response response = client.preparePut(plain("/same-origin")).setBody("payload") + .execute().get(TIMEOUT, TimeUnit.SECONDS); + assertEquals(200, response.getStatusCode()); + assertEquals("payload", bodyOnTarget.get()); + } + } + + @Test + public void theTwoArmsAreIndependent() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(followingConfig().setRefuseSchemeDowngradeOnRedirect(true))) { + Response response = client.preparePut(plain("/cross-origin")).setBody("payload") + .execute().get(TIMEOUT, TimeUnit.SECONDS); + assertEquals(200, response.getStatusCode(), "the downgrade arm must not refuse a cross-origin body"); + } + } + + // ---------------------------------------------------------------- how the refusal terminates + + /** + * Both handlers gate their {@code IOExceptionFilter} replay on {@link IOException}, so a refusal that was + * one would come back as further attempts to send the content. + */ + @Test + public void refusalIsNotAnIOExceptionAndIsNotReplayed() throws Exception { + AtomicInteger filterCalls = new AtomicInteger(); + DefaultAsyncHttpClientConfig.Builder builder = followingConfig() + .setRefuseSchemeDowngradeOnRedirect(true) + .addIOExceptionFilter(new IOExceptionFilter() { + @Override + public FilterContext filter(FilterContext ctx) { + filterCalls.incrementAndGet(); + return ctx; + } + }); + + try (AsyncHttpClient client = asyncHttpClient(builder)) { + Throwable cause = expectRefusal(client.prepareGet(secure("/downgrade"))); + assertFalse(cause instanceof IOException); + assertEquals(0, filterCalls.get(), "a refusal must not enter the IOExceptionFilter chain"); + } + } + + @Test + public void theRedirectBudgetIsCheckedFirst() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(followingConfig() + .setMaxRedirects(1) + .setRefuseSchemeDowngradeOnRedirect(true))) { + ExecutionException failure = assertThrows(ExecutionException.class, + () -> client.prepareGet(secure("/downgrade")).execute().get(TIMEOUT, TimeUnit.SECONDS)); + assertInstanceOf(MaxRedirectException.class, failure.getCause()); + } + } + + @Test + public void theMessageCarriesNeitherPathQueryNorUserInfo() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(followingConfig().setRefuseSchemeDowngradeOnRedirect(true))) { + RedirectRefusedException refusal = expectRefusal( + client.prepareGet("https://user:pw@localhost:" + port2 + "/downgrade?token=CALLER_SECRET")); + + String message = refusal.getMessage(); + assertFalse(message.contains("CALLER_SECRET"), message); + assertFalse(message.contains("SERVER_SECRET"), message); + assertFalse(message.contains("user:pw"), message); + assertTrue(message.contains("https://localhost:" + port2), message); + assertTrue(message.contains("http://127.0.0.1:" + port1), message); + } + } + + // ---------------------------------------------------------------- per-request override + + @Test + public void aRequestCanRefuseWhereTheClientAllows() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(followingConfig())) { + expectRefusal(client.prepareGet(secure("/downgrade")).setRefuseSchemeDowngradeOnRedirect(true)); + assertFalse(targetHit.get()); + } + } + + @Test + public void aRequestCannotAllowWhereTheClientRefuses() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(followingConfig().setRefuseSchemeDowngradeOnRedirect(true))) { + expectRefusal(client.prepareGet(secure("/downgrade")).setRefuseSchemeDowngradeOnRedirect(false)); + assertFalse(targetHit.get(), "a request must not be able to relax the client's posture"); + } + } + + @Test + public void anOverrideSurvivesAHopThatKeepsTheBody() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(followingConfig())) { + expectRefusal(client.preparePut(plain("/hop1")).setBody("payload") + .setRefuseCrossOriginBodyOnRedirect(true)); + assertFalse(targetHit.get(), "the override must still hold on the second hop"); + } + } + + /** + * A 303 drops the body and rebuilds from an empty builder rather than from the request, which is the only + * branch where the override has to be copied across by hand. The client leaves both arms off, so the + * refusal on the second hop can only come from the override surviving the rebuild. + */ + @Test + public void anOverrideSurvivesTheRebuildOnABodylessHop() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(followingConfig())) { + expectRefusal(client.preparePost(secure("/see-other-then-downgrade")).setBody("payload") + .setRefuseSchemeDowngradeOnRedirect(true)); + assertFalse(targetHit.get()); + } + } + + // ---------------------------------------------------------------- fixture + + private DefaultAsyncHttpClientConfig.Builder followingConfig() { + return config().setFollowRedirect(true).setUseInsecureTrustManager(true).setMaxRedirects(5); + } + + private DefaultAsyncHttpClientConfig.Builder strictConfig() { + return followingConfig().setRefuseSchemeDowngradeOnRedirect(true).setRefuseCrossOriginBodyOnRedirect(true); + } + + private String plain(String path) { + return "http://localhost:" + port1 + path; + } + + private String secure(String path) { + return "https://localhost:" + port2 + path; + } + + private static RedirectRefusedException expectRefusal(BoundRequestBuilder request) { + ExecutionException failure = assertThrows(ExecutionException.class, + () -> request.execute().get(TIMEOUT, TimeUnit.SECONDS)); + return assertInstanceOf(RedirectRefusedException.class, failure.getCause()); + } + + private class RedirectHandler extends AbstractHandler { + + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, + HttpServletResponse response) throws IOException, ServletException { + // Drain first: answering mid-write makes the write fail before the refusal surfaces, which makes + // the reported cause racy even though the refusal itself still holds. + String body = drain(request); + baseRequest.setHandled(true); + + // localhost and 127.0.0.1 are different origins to the gate but the same server here, which is how + // a cross-origin hop is staged without a second listener. + String path = stripPrefix(target); + switch (path) { + case "/downgrade": + redirect(response, 307, "http://127.0.0.1:" + port1 + "/target?token=SERVER_SECRET"); + return; + case "/upgrade": + redirect(response, 302, "https://localhost:" + port2 + "/target"); + return; + case "/upgrade-same-port": + redirect(response, 307, "https://localhost:" + port1 + "/target"); + return; + case "/cross-origin-301": + redirect(response, 301, "http://127.0.0.1:" + port1 + "/target"); + return; + case "/cross-origin-302": + redirect(response, 302, "http://127.0.0.1:" + port1 + "/target"); + return; + case "/cross-origin-308": + redirect(response, 308, "http://127.0.0.1:" + port1 + "/target"); + return; + case "/cross-origin": + redirect(response, 307, "http://127.0.0.1:" + port1 + "/target"); + return; + case "/same-origin": + redirect(response, 307, "http://localhost:" + port1 + "/target"); + return; + case "/see-other": + redirect(response, 303, "http://127.0.0.1:" + port1 + "/target"); + return; + case "/see-other-then-downgrade": + redirect(response, 303, "https://localhost:" + port2 + "/downgrade"); + return; + case "/hop1": + redirect(response, 307, "http://localhost:" + port1 + "/hop2"); + return; + case "/hop2": + redirect(response, 307, "http://127.0.0.1:" + port1 + "/target"); + return; + default: + targetHit.set(true); + methodOnTarget.set(request.getMethod()); + bodyOnTarget.set(body); + response.setStatus(200); + response.getOutputStream().write("target".getBytes(StandardCharsets.UTF_8)); + } + } + + private String stripPrefix(String target) { + int query = target.indexOf('?'); + return query < 0 ? target : target.substring(0, query); + } + + private void redirect(HttpServletResponse response, int status, String location) throws IOException { + response.setStatus(status); + response.setHeader("Location", location); + response.getOutputStream().write("moved".getBytes(StandardCharsets.UTF_8)); + } + + private String drain(HttpServletRequest request) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + byte[] buffer = new byte[1024]; + try (InputStream in = request.getInputStream()) { + for (int read = in.read(buffer); read != -1; read = in.read(buffer)) { + out.write(buffer, 0, read); + } + } + return new String(out.toByteArray(), StandardCharsets.UTF_8); + } + } +} diff --git a/client/src/test/java/org/asynchttpclient/RequestBuilderTest.java b/client/src/test/java/org/asynchttpclient/RequestBuilderTest.java index 92bd21e42..8a3206d08 100644 --- a/client/src/test/java/org/asynchttpclient/RequestBuilderTest.java +++ b/client/src/test/java/org/asynchttpclient/RequestBuilderTest.java @@ -35,6 +35,7 @@ import static org.asynchttpclient.Dsl.post; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; public class RequestBuilderTest { @@ -432,4 +433,39 @@ public void testRequestToStringRedactsSensitiveHeaders() { assertTrue(value.contains("Cookie: ")); assertTrue(value.contains("request-id")); } + + @Test + public void testRedirectRefusalOverridesSurviveToBuilder() { + Request original = new RequestBuilder("PUT") + .setUrl("https://example.com/upload") + .setRefuseSchemeDowngradeOnRedirect(true) + .setRefuseCrossOriginBodyOnRedirect(false) + .build(); + + // Deliberately one of each, so a swapped argument list cannot pass. + Request derived = original.toBuilder().build(); + assertEquals(Boolean.TRUE, derived.getRefuseSchemeDowngradeOnRedirect()); + assertEquals(Boolean.FALSE, derived.getRefuseCrossOriginBodyOnRedirect()); + } + + @Test + public void testRedirectRefusalOverridesSurviveASignatureCalculator() { + Request request = new RequestBuilder("PUT") + .setUrl("https://example.com/upload") + .setRefuseSchemeDowngradeOnRedirect(true) + .setRefuseCrossOriginBodyOnRedirect(false) + .setSignatureCalculator((toSign, builder) -> builder.addHeader("X-Signature", "computed")) + .build(); + + assertEquals("computed", request.getHeaders().get("X-Signature")); + assertEquals(Boolean.TRUE, request.getRefuseSchemeDowngradeOnRedirect()); + assertEquals(Boolean.FALSE, request.getRefuseCrossOriginBodyOnRedirect()); + } + + @Test + public void testRedirectRefusalOverridesDefaultToNull() { + Request request = new RequestBuilder("GET").setUrl("https://example.com/").build(); + assertNull(request.getRefuseSchemeDowngradeOnRedirect()); + assertNull(request.getRefuseCrossOriginBodyOnRedirect()); + } } diff --git a/client/src/test/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptorTest.java b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptorTest.java index 797cceea3..6169d7838 100644 --- a/client/src/test/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptorTest.java +++ b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptorTest.java @@ -15,6 +15,7 @@ */ package org.asynchttpclient.netty.handler.intercept; +import org.asynchttpclient.uri.Uri; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -27,6 +28,72 @@ */ public class Redirect30xInterceptorTest { + private static Uri host(String scheme, String host, int port) { + return new Uri(scheme, null, host, port, "/", null, null); + } + + @Test + public void sameOriginFoldsHostCaseButOnlyInAscii() { + assertTrue(Redirect30xInterceptor.sameOrigin(host("https", "example.com", 443), + host("https", "EXAMPLE.com", 443))); + + // The one case an integration test cannot reach, because Netty decodes header values byte per char: + // a non-ASCII host on the caller's own request URI. String.equalsIgnoreCase would call these equal, + // and \\u0130.example punycodes to xn--i-9bb.example, a different host. + assertFalse(Redirect30xInterceptor.sameOrigin(host("https", "i.example", 443), + host("https", "\u0130.example", 443))); + } + + @Test + public void sameOriginResolvesTheDefaultPort() { + assertTrue(Redirect30xInterceptor.sameOrigin(host("https", "example.com", -1), + host("https", "example.com", 443))); + assertTrue(Redirect30xInterceptor.sameOrigin(host("ws", "example.com", -1), + host("ws", "example.com", 80))); + } + + @Test + public void secureUpgradeCoversTheDefaultPortPair() { + assertTrue(Redirect30xInterceptor.secureUpgrade(host("http", "example.com", -1), + host("https", "example.com", -1))); + assertTrue(Redirect30xInterceptor.secureUpgrade(host("ws", "example.com", -1), + host("wss", "example.com", -1))); + // RFC 6797 section 8.3 preserves an explicit port rather than inventing one. + assertTrue(Redirect30xInterceptor.secureUpgrade(host("http", "example.com", 8080), + host("https", "example.com", 8080))); + // Both spellings of the default pair, since getExplicitPort resolves -1 through the scheme default. + assertTrue(Redirect30xInterceptor.secureUpgrade(host("http", "example.com", 80), + host("https", "example.com", -1))); + assertTrue(Redirect30xInterceptor.secureUpgrade(host("http", "example.com", -1), + host("https", "example.com", 443))); + // Deliberately looser than RFC 6797 section 8.3, which would map an explicit 80 to 443. Keeping the + // port means the hop lands on the endpoint the redirect was served from, so it reaches nobody new. + assertTrue(Redirect30xInterceptor.secureUpgrade(host("http", "example.com", 80), + host("https", "example.com", 80))); + assertTrue(Redirect30xInterceptor.secureUpgrade(host("http", "example.com", -1), + host("https", "example.com", 80))); + } + + @Test + public void secureUpgradeRefusesAnythingButTheSameHostGoingSecure() { + assertFalse(Redirect30xInterceptor.secureUpgrade(host("http", "example.com", 8080), + host("https", "example.com", 9999)), "a port change is not an upgrade"); + assertFalse(Redirect30xInterceptor.secureUpgrade(host("http", "example.com", -1), + host("https", "other.example", -1)), "the host has to match"); + assertFalse(Redirect30xInterceptor.secureUpgrade(host("https", "example.com", -1), + host("http", "example.com", -1)), "a downgrade is not an upgrade"); + assertFalse(Redirect30xInterceptor.secureUpgrade(host("http", "example.com", -1), + host("wss", "example.com", -1)), "only http to https and ws to wss count"); + } + + @Test + public void sameOriginSeparatesSchemeAndPort() { + assertFalse(Redirect30xInterceptor.sameOrigin(host("https", "example.com", 443), + host("http", "example.com", 443))); + assertFalse(Redirect30xInterceptor.sameOrigin(host("https", "example.com", 443), + host("https", "example.com", 8443))); + } + @Test public void acceptsTheFollowedRedirectStatuses() { for (int statusCode : new int[]{301, 302, 303, 307, 308}) { diff --git a/client/src/test/java/org/asynchttpclient/ws/WebSocketRedirectRefusalTest.java b/client/src/test/java/org/asynchttpclient/ws/WebSocketRedirectRefusalTest.java new file mode 100644 index 000000000..7e82bbfe0 --- /dev/null +++ b/client/src/test/java/org/asynchttpclient/ws/WebSocketRedirectRefusalTest.java @@ -0,0 +1,146 @@ +/* + * 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.ws; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.asynchttpclient.AsyncHttpClient; +import org.asynchttpclient.DefaultAsyncHttpClientConfig; +import org.asynchttpclient.handler.RedirectRefusedException; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.server.handler.AbstractHandler; +import org.eclipse.jetty.server.handler.HandlerList; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +import java.io.IOException; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +import static org.asynchttpclient.Dsl.asyncHttpClient; +import static org.asynchttpclient.Dsl.config; +import static org.asynchttpclient.test.TestUtils.addHttpConnector; +import static org.asynchttpclient.test.TestUtils.addHttpsConnector; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * {@code wss} to {@code ws} is a cleartext downgrade that AsyncHttpClient follows by default, and RFC 6455 + * section 4.1 leaves a client free not to. The WebSocket path reaches the same interceptor as HTTP, via + * {@code WebSocketHandler}, so both arms apply there too. + *

+ * Both fixture methods are re-annotated here. The base class drops the annotation when it overrides + * {@code tearDownGlobal}, which stops JUnit treating it as a lifecycle method, so declaring only half the + * pair starts a server per test and stops none. + */ +public class WebSocketRedirectRefusalTest extends AbstractBasicWebSocketTest { + + @Override + @BeforeEach + public void setUpGlobal() throws Exception { + server = new Server(); + ServerConnector plain = addHttpConnector(server); + ServerConnector secure = addHttpsConnector(server); + + HandlerList handlers = new HandlerList(); + handlers.addHandler(new AbstractHandler() { + @Override + public void handle(String path, Request request, HttpServletRequest servletRequest, + HttpServletResponse response) throws IOException { + if (request.getLocalPort() == port2) { + response.sendRedirect("ws://localhost:" + port1 + "/"); + } + } + }); + handlers.addHandler(configureHandler()); + server.setHandler(handlers); + + server.start(); + port1 = plain.getLocalPort(); + port2 = secure.getLocalPort(); + } + + @Override + @AfterEach + public void tearDownGlobal() throws Exception { + if (server != null) { + server.stop(); + } + } + + @Test + @Timeout(unit = TimeUnit.MILLISECONDS, value = 60000) + public void downgradeToCleartextIsRefused() throws Exception { + Outcome outcome = connect(config() + .setFollowRedirect(true) + .setUseInsecureTrustManager(true) + .setRefuseSchemeDowngradeOnRedirect(true)); + + assertFalse(outcome.opened.get(), "the cleartext WebSocket must never open"); + assertInstanceOf(RedirectRefusedException.class, outcome.failure.get()); + } + + @Test + @Timeout(unit = TimeUnit.MILLISECONDS, value = 60000) + public void downgradeToCleartextIsFollowedByDefault() throws Exception { + Outcome outcome = connect(config() + .setFollowRedirect(true) + .setUseInsecureTrustManager(true)); + + assertTrue(outcome.opened.get(), "the default posture still follows the hop"); + } + + private Outcome connect(DefaultAsyncHttpClientConfig.Builder builder) throws Exception { + Outcome outcome = new Outcome(); + try (AsyncHttpClient client = asyncHttpClient(builder)) { + client.prepareGet("wss://localhost:" + port2 + "/") + .execute(new WebSocketUpgradeHandler.Builder().addWebSocketListener(new WebSocketListener() { + + @Override + public void onOpen(WebSocket websocket) { + outcome.opened.set(true); + outcome.done.countDown(); + } + + @Override + public void onClose(WebSocket websocket, int code, String reason) { + } + + @Override + public void onError(Throwable t) { + outcome.failure.set(t); + outcome.done.countDown(); + } + }).build()); + + assertTrue(outcome.done.await(30, TimeUnit.SECONDS), "neither onOpen nor onError fired"); + } + return outcome; + } + + private static class Outcome { + final CountDownLatch done = new CountDownLatch(1); + final AtomicBoolean opened = new AtomicBoolean(); + final AtomicReference failure = new AtomicReference<>(); + } +} From b4ff85cce784d1be56534c0db08de5e857a02f75 Mon Sep 17 00:00:00 2001 From: Aayush Atharva Date: Sun, 20 Sep 2026 13:42:48 +0000 Subject: [PATCH 2/4] Carry the refusal reason on the exception --- .../handler/RedirectRefusedException.java | 68 +++++++++++++++++-- .../intercept/Redirect30xInterceptor.java | 13 +--- .../asynchttpclient/RedirectRefusalTest.java | 23 +++++++ 3 files changed, 87 insertions(+), 17 deletions(-) diff --git a/client/src/main/java/org/asynchttpclient/handler/RedirectRefusedException.java b/client/src/main/java/org/asynchttpclient/handler/RedirectRefusedException.java index 4f61de0fd..cf918dbf3 100644 --- a/client/src/main/java/org/asynchttpclient/handler/RedirectRefusedException.java +++ b/client/src/main/java/org/asynchttpclient/handler/RedirectRefusedException.java @@ -36,30 +36,74 @@ * except a 301 or non-strict 302 on a {@code POST}, preserved both the method and the content, so the same * request can be aimed somewhere else unchanged. A 303 is defined as an indirect response to the original * request, typically the output of a {@code POST} that has already run, so the origin has very likely applied - * it (RFC 9110 section 15.4.4). No status proves it did not, though, so a non-idempotent method still - * needs the caller's judgement (section 9.2.2). + * it (RFC 9110 section 15.4.4). No status proves it did not, though, so a non-idempotent method still needs + * the caller's judgement (section 9.2.2). */ -public class RedirectRefusedException extends Exception { +public final class RedirectRefusedException extends Exception { private static final long serialVersionUID = 1L; + /** + * Which gate refused the redirect. Constants are only ever appended, so a caller switching on this must + * still handle a value it does not know. + */ + public enum Reason { + + /** + * The hop left a secured scheme for one that is not. + */ + SCHEME_DOWNGRADE("the target scheme is not secured"), + + /** + * The hop would have resent the request content to an origin the caller never addressed. + */ + CROSS_ORIGIN_BODY("it would resend the request content to another origin"); + + // Carried as a field rather than switched on, so a constant added later cannot compile without one. + private final String description; + + Reason(String description) { + this.description = description; + } + } + + private final Reason reason; private final int statusCode; - // Uri is not Serializable. getMessage() and getStatusCode() carry the same facts and do survive. + // Uri is not Serializable. getMessage(), getReason() and getStatusCode() do survive. + private final transient @Nullable Uri sourceUri; private final transient @Nullable Uri targetUri; /** - * @param message the refusal, naming both origins by base URL only + * @param reason which gate refused the redirect * @param statusCode the status code of the redirect response that was refused + * @param sourceUri the URI this hop was leaving * @param targetUri the redirect target that was refused */ - public RedirectRefusedException(String message, int statusCode, Uri targetUri) { + public RedirectRefusedException(Reason reason, int statusCode, Uri sourceUri, Uri targetUri) { // No stack trace: the frames would all be Netty's event loop, and the caller's ExecutionException // already carries theirs. - super(message, null, true, false); + super(message(reason, statusCode, sourceUri, targetUri), null, true, false); + this.reason = reason; this.statusCode = statusCode; + this.sourceUri = sourceUri; this.targetUri = targetUri; } + // getBaseUrl() on both sides: toBaseUrl() keeps the path and toString() keeps userinfo, neither of which + // belongs in a message headed for a log. Built here rather than by the caller so a second refusal site + // cannot reintroduce that, and eagerly rather than from getMessage() so it survives serialization. + private static String message(Reason reason, int statusCode, Uri sourceUri, Uri targetUri) { + return "Refusing to follow the " + statusCode + " redirect from " + sourceUri.getBaseUrl() + + " to " + targetUri.getBaseUrl() + ": " + reason.description; + } + + /** + * @return which gate refused the redirect + */ + public Reason getReason() { + return reason; + } + /** * @return the status code of the redirect response that was refused */ @@ -67,6 +111,16 @@ public int getStatusCode() { return statusCode; } + /** + * The URI this hop was leaving, which is the request actually sent on this leg rather than the one the + * caller built. Prefer {@link Uri#getBaseUrl()} when logging it, as {@link #getMessage()} already does. + * + * @return the URI the refused redirect was leaving, or null on an instance that was deserialized + */ + public @Nullable Uri getSourceUri() { + return sourceUri; + } + /** * The refused target. Its path, query and userinfo are server-chosen, so prefer {@link Uri#getBaseUrl()} * when logging it, as {@link #getMessage()} already does. diff --git a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java index e106d5be8..13f3fc7ab 100644 --- a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java +++ b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java @@ -31,6 +31,7 @@ import org.asynchttpclient.cookie.CookieStore; import org.asynchttpclient.handler.MaxRedirectException; import org.asynchttpclient.handler.RedirectRefusedException; +import org.asynchttpclient.handler.RedirectRefusedException.Reason; import org.asynchttpclient.netty.NettyResponseFuture; import org.asynchttpclient.netty.channel.ChannelManager; import org.asynchttpclient.netty.channel.PrincipalScopedPartitionKey; @@ -155,15 +156,14 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture // Refuse here, before the next request is built and before ensureBodyReplayable can throw an // IOException that an IOExceptionFilter would replay. if (schemeDowngrade && refuseSchemeDowngrade(request)) { - throw refused(statusCode, currentUri, newUri, "the target scheme is not secured"); + throw new RedirectRefusedException(Reason.SCHEME_DOWNGRADE, statusCode, currentUri, newUri); } BodyRepresentation bodyRepresentation = keepBody ? selectedBodyRepresentation(request) : BodyRepresentation.NONE; if (keepBody && refuseCrossOriginBody(request) && !sameOrigin(currentUri, newUri) && !secureUpgrade(currentUri, newUri) && bodyRepresentation != BodyRepresentation.NONE) { - throw refused(statusCode, currentUri, newUri, - "it would resend the request content to another origin"); + throw new RedirectRefusedException(Reason.CROSS_ORIGIN_BODY, statusCode, currentUri, newUri); } if (LOGGER.isDebugEnabled()) { @@ -296,13 +296,6 @@ private boolean refuseCrossOriginBody(Request request) { return refuseCrossOriginBodyOnRedirect || Boolean.TRUE.equals(request.getRefuseCrossOriginBodyOnRedirect()); } - private static RedirectRefusedException refused(int statusCode, Uri from, Uri to, String reason) { - // getBaseUrl() on both sides: toBaseUrl() keeps the path and toString() keeps userinfo, neither of - // which belongs in a message headed for a log. - return new RedirectRefusedException("Refusing to follow the " + statusCode + " redirect from " - + from.getBaseUrl() + " to " + to.getBaseUrl() + ": " + reason, statusCode, to); - } - /** * Same scheme, host and effective port, per RFC 6454 section 4. *

diff --git a/client/src/test/java/org/asynchttpclient/RedirectRefusalTest.java b/client/src/test/java/org/asynchttpclient/RedirectRefusalTest.java index 6d5de0077..3e63f7e4a 100644 --- a/client/src/test/java/org/asynchttpclient/RedirectRefusalTest.java +++ b/client/src/test/java/org/asynchttpclient/RedirectRefusalTest.java @@ -23,6 +23,7 @@ import org.asynchttpclient.filter.IOExceptionFilter; import org.asynchttpclient.handler.MaxRedirectException; import org.asynchttpclient.handler.RedirectRefusedException; +import org.asynchttpclient.uri.Uri; import org.asynchttpclient.request.body.generator.InputStreamBodyGenerator; import org.asynchttpclient.request.body.multipart.StringPart; import org.eclipse.jetty.server.Request; @@ -55,6 +56,7 @@ import java.util.concurrent.atomic.AtomicReference; import static java.util.Collections.singletonList; +import static java.util.Objects.requireNonNull; import static org.asynchttpclient.Dsl.asyncHttpClient; import static org.asynchttpclient.Dsl.config; import static org.asynchttpclient.test.TestUtils.addHttpConnector; @@ -113,6 +115,9 @@ public void downgradeOnBodylessGetIsRefused() throws Exception { try (AsyncHttpClient client = asyncHttpClient(followingConfig().setRefuseSchemeDowngradeOnRedirect(true))) { RedirectRefusedException refusal = expectRefusal(client.prepareGet(secure("/downgrade"))); assertEquals(307, refusal.getStatusCode()); + assertEquals(RedirectRefusedException.Reason.SCHEME_DOWNGRADE, refusal.getReason()); + assertEquals("https", requireNonNull(refusal.getSourceUri()).getScheme()); + assertEquals("http", requireNonNull(refusal.getTargetUri()).getScheme()); assertFalse(targetHit.get(), "the refused target must not be reached"); } } @@ -196,6 +201,7 @@ public void crossOriginBodyReplayIsRefused() throws Exception { try (AsyncHttpClient client = asyncHttpClient(followingConfig().setRefuseCrossOriginBodyOnRedirect(true))) { RedirectRefusedException refusal = expectRefusal(client.preparePut(plain("/cross-origin")).setBody("payload")); assertEquals(307, refusal.getStatusCode()); + assertEquals(RedirectRefusedException.Reason.CROSS_ORIGIN_BODY, refusal.getReason()); assertFalse(targetHit.get(), "the content must not reach the other origin"); } } @@ -379,6 +385,23 @@ public void theMessageCarriesNeitherPathQueryNorUserInfo() throws Exception { } } + @Test + public void theMessageIsBuiltFromBaseUrlsWhoeverConstructsIt() { + Uri source = Uri.create("https://user:pw@localhost:8443/secret/path?token=CALLER_SECRET"); + Uri target = Uri.create("http://admin:hunter2@127.0.0.1:8080/other?token=SERVER_SECRET"); + + String message = new RedirectRefusedException( + RedirectRefusedException.Reason.SCHEME_DOWNGRADE, 307, source, target).getMessage(); + + assertFalse(message.contains("CALLER_SECRET"), message); + assertFalse(message.contains("SERVER_SECRET"), message); + assertFalse(message.contains("user:pw"), message); + assertFalse(message.contains("hunter2"), message); + assertFalse(message.contains("/secret/path"), message); + assertTrue(message.contains("https://localhost:8443"), message); + assertTrue(message.contains("http://127.0.0.1:8080"), message); + } + // ---------------------------------------------------------------- per-request override @Test From 8802367b1809361d8d362eacdf57c6470ddb7b3d Mon Sep 17 00:00:00 2001 From: Aayush Atharva Date: Sun, 20 Sep 2026 15:42:21 +0000 Subject: [PATCH 3/4] Defer the test server codec until ALPN settles --- .../org/asynchttpclient/BasicHttp2Test.java | 41 ++++++++++++++----- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/client/src/test/java/org/asynchttpclient/BasicHttp2Test.java b/client/src/test/java/org/asynchttpclient/BasicHttp2Test.java index b4d35b2d4..ddfa09036 100644 --- a/client/src/test/java/org/asynchttpclient/BasicHttp2Test.java +++ b/client/src/test/java/org/asynchttpclient/BasicHttp2Test.java @@ -37,6 +37,7 @@ import io.netty.handler.codec.http2.DefaultHttp2ResetFrame; import io.netty.handler.codec.http2.Http2DataFrame; import io.netty.handler.codec.http2.Http2Error; +import io.netty.handler.codec.http2.Http2FrameCodec; import io.netty.handler.codec.http2.Http2FrameCodecBuilder; import io.netty.util.ReferenceCountUtil; import io.netty.handler.codec.http2.Http2Headers; @@ -45,6 +46,7 @@ import io.netty.handler.codec.http2.Http2StreamChannel; import io.netty.handler.ssl.ApplicationProtocolConfig; import io.netty.handler.ssl.ApplicationProtocolNames; +import io.netty.handler.ssl.ApplicationProtocolNegotiationHandler; import io.netty.handler.ssl.SslContext; import io.netty.handler.ssl.SslContextBuilder; import io.netty.pkitesting.CertificateBuilder; @@ -57,6 +59,7 @@ import org.eclipse.jetty.proxy.ConnectHandler; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; +import org.jetbrains.annotations.Nullable; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -393,13 +396,21 @@ protected void initChannel(Channel ch) { serverChildChannels.add(ch); ch.pipeline() .addLast("ssl", serverSslCtx.newHandler(ch.alloc())) - .addLast(Http2FrameCodecBuilder.forServer().build()) - .addLast(new Http2MultiplexHandler(new ChannelInitializer() { + .addLast(new ApplicationProtocolNegotiationHandler(ApplicationProtocolNames.HTTP_2) { @Override - protected void initChannel(Http2StreamChannel streamCh) { - streamCh.pipeline().addLast(new Http2TestServerHandler()); + protected void configurePipeline(ChannelHandlerContext ctx, String protocol) { + ctx.pipeline() + .addLast(Http2FrameCodecBuilder.forServer().build()) + .addLast(new Http2MultiplexHandler( + new ChannelInitializer() { + @Override + protected void initChannel(Http2StreamChannel streamCh) { + streamCh.pipeline() + .addLast(new Http2TestServerHandler()); + } + })); } - })); + }); } }); @@ -421,6 +432,15 @@ public void stopServer() throws InterruptedException { ReferenceCountUtil.release(serverSslCtx); } + private @Nullable Channel negotiatedHttp2Channel() { + for (Channel child : serverChildChannels) { + if (child.pipeline().get(Http2FrameCodec.class) != null) { + return child; + } + } + return null; + } + private String httpsUrl(String path) { return "https://localhost:" + serverPort + path; } @@ -703,17 +723,18 @@ public void http2RoundRobinGoawayReleasesPermitForReplacementConnection() throws // Long-running stream that keeps its connection (and, before the fix, the only permit) busy. client.executeRequest(org.asynchttpclient.Dsl.get(httpsUrl("/delay/30000")).setNameResolver(resolver)); - // Wait until the server has accepted the connection. + // Wait for a connection that has settled on HTTP/2; one that has only been accepted has no + // codec yet and cannot encode the GOAWAY below. long deadline = System.currentTimeMillis() + 5000; - while (serverChildChannels.size() < 1 && System.currentTimeMillis() < deadline) { + Channel parent = negotiatedHttp2Channel(); + while (parent == null && System.currentTimeMillis() < deadline) { Thread.sleep(20); + parent = negotiatedHttp2Channel(); } - assertEquals(1, serverChildChannels.size(), "exactly one HTTP/2 connection should be established"); - Thread.sleep(300); + assertNotNull(parent, "exactly one HTTP/2 connection should be established"); // GOAWAY with a high lastStreamId leaves the in-flight stream running, so the connection // stays open and draining. - Channel parent = serverChildChannels.iterator().next(); parent.writeAndFlush(new io.netty.handler.codec.http2.DefaultHttp2GoAwayFrame(Http2Error.NO_ERROR) .setExtraStreamIds(1000)).sync(); Thread.sleep(300); From 2f26a695271929a6cfc2348a3ec1e769f1ed646a Mon Sep 17 00:00:00 2001 From: Aayush Atharva Date: Sun, 20 Sep 2026 16:56:22 +0000 Subject: [PATCH 4/4] Trim comments and correct two test defects --- .../AsyncHttpClientConfig.java | 38 +++++------ .../DefaultAsyncHttpClientConfig.java | 4 +- .../java/org/asynchttpclient/Request.java | 16 ++--- .../handler/RedirectRefusedException.java | 25 +++----- .../intercept/Redirect30xInterceptor.java | 63 +++++++------------ .../org/asynchttpclient/BasicHttp2Test.java | 15 +++-- .../RedirectRefusalConfigTest.java | 23 ++++++- .../asynchttpclient/RedirectRefusalTest.java | 40 ++++++------ .../intercept/Redirect30xInterceptorTest.java | 13 ++-- .../ws/WebSocketRedirectRefusalTest.java | 10 ++- 10 files changed, 125 insertions(+), 122 deletions(-) diff --git a/client/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java b/client/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java index 573539971..38724e1aa 100644 --- a/client/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java +++ b/client/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java @@ -185,16 +185,16 @@ default boolean isUseAbsoluteRequestDeadline() { int getMaxRedirects(); /** - * Whether to refuse a redirect that leaves an {@code https} or {@code wss} exchange for a target whose - * scheme is not secured, instead of following it. The redirected request is never sent and the exchange - * fails with a {@link org.asynchttpclient.handler.RedirectRefusedException}. + * Whether to refuse a redirect that leaves an {@code https} or {@code wss} exchange for an unsecured + * scheme. The redirected request is never sent; the exchange fails with a + * {@link org.asynchttpclient.handler.RedirectRefusedException}. *

- * Applies to every hop, bodies or not, and only once {@link #isFollowRedirect()} and - * {@link #getMaxRedirects()} have already let the hop through. It says nothing about an exchange that - * began in cleartext: there is no downgrade to refuse there. + * Applies to every hop, bodies or not, and is judged against the scheme of the request sent on that hop, + * so a cleartext hop has no downgrade to refuse but a later secured one in the same exchange still does. *

* {@link org.asynchttpclient.Request#getRefuseSchemeDowngradeOnRedirect()} can refuse a hop this allows, - * but cannot permit one this refuses. + * but cannot permit one this refuses. An implementation that wraps another configuration has to forward + * this, or the operator's setting on the wrapped one is lost to the {@code false} default here. * * @return true to refuse such a hop, false to follow it */ @@ -203,22 +203,22 @@ default boolean isRefuseSchemeDowngradeOnRedirect() { } /** - * Whether to refuse a redirect that would resend this request's content to a different origin, instead of - * following it. It applies only to a hop that keeps the body and only when the request actually carries - * content, so an ordinary bodyless redirect across origins is untouched, as is a same-origin hop. A - * {@code GET} that does carry content is not untouched: it will be refused. + * Whether to refuse a redirect that would resend this request's content to a different origin. It applies + * only to a hop that keeps the body and only when the request carries content, so a bodyless cross-origin + * hop and a same-origin hop are untouched, while a {@code GET} carrying content is refused like any + * other. Moving the same host onto TLS is exempt, keeping the port or moving between the two scheme + * defaults. The exempted hop keeps the host, not the path: the target path and query are the server's + * choice. *

- * Moving the same host onto TLS is exempt, so {@code http://host/x} to {@code https://host/x} is followed - * even though RFC 6454 counts it as a different origin. The exemption needs the host to match and the port - * to be unchanged or both at their scheme's default, so {@code :8080} to {@code :9999} is still refused. - * The hop is still not same-base, so credentials are stripped from it as they always were. + * The hop is refused rather than stripped of its body, because a {@code PUT} arriving empty would + * overwrite the target with nothing. Weigh that against what it blocks: an upload answered with a + * redirect to another host stops working. *

- * Refusing rather than dropping the body is deliberate: a {@code PUT} that arrived empty would overwrite - * the target with nothing. Weigh that against what it blocks: an upload answered with a redirect to - * another host, which some storage services use to route a request to its home region, stops working. + * The hop is still not same-base, so credentials are stripped from it as they always were. *

* {@link org.asynchttpclient.Request#getRefuseCrossOriginBodyOnRedirect()} can refuse a hop this allows, - * but cannot permit one this refuses. + * but cannot permit one this refuses. An implementation that wraps another configuration has to forward + * this, or the operator's setting on the wrapped one is lost to the {@code false} default here. * * @return true to refuse such a hop, false to follow it */ diff --git a/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClientConfig.java b/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClientConfig.java index 8fc3426b7..9b37c8d7b 100644 --- a/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClientConfig.java +++ b/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClientConfig.java @@ -84,6 +84,8 @@ import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultMaxRequestRetry; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultPooledConnectionIdleTimeout; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultReadTimeout; +import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultRefuseCrossOriginBodyOnRedirect; +import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultRefuseSchemeDowngradeOnRedirect; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultLoadBalance; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultRequestTimeout; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultShutdownQuietPeriod; @@ -95,8 +97,6 @@ import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultSoSndBuf; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultSslSessionCacheSize; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultSslSessionTimeout; -import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultRefuseCrossOriginBodyOnRedirect; -import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultRefuseSchemeDowngradeOnRedirect; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultStrict302Handling; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultTcpNoDelay; import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultThreadPoolName; diff --git a/client/src/main/java/org/asynchttpclient/Request.java b/client/src/main/java/org/asynchttpclient/Request.java index 50569afab..e16b47ddc 100644 --- a/client/src/main/java/org/asynchttpclient/Request.java +++ b/client/src/main/java/org/asynchttpclient/Request.java @@ -186,11 +186,9 @@ default Boolean getUseAbsoluteRequestDeadline() { /** * Refuses a scheme downgrade on this request and the hops it leads to, even when * {@link AsyncHttpClientConfig#isRefuseSchemeDowngradeOnRedirect()} is off. Tightening only, so - * {@code false} behaves as null and cannot re-enable a hop the client configuration refuses. - *

- * Like any per-request setting, a filter that builds a fresh {@link Request} rather than deriving one from - * {@link #toBuilder()} replaces this along with everything else; the client-wide option is what holds a - * posture across replays. + * {@code false} behaves as null and cannot re-enable a hop the client configuration refuses. A filter + * that builds a fresh request rather than deriving one from {@link #toBuilder()} drops it, so the + * client-wide option is what holds a posture across replays. * * @return true to refuse, or null or false to use the config value */ @@ -202,11 +200,9 @@ default Boolean getRefuseSchemeDowngradeOnRedirect() { /** * Refuses a cross-origin body replay on this request and the hops it leads to, even when * {@link AsyncHttpClientConfig#isRefuseCrossOriginBodyOnRedirect()} is off. Tightening only, so - * {@code false} behaves as null and cannot re-enable a hop the client configuration refuses. - *

- * Like any per-request setting, a filter that builds a fresh {@link Request} rather than deriving one from - * {@link #toBuilder()} replaces this along with everything else; the client-wide option is what holds a - * posture across replays. + * {@code false} behaves as null and cannot re-enable a hop the client configuration refuses. A filter + * that builds a fresh request rather than deriving one from {@link #toBuilder()} drops it, so the + * client-wide option is what holds a posture across replays. * * @return true to refuse, or null or false to use the config value */ diff --git a/client/src/main/java/org/asynchttpclient/handler/RedirectRefusedException.java b/client/src/main/java/org/asynchttpclient/handler/RedirectRefusedException.java index cf918dbf3..7fc5db043 100644 --- a/client/src/main/java/org/asynchttpclient/handler/RedirectRefusedException.java +++ b/client/src/main/java/org/asynchttpclient/handler/RedirectRefusedException.java @@ -32,12 +32,11 @@ * replay on that type, so a refusal that was one would come back as several more attempts to send the content * the caller asked to send once. {@link MaxRedirectException} avoids it for the same reason. * - *

Whether re-sending by hand is safe depends on {@link #getStatusCode()}. Every refusal except a 303, and - * except a 301 or non-strict 302 on a {@code POST}, preserved both the method and the content, so the same - * request can be aimed somewhere else unchanged. A 303 is defined as an indirect response to the original - * request, typically the output of a {@code POST} that has already run, so the origin has very likely applied - * it (RFC 9110 section 15.4.4). No status proves it did not, though, so a non-idempotent method still needs - * the caller's judgement (section 9.2.2). + *

Whether re-sending by hand is safe depends on {@link #getStatusCode()}. Every refusal except a 303, + * and except a 301 or non-strict 302 on a {@code POST}, preserved both the method and the content, so the + * same request can be aimed somewhere else unchanged. A 303 is intended as an indirect response to a + * request the origin has very likely already applied (RFC 9110 section 15.4.4), and no status proves + * otherwise, so a non-idempotent method still needs the caller's judgement (section 9.2.2). */ public final class RedirectRefusedException extends Exception { @@ -49,17 +48,13 @@ public final class RedirectRefusedException extends Exception { */ public enum Reason { - /** - * The hop left a secured scheme for one that is not. - */ + /** The hop left a secured scheme for one that is not. */ SCHEME_DOWNGRADE("the target scheme is not secured"), - /** - * The hop would have resent the request content to an origin the caller never addressed. - */ + /** The hop would have resent the request content to an origin the caller never addressed. */ CROSS_ORIGIN_BODY("it would resend the request content to another origin"); - // Carried as a field rather than switched on, so a constant added later cannot compile without one. + // A field, not a switch: a constant added later cannot compile without a description. private final String description; Reason(String description) { @@ -90,8 +85,8 @@ public RedirectRefusedException(Reason reason, int statusCode, Uri sourceUri, Ur } // getBaseUrl() on both sides: toBaseUrl() keeps the path and toString() keeps userinfo, neither of which - // belongs in a message headed for a log. Built here rather than by the caller so a second refusal site - // cannot reintroduce that, and eagerly rather than from getMessage() so it survives serialization. + // belongs in a log. Built here rather than by the caller so a second refusal site cannot reintroduce + // that, and eagerly rather than in getMessage() so it survives serialization. private static String message(Reason reason, int statusCode, Uri sourceUri, Uri targetUri) { return "Refusing to follow the " + statusCode + " redirect from " + sourceUri.getBaseUrl() + " to " + targetUri.getBaseUrl() + ": " + reason.description; diff --git a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java index 13f3fc7ab..2f6058af7 100644 --- a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java +++ b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java @@ -143,18 +143,16 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture HttpHeaders responseHeaders = response.headers(); String location = responseHeaders.get(LOCATION); - // The gates below must judge the same URI this hop resolves against, so both come from the - // request actually sent on this leg (RFC 9110 section 15.4 item 1). A 401 or 407 retry - // rebuilds that request without touching the future's own, so the two agree by each - // interceptor's care rather than by construction. + // Location resolves against the target URI of the request actually sent on this leg + // (RFC 9110 section 15.4, modification 1), and the gates below must judge that same URI. + // A 401 or 407 retry leaves the future's own target behind, so do not read it here. Uri currentUri = request.getUri(); Uri newUri = Uri.create(currentUri, location); boolean sameBase = currentUri.isSameBase(newUri); boolean schemeDowngrade = currentUri.isSecured() && !newUri.isSecured(); - // Refuse here, before the next request is built and before ensureBodyReplayable can throw an - // IOException that an IOExceptionFilter would replay. + // Refuse before ensureBodyReplayable, whose IOException an IOExceptionFilter would replay. if (schemeDowngrade && refuseSchemeDowngrade(request)) { throw new RedirectRefusedException(Reason.SCHEME_DOWNGRADE, statusCode, currentUri, newUri); } @@ -212,8 +210,8 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture requestBuilder.setUseAbsoluteRequestDeadline(useAbsoluteRequestDeadline); } - // The gate reads these off the request each hop, and the !keepBody branch starts from an - // empty builder, so without this the caller's choice lapses from the second hop on. + // The !keepBody branch builds from an empty builder, so carry these forward or the caller's + // choice lapses from the second hop on. Boolean refuseSchemeDowngrade = request.getRefuseSchemeDowngradeOnRedirect(); if (refuseSchemeDowngrade != null) { requestBuilder.setRefuseSchemeDowngradeOnRedirect(refuseSchemeDowngrade); @@ -297,18 +295,12 @@ private boolean refuseCrossOriginBody(Request request) { } /** - * Same scheme, host and effective port, per RFC 6454 section 4. - *

- * Not {@link Uri#isSameBase(Uri)}, which compares hosts with {@link String#equals}: a host differing only - * in case is still the same origin, and refusing there would break a legitimate upload. The fold is - * ASCII-only because that is the {@code i;ascii-casemap} collation section 4 step 5 asks for; - * {@link String#equalsIgnoreCase} folds Unicode and would call {@code i.example} equal to a host starting - * with {@code \\u0130}, which punycodes to a different host. Nothing here does IDNA - * either, so a Unicode host and its A-label spelling read as different origins, erring towards refusal. - * - * @param from the URI this hop is leaving - * @param to the redirect target - * @return whether the two are the same origin + * Same scheme, host and effective port, per RFC 6454 section 4. Not {@link Uri#isSameBase(Uri)}, which + * compares hosts with {@link String#equals}. Hosts fold ASCII-only, the {@code i;ascii-casemap} collation + * that step 5 of that section asks for: {@link String#equalsIgnoreCase} + * folds Unicode and would call {@code i.example} equal to a host starting {@code U+0130}, a different + * host. Nothing here does IDNA either, so a Unicode host and its A-label read as different origins, + * erring towards refusal. */ static boolean sameOrigin(Uri from, Uri to) { return from.getScheme().equals(to.getScheme()) @@ -317,26 +309,19 @@ static boolean sameOrigin(Uri from, Uri to) { } /** - * Whether this hop only swaps the same host onto TLS. This is a deliberate exemption from the origin - * rule, not something RFC 6454 allows: section 4 puts the scheme and port in the triple, so an upgrade is - * a different origin. It is our own policy, because refusing a move onto TLS would cost confidentiality - * rather than protect it. - *

- * The host is compared, not trusted: a {@code Location} naming anything else stays cross-origin. The port - * rule is modelled on the upgrade mapping in RFC 6797 section 8.3, which is not binding here (that - * section is scoped to a Known HSTS Host, and we keep no HSTS state): the same port, or both at their - * scheme's default. So {@code :8080} to {@code :9999} is not an upgrade, while an explicit {@code :80} to - * {@code :80} is, which section 8.3 would have mapped to {@code :443}. That last case keeps the very - * endpoint the redirect was served from, so it reaches no party the request was not already aimed at. + * Whether this hop only swaps the same host onto TLS. A deliberate exemption from the origin rule rather + * than something RFC 6454 allows, because refusing a move onto TLS would cost confidentiality rather than + * protect it. The host is compared, not trusted; the path and query are not compared at all, so the + * content lands wherever the redirect says on that host. The port must be unchanged or both sides at + * their scheme's default, so {@code :8080} to {@code :9999} is a different endpoint and stays refused. + * That is modelled on the port mapping in RFC 6797 section 8.3 but looser than it: 8.3 is scoped to a + * Known HSTS Host and we keep no HSTS state, and it would have mapped an explicit {@code :80} to + * {@code :443} where this accepts {@code :80} to {@code :80}, which is the endpoint the redirect was + * served from. *

- * Only the content arm consults this. The hop is still not {@code isSameBase}, so {@code Authorization}, - * {@code Proxy-Authorization} and {@code Cookie} are stripped and the realm is dropped, as on any - * cross-base hop; cookies are then re-derived from the {@link org.asynchttpclient.cookie.CookieStore} - * against the new URI, which on an upgraded hop includes the host's {@code Secure} ones. - * - * @param from the URI this hop is leaving - * @param to the redirect target - * @return whether this is the same host moving to its secured scheme + * Only the content arm consults this; the hop is still cross-base, so credentials are stripped as usual + * and cookies are re-derived from the {@link org.asynchttpclient.cookie.CookieStore} against the new URI + * - which on an upgraded hop includes the host's {@code Secure} ones. */ static boolean secureUpgrade(Uri from, Uri to) { // Named pairs, not isSecured(), which would also admit http to wss. Schemes compare exactly because diff --git a/client/src/test/java/org/asynchttpclient/BasicHttp2Test.java b/client/src/test/java/org/asynchttpclient/BasicHttp2Test.java index ddfa09036..c61f6a916 100644 --- a/client/src/test/java/org/asynchttpclient/BasicHttp2Test.java +++ b/client/src/test/java/org/asynchttpclient/BasicHttp2Test.java @@ -16,6 +16,7 @@ package org.asynchttpclient; import io.netty.bootstrap.ServerBootstrap; +import io.github.nettyplus.leakdetector.junit.NettyLeakDetectorExtension; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; import io.netty.channel.Channel; @@ -62,6 +63,7 @@ import org.jetbrains.annotations.Nullable; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.Test; import java.net.URLDecoder; @@ -113,6 +115,7 @@ *

  • Falls back to HTTP/1.1 when HTTP/2 is disabled
    • * */ +@ExtendWith(NettyLeakDetectorExtension.class) public class BasicHttp2Test { // Event constants (from HttpTest/EventCollectingHandler) @@ -396,9 +399,12 @@ protected void initChannel(Channel ch) { serverChildChannels.add(ch); ch.pipeline() .addLast("ssl", serverSslCtx.newHandler(ch.alloc())) - .addLast(new ApplicationProtocolNegotiationHandler(ApplicationProtocolNames.HTTP_2) { + .addLast(new ApplicationProtocolNegotiationHandler(ApplicationProtocolNames.HTTP_1_1) { @Override protected void configurePipeline(ChannelHandlerContext ctx, String protocol) { + if (!ApplicationProtocolNames.HTTP_2.equals(protocol)) { + throw new IllegalStateException("unexpected ALPN protocol: " + protocol); + } ctx.pipeline() .addLast(Http2FrameCodecBuilder.forServer().build()) .addLast(new Http2MultiplexHandler( @@ -731,7 +737,8 @@ public void http2RoundRobinGoawayReleasesPermitForReplacementConnection() throws Thread.sleep(20); parent = negotiatedHttp2Channel(); } - assertNotNull(parent, "exactly one HTTP/2 connection should be established"); + assertNotNull(parent, "no HTTP/2 connection settled within 5s"); + assertEquals(1, serverChildChannels.size(), "exactly one HTTP/2 connection should be established"); // GOAWAY with a high lastStreamId leaves the in-flight stream running, so the connection // stays open and draining. @@ -1609,8 +1616,7 @@ public void http2SequentialRequestsWithMaxConnectionsPerHostOne() throws Excepti } /** - * The refusal has to terminate an HTTP/2 stream as cleanly as an HTTP/1.1 connection, and Http2Handler - * gates its own IOExceptionFilter replay on the exception type, so both arms are worth pinning here. + * Http2Handler gates its own IOExceptionFilter replay on the exception type, so both arms are pinned. */ @Test public void schemeDowngradeRefusalAppliesOverHttp2() throws Exception { @@ -1634,5 +1640,4 @@ public void crossOriginBodyRefusalAppliesOverHttp2() throws Exception { assertInstanceOf(RedirectRefusedException.class, failure.getCause()); } } - } diff --git a/client/src/test/java/org/asynchttpclient/RedirectRefusalConfigTest.java b/client/src/test/java/org/asynchttpclient/RedirectRefusalConfigTest.java index a18afded8..5653664ab 100644 --- a/client/src/test/java/org/asynchttpclient/RedirectRefusalConfigTest.java +++ b/client/src/test/java/org/asynchttpclient/RedirectRefusalConfigTest.java @@ -15,9 +15,11 @@ */ package org.asynchttpclient; +import org.asynchttpclient.config.AsyncHttpClientConfigHelper; import org.junit.jupiter.api.Test; import static org.asynchttpclient.Dsl.config; +import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.ASYNC_CLIENT_CONFIG_ROOT; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -42,8 +44,27 @@ public void theBuilderSetsEachArmOnItsOwn() { } /** - * The copy constructor is how a framework layer usually derives a client. + * The Builder's field initialisers are the only wiring between the properties and a built config, and + * nothing else here would notice losing them: an uninitialised boolean is false, which is the default. */ + @Test + public void thePropertiesReachABuiltConfig() { + String downgrade = ASYNC_CLIENT_CONFIG_ROOT + "refuseSchemeDowngradeOnRedirect"; + String crossOrigin = ASYNC_CLIENT_CONFIG_ROOT + "refuseCrossOriginBodyOnRedirect"; + System.setProperty(downgrade, "true"); + System.setProperty(crossOrigin, "true"); + AsyncHttpClientConfigHelper.reloadProperties(); + try { + AsyncHttpClientConfig built = config().build(); + assertTrue(built.isRefuseSchemeDowngradeOnRedirect()); + assertTrue(built.isRefuseCrossOriginBodyOnRedirect()); + } finally { + System.clearProperty(downgrade); + System.clearProperty(crossOrigin); + AsyncHttpClientConfigHelper.reloadProperties(); + } + } + @Test public void theCopyConstructorKeepsBoth() { AsyncHttpClientConfig original = config() diff --git a/client/src/test/java/org/asynchttpclient/RedirectRefusalTest.java b/client/src/test/java/org/asynchttpclient/RedirectRefusalTest.java index 3e63f7e4a..55799b4c9 100644 --- a/client/src/test/java/org/asynchttpclient/RedirectRefusalTest.java +++ b/client/src/test/java/org/asynchttpclient/RedirectRefusalTest.java @@ -24,6 +24,7 @@ import org.asynchttpclient.handler.MaxRedirectException; import org.asynchttpclient.handler.RedirectRefusedException; import org.asynchttpclient.uri.Uri; +import org.asynchttpclient.request.body.generator.FileBodyGenerator; import org.asynchttpclient.request.body.generator.InputStreamBodyGenerator; import org.asynchttpclient.request.body.multipart.StringPart; import org.eclipse.jetty.server.Request; @@ -70,9 +71,6 @@ /** * The redirect refusal gate in {@code Redirect30xInterceptor}. - *

    - * {@code setUpGlobal()} stays on {@link BeforeAll} so one server serves the class and the inherited - * {@code @AfterAll} stops it. */ public class RedirectRefusalTest extends AbstractBasicTest { @@ -158,10 +156,7 @@ public void anUpgradeThatKeepsItsPortIsNotRefused() throws Exception { } } - /** - * The exemption wants the same port or both scheme defaults, and the two test connectors are on unrelated - * ephemeral ports, so this hop is a port change rather than an upgrade and stays refused. - */ + // The two connectors are on unrelated ephemeral ports, so this is a port change, not an upgrade. @Test public void anUpgradeThatAlsoChangesThePortIsStillRefused() throws Exception { try (AsyncHttpClient client = asyncHttpClient(followingConfig().setRefuseCrossOriginBodyOnRedirect(true))) { @@ -207,8 +202,7 @@ public void crossOriginBodyReplayIsRefused() throws Exception { } /** - * The arm has to hold for every status that keeps method and content, not just the 307 the other tests - * use. 301 and 302 keep both here because the legacy rewrite is limited to POST. + * 301 and 302 keep method and content here, because the legacy rewrite is limited to POST. */ @ParameterizedTest @ValueSource(ints = {301, 302, 308}) @@ -222,8 +216,7 @@ public void crossOriginBodyReplayIsRefusedOnEveryBodyPreservingStatus(int status } /** - * hasContent reads the request rather than the wire, so every representation the factory can write has to - * be recognised. A representation added there without a row here fails open. + * The gate reads the request rather than the wire, so each body representation has to be recognised. */ @ParameterizedTest(name = "{0}") @MethodSource("bodyRepresentations") @@ -238,9 +231,7 @@ public void crossOriginBodyReplayIsRefusedForEveryRepresentation(String name, Co } /** - * The gate sits above ensureBodyReplayable, which would throw an IOException for a stream that cannot be - * replayed. Getting the refusal rather than that IOException is what proves the order, and the filter - * count proves the IOException path was never entered. + * Getting the refusal rather than ensureBodyReplayable's IOException is what proves the gate runs first. */ @Test public void aConsumedStreamIsRefusedRatherThanFailingReplayable() throws Exception { @@ -277,6 +268,10 @@ private static Stream bodyRepresentations() throws IOException { Arguments.of("file", (Consumer) rb -> rb.setBody(file)), Arguments.of("formParams", (Consumer) rb -> rb.setFormParams(singletonList(new Param("k", "v")))), Arguments.of("bodyParts", (Consumer) rb -> rb.addBodyPart(new StringPart("k", "v"))), + Arguments.of("compositeByteData", (Consumer) rb -> + rb.setBody(singletonList(payload))), + Arguments.of("fileBodyGenerator", (Consumer) rb -> + rb.setBody(new FileBodyGenerator(file))), Arguments.of("bodyGenerator", (Consumer) rb -> rb.setBody(new InputStreamBodyGenerator(new ByteArrayInputStream(payload))))); } @@ -336,8 +331,7 @@ public void theTwoArmsAreIndependent() throws Exception { // ---------------------------------------------------------------- how the refusal terminates /** - * Both handlers gate their {@code IOExceptionFilter} replay on {@link IOException}, so a refusal that was - * one would come back as further attempts to send the content. + * A refusal that was an IOException would come back as further attempts to send the content. */ @Test public void refusalIsNotAnIOExceptionAndIsNotReplayed() throws Exception { @@ -429,10 +423,18 @@ public void anOverrideSurvivesAHopThatKeepsTheBody() throws Exception { } } + @Test + public void aRequestCannotAllowTheCrossOriginBodyHopWhereTheClientRefuses() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(followingConfig().setRefuseCrossOriginBodyOnRedirect(true))) { + expectRefusal(client.preparePut(plain("/cross-origin")).setBody("payload") + .setRefuseCrossOriginBodyOnRedirect(false)); + assertFalse(targetHit.get(), "a request must not be able to relax the client's posture"); + } + } + /** - * A 303 drops the body and rebuilds from an empty builder rather than from the request, which is the only - * branch where the override has to be copied across by hand. The client leaves both arms off, so the - * refusal on the second hop can only come from the override surviving the rebuild. + * A 303 rebuilds from an empty builder, the only branch where the override is copied across by hand. + * Both client arms are off, so a refusal can only come from the override surviving that rebuild. */ @Test public void anOverrideSurvivesTheRebuildOnABodylessHop() throws Exception { diff --git a/client/src/test/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptorTest.java b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptorTest.java index 6169d7838..74caa7b21 100644 --- a/client/src/test/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptorTest.java +++ b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptorTest.java @@ -37,9 +37,9 @@ public void sameOriginFoldsHostCaseButOnlyInAscii() { assertTrue(Redirect30xInterceptor.sameOrigin(host("https", "example.com", 443), host("https", "EXAMPLE.com", 443))); - // The one case an integration test cannot reach, because Netty decodes header values byte per char: - // a non-ASCII host on the caller's own request URI. String.equalsIgnoreCase would call these equal, - // and \\u0130.example punycodes to xn--i-9bb.example, a different host. + // Not reachable from an integration test: Netty decodes header values byte per char, so a non-ASCII + // host can only come from the caller's own URI. String.equalsIgnoreCase would call these two equal; + // U+0130.example punycodes to xn--i-9bb.example, a different host. assertFalse(Redirect30xInterceptor.sameOrigin(host("https", "i.example", 443), host("https", "\u0130.example", 443))); } @@ -58,7 +58,6 @@ public void secureUpgradeCoversTheDefaultPortPair() { host("https", "example.com", -1))); assertTrue(Redirect30xInterceptor.secureUpgrade(host("ws", "example.com", -1), host("wss", "example.com", -1))); - // RFC 6797 section 8.3 preserves an explicit port rather than inventing one. assertTrue(Redirect30xInterceptor.secureUpgrade(host("http", "example.com", 8080), host("https", "example.com", 8080))); // Both spellings of the default pair, since getExplicitPort resolves -1 through the scheme default. @@ -66,8 +65,8 @@ public void secureUpgradeCoversTheDefaultPortPair() { host("https", "example.com", -1))); assertTrue(Redirect30xInterceptor.secureUpgrade(host("http", "example.com", -1), host("https", "example.com", 443))); - // Deliberately looser than RFC 6797 section 8.3, which would map an explicit 80 to 443. Keeping the - // port means the hop lands on the endpoint the redirect was served from, so it reaches nobody new. + // Deliberately looser than RFC 6797 section 8.3: keeping the port lands on the endpoint the redirect + // was served from, so it reaches nobody new. assertTrue(Redirect30xInterceptor.secureUpgrade(host("http", "example.com", 80), host("https", "example.com", 80))); assertTrue(Redirect30xInterceptor.secureUpgrade(host("http", "example.com", -1), @@ -78,6 +77,8 @@ public void secureUpgradeCoversTheDefaultPortPair() { public void secureUpgradeRefusesAnythingButTheSameHostGoingSecure() { assertFalse(Redirect30xInterceptor.secureUpgrade(host("http", "example.com", 8080), host("https", "example.com", 9999)), "a port change is not an upgrade"); + assertFalse(Redirect30xInterceptor.secureUpgrade(host("http", "example.com", -1), + host("https", "example.com", 8443)), "a default port moving to a non-default one is not"); assertFalse(Redirect30xInterceptor.secureUpgrade(host("http", "example.com", -1), host("https", "other.example", -1)), "the host has to match"); assertFalse(Redirect30xInterceptor.secureUpgrade(host("https", "example.com", -1), diff --git a/client/src/test/java/org/asynchttpclient/ws/WebSocketRedirectRefusalTest.java b/client/src/test/java/org/asynchttpclient/ws/WebSocketRedirectRefusalTest.java index 7e82bbfe0..0d5d8c9a4 100644 --- a/client/src/test/java/org/asynchttpclient/ws/WebSocketRedirectRefusalTest.java +++ b/client/src/test/java/org/asynchttpclient/ws/WebSocketRedirectRefusalTest.java @@ -45,13 +45,11 @@ import static org.junit.jupiter.api.Assertions.assertTrue; /** - * {@code wss} to {@code ws} is a cleartext downgrade that AsyncHttpClient follows by default, and RFC 6455 - * section 4.1 leaves a client free not to. The WebSocket path reaches the same interceptor as HTTP, via - * {@code WebSocketHandler}, so both arms apply there too. + * {@code wss} to {@code ws} is a cleartext downgrade, and RFC 6455 section 4.1 leaves a client free not to + * follow it. The WebSocket path reaches the same interceptor as HTTP. *

    - * Both fixture methods are re-annotated here. The base class drops the annotation when it overrides - * {@code tearDownGlobal}, which stops JUnit treating it as a lifecycle method, so declaring only half the - * pair starts a server per test and stops none. + * Both fixture methods are re-annotated because the base class drops the annotation when it overrides + * {@code tearDownGlobal}, so declaring only half the pair starts a server per test and stops none. */ public class WebSocketRedirectRefusalTest extends AbstractBasicWebSocketTest {