From 15ece6b5c5fd3fd33e3d3f67420feee84de19af9 Mon Sep 17 00:00:00 2001 From: Pavel Ptashyts <49400901+pavel-ptashyts@users.noreply.github.com> Date: Sat, 7 Nov 2026 20:13:18 +0100 Subject: [PATCH 1/3] Add stored cookies without rebuilding the request When the cookie store had a cookie for the request URI, executeRequest ran request.toBuilder() and build() to add it. That copied every header into a new header set, copied the cookie, form param and body part lists twice and encoded the URI again, on every request that carries a session cookie. The URI step was also a bug. toBuilder() always uses the fixing URI encoder, so a request built with URL encoding disabled was sent encoded, but only when the store had a cookie for it: "/a|b?q={x}" went out as "/a%7Cb?q=%7Bx%7D". A DefaultRequest is now copied with only its cookie list replaced and its URI kept as built, and is returned unchanged when it already carries a cookie of every stored name. Other Request implementations still go through toBuilder(), as they may override what it copies. Claude Code on behalf of Pavel Ptashyts Co-Authored-By: Claude Opus 5.5 --- .../bench/StoredCookieRequestBenchmark.java | 115 +++++++++++++++ .../DefaultAsyncHttpClient.java | 44 +++++- .../org/asynchttpclient/DefaultRequest.java | 13 ++ .../DefaultAsyncHttpClientTest.java | 133 ++++++++++++++++++ 4 files changed, 300 insertions(+), 5 deletions(-) create mode 100644 client/src/jmh/java/org/asynchttpclient/bench/StoredCookieRequestBenchmark.java diff --git a/client/src/jmh/java/org/asynchttpclient/bench/StoredCookieRequestBenchmark.java b/client/src/jmh/java/org/asynchttpclient/bench/StoredCookieRequestBenchmark.java new file mode 100644 index 0000000000..d4cea91cb9 --- /dev/null +++ b/client/src/jmh/java/org/asynchttpclient/bench/StoredCookieRequestBenchmark.java @@ -0,0 +1,115 @@ +/* + * Copyright (c) 2026 AsyncHttpClient Project. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.asynchttpclient.bench; + +import io.netty.handler.codec.http.cookie.DefaultCookie; +import org.asynchttpclient.AsyncCompletionHandlerBase; +import org.asynchttpclient.AsyncHttpClient; +import org.asynchttpclient.ListenableFuture; +import org.asynchttpclient.Request; +import org.asynchttpclient.RequestBuilder; +import org.asynchttpclient.Response; +import org.asynchttpclient.cookie.ThreadSafeCookieStore; +import org.asynchttpclient.exception.FilterException; +import org.asynchttpclient.filter.FilterContext; +import org.asynchttpclient.filter.RequestFilter; +import org.asynchttpclient.uri.Uri; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; + +import java.io.IOException; +import java.util.concurrent.TimeUnit; + +import static org.asynchttpclient.Dsl.asyncHttpClient; +import static org.asynchttpclient.Dsl.config; + +/** + * Measures {@code DefaultAsyncHttpClient.executeRequest} up to the request filters, for a request whose URL has a + * cookie in the client's cookie store ({@code storedCookie}) and for one whose URL has none ({@code noStoredCookie}). + * The difference between the two is what adding the stored cookies to the request costs. + *

+ * A filter ends each call by throwing a preallocated exception, so nothing reaches the network; the failed future + * that {@code executeRequest} then builds is the same in both arms. + */ +@State(Scope.Thread) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +public class StoredCookieRequestBenchmark { + + private static final FilterException STOP = new FilterException("stop"); + + @Param({"4", "16"}) + public int headerCount; + + private AsyncHttpClient client; + private Request withStoredCookie; + private Request withoutStoredCookie; + private AsyncCompletionHandlerBase handler; + + @Setup(Level.Trial) + public void setup() { + ThreadSafeCookieStore cookieStore = new ThreadSafeCookieStore(); + DefaultCookie session = new DefaultCookie("SESSION", "0123456789abcdef0123456789abcdef"); + session.setPath("/"); + cookieStore.add(Uri.create("https://api.example.com/"), session); + + RequestFilter stop = new RequestFilter() { + @Override + public FilterContext filter(FilterContext ctx) throws FilterException { + throw STOP; + } + }; + client = asyncHttpClient(config().setCookieStore(cookieStore).addRequestFilter(stop)); + withStoredCookie = request("https://api.example.com/v1/items?page=2&size=50"); + withoutStoredCookie = request("https://other.example.org/v1/items?page=2&size=50"); + handler = new AsyncCompletionHandlerBase() { + @Override + public void onThrowable(Throwable t) { + } + }; + } + + private Request request(String url) { + RequestBuilder builder = new RequestBuilder("GET").setUrl(url); + for (int i = 0; i < headerCount; i++) { + builder.addHeader("X-Header-" + i, "value-" + i); + } + return builder.build(); + } + + @TearDown(Level.Trial) + public void tearDown() throws IOException { + client.close(); + } + + @Benchmark + public ListenableFuture storedCookie() { + return client.executeRequest(withStoredCookie, handler); + } + + @Benchmark + public ListenableFuture noStoredCookie() { + return client.executeRequest(withoutStoredCookie, handler); + } +} diff --git a/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClient.java b/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClient.java index 3b417a5a39..e1f2c3dff0 100644 --- a/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClient.java +++ b/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClient.java @@ -34,6 +34,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; @@ -233,11 +234,7 @@ public ListenableFuture executeRequest(Request request, AsyncHandler h try { List cookies = config.getCookieStore().get(request.getUri()); if (!cookies.isEmpty()) { - RequestBuilder requestBuilder = request.toBuilder(); - for (Cookie cookie : cookies) { - requestBuilder.addCookieIfUnset(cookie); - } - request = requestBuilder.build(); + request = addCookiesIfUnset(request, cookies); } } catch (Exception e) { handler.onThrowable(e); @@ -275,6 +272,43 @@ public ListenableFuture executeRequest(RequestBuilder requestBuilder) return executeRequest(requestBuilder.build()); } + /** + * Adds each stored cookie whose name the request does not carry yet, keeping the first of a name. A + * {@link DefaultRequest} is copied with only its cookie list replaced, which leaves its URI as built; any other + * {@link Request} goes through {@link Request#toBuilder()}, as it may override what the builder copies. + */ + private static Request addCookiesIfUnset(Request request, List storedCookies) { + if (request.getClass() != DefaultRequest.class) { + RequestBuilder requestBuilder = request.toBuilder(); + for (Cookie cookie : storedCookies) { + requestBuilder.addCookieIfUnset(cookie); + } + return requestBuilder.build(); + } + + List requestCookies = request.getCookies(); + List cookies = null; + for (Cookie cookie : storedCookies) { + if (!hasCookieNamed(cookies == null ? requestCookies : cookies, cookie.name())) { + if (cookies == null) { + cookies = new ArrayList<>(requestCookies.size() + storedCookies.size()); + cookies.addAll(requestCookies); + } + cookies.add(cookie); + } + } + return cookies == null ? request : ((DefaultRequest) request).withCookies(cookies); + } + + private static boolean hasCookieNamed(List cookies, String name) { + for (Cookie cookie : cookies) { + if (cookie.name().equals(name)) { + return true; + } + } + return false; + } + private ListenableFuture execute(Request request, final AsyncHandler asyncHandler) { try { return requestSender.sendRequest(request, asyncHandler, null); diff --git a/client/src/main/java/org/asynchttpclient/DefaultRequest.java b/client/src/main/java/org/asynchttpclient/DefaultRequest.java index 7df31fb95e..90e883e58b 100644 --- a/client/src/main/java/org/asynchttpclient/DefaultRequest.java +++ b/client/src/main/java/org/asynchttpclient/DefaultRequest.java @@ -181,6 +181,19 @@ public DefaultRequest(String method, this.nameResolver = nameResolver; } + /** + * Returns a request that differs from this one only by its cookies. Unlike a round trip through + * {@link #toBuilder()}, the URI is kept as built rather than encoded again, so a request built with URL + * encoding disabled is sent as written, and the other fields are shared rather than copied. + */ + DefaultRequest withCookies(List cookies) { + return new DefaultRequest(method, uri, address, localAddress, headers, cookies, byteData, compositeByteData, + stringData, byteBufferData, byteBufData, streamData, bodyGenerator, formParams, bodyParts, virtualHost, + proxyServer, realm, file, followRedirect, requestTimeout, readTimeout, rangeOffset, charset, + channelPoolPartitioning, nameResolver, useAbsoluteRequestDeadline, refuseSchemeDowngradeOnRedirect, + refuseCrossOriginBodyOnRedirect); + } + @Override public String getUrl() { return uri.toUrl(); diff --git a/client/src/test/java/org/asynchttpclient/DefaultAsyncHttpClientTest.java b/client/src/test/java/org/asynchttpclient/DefaultAsyncHttpClientTest.java index 3fd73a1ba8..c659cba60b 100644 --- a/client/src/test/java/org/asynchttpclient/DefaultAsyncHttpClientTest.java +++ b/client/src/test/java/org/asynchttpclient/DefaultAsyncHttpClientTest.java @@ -28,25 +28,47 @@ import io.netty.channel.nio.NioIoHandler; import io.netty.channel.uring.IoUring; import io.netty.channel.uring.IoUringIoHandler; +import io.netty.handler.codec.http.cookie.Cookie; +import io.netty.handler.codec.http.cookie.DefaultCookie; import io.netty.util.Timer; import org.asynchttpclient.cookie.CookieEvictionTask; import org.asynchttpclient.cookie.CookieStore; import org.asynchttpclient.cookie.ThreadSafeCookieStore; +import org.asynchttpclient.exception.FilterException; +import org.asynchttpclient.filter.FilterContext; +import org.asynchttpclient.filter.RequestFilter; import org.asynchttpclient.testserver.HttpServer; +import org.asynchttpclient.uri.Uri; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.EnabledOnOs; import org.junit.jupiter.api.condition.OS; import java.io.IOException; +import java.net.InetAddress; +import java.time.Duration; +import java.util.List; +import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; +import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_TYPE; +import static java.nio.charset.StandardCharsets.UTF_16; import static org.asynchttpclient.Dsl.asyncHttpClient; +import static org.asynchttpclient.Dsl.basicAuthRealm; import static org.asynchttpclient.Dsl.config; +import static org.asynchttpclient.Dsl.get; +import static org.asynchttpclient.Dsl.post; +import static org.asynchttpclient.Dsl.proxyServer; import static org.asynchttpclient.test.TestUtils.createSslEngineFactory; +import static org.asynchttpclient.util.HttpConstants.Methods.GET; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -280,4 +302,115 @@ public void testDisablingCookieStore() throws IOException { // } } + + @Test + public void testStoredCookiesKeepRawUrlAsBuilt() throws IOException { + String url = "http://localhost/a|b?q={x}"; + Request request = new RequestBuilder(GET, true).setUrl(url).build(); + assertEquals(url, request.getUrl()); + assertNotEquals(url, new RequestBuilder(GET).setUrl(url).build().getUrl(), "the URL must be one encoding would change"); + + Request sent = sendWithStoredCookies(request, cookie("session", "s")); + + assertEquals(url, sent.getUrl()); + assertEquals(List.of("session=s"), cookieStrings(sent)); + } + + @Test + public void testStoredCookiesDoNotReplaceRequestCookies() throws IOException { + Request request = get("http://localhost/").addCookie(cookie("a", "request")).build(); + + Request sent = sendWithStoredCookies(request, cookie("a", "store"), cookie("b", "store")); + + assertEquals(List.of("a=request", "b=store"), cookieStrings(sent)); + assertEquals(List.of("a=request"), cookieStrings(request)); + } + + @Test + public void testStoredCookiesAlreadySetLeaveRequestAsIs() throws IOException { + Request request = get("http://localhost/").addCookie(cookie("a", "request")).build(); + + Request sent = sendWithStoredCookies(request, cookie("a", "store")); + + assertSame(request, sent); + } + + @Test + public void testStoredCookiesKeepEveryOtherRequestField() throws Exception { + Request request = post("http://localhost/path?x=1") + .setAddress(InetAddress.getByAddress("localhost", new byte[]{127, 0, 0, 1})) + .setLocalAddress(InetAddress.getByAddress(new byte[]{127, 0, 0, 2})) + .setHeader("X-Custom", "v") + .setHeader(CONTENT_TYPE, "text/plain") + .setBody("body") + .setVirtualHost("vhost") + .setProxyServer(proxyServer("proxy", 8080)) + .setRealm(basicAuthRealm("user", "pass")) + .setFollowRedirect(true) + .setUseAbsoluteRequestDeadline(true) + .setRefuseSchemeDowngradeOnRedirect(true) + .setRefuseCrossOriginBodyOnRedirect(true) + .setRequestTimeout(Duration.ofSeconds(3)) + .setReadTimeout(Duration.ofSeconds(2)) + .setCharset(UTF_16) + .build(); + + Request sent = sendWithStoredCookies(request, cookie("session", "s")); + + assertNotSame(request, sent); + assertEquals(List.of("session=s"), cookieStrings(sent)); + assertEquals(request.getMethod(), sent.getMethod()); + assertEquals(request.getUri(), sent.getUri()); + assertEquals(request.getAddress(), sent.getAddress()); + assertEquals(request.getLocalAddress(), sent.getLocalAddress()); + assertEquals(request.getHeaders().entries().toString(), sent.getHeaders().entries().toString()); + assertEquals(request.getStringData(), sent.getStringData()); + assertEquals(request.getVirtualHost(), sent.getVirtualHost()); + assertSame(request.getProxyServer(), sent.getProxyServer()); + assertSame(request.getRealm(), sent.getRealm()); + assertEquals(request.getFollowRedirect(), sent.getFollowRedirect()); + assertEquals(request.getUseAbsoluteRequestDeadline(), sent.getUseAbsoluteRequestDeadline()); + assertEquals(request.getRefuseSchemeDowngradeOnRedirect(), sent.getRefuseSchemeDowngradeOnRedirect()); + assertEquals(request.getRefuseCrossOriginBodyOnRedirect(), sent.getRefuseCrossOriginBodyOnRedirect()); + assertEquals(request.getRequestTimeout(), sent.getRequestTimeout()); + assertEquals(request.getReadTimeout(), sent.getReadTimeout()); + assertEquals(request.getRangeOffset(), sent.getRangeOffset()); + assertEquals(request.getCharset(), sent.getCharset()); + assertSame(request.getChannelPoolPartitioning(), sent.getChannelPoolPartitioning()); + assertSame(request.getNameResolver(), sent.getNameResolver()); + assertEquals(request.getQueryParams(), sent.getQueryParams()); + } + + /** Executes the request on a client whose store holds these cookies for localhost and returns what reached the filters. */ + private static Request sendWithStoredCookies(Request request, Cookie... storedCookies) throws IOException { + CookieStore cookieStore = new ThreadSafeCookieStore(); + for (Cookie cookie : storedCookies) { + cookieStore.add(Uri.create("http://localhost/"), cookie); + } + AtomicReference sent = new AtomicReference<>(); + RequestFilter capture = new RequestFilter() { + @Override + public FilterContext filter(FilterContext ctx) throws FilterException { + sent.set(ctx.getRequest()); + throw new FilterException("captured"); + } + }; + AsyncHttpClientConfig config = config().setCookieStore(cookieStore).addRequestFilter(capture).build(); + try (AsyncHttpClient client = asyncHttpClient(config)) { + ListenableFuture future = client.executeRequest(request); + assertThrows(ExecutionException.class, future::get); + } + assertNotNull(sent.get()); + return sent.get(); + } + + private static Cookie cookie(String name, String value) { + DefaultCookie cookie = new DefaultCookie(name, value); + cookie.setPath("/"); + return cookie; + } + + private static List cookieStrings(Request request) { + return request.getCookies().stream().map(c -> c.name() + '=' + c.value()).collect(Collectors.toList()); + } } From 3c843f5b8ee1cefd871bd13d65d0e9be674f664c Mon Sep 17 00:00:00 2001 From: Pavel Ptashyts <49400901+pavel-ptashyts@users.noreply.github.com> Date: Sat, 7 Nov 2026 20:22:45 +0100 Subject: [PATCH 2/3] Cover every copied field in stored cookie tests The field test set all four Boolean flags to true, so swapping two adjacent Boolean parameters in DefaultRequest.withCookies would still pass. It now uses two requests in which every pair of flags differs, and compares the body fields too. New tests check that the first stored cookie of a name wins and that a DefaultRequest subclass still goes through toBuilder(). Claude Code on behalf of Pavel Ptashyts Co-Authored-By: Claude Opus 5.5 --- .../DefaultAsyncHttpClientTest.java | 127 +++++++++++++----- 1 file changed, 90 insertions(+), 37 deletions(-) diff --git a/client/src/test/java/org/asynchttpclient/DefaultAsyncHttpClientTest.java b/client/src/test/java/org/asynchttpclient/DefaultAsyncHttpClientTest.java index c659cba60b..cf4b64a178 100644 --- a/client/src/test/java/org/asynchttpclient/DefaultAsyncHttpClientTest.java +++ b/client/src/test/java/org/asynchttpclient/DefaultAsyncHttpClientTest.java @@ -49,6 +49,7 @@ import java.util.List; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; @@ -336,49 +337,101 @@ public void testStoredCookiesAlreadySetLeaveRequestAsIs() throws IOException { } @Test - public void testStoredCookiesKeepEveryOtherRequestField() throws Exception { - Request request = post("http://localhost/path?x=1") - .setAddress(InetAddress.getByAddress("localhost", new byte[]{127, 0, 0, 1})) - .setLocalAddress(InetAddress.getByAddress(new byte[]{127, 0, 0, 2})) - .setHeader("X-Custom", "v") + public void testStoredCookiesKeepFirstStoredCookieOfAName() throws IOException { + Request request = get("http://localhost/x/y").build(); + DefaultCookie longerPath = new DefaultCookie("a", "longer-path"); + longerPath.setPath("/x"); + + Request sent = sendWithStoredCookies(request, cookie("a", "root-path"), longerPath); + + assertEquals(List.of("a=longer-path"), cookieStrings(sent)); + } + + @Test + public void testStoredCookiesKeepOtherRequestFields() throws Exception { + // The flags are adjacent Boolean parameters of DefaultRequest: each pair differs in one of the two requests. + RequestBuilder stringBody = post("http://localhost/path?x=1") .setHeader(CONTENT_TYPE, "text/plain") .setBody("body") - .setVirtualHost("vhost") - .setProxyServer(proxyServer("proxy", 8080)) - .setRealm(basicAuthRealm("user", "pass")) .setFollowRedirect(true) + .setUseAbsoluteRequestDeadline(false) + .setRefuseCrossOriginBodyOnRedirect(true); + RequestBuilder formBody = post("http://localhost/path?x=1") + .addFormParam("p", "v") .setUseAbsoluteRequestDeadline(true) - .setRefuseSchemeDowngradeOnRedirect(true) - .setRefuseCrossOriginBodyOnRedirect(true) - .setRequestTimeout(Duration.ofSeconds(3)) - .setReadTimeout(Duration.ofSeconds(2)) - .setCharset(UTF_16) - .build(); + .setRefuseSchemeDowngradeOnRedirect(false) + .setRefuseCrossOriginBodyOnRedirect(false); + + for (RequestBuilder builder : List.of(stringBody, formBody)) { + Request request = builder + .setAddress(InetAddress.getByAddress("localhost", new byte[]{127, 0, 0, 1})) + .setLocalAddress(InetAddress.getByAddress(new byte[]{127, 0, 0, 2})) + .setHeader("X-Custom", "v") + .setVirtualHost("vhost") + .setProxyServer(proxyServer("proxy", 8080)) + .setRealm(basicAuthRealm("user", "pass")) + .setRequestTimeout(Duration.ofSeconds(3)) + .setReadTimeout(Duration.ofSeconds(2)) + .setCharset(UTF_16) + .build(); + + Request sent = sendWithStoredCookies(request, cookie("session", "s")); + + assertNotSame(request, sent); + assertEquals(List.of("session=s"), cookieStrings(sent)); + assertEquals(request.getMethod(), sent.getMethod()); + assertEquals(request.getUri(), sent.getUri()); + assertEquals(request.getAddress(), sent.getAddress()); + assertEquals(request.getLocalAddress(), sent.getLocalAddress()); + assertEquals(request.getHeaders().entries().toString(), sent.getHeaders().entries().toString()); + assertSame(request.getByteData(), sent.getByteData()); + assertSame(request.getCompositeByteData(), sent.getCompositeByteData()); + assertSame(request.getStringData(), sent.getStringData()); + assertSame(request.getByteBufferData(), sent.getByteBufferData()); + assertSame(request.getByteBufData(), sent.getByteBufData()); + assertSame(request.getStreamData(), sent.getStreamData()); + assertSame(request.getBodyGenerator(), sent.getBodyGenerator()); + assertEquals(request.getFormParams(), sent.getFormParams()); + assertEquals(request.getBodyParts(), sent.getBodyParts()); + assertEquals(request.getVirtualHost(), sent.getVirtualHost()); + assertSame(request.getProxyServer(), sent.getProxyServer()); + assertSame(request.getRealm(), sent.getRealm()); + assertSame(request.getFile(), sent.getFile()); + assertEquals(request.getFollowRedirect(), sent.getFollowRedirect()); + assertEquals(request.getUseAbsoluteRequestDeadline(), sent.getUseAbsoluteRequestDeadline()); + assertEquals(request.getRefuseSchemeDowngradeOnRedirect(), sent.getRefuseSchemeDowngradeOnRedirect()); + assertEquals(request.getRefuseCrossOriginBodyOnRedirect(), sent.getRefuseCrossOriginBodyOnRedirect()); + assertEquals(request.getRequestTimeout(), sent.getRequestTimeout()); + assertEquals(request.getReadTimeout(), sent.getReadTimeout()); + assertEquals(request.getRangeOffset(), sent.getRangeOffset()); + assertEquals(request.getCharset(), sent.getCharset()); + assertSame(request.getChannelPoolPartitioning(), sent.getChannelPoolPartitioning()); + assertSame(request.getNameResolver(), sent.getNameResolver()); + assertEquals(request.getQueryParams(), sent.getQueryParams()); + } + } - Request sent = sendWithStoredCookies(request, cookie("session", "s")); + @Test + public void testStoredCookiesGoThroughToBuilderForRequestSubclass() throws IOException { + Request built = get("http://localhost/").addCookie(cookie("a", "request")).build(); + AtomicInteger toBuilderCalls = new AtomicInteger(); + Request request = new DefaultRequest(built.getMethod(), built.getUri(), built.getAddress(), built.getLocalAddress(), + built.getHeaders(), built.getCookies(), built.getByteData(), built.getCompositeByteData(), built.getStringData(), + built.getByteBufferData(), built.getByteBufData(), built.getStreamData(), built.getBodyGenerator(), + built.getFormParams(), built.getBodyParts(), built.getVirtualHost(), built.getProxyServer(), built.getRealm(), + built.getFile(), built.getFollowRedirect(), built.getRequestTimeout(), built.getReadTimeout(), + built.getRangeOffset(), built.getCharset(), built.getChannelPoolPartitioning(), built.getNameResolver()) { + @Override + public RequestBuilder toBuilder() { + toBuilderCalls.incrementAndGet(); + return super.toBuilder(); + } + }; - assertNotSame(request, sent); - assertEquals(List.of("session=s"), cookieStrings(sent)); - assertEquals(request.getMethod(), sent.getMethod()); - assertEquals(request.getUri(), sent.getUri()); - assertEquals(request.getAddress(), sent.getAddress()); - assertEquals(request.getLocalAddress(), sent.getLocalAddress()); - assertEquals(request.getHeaders().entries().toString(), sent.getHeaders().entries().toString()); - assertEquals(request.getStringData(), sent.getStringData()); - assertEquals(request.getVirtualHost(), sent.getVirtualHost()); - assertSame(request.getProxyServer(), sent.getProxyServer()); - assertSame(request.getRealm(), sent.getRealm()); - assertEquals(request.getFollowRedirect(), sent.getFollowRedirect()); - assertEquals(request.getUseAbsoluteRequestDeadline(), sent.getUseAbsoluteRequestDeadline()); - assertEquals(request.getRefuseSchemeDowngradeOnRedirect(), sent.getRefuseSchemeDowngradeOnRedirect()); - assertEquals(request.getRefuseCrossOriginBodyOnRedirect(), sent.getRefuseCrossOriginBodyOnRedirect()); - assertEquals(request.getRequestTimeout(), sent.getRequestTimeout()); - assertEquals(request.getReadTimeout(), sent.getReadTimeout()); - assertEquals(request.getRangeOffset(), sent.getRangeOffset()); - assertEquals(request.getCharset(), sent.getCharset()); - assertSame(request.getChannelPoolPartitioning(), sent.getChannelPoolPartitioning()); - assertSame(request.getNameResolver(), sent.getNameResolver()); - assertEquals(request.getQueryParams(), sent.getQueryParams()); + Request sent = sendWithStoredCookies(request, cookie("a", "store"), cookie("b", "store")); + + assertEquals(1, toBuilderCalls.get()); + assertEquals(List.of("a=request", "b=store"), cookieStrings(sent)); } /** Executes the request on a client whose store holds these cookies for localhost and returns what reached the filters. */ From f7ff0168454224193f4aeb2c6cbb98138858d509 Mon Sep 17 00:00:00 2001 From: Pavel Ptashyts <49400901+pavel-ptashyts@users.noreply.github.com> Date: Sat, 7 Nov 2026 20:27:29 +0100 Subject: [PATCH 3/3] Treat a null request cookie list as empty A DefaultRequest made through its public constructor may carry a null cookie list. The toBuilder() path treated it as empty, so the new path failed with a NullPointerException only when the cookie store had a cookie for the request, while the same request without a stored cookie was sent. Claude Code on behalf of Pavel Ptashyts Co-Authored-By: Claude Opus 5.5 --- .../asynchttpclient/DefaultAsyncHttpClient.java | 5 ++++- .../DefaultAsyncHttpClientTest.java | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClient.java b/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClient.java index e1f2c3dff0..3535fd4b26 100644 --- a/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClient.java +++ b/client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClient.java @@ -35,6 +35,7 @@ import org.slf4j.LoggerFactory; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; @@ -51,6 +52,7 @@ import static org.asynchttpclient.util.HttpConstants.Methods.POST; import static org.asynchttpclient.util.HttpConstants.Methods.PUT; import static org.asynchttpclient.util.HttpConstants.Methods.TRACE; +import static org.asynchttpclient.util.MiscUtils.isNonEmpty; /** * Default and threadsafe implementation of {@link AsyncHttpClient}. @@ -286,7 +288,8 @@ private static Request addCookiesIfUnset(Request request, List storedCoo return requestBuilder.build(); } - List requestCookies = request.getCookies(); + // A DefaultRequest from the public constructor may carry null, which the builder path treated as empty. + List requestCookies = isNonEmpty(request.getCookies()) ? request.getCookies() : Collections.emptyList(); List cookies = null; for (Cookie cookie : storedCookies) { if (!hasCookieNamed(cookies == null ? requestCookies : cookies, cookie.name())) { diff --git a/client/src/test/java/org/asynchttpclient/DefaultAsyncHttpClientTest.java b/client/src/test/java/org/asynchttpclient/DefaultAsyncHttpClientTest.java index cf4b64a178..956c1a3229 100644 --- a/client/src/test/java/org/asynchttpclient/DefaultAsyncHttpClientTest.java +++ b/client/src/test/java/org/asynchttpclient/DefaultAsyncHttpClientTest.java @@ -434,6 +434,21 @@ public RequestBuilder toBuilder() { assertEquals(List.of("a=request", "b=store"), cookieStrings(sent)); } + @Test + public void testStoredCookiesAddedToRequestWithNullCookieList() throws IOException { + Request built = get("http://localhost/").build(); + Request request = new DefaultRequest(built.getMethod(), built.getUri(), built.getAddress(), built.getLocalAddress(), + built.getHeaders(), null, built.getByteData(), built.getCompositeByteData(), built.getStringData(), + built.getByteBufferData(), built.getByteBufData(), built.getStreamData(), built.getBodyGenerator(), + built.getFormParams(), built.getBodyParts(), built.getVirtualHost(), built.getProxyServer(), built.getRealm(), + built.getFile(), built.getFollowRedirect(), built.getRequestTimeout(), built.getReadTimeout(), + built.getRangeOffset(), built.getCharset(), built.getChannelPoolPartitioning(), built.getNameResolver()); + + Request sent = sendWithStoredCookies(request, cookie("session", "s")); + + assertEquals(List.of("session=s"), cookieStrings(sent)); + } + /** Executes the request on a client whose store holds these cookies for localhost and returns what reached the filters. */ private static Request sendWithStoredCookies(Request request, Cookie... storedCookies) throws IOException { CookieStore cookieStore = new ThreadSafeCookieStore();