From 94bacf1cb97a8f7dee5d1d7cda2eb139425f750e Mon Sep 17 00:00:00 2001 From: Matthias Kurz Date: Fri, 4 Sep 2026 11:24:10 +0200 Subject: [PATCH] Preserve QUERY redirects per RFC 10008 RFC 10008 requires QUERY requests to retain their method and content across 301, 302, 307, and 308 redirects. AHC treated QUERY like POST on 301 and non-strict 302, changing it to GET and dropping its content. Preserve QUERY while keeping established behavior for other methods. Add the standardized method constant and regression coverage for every redirect status, strict 302, repeatable and non-repeatable bodies, and cross-origin credential stripping. Cross-origin QUERY redirects retain AHC's existing keep-body trust model. A broader change limiting legacy redirect rewrites to POST remains a separate compatibility decision. OpenAI Codex on behalf of Matthias Kurz. Co-Authored-By: OpenAI Codex --- .../intercept/Redirect30xInterceptor.java | 20 +- .../asynchttpclient/util/HttpConstants.java | 1 + .../org/asynchttpclient/RedirectBodyTest.java | 172 ++++++++++++++++++ .../RedirectCredentialSecurityTest.java | 54 ++++++ 4 files changed, 242 insertions(+), 5 deletions(-) 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 1b18cd9741..1450f23616 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 @@ -56,6 +56,7 @@ import static org.asynchttpclient.util.HttpConstants.Methods.GET; import static org.asynchttpclient.util.HttpConstants.Methods.HEAD; import static org.asynchttpclient.util.HttpConstants.Methods.OPTIONS; +import static org.asynchttpclient.util.HttpConstants.Methods.QUERY; import static org.asynchttpclient.util.HttpConstants.ResponseStatusCodes.FOUND_302; import static org.asynchttpclient.util.HttpConstants.ResponseStatusCodes.MOVED_PERMANENTLY_301; import static org.asynchttpclient.util.HttpConstants.ResponseStatusCodes.PERMANENT_REDIRECT_308; @@ -116,11 +117,20 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture future.setScramContext(null); String originalMethod = request.getMethod(); - boolean switchToGet = !originalMethod.equals(GET) && - !originalMethod.equals(OPTIONS) && - !originalMethod.equals(HEAD) && - (statusCode == MOVED_PERMANENTLY_301 || statusCode == SEE_OTHER_303 || statusCode == FOUND_302 && !config.isStrict302Handling()); - boolean keepBody = statusCode == TEMPORARY_REDIRECT_307 || statusCode == PERMANENT_REDIRECT_308 || statusCode == FOUND_302 && config.isStrict302Handling(); + boolean isQuery = QUERY.equals(originalMethod); + boolean methodAlreadyPreserved = originalMethod.equals(GET) || + originalMethod.equals(OPTIONS) || originalMethod.equals(HEAD); + boolean strict302 = statusCode == FOUND_302 && config.isStrict302Handling(); + // RFC 10008 section 2.5 excludes QUERY from the legacy POST-to-GET behavior. + boolean queryRedirect = isQuery && + (statusCode == MOVED_PERMANENTLY_301 || statusCode == FOUND_302); + boolean legacyRedirectToGet = statusCode == MOVED_PERMANENTLY_301 || + (statusCode == FOUND_302 && !strict302); + boolean switchToGet = !methodAlreadyPreserved && + (statusCode == SEE_OTHER_303 || (!isQuery && legacyRedirectToGet)); + boolean keepBody = queryRedirect || + statusCode == TEMPORARY_REDIRECT_307 || statusCode == PERMANENT_REDIRECT_308 || + strict302; HttpHeaders responseHeaders = response.headers(); String location = responseHeaders.get(LOCATION); diff --git a/client/src/main/java/org/asynchttpclient/util/HttpConstants.java b/client/src/main/java/org/asynchttpclient/util/HttpConstants.java index 4a1a128650..38fc140f78 100644 --- a/client/src/main/java/org/asynchttpclient/util/HttpConstants.java +++ b/client/src/main/java/org/asynchttpclient/util/HttpConstants.java @@ -33,6 +33,7 @@ public static final class Methods { public static final String PATCH = HttpMethod.PATCH.name(); public static final String POST = HttpMethod.POST.name(); public static final String PUT = HttpMethod.PUT.name(); + public static final String QUERY = HttpMethod.QUERY.name(); public static final String TRACE = HttpMethod.TRACE.name(); private Methods() { diff --git a/client/src/test/java/org/asynchttpclient/RedirectBodyTest.java b/client/src/test/java/org/asynchttpclient/RedirectBodyTest.java index 1360091e0b..6ab7fd3e23 100644 --- a/client/src/test/java/org/asynchttpclient/RedirectBodyTest.java +++ b/client/src/test/java/org/asynchttpclient/RedirectBodyTest.java @@ -23,12 +23,15 @@ import org.apache.commons.io.IOUtils; import org.asynchttpclient.filter.FilterContext; import org.asynchttpclient.filter.ResponseFilter; +import org.asynchttpclient.request.body.generator.ByteArrayBodyGenerator; import org.asynchttpclient.request.body.generator.InputStreamBodyGenerator; import org.asynchttpclient.request.body.multipart.InputStreamPart; import org.asynchttpclient.request.body.multipart.StringPart; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.AbstractHandler; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import java.io.ByteArrayInputStream; import java.io.FilterInputStream; @@ -49,6 +52,9 @@ import static io.netty.handler.codec.http.HttpHeaderNames.LOCATION; import static org.asynchttpclient.Dsl.asyncHttpClient; import static org.asynchttpclient.Dsl.config; +import static org.asynchttpclient.util.HttpConstants.Methods.GET; +import static org.asynchttpclient.util.HttpConstants.Methods.POST; +import static org.asynchttpclient.util.HttpConstants.Methods.QUERY; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; @@ -64,6 +70,7 @@ public class RedirectBodyTest extends AbstractBasicTest { private static final List receivedContentLengths = new CopyOnWriteArrayList<>(); private static volatile boolean redirectAlreadyPerformed; private static volatile String receivedContentType; + private static volatile String receivedMethod; private static volatile Path fileToDeleteBeforeRedirect; @BeforeEach @@ -71,6 +78,7 @@ public void setUp() { receivedContentLengths.clear(); redirectAlreadyPerformed = false; receivedContentType = null; + receivedMethod = null; fileToDeleteBeforeRedirect = null; } @@ -94,6 +102,7 @@ public void handle(String pathInContext, Request request, HttpServletRequest htt } else { receivedContentType = request.getContentType(); + receivedMethod = request.getMethod(); httpResponse.setStatus(200); httpResponse.setContentLength(body.length); if (body.length > 0) { @@ -114,6 +123,7 @@ public void regular301LosesBody() throws Exception { Response response = c.preparePost(getTargetUrl()).setHeader(CONTENT_TYPE, contentType).setBody(body).setHeader("X-REDIRECT", "301").execute().get(TIMEOUT, TimeUnit.SECONDS); assertEquals(response.getResponseBody(), ""); + assertEquals(GET, receivedMethod); assertNull(receivedContentType); } } @@ -126,6 +136,7 @@ public void regular302LosesBody() throws Exception { Response response = c.preparePost(getTargetUrl()).setHeader(CONTENT_TYPE, contentType).setBody(body).setHeader("X-REDIRECT", "302").execute().get(TIMEOUT, TimeUnit.SECONDS); assertEquals(response.getResponseBody(), ""); + assertEquals(GET, receivedMethod); assertNull(receivedContentType); } } @@ -138,10 +149,24 @@ public void regular302StrictKeepsBody() throws Exception { Response response = c.preparePost(getTargetUrl()).setHeader(CONTENT_TYPE, contentType).setBody(body).setHeader("X-REDIRECT", "302").execute().get(TIMEOUT, TimeUnit.SECONDS); assertEquals(response.getResponseBody(), body); + assertEquals(POST, receivedMethod); assertEquals(receivedContentType, contentType); } } + @RepeatedIfExceptionsTest(repeats = 5) + public void regular303SwitchesToGetAndLosesBody() throws Exception { + try (AsyncHttpClient c = asyncHttpClient(config().setFollowRedirect(true))) { + String body = "hello there"; + String contentType = "text/plain; charset=UTF-8"; + + Response response = c.preparePost(getTargetUrl()).setHeader(CONTENT_TYPE, contentType).setBody(body).setHeader("X-REDIRECT", "303").execute().get(TIMEOUT, TimeUnit.SECONDS); + assertEquals("", response.getResponseBody()); + assertEquals(GET, receivedMethod); + assertNull(receivedContentType); + } + } + @RepeatedIfExceptionsTest(repeats = 5) public void regular307KeepsBody() throws Exception { try (AsyncHttpClient c = asyncHttpClient(config().setFollowRedirect(true))) { @@ -150,10 +175,138 @@ public void regular307KeepsBody() throws Exception { Response response = c.preparePost(getTargetUrl()).setHeader(CONTENT_TYPE, contentType).setBody(body).setHeader("X-REDIRECT", "307").execute().get(TIMEOUT, TimeUnit.SECONDS); assertEquals(response.getResponseBody(), body); + assertEquals(POST, receivedMethod); assertEquals(receivedContentType, contentType); } } + @RepeatedIfExceptionsTest(repeats = 5) + public void regular308KeepsBody() throws Exception { + try (AsyncHttpClient c = asyncHttpClient(config().setFollowRedirect(true))) { + String body = "hello there"; + String contentType = "text/plain; charset=UTF-8"; + + Response response = c.preparePost(getTargetUrl()).setHeader(CONTENT_TYPE, contentType).setBody(body).setHeader("X-REDIRECT", "308").execute().get(TIMEOUT, TimeUnit.SECONDS); + assertEquals(body, response.getResponseBody()); + assertEquals(POST, receivedMethod); + assertEquals(contentType, receivedContentType); + } + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void query301KeepsMethodAndBody() throws Exception { + queryRedirectKeepsMethodAndBody(301, false); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void query302KeepsMethodAndBody() throws Exception { + queryRedirectKeepsMethodAndBody(302, false); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void query302StrictKeepsMethodAndBody() throws Exception { + queryRedirectKeepsMethodAndBody(302, true); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void query303SwitchesToGetAndDropsBody() throws Exception { + try (AsyncHttpClient c = asyncHttpClient(config().setFollowRedirect(true))) { + String body = "hello there"; + String contentType = "text/plain; charset=UTF-8"; + + Response response = c.prepare(QUERY, getTargetUrl()) + .setHeader(CONTENT_TYPE, contentType) + .setBody(body) + .setHeader("X-REDIRECT", "303") + .execute() + .get(TIMEOUT, TimeUnit.SECONDS); + assertEquals("", response.getResponseBody()); + assertEquals(GET, receivedMethod); + assertNull(receivedContentType); + } + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void query307KeepsMethodAndBody() throws Exception { + queryRedirectKeepsMethodAndBody(307, false); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void query308KeepsMethodAndBody() throws Exception { + queryRedirectKeepsMethodAndBody(308, false); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void query301KeepsRepeatableBodyGenerator() throws Exception { + try (AsyncHttpClient c = asyncHttpClient(config().setFollowRedirect(true))) { + byte[] body = "hello there".getBytes(UTF_8); + String contentType = "text/plain; charset=UTF-8"; + + Response response = c.prepare(QUERY, getTargetUrl()) + .setHeader(CONTENT_TYPE, contentType) + .setBody(new ByteArrayBodyGenerator(body)) + .setHeader("X-REDIRECT", "301") + .execute() + .get(TIMEOUT, TimeUnit.SECONDS); + assertEquals("hello there", response.getResponseBody()); + assertEquals(QUERY, receivedMethod); + assertEquals(contentType, receivedContentType); + } + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void query301WithNonRepeatableBodyGeneratorFailsPromptly() throws Exception { + try (InputStream body = new FilterInputStream(new ByteArrayInputStream(REDIRECT_BODY)) { + @Override + public boolean markSupported() { + return false; + } + + @Override + public synchronized void reset() throws IOException { + throw new IOException("reset not supported"); + } + }; + AsyncHttpClient c = asyncHttpClient(config().setFollowRedirect(true))) { + ExecutionException thrown = assertThrows(ExecutionException.class, + () -> c.prepare(QUERY, getTargetUrl()) + .setBody(new InputStreamBodyGenerator(body)) + .setHeader("X-REDIRECT", "301") + .execute() + .get(TIMEOUT, TimeUnit.SECONDS)); + + IOException cause = assertInstanceOf(IOException.class, thrown.getCause()); + assertEquals("HTTP/1 request body InputStream already consumed and cannot be reset for a retry", + cause.getMessage()); + } + } + + @ParameterizedTest(name = "{0} on {1} keeps the existing GET rewrite") + @CsvSource({ + "PUT, 301", + "PUT, 302", + "PATCH, 301", + "PATCH, 302", + "DELETE, 301", + "DELETE, 302" + }) + public void putPatchAndDelete301And302KeepExistingBehavior(String method, int statusCode) throws Exception { + try (AsyncHttpClient c = asyncHttpClient(config().setFollowRedirect(true))) { + String body = "hello there"; + String contentType = "text/plain; charset=UTF-8"; + + Response response = c.prepare(method, getTargetUrl()) + .setHeader(CONTENT_TYPE, contentType) + .setBody(body) + .setHeader("X-REDIRECT", Integer.toString(statusCode)) + .execute() + .get(TIMEOUT, TimeUnit.SECONDS); + assertEquals("", response.getResponseBody()); + assertEquals(GET, receivedMethod); + assertNull(receivedContentType); + } + } + @RepeatedIfExceptionsTest(repeats = 5) public void redirectPreservesPerRequestSettings() throws Exception { Duration readTimeout = Duration.ofSeconds(7); @@ -424,6 +577,25 @@ public void inputStreamMultipart307FailsPromptly() throws Exception { } } + private void queryRedirectKeepsMethodAndBody(int statusCode, boolean strict302Handling) throws Exception { + try (AsyncHttpClient c = asyncHttpClient(config() + .setFollowRedirect(true) + .setStrict302Handling(strict302Handling))) { + String body = "hello there"; + String contentType = "text/plain; charset=UTF-8"; + + Response response = c.prepare(QUERY, getTargetUrl()) + .setHeader(CONTENT_TYPE, contentType) + .setBody(body) + .setHeader("X-REDIRECT", Integer.toString(statusCode)) + .execute() + .get(TIMEOUT, TimeUnit.SECONDS); + assertEquals(body, response.getResponseBody()); + assertEquals(QUERY, receivedMethod); + assertEquals(contentType, receivedContentType); + } + } + private static Response execute307(BoundRequestBuilder requestBuilder) throws Exception { return requestBuilder .setHeader(CONTENT_TYPE, CONTENT_TYPE_VALUE) diff --git a/client/src/test/java/org/asynchttpclient/RedirectCredentialSecurityTest.java b/client/src/test/java/org/asynchttpclient/RedirectCredentialSecurityTest.java index 99119925ed..faada9961c 100644 --- a/client/src/test/java/org/asynchttpclient/RedirectCredentialSecurityTest.java +++ b/client/src/test/java/org/asynchttpclient/RedirectCredentialSecurityTest.java @@ -36,6 +36,7 @@ import java.util.concurrent.atomic.AtomicReference; import static org.asynchttpclient.Dsl.basicAuthRealm; +import static org.asynchttpclient.util.HttpConstants.Methods.QUERY; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -65,6 +66,11 @@ public class RedirectCredentialSecurityTest { private static final AtomicReference cookieOn307Target = new AtomicReference<>(); private static final AtomicReference authOn308Target = new AtomicReference<>(); private static final AtomicReference bodyOn308Target = new AtomicReference<>(); + private static final AtomicReference query301AuthOnTarget = new AtomicReference<>(); + private static final AtomicReference query301CookieOnTarget = new AtomicReference<>(); + private static final AtomicReference query301ContentTypeOnTarget = new AtomicReference<>(); + private static final AtomicReference query301MethodOnTarget = new AtomicReference<>(); + private static final AtomicReference query301BodyOnTarget = new AtomicReference<>(); private static final AtomicReference lastCookieHeaderOnA = new AtomicReference<>(); private static final AtomicReference lastCookieHeaderOnB = new AtomicReference<>(); private static final AtomicReference cookieAtChainStep2 = new AtomicReference<>(); @@ -189,6 +195,24 @@ public static void startServers() throws Exception { exchange.close(); }); + serverA.createContext("/redirect-query-301-to-b", exchange -> { + exchange.getRequestBody().readAllBytes(); + exchange.getResponseHeaders().add("Location", "http://127.0.0.1:" + portB + "/target-query-301"); + exchange.sendResponseHeaders(301, -1); + exchange.close(); + }); + + serverB.createContext("/target-query-301", exchange -> { + query301AuthOnTarget.set(exchange.getRequestHeaders().getFirst("Authorization")); + query301CookieOnTarget.set(exchange.getRequestHeaders().getFirst("Cookie")); + query301ContentTypeOnTarget.set(exchange.getRequestHeaders().getFirst("Content-Type")); + query301MethodOnTarget.set(exchange.getRequestMethod()); + query301BodyOnTarget.set(new String(exchange.getRequestBody().readAllBytes(), StandardCharsets.UTF_8)); + exchange.sendResponseHeaders(200, 0); + exchange.getResponseBody().close(); + exchange.close(); + }); + // Endpoint reused by the HTTPS-to-HTTP downgrade test (target on server B over plain HTTP) serverB.createContext("/target-after-downgrade", exchange -> { authAfterHttpsDowngrade.set(exchange.getRequestHeaders().getFirst("Authorization")); @@ -511,6 +535,36 @@ void redirect308CrossDomainStripsAuthButPreservesBody() throws Exception { } } + @Test + void query301CrossOriginStripsCredentialsAndPreservesRequest() throws Exception { + DefaultAsyncHttpClientConfig config = new DefaultAsyncHttpClientConfig.Builder() + .setFollowRedirect(true) + .build(); + try (DefaultAsyncHttpClient client = new DefaultAsyncHttpClient(config)) { + query301AuthOnTarget.set(null); + query301CookieOnTarget.set(null); + query301ContentTypeOnTarget.set(null); + query301MethodOnTarget.set(null); + query301BodyOnTarget.set(null); + + client.prepare(QUERY, "http://127.0.0.1:" + portA + "/redirect-query-301-to-b") + .setHeader("Authorization", "Bearer secret-token") + .setHeader("Cookie", "session=secret-session") + .setHeader("Content-Type", "application/query") + .setBody("sensitive-query") + .execute() + .get(5, TimeUnit.SECONDS); + + assertNull(query301AuthOnTarget.get(), + "Authorization must be stripped on a cross-origin QUERY redirect"); + assertNull(query301CookieOnTarget.get(), + "Cookie must be stripped on a cross-origin QUERY redirect"); + assertEquals(QUERY, query301MethodOnTarget.get()); + assertEquals("application/query", query301ContentTypeOnTarget.get()); + assertEquals("sensitive-query", query301BodyOnTarget.get()); + } + } + /** * Cross-domain redirect (different port) must strip a user-supplied Cookie header. * Regression test for GHSA-fmxf-pm6p-7xgm.