From feac41197aa99ab5b08bc6c6704a5a93df1079e3 Mon Sep 17 00:00:00 2001 From: Matthias Kurz Date: Wed, 7 Oct 2026 09:48:23 +0200 Subject: [PATCH 1/7] Keep caller Cookie headers as written on retries A redirect or authentication retry decoded a caller-set Cookie header leniently, then re-encoded it strictly, whenever the cookie store also contributed cookies to the hop. Values accepted on the first hop could therefore fail on the next one, pairs could be reordered, and valueless or unbalanced-quote pairs were dropped. Keep caller-set header pairs as text and cut out only the pairs whose names the response replaced. The rest stays as written, separators included, so prefs={"a":"x;y"} or A=1;B=2 is not rewritten, and a header that loses no pair is returned unchanged. Cookie objects are still encoded normally, and caller cookies are still stripped at cross-origin redirect boundaries. Find the pairs by splitting on every ';', as NettyRequestFactory.mergeCookies does when it decides which names the caller set; RFC 6265 does not allow a ';' in a cookie value, quoted or not. Treating a double quote as opening a quoted string would let a header such as X=a"b; SID=old hide a SID that a redirect, 401 or 407 response rotated or deleted, so that the next request resent the stale session. Restore the Javadoc of CallerCookies.of, and fix the comment on the Cookie header in Redirect30xInterceptor, which still said the caller's pairs travel in the cookie list. Add tests for redirects with cookie rotation, Digest, proxy and NTLM retries, the lax encoder, several caller Cookie headers, a raw pair deleted with Max-Age=0, quotes in front of a replaced pair, and byte-exact kept text. Follow-up to fd9763620 (GHSA-2jwh-9rmr-j4xf). OpenAI Codex and Claude Code on behalf of Matthias Kurz Co-Authored-By: Codex Co-Authored-By: Claude Opus 5.5 --- .../handler/intercept/CallerCookies.java | 109 +++++++++++---- .../intercept/Redirect30xInterceptor.java | 17 ++- .../intercept/AuthRetryCookieTest.java | 87 +++++++++++- .../intercept/RedirectCookieRotationTest.java | 128 ++++++++++++++++++ 4 files changed, 300 insertions(+), 41 deletions(-) diff --git a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/CallerCookies.java b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/CallerCookies.java index b38b1685a..84eeb826f 100644 --- a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/CallerCookies.java +++ b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/CallerCookies.java @@ -18,14 +18,12 @@ import io.netty.handler.codec.http.HttpResponse; import io.netty.handler.codec.http.cookie.ClientCookieDecoder; import io.netty.handler.codec.http.cookie.Cookie; -import io.netty.handler.codec.http.cookie.ServerCookieDecoder; import org.asynchttpclient.Request; import org.asynchttpclient.RequestBuilder; import org.asynchttpclient.cookie.CookieStore; import org.asynchttpclient.uri.Uri; import java.util.ArrayList; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Locale; @@ -44,16 +42,28 @@ private CallerCookies() { } /** - * The request's cookies minus the ones the store put there: those this response set, rotated or deleted, - * and those the store still holds with the same value. A caller's cookie sharing only a name with a stored - * one stays the caller's. + * The request's cookie objects minus the ones the store put there: those this response set, rotated or + * deleted, and those the store still holds with the same value. A caller's cookie sharing only a name with + * a stored one stays the caller's. A Cookie header the caller set is not among these; see + * {@link #headerCookies}. */ - static List of(Request request, HttpResponse response, CookieStore cookieStore, Uri next, - ClientCookieDecoder cookieDecoder) { - List cookies = withHeaderCookies(request); + private static List of(Request request, CookieStore cookieStore, Set setByResponse) { + List cookies = request.getCookies(); if (cookies.isEmpty()) { return cookies; } + List stored = cookieStore.get(request.getUri()); + List callers = new ArrayList<>(cookies.size()); + for (Cookie cookie : cookies) { + if (!setByResponse.contains(cookie.name().toLowerCase(Locale.ROOT)) && !holdsSameValue(stored, cookie)) { + callers.add(cookie); + } + } + return callers; + } + + private static Set replacedNames(HttpResponse response, CookieStore cookieStore, Uri next, + ClientCookieDecoder cookieDecoder) { // Only what the store took counts: a Set-Cookie it refused, or one for another path, leaves the // caller's cookie of that name in place. Names folded as the store keys them, so a SID set here also // replaces a request's sid. @@ -81,14 +91,7 @@ static List of(Request request, HttpResponse response, CookieStore cooki setByResponse.add(cookie.name().toLowerCase(Locale.ROOT)); } } - List stored = cookieStore.get(request.getUri()); - List callers = new ArrayList<>(cookies.size()); - for (Cookie cookie : cookies) { - if (!setByResponse.contains(cookie.name().toLowerCase(Locale.ROOT)) && !holdsSameValue(stored, cookie)) { - callers.add(cookie); - } - } - return callers; + return setByResponse; } /** @@ -96,28 +99,76 @@ static List of(Request request, HttpResponse response, CookieStore cooki */ static void refresh(RequestBuilder retry, Request request, HttpResponse response, CookieStore cookieStore, ClientCookieDecoder cookieDecoder) { - retry.setCookies(of(request, response, cookieStore, request.getUri(), cookieDecoder)); - retry.setHeader(COOKIE, Collections.emptyList()); - for (Cookie cookie : cookieStore.get(request.getUri())) { + refresh(retry, request, response, cookieStore, request.getUri(), cookieDecoder); + } + + static void refresh(RequestBuilder retry, Request request, HttpResponse response, CookieStore cookieStore, + Uri next, ClientCookieDecoder cookieDecoder) { + Set setByResponse = replacedNames(response, cookieStore, next, cookieDecoder); + retry.setCookies(of(request, cookieStore, setByResponse)); + retry.setHeader(COOKIE, headerCookies(request.getHeaders().getAll(COOKIE), setByResponse)); + for (Cookie cookie : cookieStore.get(next)) { retry.addCookieIfUnset(cookie); } } /** - * The request's cookies plus the pairs of a Cookie header the caller set. A hop drops the raw header and - * sends these instead, reconciled against the response, or the header would outrank a cookie it rotated. - * Only the request the caller built sends the header as written. + * The caller's Cookie header values minus the pairs this response replaced, kept as text: a lenient decode + * followed by strict encoding can reject a value that was sent on the first attempt. + *

+ * Every ';' ends a pair, inside quotes too: RFC 6265 does not allow one in a cookie value, and + * {@code NettyRequestFactory#mergeCookies} splits the same way when it decides which names the caller set. + * Honouring quotes here would let a stray '"' hide a replaced pair in its neighbour's value, to be resent + * stale while the merge still counts its name as the caller's and drops the store's new value. Only the + * replaced pairs are cut out, so the rest keeps its bytes and a header that loses none is returned as is. */ - private static List withHeaderCookies(Request request) { - List headers = request.getHeaders().getAll(COOKIE); - if (headers.isEmpty()) { - return request.getCookies(); + private static List headerCookies(List headers, Set setByResponse) { + if (setByResponse.isEmpty() || headers.isEmpty()) { + return headers; } - List cookies = new ArrayList<>(request.getCookies()); + List remaining = new ArrayList<>(headers.size()); for (String header : headers) { - cookies.addAll(ServerCookieDecoder.LAX.decodeAll(header)); + StringBuilder kept = new StringBuilder(header.length()); + boolean removed = false; + int start = 0; + while (start <= header.length()) { + int end = header.indexOf(';', start); + if (end < 0) { + end = header.length(); + } + int nameEnd = start; + while (nameEnd < end && header.charAt(nameEnd) != '=') { + nameEnd++; + } + String name = header.substring(start, nameEnd).trim(); + if (!name.isEmpty() && setByResponse.contains(name.toLowerCase(Locale.ROOT))) { + removed = true; + } else { + if (kept.length() > 0) { + kept.append(';'); + } + kept.append(header, start, end); + } + start = end + 1; + } + if (!removed) { + remaining.add(header); + continue; + } + // Cutting the first or last pair leaves the whitespace or ';' that separated it at that end. + int from = 0; + int to = kept.length(); + while (from < to && (kept.charAt(from) == ';' || kept.charAt(from) <= ' ')) { + from++; + } + while (to > from && (kept.charAt(to - 1) == ';' || kept.charAt(to - 1) <= ' ')) { + to--; + } + if (from < to) { + remaining.add(kept.substring(from, to)); + } } - return cookies; + return remaining; } private static boolean holdsSameValue(List stored, Cookie cookie) { 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 a409bfb02..d1ec092fd 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 @@ -207,9 +207,8 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture CookieStore cookieStore = config.getCookieStore(); if (stripAuth) { requestBuilder.resetCookies(); - } else { - requestBuilder.setCookies(cookieStore == null - ? request.getCookies() : CallerCookies.of(request, response, cookieStore, newUri, cookieDecoder)); + } else if (cookieStore == null) { + requestBuilder.setCookies(request.getCookies()); } requestBuilder.setMethod(switchToGet ? GET : originalMethod) @@ -253,9 +252,13 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture final boolean initialConnectionKeepAlive = future.isKeepAlive(); if (cookieStore != null) { - // Update request's cookies assuming that cookie store is already updated by Interceptors - for (Cookie cookie : cookieStore.get(newUri)) { - requestBuilder.addCookieIfUnset(cookie); + if (stripAuth) { + // Only cookies scoped to the new URI may cross this security boundary. + for (Cookie cookie : cookieStore.get(newUri)) { + requestBuilder.addCookieIfUnset(cookie); + } + } else { + CallerCookies.refresh(requestBuilder, request, response, cookieStore, newUri, cookieDecoder); } } @@ -481,7 +484,7 @@ private static HttpHeaders propagatedHeaders(Request request, Realm realm, boole .remove(PROXY_AUTHORIZATION); } if (cookiesReconciled) { - // CallerCookies carries its pairs in the cookie list. + // Unless stripped above, CallerCookies.refresh puts it back minus the pairs this response replaced. headers.remove(COOKIE); } return headers; diff --git a/client/src/test/java/org/asynchttpclient/netty/handler/intercept/AuthRetryCookieTest.java b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/AuthRetryCookieTest.java index 36d5a0e87..86034e01d 100644 --- a/client/src/test/java/org/asynchttpclient/netty/handler/intercept/AuthRetryCookieTest.java +++ b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/AuthRetryCookieTest.java @@ -30,17 +30,23 @@ import java.util.HashSet; import java.util.Set; import java.util.concurrent.TimeUnit; +import java.util.function.Function; import static org.asynchttpclient.Dsl.asyncHttpClient; import static org.asynchttpclient.Dsl.basicAuthRealm; +import static org.asynchttpclient.Dsl.digestAuthRealm; +import static org.asynchttpclient.Dsl.ntlmAuthRealm; +import static org.asynchttpclient.Dsl.proxyServer; import static org.junit.jupiter.api.Assertions.assertEquals; /** - * The retry after a 401 must carry the cookies the challenge set, not the ones the first attempt went out with. + * The retry after a 401 or 407 must carry the cookies the challenge set, not the ones the first attempt went out + * with. */ public class AuthRetryCookieTest extends AbstractBasicTest { private static final String RECEIVED_COOKIE = "received-cookie"; + private static final String NTLM_TYPE_2 = "TlRMTVNTUAACAAAAAAAAACgAAAABggAAU3J2Tm9uY2UAAAAAAAAAAA=="; @Override public AbstractHandler configureHandler() { @@ -48,12 +54,28 @@ public AbstractHandler configureHandler() { @Override public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException { + String authorization = request.getHeader("Authorization"); if ("/seed".equals(target)) { response.addHeader("Set-Cookie", "SID=old; Path=/"); - } else if (request.getHeader("Authorization") == null) { - response.addHeader("Set-Cookie", "SID=new; Path=/"); - response.setHeader("WWW-Authenticate", "Basic realm=\"test\""); - response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); + } else if ("/digest".equals(target) && authorization == null) { + challenge(response, HttpServletResponse.SC_UNAUTHORIZED, "WWW-Authenticate", "SID=new; Path=/", + "Digest realm=\"test\", nonce=\"dcd98b7102dd2f0e8b11d0f600bfb0c093\", qop=\"auth\""); + } else if ("/ntlm".equals(target) && authorization == null) { + challenge(response, HttpServletResponse.SC_UNAUTHORIZED, "WWW-Authenticate", "SID=mid; Path=/", + "NTLM"); + } else if ("/ntlm".equals(target) && authorization.startsWith("NTLM TlRMTVNTUAAB") + && "X=a b; SID=mid".equals(request.getHeader("Cookie"))) { + // The second round of the handshake rotates the session again. Only a first retry that + // carried the first rotation gets here; any other falls through and echoes what it sent. + challenge(response, HttpServletResponse.SC_UNAUTHORIZED, "WWW-Authenticate", "SID=new; Path=/", + "NTLM " + NTLM_TYPE_2); + } else if ("/proxied".equals(target) && request.getHeader("Proxy-Authorization") == null) { + // This server is the proxy too: the request is sent to it in absolute form. + challenge(response, HttpServletResponse.SC_PROXY_AUTHENTICATION_REQUIRED, "Proxy-Authenticate", + "SID=new; Path=/", "Basic realm=\"proxy\""); + } else if ("/protected".equals(target) && authorization == null) { + challenge(response, HttpServletResponse.SC_UNAUTHORIZED, "WWW-Authenticate", "SID=new; Path=/", + "Basic realm=\"test\""); } else { String cookie = request.getHeader("Cookie"); if (cookie != null) { @@ -87,6 +109,61 @@ void aCallerSetCookieHeaderDoesNotOutrankTheSessionTheChallengeSet() throws Exce } } + @Test + void aRawCookieWithANonStrictValueSurvivesTheRetry() throws Exception { + try (AsyncHttpClient client = asyncHttpClient()) { + String header = client.prepareGet(url("/protected")) + .setRealm(basicAuthRealm("user", "pass").setUsePreemptiveAuth(false)) + .setHeader("Cookie", "SID=old; X=a b") + .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); + assertEquals(new HashSet<>(Arrays.asList("X=a b", "SID=new")), new HashSet<>(Arrays.asList(header.split("; ")))); + } + } + + @Test + void aRawCookieSurvivesADigestRetryBesideTheSessionTheChallengeSet() throws Exception { + assertEquals("X=a b; SID=new", rawCookieOnRetry("SID=old; X=a b", client -> client.prepareGet(url("/digest")) + .setRealm(digestAuthRealm("user", "pass").setUsePreemptiveAuth(false)))); + } + + @Test + void aRawCookieSurvivesAProxyAuthenticationRetryBesideTheSessionTheChallengeSet() throws Exception { + assertEquals("X=a b; SID=new", rawCookieOnRetry("SID=old; X=a b", client -> client.prepareGet(url("/proxied")) + .setProxyServer(proxyServer("localhost", port2) + .setRealm(basicAuthRealm("user", "pass").setUsePreemptiveAuth(false))))); + } + + /** NTLM takes two challenges, and the retry after each carries what that one set. */ + @Test + void aRawCookieSurvivesEveryRoundOfAnNtlmHandshake() throws Exception { + assertEquals("X=a b; SID=new", rawCookieOnRetry("SID=old; X=a b", client -> client.prepareGet(url("/ntlm")) + .setRealm(ntlmAuthRealm("Zaphod", "Beeblebrox").setNtlmDomain("Ursa-Minor").setNtlmHost("LightCity")))); + } + + /** Every ';' ends a raw pair, so a quote does not hide the session the challenge set in its neighbour's value. */ + @Test + void aQuoteInARawValueDoesNotShieldTheSessionTheChallengeSet() throws Exception { + assertEquals("X=\"a; SID=new", rawCookieOnRetry("X=\"a; SID=old", client -> client.prepareGet(url("/protected")) + .setRealm(basicAuthRealm("user", "pass").setUsePreemptiveAuth(false)))); + assertEquals("X=a\"b; SID=new", rawCookieOnRetry("X=a\"b; SID=old", client -> client.prepareGet(url("/digest")) + .setRealm(digestAuthRealm("user", "pass").setUsePreemptiveAuth(false)))); + } + + private static String rawCookieOnRetry(String header, Function request) + throws Exception { + try (AsyncHttpClient client = asyncHttpClient()) { + return request.apply(client).setHeader("Cookie", header) + .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); + } + } + + private static void challenge(HttpServletResponse response, int status, String authenticate, String setCookie, + String challenge) { + response.addHeader("Set-Cookie", setCookie); + response.setHeader(authenticate, challenge); + response.setStatus(status); + } + private Set cookiesOnRetry(DefaultCookie callerCookie) throws Exception { try (AsyncHttpClient client = asyncHttpClient()) { client.prepareGet(url("/seed")).execute().get(TIMEOUT, TimeUnit.SECONDS); diff --git a/client/src/test/java/org/asynchttpclient/netty/handler/intercept/RedirectCookieRotationTest.java b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/RedirectCookieRotationTest.java index 1580f2f07..d6bc1881a 100644 --- a/client/src/test/java/org/asynchttpclient/netty/handler/intercept/RedirectCookieRotationTest.java +++ b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/RedirectCookieRotationTest.java @@ -204,6 +204,126 @@ void aCallerSetCookieHeaderDoesNotOutrankTheSessionTheRedirectRotated() throws E } } + @Test + void aRawCookieWithAValueTheStrictEncoderRejectsSurvivesARedirect() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(config().setFollowRedirect(true))) { + String received = client.prepareGet(url("/bounce")) + .setHeader("Cookie", "X=a b; flag; Y=1") + .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); + assertEquals("X=a b; flag; Y=1", received); + } + } + + @Test + void aRawCookieWithANonStrictValueSurvivesA307() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(config().setFollowRedirect(true))) { + String received = client.preparePost(url("/login-307")) + .setHeader("Cookie", "SID=old; X=a b") + .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); + assertEquals(new HashSet<>(Arrays.asList("X=a b", "SID=new")), cookiesSent(received)); + } + } + + @Test + void aRotatedCookieDoesNotDisplaceAnUnrelatedRawValue() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(config().setFollowRedirect(true))) { + String received = client.prepareGet(url("/login")) + .setHeader("Cookie", "SID=old; X=a b") + .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); + assertEquals(new HashSet<>(Arrays.asList("X=a b", "SID=new")), cookiesSent(received)); + } + } + + @Test + void rotationPreservesQuotedAndValuelessRawPairs() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(config().setFollowRedirect(true))) { + String received = client.prepareGet(url("/login")) + .setHeader("Cookie", "SID=old; X=\"a;b\"; flag; Y=a b") + .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); + assertEquals("X=\"a;b\"; flag; Y=a b; SID=new", received); + } + } + + @Test + void aRawCookieTheRedirectDeletedIsNotResent() throws Exception { + assertEquals("X=a b", withRawCookie("SID=old; X=a b", client -> client.prepareGet(url("/logout")))); + assertNull(withRawCookie("SID=old", client -> client.prepareGet(url("/logout")))); + } + + /** + * Every ';' ends a raw pair, as it does where the request factory merges the header: a quote must not hide a + * pair the redirect replaced in its neighbour's value, or the stale one is resent and the new one dropped. + */ + @Test + void aQuoteInARawValueDoesNotShieldTheSessionTheRedirectRotated() throws Exception { + assertEquals("X=a\"b; SID=new", withRawCookie("X=a\"b; SID=old", client -> client.prepareGet(url("/login")))); + assertEquals("X=\"a; SID=new", withRawCookie("X=\"a; SID=old\"", client -> client.prepareGet(url("/login")))); + } + + @Test + void aQuoteInARawValueDoesNotShieldACookieTheRedirectDeleted() throws Exception { + assertEquals("X=\"a", withRawCookie("X=\"a; SID=old", client -> client.prepareGet(url("/logout")))); + } + + /** Only the replaced pair is cut out; what is left, separators included, goes out as the caller wrote it. */ + @Test + void theRawPairsARotationKeepsAreSentByteForByte() throws Exception { + assertEquals("prefs={\"a\":\"x;y\"}; SID=new", + withRawCookie("prefs={\"a\":\"x;y\"}; SID=old", client -> client.prepareGet(url("/login")))); + assertEquals("A=1;B=2; SID=new", withRawCookie("A=1;B=2; SID=old", client -> client.prepareGet(url("/login")))); + assertEquals("A=1;B=2; SID=new", withRawCookie("A=1;SID=old;B=2", client -> client.prepareGet(url("/login")))); + assertEquals("A=1; SID=new", withRawCookie("SID=a; A=1; sid=b", client -> client.prepareGet(url("/login")))); + } + + @Test + void aRotationReachesEveryCookieHeaderTheCallerSet() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(config().setFollowRedirect(true))) { + String received = client.prepareGet(url("/login")) + .addHeader("Cookie", "X=a b; SID=old") + .addHeader("Cookie", "A=1;B=2") + .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); + assertEquals("X=a b; A=1;B=2; SID=new", received); + } + try (AsyncHttpClient client = asyncHttpClient(config().setFollowRedirect(true))) { + String received = client.prepareGet(url("/login")) + .addHeader("Cookie", "A=1;B=2") + .addHeader("Cookie", "SID=old") + .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); + assertEquals("A=1;B=2; SID=new", received); + } + } + + @Test + void aRawCookieSurvivesARotationWithTheLaxEncoder() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(config().setFollowRedirect(true).setUseLaxCookieEncoder(true))) { + String received = client.prepareGet(url("/login")) + .setHeader("Cookie", "flag; X=a b; SID=old") + .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); + assertEquals("flag; X=a b; SID=new", received); + } + } + + @Test + void rawCookieStillWinsOverAStoredCookieOfTheSameName() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(config().setFollowRedirect(true))) { + client.prepareGet(url("/seed")).execute().get(TIMEOUT, TimeUnit.SECONDS); + String received = client.prepareGet(url("/bounce")) + .setHeader("Cookie", "SID=mine; X=a b") + .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); + assertEquals("SID=mine; X=a b", received); + } + } + + @Test + void rawCookieDoesNotCrossAnOriginBoundary() throws Exception { + try (AsyncHttpClient client = asyncHttpClient(config().setFollowRedirect(true))) { + String received = client.prepareGet(url("/elsewhere")) + .setHeader("Cookie", "X=a b") + .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); + assertNull(received); + } + } + /** A Set-Cookie the store refused, or filed for another path, does not replace the caller's cookie. */ @Test void aSetCookieThatDoesNotReachTheNextHopLeavesTheCallersCookie() throws Exception { @@ -242,6 +362,14 @@ private String withCallerCookie(String name, String value, Function request) + throws Exception { + try (AsyncHttpClient client = asyncHttpClient(config().setFollowRedirect(true))) { + return request.apply(client).setHeader("Cookie", header) + .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); + } + } + private static Set cookiesSent(String header) { return header == null ? new HashSet<>() : new HashSet<>(Arrays.asList(header.split("; "))); } From 3f3b8de2c06507c1e03e259e5d0f9f3d39b7fd1a Mon Sep 17 00:00:00 2001 From: Matthias Kurz Date: Thu, 22 Oct 2026 14:12:08 +0200 Subject: [PATCH 2/7] Count a cookie deletion only where it applies A redirect or authentication retry drops the request's cookies that the response replaced, so that the store's current values are sent instead. Every Set-Cookie with Max-Age<=0 counted, whatever its Domain and Path. A deletion for another path then cut the caller's cookie, and the store's cookie of that name, if it held one, was sent in its place. Count a deletion only if its Domain and Path match the next request or the one it answered, as the store matches them. The latter keeps the cookie that the response deleted from following a redirect to another path: once gone from the store, the store's cookie can no longer be told from the caller's, and a caller's pair may be a copy of it. Cookie objects and Cookie header pairs follow the same rule. 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. The store's other checks, such as a public suffix as Domain or a Secure cookie over plaintext, are not repeated. A deletion that the store refuses for such a reason still counts, as before. Claude Code on behalf of Matthias Kurz Co-Authored-By: Claude Opus 5.5 --- .../handler/intercept/CallerCookies.java | 58 +++++++++++++-- .../intercept/AuthRetryCookieTest.java | 19 +++++ .../intercept/RedirectCookieRotationTest.java | 71 +++++++++++++++++++ 3 files changed, 141 insertions(+), 7 deletions(-) diff --git a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/CallerCookies.java b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/CallerCookies.java index 84eeb826f..661eb4326 100644 --- a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/CallerCookies.java +++ b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/CallerCookies.java @@ -62,10 +62,12 @@ private static List of(Request request, CookieStore cookieStore, Set replacedNames(HttpResponse response, CookieStore cookieStore, Uri next, + private static Set replacedNames(HttpResponse response, CookieStore cookieStore, Uri current, Uri next, ClientCookieDecoder cookieDecoder) { - // Only what the store took counts: a Set-Cookie it refused, or one for another path, leaves the - // caller's cookie of that name in place. Names folded as the store keys them, so a SID set here also + // A Set-Cookie replaces the caller's cookie of that name only where its Domain and Path apply, matched as + // the store matches them, and one that does not delete only if the store now holds its value for the next + // request. The store's other checks are not repeated: a deletion it refuses for another reason, such as a + // public suffix as Domain, still counts. Names folded as the store keys them, so a SID set here also // replaces a request's sid. Set setByResponse = new HashSet<>(); List nextCookies = null; @@ -80,15 +82,21 @@ private static Set replacedNames(HttpResponse response, CookieStore cook if (cookie == null) { continue; } + String name = cookie.name().toLowerCase(Locale.ROOT); if (cookie.maxAge() != Cookie.UNDEFINED_MAX_AGE && cookie.maxAge() <= 0) { - setByResponse.add(cookie.name().toLowerCase(Locale.ROOT)); + // A deletion also counts if it applies only to the request it answered. The cookie it deleted + // there was sent with that request: once gone from the store, it can no longer be told from + // the caller's, and a caller's pair may be a copy of it. + if (appliesTo(cookie, current, current) || appliesTo(cookie, current, next)) { + setByResponse.add(name); + } continue; } if (nextCookies == null) { nextCookies = cookieStore.get(next); } - if (holdsSameValue(nextCookies, cookie)) { - setByResponse.add(cookie.name().toLowerCase(Locale.ROOT)); + if (appliesTo(cookie, current, next) && holdsSameValue(nextCookies, cookie)) { + setByResponse.add(name); } } return setByResponse; @@ -104,7 +112,7 @@ static void refresh(RequestBuilder retry, Request request, HttpResponse response static void refresh(RequestBuilder retry, Request request, HttpResponse response, CookieStore cookieStore, Uri next, ClientCookieDecoder cookieDecoder) { - Set setByResponse = replacedNames(response, cookieStore, next, cookieDecoder); + Set setByResponse = replacedNames(response, cookieStore, request.getUri(), next, cookieDecoder); retry.setCookies(of(request, cookieStore, setByResponse)); retry.setHeader(COOKIE, headerCookies(request.getHeaders().getAll(COOKIE), setByResponse)); for (Cookie cookie : cookieStore.get(next)) { @@ -171,6 +179,42 @@ private static List headerCookies(List headers, Set setB return remaining; } + /** + * Whether the Domain and Path of a cookie in the response to {@code answered} match {@code uri}, as the store + * matches them (RFC 6265 sections 5.1.3 and 5.1.4). Only for a {@code uri} on the same host, the only kind a + * retry or a redirect that keeps the caller's cookies goes to. + */ + private static boolean appliesTo(Cookie cookie, Uri answered, Uri uri) { + String domain = cookie.domain() == null ? "" : canonicalHost(cookie.domain()); + if (domain.startsWith(".")) { + domain = domain.substring(1); + } + if (!domain.isEmpty()) { + String host = canonicalHost(uri.getHost()); + if (!host.equals(domain) && !host.endsWith('.' + domain)) { + return false; + } + } + String path = cookie.path(); + if (path == null || path.isEmpty() || path.charAt(0) != '/') { + String answeredPath = answered.getPath(); + int lastSlash = answeredPath.lastIndexOf('/'); + path = answeredPath.startsWith("/") && lastSlash > 0 ? answeredPath.substring(0, lastSlash) : "/"; + } + String uriPath = uri.getPath().isEmpty() ? "/" : uri.getPath(); + return uriPath.startsWith(path) && (uriPath.length() == path.length() + || path.charAt(path.length() - 1) == '/' || uriPath.charAt(path.length()) == '/'); + } + + private static String canonicalHost(String host) { + String lowered = host.toLowerCase(Locale.ROOT); + int end = lowered.length(); + while (end > 0 && lowered.charAt(end - 1) == '.') { + end--; + } + return lowered.substring(0, end); + } + private static boolean holdsSameValue(List stored, Cookie cookie) { for (Cookie candidate : stored) { if (candidate.name().equals(cookie.name()) && candidate.value().equals(cookie.value())) { diff --git a/client/src/test/java/org/asynchttpclient/netty/handler/intercept/AuthRetryCookieTest.java b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/AuthRetryCookieTest.java index 86034e01d..04f1aab23 100644 --- a/client/src/test/java/org/asynchttpclient/netty/handler/intercept/AuthRetryCookieTest.java +++ b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/AuthRetryCookieTest.java @@ -73,6 +73,9 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques // This server is the proxy too: the request is sent to it in absolute form. challenge(response, HttpServletResponse.SC_PROXY_AUTHENTICATION_REQUIRED, "Proxy-Authenticate", "SID=new; Path=/", "Basic realm=\"proxy\""); + } else if ("/protected-delete-elsewhere".equals(target) && authorization == null) { + challenge(response, HttpServletResponse.SC_UNAUTHORIZED, "WWW-Authenticate", + "SID=; Path=/elsewhere; Max-Age=0", "Basic realm=\"test\""); } else if ("/protected".equals(target) && authorization == null) { challenge(response, HttpServletResponse.SC_UNAUTHORIZED, "WWW-Authenticate", "SID=new; Path=/", "Basic realm=\"test\""); @@ -149,6 +152,22 @@ void aQuoteInARawValueDoesNotShieldTheSessionTheChallengeSet() throws Exception .setRealm(digestAuthRealm("user", "pass").setUsePreemptiveAuth(false)))); } + /** A deletion for another path leaves the store's cookie of that name, which must not replace the caller's. */ + @Test + void aDeletionForAnotherPathLeavesTheCallersCookieOnTheRetry() throws Exception { + try (AsyncHttpClient client = asyncHttpClient()) { + client.prepareGet(url("/seed")).execute().get(TIMEOUT, TimeUnit.SECONDS); + assertEquals("SID=mine", client.prepareGet(url("/protected-delete-elsewhere")) + .setRealm(basicAuthRealm("user", "pass").setUsePreemptiveAuth(false)) + .setHeader("Cookie", "SID=mine") + .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE), "Cookie header"); + assertEquals("SID=mine", client.prepareGet(url("/protected-delete-elsewhere")) + .setRealm(basicAuthRealm("user", "pass").setUsePreemptiveAuth(false)) + .addCookie(new DefaultCookie("SID", "mine")) + .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE), "cookie object"); + } + } + private static String rawCookieOnRetry(String header, Function request) throws Exception { try (AsyncHttpClient client = asyncHttpClient()) { diff --git a/client/src/test/java/org/asynchttpclient/netty/handler/intercept/RedirectCookieRotationTest.java b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/RedirectCookieRotationTest.java index d6bc1881a..eacc1abd2 100644 --- a/client/src/test/java/org/asynchttpclient/netty/handler/intercept/RedirectCookieRotationTest.java +++ b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/RedirectCookieRotationTest.java @@ -90,6 +90,22 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques case "/other-path": redirect(response, HttpServletResponse.SC_FOUND, "SID=other; Path=/elsewhere", "/home"); break; + case "/other-path-same-value": + redirect(response, HttpServletResponse.SC_FOUND, "SID=old; Path=/elsewhere", "/home"); + break; + case "/reject-domain-delete": + redirect(response, HttpServletResponse.SC_FOUND, "SID=; Domain=example.org; Path=/; Max-Age=0", + "/home"); + break; + case "/other-path-delete": + redirect(response, HttpServletResponse.SC_FOUND, "SID=; Path=/elsewhere; Max-Age=0", "/home"); + break; + case "/seed-root-p": + response.addHeader("Set-Cookie", "P=root; Path=/"); + break; + case "/p/logout": + redirect(response, HttpServletResponse.SC_FOUND, "P=; Path=/p; Max-Age=0", "/home"); + break; case "/rotate-lower": redirect(response, HttpServletResponse.SC_FOUND, "sid=new; Path=/", "/home"); break; @@ -331,6 +347,40 @@ void aSetCookieThatDoesNotReachTheNextHopLeavesTheCallersCookie() throws Excepti "refused: Domain does not match"); assertEquals("SID=mine", withCallerCookie("SID", "mine", client -> client.prepareGet(url("/other-path"))), "stored for /elsewhere, not sent to /home"); + assertEquals("SID=mine", withCallerCookie("SID", "mine", + client -> client.prepareGet(url("/other-path-same-value"))), + "stored for /elsewhere, with the value of the stored cookie sent to /home"); + } + + /** Nor does a deletion the store refused or that applies elsewhere, or the stored value takes its place. */ + @Test + void aDeletionThatDoesNotReachTheNextHopLeavesTheCallersCookie() throws Exception { + for (String path : Arrays.asList("/reject-domain-delete", "/other-path-delete")) { + assertEquals("SID=mine", withCallerCookie("SID", "mine", client -> client.prepareGet(url(path))), path); + assertEquals("SID=mine", withRawCookieAfterSeeding("SID=mine", client -> client.prepareGet(url(path))), + path + ", Cookie header"); + } + assertEquals("SID=mine", withRawCookie("SID=mine", client -> client.prepareGet(url("/other-path-delete"))), + "nothing stored"); + } + + /** + * A deletion that applies only to the request it answered still removes the cookie sent with that request, from + * the store or the caller's header, so it does not follow the redirect. + */ + @Test + void aCookieDeletedForThePathItWasSentToDoesNotFollowTheRedirect() throws Exception { + assertEquals("SID=old", afterSeeding(client -> client.prepareGet(url("/p/logout"))), "cookie object"); + assertEquals("SID=old", withRawCookieAfterSeeding("P=scoped", client -> client.prepareGet(url("/p/logout"))), + "Cookie header"); + } + + /** A stored cookie of that name for the next hop still follows, as it would without the caller's header. */ + @Test + void aCookieDeletedForThePathItWasSentToLeavesTheOneStoredForTheNextHop() throws Exception { + Set expected = new HashSet<>(Arrays.asList("SID=old", "P=root")); + assertEquals(expected, cookiesSent(logoutBesideARootCookie(null)), "cookie object"); + assertEquals(expected, cookiesSent(logoutBesideARootCookie("P=scoped")), "Cookie header"); } // Without a cookie store every cookie on the request is the caller's own. @@ -370,6 +420,27 @@ private String withRawCookie(String header, Function request) + throws Exception { + try (AsyncHttpClient client = asyncHttpClient(config().setFollowRedirect(true))) { + client.prepareGet(url("/seed")).execute().get(TIMEOUT, TimeUnit.SECONDS); + return request.apply(client).setHeader("Cookie", header) + .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); + } + } + private static Set cookiesSent(String header) { return header == null ? new HashSet<>() : new HashSet<>(Arrays.asList(header.split("; "))); } From 0bac96ea473198fdc3cc130a0829cd8eb0fed83b Mon Sep 17 00:00:00 2001 From: Matthias Kurz Date: Thu, 22 Oct 2026 13:29:27 +0200 Subject: [PATCH 3/7] Test a raw pair equal to a path-scoped cookie A pair of a caller's Cookie header now follows a same-origin redirect as written, even when the store holds a cookie with the same name and value. Before, such a pair counted as the store's, and was dropped once the redirect left that cookie's Path. Pin the new behavior: a caller who wrote the pair meant to send it. Claude Code on behalf of Matthias Kurz Co-Authored-By: Claude Opus 5.5 --- .../handler/intercept/RedirectCookieRotationTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/client/src/test/java/org/asynchttpclient/netty/handler/intercept/RedirectCookieRotationTest.java b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/RedirectCookieRotationTest.java index eacc1abd2..2ba333730 100644 --- a/client/src/test/java/org/asynchttpclient/netty/handler/intercept/RedirectCookieRotationTest.java +++ b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/RedirectCookieRotationTest.java @@ -158,6 +158,13 @@ void aPathScopedCookieDoesNotFollowARedirectOutOfItsPath() throws Exception { assertFalse(received != null && received.contains("P="), "sent to /q/b: " + received); } + /** A Cookie header pair is the caller's even when the store holds the same cookie, so it may leave its path. */ + @Test + void aRawPairEqualToAStoredCookieFollowsARedirectOutOfItsPath() throws Exception { + assertEquals(new HashSet<>(Arrays.asList("P=scoped", "SID=old")), + cookiesSent(withRawCookieAfterSeeding("P=scoped", client -> client.prepareGet(url("/p/a"))))); + } + /** The store folds cookie names, so a redirect naming SID as sid still replaces it. */ @Test void aRedirectReplacesACookieItNamesInAnotherCase() throws Exception { From d1d8552af487706691589f26ebcfb4b29716db00 Mon Sep 17 00:00:00 2001 From: Matthias Kurz Date: Thu, 22 Oct 2026 13:31:13 +0200 Subject: [PATCH 4/7] Split Cookie header pairs in one place NettyRequestFactory and CallerCookies each split a caller's Cookie header into pairs and named them, in two loops that had to agree. Move the split and the naming to HttpUtils and use them in both. CallerCookies still folds names as the store does, while the merge compares them as written; the Javadoc now says so instead of claiming that both work the same way. CallerCookies also checks a header's pair names before it builds anything, so a header without a replaced pair is no longer copied. Claude Code on behalf of Matthias Kurz Co-Authored-By: Claude Opus 5.5 --- .../handler/intercept/CallerCookies.java | 60 ++++++++++--------- .../netty/request/NettyRequestFactory.java | 9 +-- .../org/asynchttpclient/util/HttpUtils.java | 17 ++++++ .../asynchttpclient/util/HttpUtilsTest.java | 9 +++ 4 files changed, 63 insertions(+), 32 deletions(-) diff --git a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/CallerCookies.java b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/CallerCookies.java index 661eb4326..37d0a592d 100644 --- a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/CallerCookies.java +++ b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/CallerCookies.java @@ -31,6 +31,8 @@ import static io.netty.handler.codec.http.HttpHeaderNames.COOKIE; import static io.netty.handler.codec.http.HttpHeaderNames.SET_COOKIE; +import static org.asynchttpclient.util.HttpUtils.cookiePairName; +import static org.asynchttpclient.util.HttpUtils.cookiePairs; /** * Tells the cookies a caller put on a request from the ones the cookie store added, for the request a redirect @@ -124,44 +126,32 @@ static void refresh(RequestBuilder retry, Request request, HttpResponse response * The caller's Cookie header values minus the pairs this response replaced, kept as text: a lenient decode * followed by strict encoding can reject a value that was sent on the first attempt. *

- * Every ';' ends a pair, inside quotes too: RFC 6265 does not allow one in a cookie value, and - * {@code NettyRequestFactory#mergeCookies} splits the same way when it decides which names the caller set. - * Honouring quotes here would let a stray '"' hide a replaced pair in its neighbour's value, to be resent - * stale while the merge still counts its name as the caller's and drops the store's new value. Only the - * replaced pairs are cut out, so the rest keeps its bytes and a header that loses none is returned as is. + * The pairs come from {@code HttpUtils.cookiePairs}, as in {@code NettyRequestFactory#mergeCookies} when it + * decides which names the caller set, so every ';' ends one. Honouring quotes here would let a stray '"' hide + * a replaced pair in its neighbour's value, to be resent stale while the merge still counts its name as the + * caller's and drops the store's new value. Unlike the merge, names are folded as the store folds them, so + * replacing SID also cuts a pair named sid. Only the replaced pairs are cut out, so the rest keeps its bytes + * and a header that loses none is returned as is. */ - private static List headerCookies(List headers, Set setByResponse) { - if (setByResponse.isEmpty() || headers.isEmpty()) { + private static List headerCookies(List headers, Set replaced) { + if (replaced.isEmpty() || headers.isEmpty()) { return headers; } List remaining = new ArrayList<>(headers.size()); for (String header : headers) { + String[] pairs = cookiePairs(header); + if (!anyReplaced(pairs, replaced)) { + remaining.add(header); + continue; + } StringBuilder kept = new StringBuilder(header.length()); - boolean removed = false; - int start = 0; - while (start <= header.length()) { - int end = header.indexOf(';', start); - if (end < 0) { - end = header.length(); - } - int nameEnd = start; - while (nameEnd < end && header.charAt(nameEnd) != '=') { - nameEnd++; - } - String name = header.substring(start, nameEnd).trim(); - if (!name.isEmpty() && setByResponse.contains(name.toLowerCase(Locale.ROOT))) { - removed = true; - } else { + for (String pair : pairs) { + if (!isReplaced(pair, replaced)) { if (kept.length() > 0) { kept.append(';'); } - kept.append(header, start, end); + kept.append(pair); } - start = end + 1; - } - if (!removed) { - remaining.add(header); - continue; } // Cutting the first or last pair leaves the whitespace or ';' that separated it at that end. int from = 0; @@ -179,6 +169,20 @@ private static List headerCookies(List headers, Set setB return remaining; } + private static boolean anyReplaced(String[] pairs, Set replaced) { + for (String pair : pairs) { + if (isReplaced(pair, replaced)) { + return true; + } + } + return false; + } + + private static boolean isReplaced(String pair, Set replaced) { + String name = cookiePairName(pair); + return !name.isEmpty() && replaced.contains(name.toLowerCase(Locale.ROOT)); + } + /** * Whether the Domain and Path of a cookie in the response to {@code answered} match {@code uri}, as the store * matches them (RFC 6265 sections 5.1.3 and 5.1.4). Only for a {@code uri} on the same host, the only kind a diff --git a/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestFactory.java b/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestFactory.java index b5ea972ab..613707030 100755 --- a/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestFactory.java +++ b/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestFactory.java @@ -80,6 +80,8 @@ import static org.asynchttpclient.util.AuthenticatorUtils.perRequestProxyAuthorizationHeader; import static org.asynchttpclient.util.HttpUtils.ACCEPT_ALL_HEADER_VALUE; import static org.asynchttpclient.util.HttpUtils.GZIP_DEFLATE; +import static org.asynchttpclient.util.HttpUtils.cookiePairName; +import static org.asynchttpclient.util.HttpUtils.cookiePairs; import static org.asynchttpclient.util.HttpUtils.filterOutBrotliFromAcceptEncoding; import static org.asynchttpclient.util.HttpUtils.filterOutZstdFromAcceptEncoding; import static org.asynchttpclient.util.HttpUtils.hostHeader; @@ -203,10 +205,9 @@ private void mergeCookies(List cookies, HttpHeaders headers) { merged.append("; "); } merged.append(callerHeader, start, end); - for (String pair : callerHeader.split(";")) { - int eq = pair.indexOf('='); - // Case-sensitive, but trimmed: "a = 1" still names "a". - String name = (eq >= 0 ? pair.substring(0, eq) : pair).trim(); + for (String pair : cookiePairs(callerHeader)) { + // Case-sensitive, unlike CallerCookies, which folds names as the store does. + String name = cookiePairName(pair); if (!name.isEmpty()) { callerNames.add(name); } diff --git a/client/src/main/java/org/asynchttpclient/util/HttpUtils.java b/client/src/main/java/org/asynchttpclient/util/HttpUtils.java index 2b970595b..308e73677 100644 --- a/client/src/main/java/org/asynchttpclient/util/HttpUtils.java +++ b/client/src/main/java/org/asynchttpclient/util/HttpUtils.java @@ -178,6 +178,23 @@ private static void encodeAndAppendFormField(StringBuilder sb, String field, Cha } } + /** + * The name=value pairs of a {@code Cookie} header value, as written. Every ';' ends a pair, inside quotes too: + * RFC 6265 does not allow one in a cookie value. + */ + public static String[] cookiePairs(String cookieHeader) { + return cookieHeader.split(";", -1); + } + + /** + * The name of a pair from {@link #cookiePairs}: the text before its first '=', or all of it, trimmed, so + * "a = 1" names "a". + */ + public static String cookiePairName(String pair) { + int eq = pair.indexOf('='); + return (eq >= 0 ? pair.substring(0, eq) : pair).trim(); + } + public static CharSequence filterOutBrotliFromAcceptEncoding(String acceptEncoding) { // we don't support Brotly ATM if (acceptEncoding.endsWith(BROTLY_ACCEPT_ENCODING_SUFFIX)) { diff --git a/client/src/test/java/org/asynchttpclient/util/HttpUtilsTest.java b/client/src/test/java/org/asynchttpclient/util/HttpUtilsTest.java index 06414d864..7ed607dde 100644 --- a/client/src/test/java/org/asynchttpclient/util/HttpUtilsTest.java +++ b/client/src/test/java/org/asynchttpclient/util/HttpUtilsTest.java @@ -36,6 +36,7 @@ import static java.nio.charset.StandardCharsets.ISO_8859_1; import static java.nio.charset.StandardCharsets.US_ASCII; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; @@ -185,4 +186,12 @@ public void computeOriginForSecuredUriWithDefaultPort() { public void computeOriginForSecuredUriWithNonDefaultPort() { assertEquals("https://foo.com:444", HttpUtils.originHeader(Uri.create("wss://foo.com:444/bar"))); } + + @Test + public void cookiePairsEndAtEverySemicolonAndKeepTheirText() { + assertArrayEquals(new String[]{"X=\"a", " b\"", " flag", " Y = 1", ""}, HttpUtils.cookiePairs("X=\"a; b\"; flag; Y = 1;")); + assertEquals("Y", HttpUtils.cookiePairName(" Y = 1")); + assertEquals("flag", HttpUtils.cookiePairName(" flag")); + assertEquals("", HttpUtils.cookiePairName("")); + } } From 0e4d781d4e1fe66619c7529f062cbdc46db904d4 Mon Sep 17 00:00:00 2001 From: Matthias Kurz Date: Thu, 22 Oct 2026 14:12:25 +0200 Subject: [PATCH 5/7] Look up the cookie store once per hop CallerCookies.refresh asked the store for the next request's cookies up to twice and for the current request's once, three lookups of the same URI on an authentication retry. Look up the next request's cookies once and pass them down, and reuse them for the current request on a retry. A redirect still needs both. Claude Code on behalf of Matthias Kurz Co-Authored-By: Claude Opus 5.5 --- .../handler/intercept/CallerCookies.java | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/CallerCookies.java b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/CallerCookies.java index 37d0a592d..ca3cc6ec3 100644 --- a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/CallerCookies.java +++ b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/CallerCookies.java @@ -49,12 +49,10 @@ private CallerCookies() { * a stored one stays the caller's. A Cookie header the caller set is not among these; see * {@link #headerCookies}. */ - private static List of(Request request, CookieStore cookieStore, Set setByResponse) { - List cookies = request.getCookies(); + private static List of(List cookies, List stored, Set setByResponse) { if (cookies.isEmpty()) { return cookies; } - List stored = cookieStore.get(request.getUri()); List callers = new ArrayList<>(cookies.size()); for (Cookie cookie : cookies) { if (!setByResponse.contains(cookie.name().toLowerCase(Locale.ROOT)) && !holdsSameValue(stored, cookie)) { @@ -64,7 +62,7 @@ private static List of(Request request, CookieStore cookieStore, Set replacedNames(HttpResponse response, CookieStore cookieStore, Uri current, Uri next, + private static Set replacedNames(HttpResponse response, List nextCookies, Uri current, Uri next, ClientCookieDecoder cookieDecoder) { // A Set-Cookie replaces the caller's cookie of that name only where its Domain and Path apply, matched as // the store matches them, and one that does not delete only if the store now holds its value for the next @@ -72,7 +70,6 @@ private static Set replacedNames(HttpResponse response, CookieStore cook // public suffix as Domain, still counts. Names folded as the store keys them, so a SID set here also // replaces a request's sid. Set setByResponse = new HashSet<>(); - List nextCookies = null; for (String header : response.headers().getAll(SET_COOKIE)) { Cookie cookie; try { @@ -94,9 +91,6 @@ private static Set replacedNames(HttpResponse response, CookieStore cook } continue; } - if (nextCookies == null) { - nextCookies = cookieStore.get(next); - } if (appliesTo(cookie, current, next) && holdsSameValue(nextCookies, cookie)) { setByResponse.add(name); } @@ -114,10 +108,15 @@ static void refresh(RequestBuilder retry, Request request, HttpResponse response static void refresh(RequestBuilder retry, Request request, HttpResponse response, CookieStore cookieStore, Uri next, ClientCookieDecoder cookieDecoder) { - Set setByResponse = replacedNames(response, cookieStore, request.getUri(), next, cookieDecoder); - retry.setCookies(of(request, cookieStore, setByResponse)); + List nextCookies = cookieStore.get(next); + Set setByResponse = replacedNames(response, nextCookies, request.getUri(), next, cookieDecoder); + List cookies = request.getCookies(); + // A retry goes to the same URI again, so one lookup serves both. + List stored = cookies.isEmpty() || next.equals(request.getUri()) + ? nextCookies : cookieStore.get(request.getUri()); + retry.setCookies(of(cookies, stored, setByResponse)); retry.setHeader(COOKIE, headerCookies(request.getHeaders().getAll(COOKIE), setByResponse)); - for (Cookie cookie : cookieStore.get(next)) { + for (Cookie cookie : nextCookies) { retry.addCookieIfUnset(cookie); } } From 68c2746f622760e14293f9ab8cb648c2c157003e Mon Sep 17 00:00:00 2001 From: Matthias Kurz Date: Thu, 22 Oct 2026 13:33:08 +0200 Subject: [PATCH 6/7] Remove a dead Cookie block from redirects propagatedHeaders removed the Cookie header whenever there was a cookie store. On a hop that strips credentials it is already gone, and on any other CallerCookies.refresh sets it again right after. Drop the block and its parameter, and say in the comment above that, with a store, CallerCookies tells the caller's cookies from the store's. Claude Code on behalf of Matthias Kurz Co-Authored-By: Claude Opus 5.5 --- .../handler/intercept/Redirect30xInterceptor.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 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 d1ec092fd..1b74f448c 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 @@ -203,7 +203,8 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture .setRangeOffset(request.getRangeOffset()); } // A same-origin hop keeps the caller's cookies but not the store's: those are its values from - // before this response and would outrank what it just set. The store adds its current ones below. + // before this response and would outrank what it just set. Without a store, every cookie is the + // caller's. With one, CallerCookies.refresh tells them apart below and adds the store's current ones. CookieStore cookieStore = config.getCookieStore(); if (stripAuth) { requestBuilder.resetCookies(); @@ -214,7 +215,7 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture requestBuilder.setMethod(switchToGet ? GET : originalMethod) .setFollowRedirect(true) .setRealm(stripAuth ? null : request.getRealm()) - .setHeaders(propagatedHeaders(request, realm, keepBody, stripAuth, cookieStore != null)) + .setHeaders(propagatedHeaders(request, realm, keepBody, stripAuth)) .setRequestTimeout(request.getRequestTimeout()) .setReadTimeout(request.getReadTimeout()); @@ -459,8 +460,8 @@ private enum BodyRepresentation { NONE } - private static HttpHeaders propagatedHeaders(Request request, Realm realm, boolean keepBody, boolean stripAuthorization, - boolean cookiesReconciled) { + private static HttpHeaders propagatedHeaders(Request request, Realm realm, boolean keepBody, + boolean stripAuthorization) { HttpHeaders headers = request.getHeaders().copy().remove(HOST); // Preserve an explicit length when the selected stream representation cannot rebuild it. @@ -483,10 +484,6 @@ private static HttpHeaders propagatedHeaders(Request request, Realm realm, boole headers.remove(AUTHORIZATION) .remove(PROXY_AUTHORIZATION); } - if (cookiesReconciled) { - // Unless stripped above, CallerCookies.refresh puts it back minus the pairs this response replaced. - headers.remove(COOKIE); - } return headers; } } From 21a2e1d0d15a6aaa604fcc732b83f1e653d8d33b Mon Sep 17 00:00:00 2001 From: Matthias Kurz Date: Thu, 22 Oct 2026 13:34:06 +0200 Subject: [PATCH 7/7] Tidy the auth retry cookie tests Two tests split the received Cookie header without a null check, so a retry without any cookie failed with a NullPointerException instead of an assertion. Use a null-safe helper for both. The proxy authentication test pinned that a Set-Cookie on the proxy's own 407 is stored against the origin and replaces the caller's cookie, existing behavior that this change should not cement. Its 407 now sets no cookie, so the test checks only that the caller's Cookie header survives the retry as written. It fails on main, where the strict encoder rejects the value "a b". Claude Code on behalf of Matthias Kurz Co-Authored-By: Claude Opus 5.5 --- .../intercept/AuthRetryCookieTest.java | 37 +++++++++---------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/client/src/test/java/org/asynchttpclient/netty/handler/intercept/AuthRetryCookieTest.java b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/AuthRetryCookieTest.java index 04f1aab23..d5fd01a8c 100644 --- a/client/src/test/java/org/asynchttpclient/netty/handler/intercept/AuthRetryCookieTest.java +++ b/client/src/test/java/org/asynchttpclient/netty/handler/intercept/AuthRetryCookieTest.java @@ -72,7 +72,7 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques } else if ("/proxied".equals(target) && request.getHeader("Proxy-Authorization") == null) { // This server is the proxy too: the request is sent to it in absolute form. challenge(response, HttpServletResponse.SC_PROXY_AUTHENTICATION_REQUIRED, "Proxy-Authenticate", - "SID=new; Path=/", "Basic realm=\"proxy\""); + null, "Basic realm=\"proxy\""); } else if ("/protected-delete-elsewhere".equals(target) && authorization == null) { challenge(response, HttpServletResponse.SC_UNAUTHORIZED, "WWW-Authenticate", "SID=; Path=/elsewhere; Max-Age=0", "Basic realm=\"test\""); @@ -103,24 +103,16 @@ void theCallersCookieSurvivesTheRetry() throws Exception { /** A caller-set Cookie header is the caller's too, but it must not outrank what the challenge rotated. */ @Test void aCallerSetCookieHeaderDoesNotOutrankTheSessionTheChallengeSet() throws Exception { - try (AsyncHttpClient client = asyncHttpClient()) { - String header = client.prepareGet(url("/protected")) - .setRealm(basicAuthRealm("user", "pass").setUsePreemptiveAuth(false)) - .setHeader("Cookie", "SID=old; X=1") - .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); - assertEquals(new HashSet<>(Arrays.asList("X=1", "SID=new")), new HashSet<>(Arrays.asList(header.split("; ")))); - } + assertEquals(new HashSet<>(Arrays.asList("X=1", "SID=new")), cookiesSent(rawCookieOnRetry("SID=old; X=1", + client -> client.prepareGet(url("/protected")) + .setRealm(basicAuthRealm("user", "pass").setUsePreemptiveAuth(false))))); } @Test void aRawCookieWithANonStrictValueSurvivesTheRetry() throws Exception { - try (AsyncHttpClient client = asyncHttpClient()) { - String header = client.prepareGet(url("/protected")) - .setRealm(basicAuthRealm("user", "pass").setUsePreemptiveAuth(false)) - .setHeader("Cookie", "SID=old; X=a b") - .execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); - assertEquals(new HashSet<>(Arrays.asList("X=a b", "SID=new")), new HashSet<>(Arrays.asList(header.split("; ")))); - } + assertEquals(new HashSet<>(Arrays.asList("X=a b", "SID=new")), cookiesSent(rawCookieOnRetry("SID=old; X=a b", + client -> client.prepareGet(url("/protected")) + .setRealm(basicAuthRealm("user", "pass").setUsePreemptiveAuth(false))))); } @Test @@ -130,8 +122,8 @@ void aRawCookieSurvivesADigestRetryBesideTheSessionTheChallengeSet() throws Exce } @Test - void aRawCookieSurvivesAProxyAuthenticationRetryBesideTheSessionTheChallengeSet() throws Exception { - assertEquals("X=a b; SID=new", rawCookieOnRetry("SID=old; X=a b", client -> client.prepareGet(url("/proxied")) + void aRawCookieSurvivesAProxyAuthenticationRetry() throws Exception { + assertEquals("SID=old; X=a b", rawCookieOnRetry("SID=old; X=a b", client -> client.prepareGet(url("/proxied")) .setProxyServer(proxyServer("localhost", port2) .setRealm(basicAuthRealm("user", "pass").setUsePreemptiveAuth(false))))); } @@ -178,7 +170,9 @@ private static String rawCookieOnRetry(String header, Function cookiesOnRetry(DefaultCookie callerCookie) throws Exception if (callerCookie != null) { request.addCookie(callerCookie); } - String header = request.execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE); - return header == null ? new HashSet<>() : new HashSet<>(Arrays.asList(header.split("; "))); + return cookiesSent(request.execute().get(TIMEOUT, TimeUnit.SECONDS).getHeader(RECEIVED_COOKIE)); } } + private static Set cookiesSent(String header) { + return header == null ? new HashSet<>() : new HashSet<>(Arrays.asList(header.split("; "))); + } + private String url(String path) { return "http://localhost:" + port1 + path; }