fix(netty): keep caller Cookie headers as written on redirects and auth retries - #2360

Summary

• On a redirect or an authentication retry, keep a caller-set Cookie header as text, removing only the pairs that the response replaced, instead of decoding it leniently and re-encoding it strictly.
• Find those pairs by splitting at every ;, as NettyRequestFactory.mergeCookies does, so that a quote in front of a replaced pair cannot hide it and resend a stale session. Both now share one split.
• Count a Set-Cookie deletion only where its Domain and Path apply, so that a deletion for another path no longer cuts the caller's cookie and lets an older stored one take its place.
• No breaking API change: two helpers are added to HttpUtils. Follow-up to fd97636 (GHSA-2jwh-9rmr-j4xf).

Problem

Since fd97636, a caller-set Cookie header is kept when the cookie store also contributes cookies to a hop, with the pairs that the response replaced removed. The cookie store is enabled by default (ThreadSafeCookieStore), so this path covers ordinary clients. It applies to redirects and to authentication retries: 401 (Basic, Digest, NTLM) and proxy 407.

To find the replaced pairs, CallerCookies decoded the caller's header leniently and re-encoded the remaining pairs strictly. That broke headers that the first request had sent without complaint:

• A value that the strict encoder rejects failed the redirect or retry, although the initial request had been sent.
• Pairs could be reordered, and valueless or unbalanced-quote pairs were dropped.

Review also found that CallerCookies counted every Set-Cookie with Max-Age<=0 as replacing the caller's cookie of that name, whatever its Domain and Path. Set-Cookie: SID=; Path=/elsewhere; Max-Age=0 on a same-origin 302, 401 or 407 response cut the caller's SID=mine, whether it was set as a cookie object or in a Cookie header. If the store held a SID for the next request, that older value was sent in its place. main behaves the same way.

Change

• Keep the caller-set header as text: remove only the pairs whose names the response replaced. Cookie objects are still encoded normally, and caller cookies are still stripped at cross-origin redirect boundaries.
• Split at every ;, as mergeCookies does when it decides which names the caller set. RFC 6265 does not allow a ; in a cookie value, quoted or not. Keeping the text while splitting like the lenient decoder, which treats a double quote as opening a quoted string, would be unsafe. A header such as X=a"b; SID=old or X="a; SID=old would become a single pair named X. A SID that a redirect, 401 or 407 response rotated, or deleted with Max-Age=0, would stay in the header. mergeCookies would still count it as the caller's and drop the store's new value, so the next request would carry the stale session. Tests pin these cases.
• One split for both: HttpUtils.cookiePairs and HttpUtils.cookiePairName now split and name the pairs for mergeCookies and CallerCookies. CallerCookies still folds names as the store does, while mergeCookies compares them as written; the Javadoc says so.
• Cut out only the replaced pairs and keep the rest as written, separators included, so prefs={"a":"x;y"} or A=1;B=2 is not rewritten.
• Return a header that loses no pair unchanged, instead of re-joining it with "; ". A header's pair names are checked before anything is built.
• Count a deletion only where it applies: its Domain and Path must match the next request or the request it answered, as the store matches them (RFC 6265 sections 5.1.3 and 5.1.4). Cookie objects and Cookie header pairs follow the same rule.
  • Matching only the next request is not enough. Take Set-Cookie: S=; Path=/app; Max-Age=0 on a logout that redirects to /login. The deleted cookie was sent with the logout request, either added by the store or as a caller's pair that may be a copy of it. Once it is gone from the store, it can no longer be told from a caller's own cookie, and it would follow the redirect.
  • The store's other checks are not repeated, such as a public suffix as Domain or a Secure cookie over plaintext.
• A Set-Cookie that is not a deletion counts only if it applies to the next request too, not just because a stored cookie for that request has the same value.
• One store lookup per hop for the next request's cookies. A retry, which goes to the same URI again, reuses it for the current request.
• Restore the Javadoc of CallerCookies.of. In Redirect30xInterceptor, remove a block that the new code made dead, and update a comment that described only the case without a cookie store.

Behavior changes

• A caller-set Cookie header reaches the next hop of a redirect or authentication retry as written, minus the pairs the response replaced. Before, it could be reordered, lose pairs, or fail the hop.
• A raw pair equal to a stored cookie now follows a same-origin redirect as written. Seed P=scoped; Path=/p, then GET /p/a with Cookie: P=scoped and a 302 to /q/b: main sends no P, because the pair counted as the store's and was dropped outside its Path. This change sends P=scoped, since the caller wrote it.
• A deletion whose Domain or Path matches neither the request it answered nor the next one no longer removes the caller's cookie, as an object or in a Cookie header.
• Unchanged: a deletion that does apply still removes the caller's cookie of that name. If the store keeps a cookie of that name for the next request, for another path, that one is sent instead, as on main and as a browser would.
• Not covered, as on main: the store's other reasons to refuse a cookie are not repeated.
  • A deletion that matches by Domain and Path but that the store refuses still removes the caller's cookie. Examples are a public suffix as Domain, or a Secure cookie over plaintext.
  • So does a refused Set-Cookie that is not a deletion, if the store already holds its value for the next request.
  • Covering these would need the store's own decision on each cookie.

Compatibility

No breaking API change. HttpUtils gains cookiePairs and cookiePairName.

AI disclosure

OpenAI Codex and Claude Code on behalf of Matthias Kurz. The first commit was written in two steps, the first with Codex and the second with Claude Code, and squashed; it carries both Co-Authored-By trailers per AGENTS.md. The review fixes were made with Claude Code, and their commits carry its Co-Authored-By trailer.

Test plan

New tests in RedirectCookieRotationTest and AuthRetryCookieTest cover:

• redirects with cookie rotation, and Digest, proxy 407 and NTLM (both rounds) retries;
• the lax encoder, several caller Cookie headers, and a raw pair deleted with Max-Age=0;
• quotes in front of a replaced pair, and byte-exact kept text;
• deletions:
  • for another path, or for another Domain, on a redirect and on a 401 retry, for cookie objects and header pairs, with and without a stored cookie of that name;
  • a logout that deletes a path-scoped cookie and redirects elsewhere, for cookie objects and header pairs, also beside a second stored cookie of that name;
• a non-deleting Set-Cookie for another path with the value of a stored cookie;
• a raw pair equal to a stored, path-scoped cookie, which follows a redirect out of its Path.

HttpUtilsTest covers the shared split. The proxy 407 test no longer sets a cookie, so it does not pin that a proxy's Set-Cookie is stored against the origin. It fails on main, where the strict encoder rejects the value.

Broken variants of the deletion rule, each run against RedirectCookieRotationTest and AuthRetryCookieTest:

• Every deletion counted, as on main: 2 failures: the deletion for another path on a 401 retry, and the deletion for another Domain.
• A deletion counted only if it applies to the next request: 2 failures, both logout tests: the deleted P=scoped follows the redirect.
• The previous revision's rule, which kept a header pair while the store held a cookie of that name for the next request: 1 failure, the logout beside a second stored cookie: the caller's P=scoped follows instead of the stored P=root.
• The Domain not checked: 1 failure, the deletion with Domain=example.org.
• A non-deleting Set-Cookie counted without checking its Path: 1 failure, the Set-Cookie for /elsewhere with the value of the stored cookie.

Runs:

• Each new commit compiles with its tests.
• ./mvnw -B -ntp -Dgpg.skip=true clean verify on JDK 11: 1,814 tests, 0 failures, 0 errors, 11 skipped; Revapi passed. No test-skipping flags were used.
• The cookie and redirect tests, compiled on JDK 11 and run on JDK 17, 21 and 25 (Maven Surefire's -Djvm): 270 tests each, 0 failures.
• All four open AHC changes merged together (raw Cookie headers on retries, ResponseBodyControl.execute, suspend/resume order, demand-bounded decompression): they merge without conflicts, together and in every pair, and ./mvnw -B -ntp -Dgpg.skip=true clean install on JDK 11 passes with 1,867 tests, 0 failures, 0 errors, 11 skipped; Revapi passed.
• Play WS against that merged build, Scala 2.13 and 3.3, JDK 17: all tests pass. A Play WS test sends Cookie: z=1; flash=stale; a=2; flag through a same-origin redirect that sets flash, with the cookie store enabled. The next hop receives z=1; a=2; flag; flash=redirect-cookie. Against AHC without this change, the valueless flag is dropped and the test fails.