Skip to content

fix(netty): preserve non-POST redirect methods #2325

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mkurz wants to merge 5 commits into
base: main
Choose a base branch
from
+259 −65
Open

fix(netty): preserve non-POST redirect methods #2325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
015ed89
Preserve non-POST redirect methods
mkurz Aug 25, 2026
ba32a32
Preserve bodies for unchanged redirect methods
mkurz Sep 5, 2026
872800a
Validate only the selected redirect body
mkurz Sep 5, 2026
78930e8
Preflight non-replayable redirect bodies
mkurz Sep 5, 2026
2710101
Strengthen cross-origin redirect coverage
mkurz Sep 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 90 additions & 52 deletions client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.util.HashSet;
import java.util.Set;

Expand All @@ -56,7 +57,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.Methods.POST;
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;
Expand Down Expand Up @@ -117,20 +118,17 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture<?>
future.setScramContext(null);

String originalMethod = request.getMethod();
boolean isQuery = QUERY.equals(originalMethod);
boolean isPost = originalMethod.equals(POST);
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);
// RFC 9110 limits the historical 301/302 POST-to-GET rewrite to POST.

@hyperxpro hyperxpro Sep 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: can we keep the RFC 10008 reference ? QUERY is still correct after this but only as a side effect of !isPost now.

@mkurz mkurz Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kept in ba32a32. The policy comment now cites RFC 9110 for the POST-scoped historical rewrite and RFC 10008 section 2.5 for QUERY, even though QUERY is handled by the general non-POST rule.

// This also preserves QUERY as required by RFC 10008 section 2.5.
boolean legacyPostToGet = isPost && (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;
(statusCode == SEE_OTHER_303 || legacyPostToGet);
boolean keepBody = statusCode != SEE_OTHER_303 && !switchToGet;

HttpHeaders responseHeaders = response.headers();
String location = responseHeaders.get(LOCATION);
Expand Down Expand Up @@ -243,66 +241,106 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture<?>
}

private static void ensureBodyReplayable(Request request) throws IOException {
for (Part part : request.getBodyParts()) {
if (part instanceof InputStreamPart) {
throw new IOException("Multipart InputStream body part '" + part.getName()
+ "' cannot be replayed after redirect");
BodyRepresentation bodyRepresentation = selectedBodyRepresentation(request);
if (bodyRepresentation == BodyRepresentation.BODY_PARTS) {
for (Part part : request.getBodyParts()) {
if (part instanceof InputStreamPart) {
throw new IOException("Multipart InputStream body part '" + part.getName()
+ "' cannot be replayed after redirect");
}
}
}

File file = selectedBodyFile(request);
File file = null;
if (bodyRepresentation == BodyRepresentation.FILE) {
file = request.getFile();
} else if (bodyRepresentation == BodyRepresentation.FILE_BODY_GENERATOR) {
file = ((FileBodyGenerator) request.getBodyGenerator()).getFile();
}
if (file != null && !file.isFile()) {
throw new IOException("Redirect request body file " + file.getAbsolutePath()
+ " is not a file or does not exist");
}
}

private static File selectedBodyFile(Request request) {
// Keep this precedence aligned with NettyRequestFactory.body. A File can remain set alongside a
// higher-priority representation, so only validate it when the original request actually sent it.
if (hasBodyBeforeFile(request)) {
return null;
InputStream inputStream = null;
if (bodyRepresentation == BodyRepresentation.STREAM_DATA) {
inputStream = request.getStreamData();
} else if (bodyRepresentation == BodyRepresentation.INPUT_STREAM_BODY_GENERATOR) {
inputStream = ((InputStreamBodyGenerator) request.getBodyGenerator()).getInputStream();
}
if (request.getFile() != null) {
return request.getFile();
if (inputStream != null && !inputStream.markSupported()) {
throw new IOException("Redirect request body InputStream does not support mark/reset"
+ " and cannot be replayed");
}
return request.getBodyGenerator() instanceof FileBodyGenerator
? ((FileBodyGenerator) request.getBodyGenerator()).getFile()
: null;
}

private static boolean hasBodyBeforeFile(Request request) {
return hasBodyBeforeStream(request)
|| request.getStreamData() != null
|| !request.getFormParams().isEmpty()
|| !request.getBodyParts().isEmpty();
}

private static boolean hasBodyBeforeStream(Request request) {
return request.getByteData() != null
|| request.getCompositeByteData() != null
|| request.getStringData() != null
|| request.getByteBufferData() != null
|| request.getByteBufData() != null;
}

private static boolean selectedBodyHasUnknownLength(Request request) {
if (hasBodyBeforeStream(request)) {
return false;
private static BodyRepresentation selectedBodyRepresentation(Request request) {
// Keep this precedence aligned with NettyRequestFactory.body. Some setters leave lower-priority
// representations in place, so redirect validation must inspect only the body that was sent.
if (request.getByteData() != null) {
return BodyRepresentation.BYTE_DATA;
}
if (request.getCompositeByteData() != null) {
return BodyRepresentation.COMPOSITE_BYTE_DATA;
}
if (request.getStringData() != null) {
return BodyRepresentation.STRING_DATA;
}
if (request.getByteBufferData() != null) {
return BodyRepresentation.BYTE_BUFFER_DATA;
}
if (request.getByteBufData() != null) {
return BodyRepresentation.BYTE_BUF_DATA;
}
if (request.getStreamData() != null) {
return true;
return BodyRepresentation.STREAM_DATA;
}
if (!request.getFormParams().isEmpty()
|| !request.getBodyParts().isEmpty()
|| request.getFile() != null) {
return false;
if (!request.getFormParams().isEmpty()) {
return BodyRepresentation.FORM_PARAMS;
}
if (!request.getBodyParts().isEmpty()) {
return BodyRepresentation.BODY_PARTS;
}
if (request.getFile() != null) {
return BodyRepresentation.FILE;
}
if (request.getBodyGenerator() instanceof FileBodyGenerator) {
return BodyRepresentation.FILE_BODY_GENERATOR;
}
if (request.getBodyGenerator() instanceof InputStreamBodyGenerator) {
return BodyRepresentation.INPUT_STREAM_BODY_GENERATOR;
}
return request.getBodyGenerator() == null
? BodyRepresentation.NONE
: BodyRepresentation.BODY_GENERATOR;
}

private static boolean selectedBodyHasUnknownLength(Request request) {
BodyRepresentation bodyRepresentation = selectedBodyRepresentation(request);
if (bodyRepresentation == BodyRepresentation.STREAM_DATA
|| bodyRepresentation == BodyRepresentation.BODY_GENERATOR) {
return true;
}
if (bodyRepresentation == BodyRepresentation.INPUT_STREAM_BODY_GENERATOR) {
return ((InputStreamBodyGenerator) request.getBodyGenerator()).getContentLength() < 0;
}
return request.getBodyGenerator() != null
&& !(request.getBodyGenerator() instanceof FileBodyGenerator);
return false;
}

private enum BodyRepresentation {
BYTE_DATA,
COMPOSITE_BYTE_DATA,
STRING_DATA,
BYTE_BUFFER_DATA,
BYTE_BUF_DATA,
STREAM_DATA,
FORM_PARAMS,
BODY_PARTS,
FILE,
FILE_BODY_GENERATOR,
INPUT_STREAM_BODY_GENERATOR,
BODY_GENERATOR,
NONE
}

private static HttpHeaders propagatedHeaders(Request request, Realm realm, boolean keepBody, boolean stripAuthorization) {
Expand Down
119 changes: 106 additions & 13 deletions client/src/test/java/org/asynchttpclient/RedirectBodyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
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.netty.handler.intercept.Redirect30xInterceptor.REDIRECT_STATUSES;
import static org.asynchttpclient.util.HttpConstants.Methods.GET;
import static org.asynchttpclient.util.HttpConstants.Methods.POST;
import static org.asynchttpclient.util.HttpConstants.Methods.QUERY;
Expand All @@ -66,9 +67,12 @@ public class RedirectBodyTest extends AbstractBasicTest {

private static final byte[] REDIRECT_BODY = "redirect body".getBytes(UTF_8);
private static final String CONTENT_TYPE_VALUE = "application/octet-stream";
private static final String NON_REPLAYABLE_STREAM_MESSAGE =
"Redirect request body InputStream does not support mark/reset and cannot be replayed";

private static final List<String> receivedContentLengths = new CopyOnWriteArrayList<>();
private static volatile boolean redirectAlreadyPerformed;
private static volatile byte[] receivedBody;
private static volatile String receivedContentType;
private static volatile String receivedMethod;
private static volatile Path fileToDeleteBeforeRedirect;
Expand All @@ -77,6 +81,7 @@ public class RedirectBodyTest extends AbstractBasicTest {
public void setUp() {
receivedContentLengths.clear();
redirectAlreadyPerformed = false;
receivedBody = null;
receivedContentType = null;
receivedMethod = null;
fileToDeleteBeforeRedirect = null;
Expand All @@ -101,6 +106,7 @@ public void handle(String pathInContext, Request request, HttpServletRequest htt
httpResponse.setHeader(LOCATION.toString(), getTargetUrl());

} else {
receivedBody = body;
receivedContentType = request.getContentType();
receivedMethod = request.getMethod();
httpResponse.setStatus(200);
Expand Down Expand Up @@ -276,34 +282,110 @@ public synchronized void reset() throws IOException {
.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());
assertEquals(NON_REPLAYABLE_STREAM_MESSAGE, cause.getMessage());
}
}

@ParameterizedTest(name = "{0} on {1} keeps the existing GET rewrite")
@RepeatedIfExceptionsTest(repeats = 5)
public void put301WithNonRepeatableBodyGeneratorFailsPromptly() throws Exception {

@hyperxpro hyperxpro Sep 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ensureBodyReplayable only looks at InputStreamPart and File, so this one fails in NettyInputStreamBody.write after we already connected to the target. Can we check streamData and InputStreamBodyGenerator up front too ?

@mkurz mkurz Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in two commits. 872800a centralizes the selected body representation using the exact precedence from NettyRequestFactory.body() and routes multipart, file, and length checks through it. That also fixed a false rejection in the merged #2316 code: a stale lower-priority multipart InputStreamPart no longer rejects a redirect when the selected body is a replayable byte array, with a regression test for that coexistence case.

78930e8dc then preflights the selected raw streamData or InputStreamBodyGenerator. If it reports no mark/reset support, the interceptor now fails before connecting to the redirect target with Redirect request body InputStream does not support mark/reset and cannot be replayed.

The preflight is necessarily partial: a stream can advertise mark support but still be closed or fail to reset after the first write, so the existing write-time replay guard remains the final authority. A generic BodyGenerator can also conceal replayability until its one-shot createBody() is called, so that path retains its existing write-time behavior. The affected tests now assert the redirect-level preflight message.

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.preparePut(getTargetUrl())
.setBody(new InputStreamBodyGenerator(body))
.setHeader("X-REDIRECT", "301")
.execute()
.get(TIMEOUT, TimeUnit.SECONDS));

IOException cause = assertInstanceOf(IOException.class, thrown.getCause());
assertEquals(NON_REPLAYABLE_STREAM_MESSAGE, cause.getMessage());
}
}

@ParameterizedTest(name = "{0} on {1} keeps its method and body")
@CsvSource({
"PUT, 301",
"PUT, 302",
"PATCH, 301",
"PATCH, 302",
"DELETE, 301",
"DELETE, 302"
"DELETE, 302",
"CUSTOM, 301",
"CUSTOM, 302",
"GET, 301",
"GET, 302",
"HEAD, 301",
"HEAD, 302",
"OPTIONS, 301",
"OPTIONS, 302"
})
public void putPatchAndDelete301And302KeepExistingBehavior(String method, int statusCode) throws Exception {
public void nonPost301And302KeepMethodAndBody(String method, int statusCode) throws Exception {

@hyperxpro hyperxpro Sep 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These all use getTargetUrl() so every case here is same origin, which is also why the suite stays green. Can we add a row that redirects to a different host ?

@mkurz mkurz Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a dedicated different-host case in 2710101. It starts the PUT at 127.0.0.1 and follows the 301 Location to localhost, then asserts that the redirected request retains PUT, its content type, and its body bytes. I kept this as a separate test rather than adding an origin flag to every method/status row so the origin-boundary condition is explicit. The security test separately verifies credential stripping across a different-port origin.

try (AsyncHttpClient c = asyncHttpClient(config().setFollowRedirect(true))) {
String body = "hello there";
String contentType = "text/plain; charset=UTF-8";

Response response = c.prepare(method, getTargetUrl())
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);
assertArrayEquals(body.getBytes(UTF_8), receivedBody);
assertEquals(method, receivedMethod);
assertEquals(contentType, receivedContentType);
}
}

@RepeatedIfExceptionsTest(repeats = 5)
public void put301AcrossDifferentHostsKeepsMethodAndBody() throws Exception {
try (AsyncHttpClient c = asyncHttpClient(config().setFollowRedirect(true))) {
String body = "hello there";
String contentType = "text/plain; charset=UTF-8";
String originalUrl = getTargetUrl().replace("localhost", "127.0.0.1");

c.preparePut(originalUrl)
.setHeader(CONTENT_TYPE, contentType)
.setBody(body)
.setHeader("X-REDIRECT", "301")
.execute()
.get(TIMEOUT, TimeUnit.SECONDS);
assertArrayEquals(body.getBytes(UTF_8), receivedBody);
assertEquals("PUT", receivedMethod);
assertEquals(contentType, receivedContentType);
}
}

@ParameterizedTest(name = "{0} on caller-added 300 keeps its method and body")
@CsvSource({"POST", "PUT"})
public void callerAddedRedirectStatusKeepsMethodAndBody(String method) throws Exception {
boolean added = REDIRECT_STATUSES.add(300);
try (AsyncHttpClient c = asyncHttpClient(config().setFollowRedirect(true))) {
String body = "hello there";
String contentType = "text/plain; charset=UTF-8";

c.prepare(method, getTargetUrl())
.setHeader(CONTENT_TYPE, contentType)
.setBody(body)
.setHeader("X-REDIRECT", "300")
.execute()
.get(TIMEOUT, TimeUnit.SECONDS);
assertArrayEquals(body.getBytes(UTF_8), receivedBody);
assertEquals(method, receivedMethod);
assertEquals(contentType, receivedContentType);
} finally {
if (added) {
REDIRECT_STATUSES.remove(300);
}
}
}

Expand Down Expand Up @@ -447,8 +529,7 @@ public synchronized void reset() throws IOException {
() -> execute307(c.preparePost(getTargetUrl()).setBody(body)));

IOException cause = assertInstanceOf(IOException.class, thrown.getCause());
assertEquals("HTTP/1 request body InputStream already consumed and cannot be reset for a retry",
cause.getMessage());
assertEquals(NON_REPLAYABLE_STREAM_MESSAGE, cause.getMessage());
}
}

Expand All @@ -463,8 +544,7 @@ public void fileInputStream307FailsPromptly() throws Exception {
() -> execute307(c.preparePost(getTargetUrl()).setBody(body)));

IOException cause = assertInstanceOf(IOException.class, thrown.getCause());
assertEquals("HTTP/1 request body InputStream already consumed and cannot be reset for a retry",
cause.getMessage());
assertEquals(NON_REPLAYABLE_STREAM_MESSAGE, cause.getMessage());
}
} finally {
Files.deleteIfExists(bodyFile);
Expand Down Expand Up @@ -527,6 +607,19 @@ public void coexistingFileAndByteArray308UsesByteArray() throws Exception {
}
}

@RepeatedIfExceptionsTest(repeats = 5)
public void coexistingMultipartStreamAndByteArray307UsesByteArray() throws Exception {
try (InputStream unusedPart = new ByteArrayInputStream("unused part".getBytes(UTF_8));
AsyncHttpClient c = asyncHttpClient(config().setFollowRedirect(true))) {
Response response = execute307(c.preparePost(getTargetUrl())
.setBody(REDIRECT_BODY)
.setBodyParts(List.of(new InputStreamPart("file", unusedPart, "unused.bin",
"unused part".length(), CONTENT_TYPE_VALUE))));

assertRedirectBody(response);
}
}

@RepeatedIfExceptionsTest(repeats = 5)
public void formParams307KeepBody() throws Exception {
try (AsyncHttpClient c = asyncHttpClient(config().setFollowRedirect(true))) {
Expand Down
Loading
Loading