fix(netty): preserve non-POST redirect methods

[mkurz]

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 actually selected for transmission and reject obviously non-replayable 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 actually sent; 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. 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 without 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. 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.
• ./mvnw -pl client -Dtest=RedirectBodyTest,RedirectCredentialSecurityTest test on JDK 11: 71 tests passed.
• ./mvnw clean verify on JDK 11: BUILD SUCCESS (full reactor, including tests, Javadocs, artifact signing, coverage, and Revapi).

Generated with OpenAI Codex.

[hyperxpro]

Overall looks solid

[hyperxpro]

schemeDowngrade is computed below but only feeds stripAuth, so an https to http 301 now replays the body in cleartext. The JDK client refuses that hop completely. Can we gate keepBody on the scheme as well ? 307 and 308 have the same problem already so maybe that part is a follow up.

[mkurz]

Thanks, this is a real concern. I did not gate keepBody on the scheme because that would preserve PUT/QUERY/etc. while silently deleting their payload, which recreates the data-corruption shape this change is intended to remove. If AHC restricts HTTPS-to-HTTP body replay, I think it should refuse the redirect itself and apply the policy uniformly to every keep-body redirect, including strict 302, 307, and 308, which can already replay bodies on a downgrade.

I updated the PR description to disclose that 301/302 can now replay content on a downgrade where the old method/body rewrite discarded it. I agree that a uniform downgrade policy is best handled as a focused follow-up rather than changing only these newly corrected paths here.

[hyperxpro]

bodylessMethod is not accurate, we do support GET and OPTIONS with a body, see BasicHttpTest.getShouldAllowBody. So the same GET keeps its body on 307 but loses it on 301. Neither changes the method, so why drop it ?

[mkurz]

Fixed in ba32a32. I renamed bodylessMethod to methodAlreadyPreserved and now derive body handling from the redirect decision: the body is kept whenever the method is preserved, except for 303. I added 301/302 coverage for GET, HEAD, and OPTIONS with explicit bodies; all three retain their method, content type, and bytes.

[hyperxpro]

REDIRECT_STATUSES is public and mutable, so a registered 300 keeps the method here but loses the body and you get a PUT with Content-Length: 0. Should this keep the body for anything that is not 303 and not POST, instead of listing the two ?

[mkurz]

Fixed in ba32a32. I used statusCode != SEE_OTHER_303 && !switchToGet for keepBody, which is slightly broader than "not 303 and not POST": it keeps the body whenever the method is preserved. That distinction matters for a caller-added 300 with POST; excluding POST there would retain POST while sending Content-Length: 0, the same corruption we want to avoid. The regression test temporarily registers 300 and covers both POST and PUT, asserting the original method, content type, and body bytes.

[hyperxpro]

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

[mkurz]

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.

[hyperxpro]

This pins the body being replayed cross origin as expected behaviour, under a name that reads like a hardening test. Can we rename it to say what it asserts ?

[mkurz]

Renamed in 2710101 to put301CrossOriginReplaysBodyAndStripsCredentials, so the test name states both sides of the behavior it pins rather than reading only as a hardening test.

[hyperxpro]

These pass even if we stopped sending Authorization on the first hop. crossDomainRedirectStripsCookieHeader records the value on A and asserts it first, we should do the same here.

[mkurz]

Fixed in 2710101. The origin handler now records Authorization and Cookie independently of the target handler, and the test first asserts the exact values on the original PUT before asserting that both are absent from the redirected request. It therefore fails if either credential was never sent on the first hop.

[hyperxpro]

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]

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.

[hyperxpro]

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]

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.