From 5b3a98ecce0f351da4945b4a7aebcd4192eade8a Mon Sep 17 00:00:00 2001 From: Aayush Atharva Date: Mon, 15 Jun 2026 22:20:11 +0000 Subject: [PATCH] Reduce per-request allocation in ThreadSafeCookieStore.get() --- .../cookie/ThreadSafeCookieStore.java | 40 +++--- .../cookie/ThreadSafeCookieStoreGetTest.java | 124 ++++++++++++++++++ 2 files changed, 145 insertions(+), 19 deletions(-) create mode 100644 client/src/test/java/org/asynchttpclient/cookie/ThreadSafeCookieStoreGetTest.java diff --git a/client/src/main/java/org/asynchttpclient/cookie/ThreadSafeCookieStore.java b/client/src/main/java/org/asynchttpclient/cookie/ThreadSafeCookieStore.java index ca58939ac..ab0e63235 100644 --- a/client/src/main/java/org/asynchttpclient/cookie/ThreadSafeCookieStore.java +++ b/client/src/main/java/org/asynchttpclient/cookie/ThreadSafeCookieStore.java @@ -205,36 +205,38 @@ private List get(String domain, String path, boolean secure) { List results = null; while (MiscUtils.isNonEmpty(subDomain)) { - final List storedCookies = getStoredCookies(subDomain, path, secure, exactDomainMatch); - subDomain = DomainUtils.getSubDomain(subDomain); - exactDomainMatch = false; - if (storedCookies.isEmpty()) { - continue; - } + // Lazily allocate a single result list and append matches straight into it; an imperative + // scan avoids the per-sub-domain-level Stream pipeline (filter/map stages, two capturing + // lambdas, a spliterator and the Collectors.toList intermediate list) that this ran on every + // cookie-enabled request. Cookie header order is re-sorted by Netty's ClientCookieEncoder, + // so the (preserved) entrySet iteration order is not observable on the wire. if (results == null) { results = new ArrayList<>(4); } - results.addAll(storedCookies); + collectStoredCookies(subDomain, path, secure, exactDomainMatch, results); + subDomain = DomainUtils.getSubDomain(subDomain); + exactDomainMatch = false; } - return results == null ? Collections.emptyList() : results; + return results == null || results.isEmpty() ? Collections.emptyList() : results; } - private List getStoredCookies(String domain, String path, boolean secure, boolean isExactMatch) { + private void collectStoredCookies(String domain, String path, boolean secure, boolean isExactMatch, List out) { final Map innerMap = cookieJar.get(domain); if (innerMap == null) { - return Collections.emptyList(); + return; } - return innerMap.entrySet().stream().filter(pair -> { - CookieKey key = pair.getKey(); - StoredCookie storedCookie = pair.getValue(); - boolean hasCookieExpired = hasCookieExpired(storedCookie.cookie, storedCookie.createdAt); - return !hasCookieExpired && - (isExactMatch || !storedCookie.hostOnly) && - pathsMatch(key.path, path) && - (secure || !storedCookie.cookie.isSecure()); - }).map(v -> v.getValue().cookie).collect(Collectors.toList()); + for (Map.Entry entry : innerMap.entrySet()) { + CookieKey key = entry.getKey(); + StoredCookie storedCookie = entry.getValue(); + if (!hasCookieExpired(storedCookie.cookie, storedCookie.createdAt) + && (isExactMatch || !storedCookie.hostOnly) + && pathsMatch(key.path, path) + && (secure || !storedCookie.cookie.isSecure())) { + out.add(storedCookie.cookie); + } + } } private void removeExpired() { diff --git a/client/src/test/java/org/asynchttpclient/cookie/ThreadSafeCookieStoreGetTest.java b/client/src/test/java/org/asynchttpclient/cookie/ThreadSafeCookieStoreGetTest.java new file mode 100644 index 000000000..6234549fa --- /dev/null +++ b/client/src/test/java/org/asynchttpclient/cookie/ThreadSafeCookieStoreGetTest.java @@ -0,0 +1,124 @@ +/* + * 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.cookie; + +import io.netty.handler.codec.http.cookie.ClientCookieDecoder; +import io.netty.handler.codec.http.cookie.Cookie; +import org.asynchttpclient.uri.Uri; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Set; +import java.util.TreeSet; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Behavior-equivalence tests for the imperative {@link ThreadSafeCookieStore#get(Uri)} scan that + * replaced the per-sub-domain Stream pipeline. The returned cookie set must be unchanged; iteration + * order is not contractual (Netty's {@code ClientCookieEncoder} re-sorts on the wire), so results are + * compared as a SET of {@code name=value} pairs. + */ +public class ThreadSafeCookieStoreGetTest { + + private static Set namesValues(List cookies) { + Set out = new TreeSet<>(); + for (Cookie c : cookies) { + out.add(c.name() + '=' + c.value()); + } + return out; + } + + private static Set setOf(String... values) { + Set out = new TreeSet<>(); + for (String v : values) { + out.add(v); + } + return out; + } + + @Test + public void returnsCookieOnExactDomainAndPath() { + ThreadSafeCookieStore store = new ThreadSafeCookieStore(); + store.add(Uri.create("http://www.foo.com/bar"), + ClientCookieDecoder.LAX.decode("ALPHA=VALUE1; Domain=www.foo.com; path=/bar")); + + assertEquals(setOf("ALPHA=VALUE1"), namesValues(store.get(Uri.create("http://www.foo.com/bar/baz")))); + } + + @Test + public void inheritsParentDomainCookieButExcludesHostOnlyCookie() { + ThreadSafeCookieStore store = new ThreadSafeCookieStore(); + // Domain cookie on .foo.com -> visible to sub.foo.com + store.add(Uri.create("http://foo.com/"), + ClientCookieDecoder.LAX.decode("DOMAINCK=DV; Domain=foo.com; Path=/")); + // Host-only cookie on foo.com (no Domain attr) -> NOT visible to a sub-domain request + store.add(Uri.create("http://foo.com/"), + ClientCookieDecoder.LAX.decode("HOSTONLY=HV; Path=/")); + + // Request to the sub-domain: sees the domain cookie, not the host-only one. + assertEquals(setOf("DOMAINCK=DV"), namesValues(store.get(Uri.create("http://sub.foo.com/")))); + // Request to the exact host: sees both. + assertEquals(setOf("DOMAINCK=DV", "HOSTONLY=HV"), namesValues(store.get(Uri.create("http://foo.com/")))); + } + + @Test + public void excludesSecureCookieOnInsecureRequest() { + ThreadSafeCookieStore store = new ThreadSafeCookieStore(); + store.add(Uri.create("https://www.foo.com/"), + ClientCookieDecoder.LAX.decode("SEC=SV; Domain=www.foo.com; Path=/; Secure")); + store.add(Uri.create("http://www.foo.com/"), + ClientCookieDecoder.LAX.decode("PLAIN=PV; Domain=www.foo.com; Path=/")); + + // Insecure request: only the non-secure cookie. + assertEquals(setOf("PLAIN=PV"), namesValues(store.get(Uri.create("http://www.foo.com/")))); + // Secure request: both. + assertEquals(setOf("SEC=SV", "PLAIN=PV"), namesValues(store.get(Uri.create("https://www.foo.com/")))); + } + + @Test + public void excludesExpiredCookie() { + ThreadSafeCookieStore store = new ThreadSafeCookieStore(); + store.add(Uri.create("http://www.foo.com/bar"), + ClientCookieDecoder.LAX.decode("LIVE=VALUE1; Domain=www.foo.com; path=/bar")); + // Max-Age=0 expires immediately; the store drops it on add, so it must never be returned. + store.add(Uri.create("http://www.foo.com/bar"), + ClientCookieDecoder.LAX.decode("DEAD=GONE; Domain=www.foo.com; path=/bar; Max-Age=0")); + + assertEquals(setOf("LIVE=VALUE1"), namesValues(store.get(Uri.create("http://www.foo.com/bar")))); + } + + @Test + public void returnsMultipleDistinctCookiesAtSameDomainPath() { + ThreadSafeCookieStore store = new ThreadSafeCookieStore(); + Uri uri = Uri.create("http://www.foo.com/bar"); + store.add(uri, ClientCookieDecoder.LAX.decode("ALPHA=AV; Domain=www.foo.com; path=/bar")); + store.add(uri, ClientCookieDecoder.LAX.decode("BETA=BV; Domain=www.foo.com; path=/bar")); + + assertEquals(setOf("ALPHA=AV", "BETA=BV"), namesValues(store.get(uri))); + } + + @Test + public void returnsEmptyForUnknownDomain() { + ThreadSafeCookieStore store = new ThreadSafeCookieStore(); + store.add(Uri.create("http://www.foo.com/"), + ClientCookieDecoder.LAX.decode("ALPHA=VALUE1; Domain=www.foo.com; Path=/")); + + List result = store.get(Uri.create("http://www.bar.com/")); + assertTrue(result.isEmpty(), "no cookies should match an unrelated domain"); + } +}