Skip to content

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

Merged
hyperxpro merged 6 commits into
AsyncHttpClient:mainfrom
mkurz:fix/non-post-redirects
Sep 9, 2026
Merged

fix(netty): preserve non-POST redirect methods#2325
hyperxpro merged 6 commits into
AsyncHttpClient:mainfrom
mkurz:fix/non-post-redirects

Conversation

@mkurz

@mkurz mkurz commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Limit AHC's historical 301/302 POST-to-GET rewrite to POST requests.
  • Keep request content whenever a redirect preserves the request method, except for 303's explicit body-dropping behavior.
  • Validate only the body representation that AHC selected for transmission and reject consumed, non-resettable streams before connecting to the redirect target.
  • Cover standard methods, extension methods, caller-added redirect statuses, and cross-origin behavior.

Problem

Redirect30xInterceptor applied the historical POST-to-GET behavior for 301 and non-strict 302 responses to every method other than GET, HEAD, and OPTIONS. A PUT, PATCH, DELETE, or extension-method request therefore became a bodyless GET after either redirect.

The body decision was also coupled to a fixed list of methods and status codes. GET, HEAD, and OPTIONS requests can mechanically carry bodies in AHC, but those bodies were dropped on 301 and 302 even though the methods were retained. A caller-added status in the public mutable REDIRECT_STATUSES set likewise retained the method while silently dropping its body.

RFC 9110 sections 15.4.2 and 15.4.3 scope the compatibility allowance to changing POST to GET. They do not permit rewriting every other method in the same way. RFC 10008 section 2.5 explicitly requires QUERY not to use the POST exceptions.

Change

Apply the legacy 301/302 rewrite only when the original method is POST. Derive body handling from the method decision: keep the body whenever the method is preserved, except for 303, which drops the body regardless. This differs deliberately from checking whether a request is "not POST": a POST receiving a caller-added status such as 300 also keeps its method and must therefore keep its body.

The resulting behavior is:

  • POST on 301 or non-strict 302: switch to GET and drop the body.
  • POST on strict 302: retain POST and the body.
  • PUT, PATCH, DELETE, QUERY, and extension methods on 301 or 302: retain the method and body.
  • GET, HEAD, and OPTIONS on 301 or 302: retain the method and any explicitly attached body.
  • 303: drop the body; methods that require rewriting switch to GET, while GET, HEAD, and OPTIONS remain unchanged.
  • 307 and 308: retain the method and body under the existing policy.
  • Caller-added redirect statuses: retain the body whenever the method is retained.

This is a deliberate compatibility change. It corrects the standards scope and removes method-preserving, body-dropping combinations while retaining the long-established POST behavior for 301 and non-strict 302.

Redirect-body validation

The replay checks now use one private BodyRepresentation selection that mirrors NettyRequestFactory.body() precedence. This matters because some request-builder setters leave lower-priority representations in place. Validation now examines only the body AHC selected for transmission; for example, a stale multipart InputStreamPart no longer rejects a redirect whose selected body is a byte array.

Selected raw InputStream bodies and InputStreamBodyGenerator instances that report no mark/reset support are rejected before AHC connects to the redirect target only if the stream has already been consumed. An early redirect in response to Expect: 100-continue can leave the stream untouched, so the target can still receive its first transmission without reset support. This preflight is intentionally partial: a stream may report mark support yet be closed or fail to reset after the first send. The existing write-time replay guard remains the final authority for those cases. A generic BodyGenerator can also produce an unknown-length or non-repeatable body, but discovering that would require calling its one-shot createBody() early, so its existing write-time behavior is unchanged.

Redirect security

For a cross-origin 301 or 302, newly preserved request content is replayed to the redirect target. This follows the body-replay trust model AHC already uses for strict 302, 307, and 308. Existing redirect security continues to strip Authorization, Proxy-Authorization, Realm credentials, user-supplied Cookie headers, and Cookie objects when the origin changes. Tests assert that credentials reach the original server, do not reach the target, and that the method, content type, and body reach the target intact.

The same consideration applies to HTTPS-to-HTTP redirects: this pull request can replay content on 301 or 302 that the previous method/body rewrite discarded. Gating only keepBody on a scheme downgrade would preserve the method while silently deleting its payload, reproducing the data-corruption shape this change removes. If AHC adopts a downgrade restriction, it should refuse the redirect itself and apply uniformly to all keep-body statuses, including strict 302, 307, and 308. That transport-policy decision is left to a focused follow-up; this pull request does not alter AHC's existing downgrade policy.

History checked

The broad conversion is established behavior rather than a recent accident. Issue #989 requested browser-compatible 301 handling, issue #1042 retained the body drop for POST, and pull request #1736 later exempted HEAD and OPTIONS from method rewriting. This change retains the established POST rule while making body preservation follow the resulting method decision. A search did not find an existing issue or pull request specifically correcting PUT, PATCH, DELETE, extension methods, or caller-added redirect statuses.

Compatibility

There is no public API change.

The following redirect behavior changes intentionally:

  • PUT, PATCH, DELETE, and extension methods on 301 and non-strict 302 retain their original method and content instead of becoming bodyless GET requests.
  • GET, HEAD, and OPTIONS requests with explicitly attached content retain it on 301 and 302 instead of sending a bodyless second request.
  • Requests followed through caller-added redirect statuses retain content whenever their method is retained, including POST on a registered 300.
  • A selected raw InputStream or InputStreamBodyGenerator that has already been consumed and lacks mark/reset support now fails before connecting to the redirect target. For newly preserved non-POST 301/302 requests, this replaces the previous silent success as a bodyless GET with an explicit replay failure. Existing keep-body redirects fail earlier and use the new redirect-level error message. Untouched streams deferred by Expect: 100-continue remain usable at the redirect target. Streams that advertise mark support but cannot actually reset still fail at write time.
  • A request with a replayable selected body and a stale, lower-priority non-replayable representation no longer fails validation.
  • Cross-origin and HTTPS-to-HTTP 301/302 redirects can now receive content that the old body-dropping behavior suppressed; credential stripping remains unchanged.

POST on 301 and non-strict 302, strict-302 method policy, 303 method policy, and 307/308 method policy remain unchanged.

AI disclosure

OpenAI Codex on behalf of Matthias Kurz. The commits include Co-Authored-By: OpenAI Codex <codex@openai.com> per AGENTS.md.

Test plan

  • The new focused regressions reproduced eight body-loss failures for GET, HEAD, OPTIONS, and caller-added 300 before the generalized body rule was applied.
  • The selected-body coexistence regression reproduced the stale multipart InputStreamPart false rejection before validation was aligned with outbound-body precedence.
  • Both deferred-stream regressions failed with the unconditional preflight and pass with the consumption check. They assert no stream reads or body bytes at the origin before its 307 response and exact body bytes at the target, for raw InputStream and InputStreamBodyGenerator bodies.
  • ./mvnw -pl client -Dtest=RedirectBodyTest,RedirectCredentialSecurityTest test on JDK 11: 73 tests passed.
  • ./mvnw clean verify on JDK 11: BUILD SUCCESS (full reactor, including tests, Javadocs, artifact signing, coverage, and Revapi).

Generated with OpenAI Codex.

RFC 9110 scopes the historical 301 and 302 POST-to-GET rewrite to
POST. AHC applied it to other methods, silently dropping content from
PUT, PATCH, DELETE, and extension requests.

Retain the established POST behavior while repeating those non-POST
requests with their bodies. Cross-origin redirects still strip
credentials even though request content is replayed.

OpenAI Codex on behalf of Matthias Kurz.

Co-Authored-By: OpenAI Codex <codex@openai.com>

@hyperxpro hyperxpro left a comment

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.

Overall looks solid

Comment thread client/src/test/java/org/asynchttpclient/RedirectBodyTest.java
Comment thread client/src/test/java/org/asynchttpclient/RedirectBodyTest.java
mkurz and others added 4 commits September 6, 2026 01:19
Keep request content whenever a redirect preserves its method, except
for the explicit body-dropping semantics of 303. This covers GET, HEAD,
OPTIONS, and caller-added redirect statuses while retaining the
historical POST rewrite for 301 and non-strict 302.

Co-Authored-By: OpenAI Codex <codex@openai.com>
Request builders can retain lower-priority body representations. Reuse
the outbound body precedence for redirect checks so stale multipart
streams do not reject replayable byte-array redirects.

Co-Authored-By: OpenAI Codex <codex@openai.com>
Reject selected raw streams and InputStream body generators that declare
no mark/reset support before opening the redirect target. The write-time
reset remains the final check for streams that advertise support but
cannot actually reset after their first send.

Co-Authored-By: OpenAI Codex <codex@openai.com>
Prove that a cross-origin PUT redirect receives credentials only on its
original leg while preserving the method, content type, and body on the
target leg. Also cover body preservation when the redirect changes the
hostname without changing the server.

Co-Authored-By: OpenAI Codex <codex@openai.com>
@mkurz
mkurz requested a review from hyperxpro September 5, 2026 23:44
@mkurz

mkurz commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@hyperxpro looking forward to your review ;)

@hyperxpro

Copy link
Copy Markdown
Member

@hyperxpro looking forward to your review ;)

Sorry took longer than expected

An early redirect can arrive before an Expect: 100-continue body is
written. Only require reset support after stream transmission starts,
matching the existing write-time guard and allowing the target to read
an untouched stream for the first time.

Cover raw streams and InputStreamBodyGenerator with a 307 origin that
receives no body before redirecting. Multipart validation retains its
separate rules because it does not update the stream-consumed flag.

Fix the deferred-stream regression reported in PR AsyncHttpClient#2325.

OpenAI Codex on behalf of Matthias Kurz.

Co-Authored-By: OpenAI Codex <codex@openai.com>
@mkurz

mkurz commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

@hyperxpro looking forward to your review ;)

Sorry took longer than expected

No worries at all - thanks for taking the time to test both stream representations and catch this.

@mkurz
mkurz requested a review from hyperxpro September 9, 2026 21:29
@mkurz

mkurz commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

@hyperxpro assuming this gets merged, would it be possible to cut a new release so I can use it downstream? Thanks!

@hyperxpro

Copy link
Copy Markdown
Member

@hyperxpro assuming this gets merged, would it be possible to cut a new release so I can use it downstream? Thanks!

I plan to cut release next week as there are some fixes I need to perform. Is it possible to use SNAPSHOT till then?

@hyperxpro
hyperxpro merged commit 05efb00 into AsyncHttpClient:main Sep 9, 2026
13 checks passed
@hyperxpro

Copy link
Copy Markdown
Member

Thanks a lot!

@mkurz

mkurz commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

@hyperxpro assuming this gets merged, would it be possible to cut a new release so I can use it downstream? Thanks!

I plan to cut release next week as there are some fixes I need to perform. Is it possible to use SNAPSHOT till then?

Next week is ok, thanks!

@mkurz
mkurz deleted the fix/non-post-redirects branch September 9, 2026 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants