Skip to content

Commit 97f3218

Browse files
committed
Reduce per-request allocation in ThreadSafeCookieStore.get()
1 parent 7644c3c commit 97f3218

2 files changed

Lines changed: 145 additions & 19 deletions

File tree

client/src/main/java/org/asynchttpclient/cookie/ThreadSafeCookieStore.java

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -205,36 +205,38 @@ private List<Cookie> get(String domain, String path, boolean secure) {
205205
List<Cookie> results = null;
206206

207207
while (MiscUtils.isNonEmpty(subDomain)) {
208-
final List<Cookie> storedCookies = getStoredCookies(subDomain, path, secure, exactDomainMatch);
209-
subDomain = DomainUtils.getSubDomain(subDomain);
210-
exactDomainMatch = false;
211-
if (storedCookies.isEmpty()) {
212-
continue;
213-
}
208+
// Lazily allocate a single result list and append matches straight into it; an imperative
209+
// scan avoids the per-sub-domain-level Stream pipeline (filter/map stages, two capturing
210+
// lambdas, a spliterator and the Collectors.toList intermediate list) that this ran on every
211+
// cookie-enabled request. Cookie header order is re-sorted by Netty's ClientCookieEncoder,
212+
// so the (preserved) entrySet iteration order is not observable on the wire.
214213
if (results == null) {
215214
results = new ArrayList<>(4);
216215
}
217-
results.addAll(storedCookies);
216+
collectStoredCookies(subDomain, path, secure, exactDomainMatch, results);
217+
subDomain = DomainUtils.getSubDomain(subDomain);
218+
exactDomainMatch = false;
218219
}
219220

220-
return results == null ? Collections.emptyList() : results;
221+
return results == null || results.isEmpty() ? Collections.emptyList() : results;
221222
}
222223

223-
private List<Cookie> getStoredCookies(String domain, String path, boolean secure, boolean isExactMatch) {
224+
private void collectStoredCookies(String domain, String path, boolean secure, boolean isExactMatch, List<Cookie> out) {
224225
final Map<CookieKey, StoredCookie> innerMap = cookieJar.get(domain);
225226
if (innerMap == null) {
226-
return Collections.emptyList();
227+
return;
227228
}
228229

229-
return innerMap.entrySet().stream().filter(pair -> {
230-
CookieKey key = pair.getKey();
231-
StoredCookie storedCookie = pair.getValue();
232-
boolean hasCookieExpired = hasCookieExpired(storedCookie.cookie, storedCookie.createdAt);
233-
return !hasCookieExpired &&
234-
(isExactMatch || !storedCookie.hostOnly) &&
235-
pathsMatch(key.path, path) &&
236-
(secure || !storedCookie.cookie.isSecure());
237-
}).map(v -> v.getValue().cookie).collect(Collectors.toList());
230+
for (Map.Entry<CookieKey, StoredCookie> entry : innerMap.entrySet()) {
231+
CookieKey key = entry.getKey();
232+
StoredCookie storedCookie = entry.getValue();
233+
if (!hasCookieExpired(storedCookie.cookie, storedCookie.createdAt)
234+
&& (isExactMatch || !storedCookie.hostOnly)
235+
&& pathsMatch(key.path, path)
236+
&& (secure || !storedCookie.cookie.isSecure())) {
237+
out.add(storedCookie.cookie);
238+
}
239+
}
238240
}
239241

240242
private void removeExpired() {
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
/*
2+
* Copyright (c) 2026 AsyncHttpClient Project. All rights reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.asynchttpclient.cookie;
17+
18+
import io.netty.handler.codec.http.cookie.ClientCookieDecoder;
19+
import io.netty.handler.codec.http.cookie.Cookie;
20+
import org.asynchttpclient.uri.Uri;
21+
import org.junit.jupiter.api.Test;
22+
23+
import java.util.List;
24+
import java.util.Set;
25+
import java.util.TreeSet;
26+
27+
import static org.junit.jupiter.api.Assertions.assertEquals;
28+
import static org.junit.jupiter.api.Assertions.assertTrue;
29+
30+
/**
31+
* Behavior-equivalence tests for the imperative {@link ThreadSafeCookieStore#get(Uri)} scan that
32+
* replaced the per-sub-domain Stream pipeline. The returned cookie set must be unchanged; iteration
33+
* order is not contractual (Netty's {@code ClientCookieEncoder} re-sorts on the wire), so results are
34+
* compared as a SET of {@code name=value} pairs.
35+
*/
36+
public class ThreadSafeCookieStoreGetTest {
37+
38+
private static Set<String> namesValues(List<Cookie> cookies) {
39+
Set<String> out = new TreeSet<>();
40+
for (Cookie c : cookies) {
41+
out.add(c.name() + '=' + c.value());
42+
}
43+
return out;
44+
}
45+
46+
private static Set<String> setOf(String... values) {
47+
Set<String> out = new TreeSet<>();
48+
for (String v : values) {
49+
out.add(v);
50+
}
51+
return out;
52+
}
53+
54+
@Test
55+
public void returnsCookieOnExactDomainAndPath() {
56+
ThreadSafeCookieStore store = new ThreadSafeCookieStore();
57+
store.add(Uri.create("http://www.foo.com/bar"),
58+
ClientCookieDecoder.LAX.decode("ALPHA=VALUE1; Domain=www.foo.com; path=/bar"));
59+
60+
assertEquals(setOf("ALPHA=VALUE1"), namesValues(store.get(Uri.create("http://www.foo.com/bar/baz"))));
61+
}
62+
63+
@Test
64+
public void inheritsParentDomainCookieButExcludesHostOnlyCookie() {
65+
ThreadSafeCookieStore store = new ThreadSafeCookieStore();
66+
// Domain cookie on .foo.com -> visible to sub.foo.com
67+
store.add(Uri.create("http://foo.com/"),
68+
ClientCookieDecoder.LAX.decode("DOMAINCK=DV; Domain=foo.com; Path=/"));
69+
// Host-only cookie on foo.com (no Domain attr) -> NOT visible to a sub-domain request
70+
store.add(Uri.create("http://foo.com/"),
71+
ClientCookieDecoder.LAX.decode("HOSTONLY=HV; Path=/"));
72+
73+
// Request to the sub-domain: sees the domain cookie, not the host-only one.
74+
assertEquals(setOf("DOMAINCK=DV"), namesValues(store.get(Uri.create("http://sub.foo.com/"))));
75+
// Request to the exact host: sees both.
76+
assertEquals(setOf("DOMAINCK=DV", "HOSTONLY=HV"), namesValues(store.get(Uri.create("http://foo.com/"))));
77+
}
78+
79+
@Test
80+
public void excludesSecureCookieOnInsecureRequest() {
81+
ThreadSafeCookieStore store = new ThreadSafeCookieStore();
82+
store.add(Uri.create("https://www.foo.com/"),
83+
ClientCookieDecoder.LAX.decode("SEC=SV; Domain=www.foo.com; Path=/; Secure"));
84+
store.add(Uri.create("http://www.foo.com/"),
85+
ClientCookieDecoder.LAX.decode("PLAIN=PV; Domain=www.foo.com; Path=/"));
86+
87+
// Insecure request: only the non-secure cookie.
88+
assertEquals(setOf("PLAIN=PV"), namesValues(store.get(Uri.create("http://www.foo.com/"))));
89+
// Secure request: both.
90+
assertEquals(setOf("SEC=SV", "PLAIN=PV"), namesValues(store.get(Uri.create("https://www.foo.com/"))));
91+
}
92+
93+
@Test
94+
public void excludesExpiredCookie() {
95+
ThreadSafeCookieStore store = new ThreadSafeCookieStore();
96+
store.add(Uri.create("http://www.foo.com/bar"),
97+
ClientCookieDecoder.LAX.decode("LIVE=VALUE1; Domain=www.foo.com; path=/bar"));
98+
// Max-Age=0 expires immediately; the store drops it on add, so it must never be returned.
99+
store.add(Uri.create("http://www.foo.com/bar"),
100+
ClientCookieDecoder.LAX.decode("DEAD=GONE; Domain=www.foo.com; path=/bar; Max-Age=0"));
101+
102+
assertEquals(setOf("LIVE=VALUE1"), namesValues(store.get(Uri.create("http://www.foo.com/bar"))));
103+
}
104+
105+
@Test
106+
public void returnsMultipleDistinctCookiesAtSameDomainPath() {
107+
ThreadSafeCookieStore store = new ThreadSafeCookieStore();
108+
Uri uri = Uri.create("http://www.foo.com/bar");
109+
store.add(uri, ClientCookieDecoder.LAX.decode("ALPHA=AV; Domain=www.foo.com; path=/bar"));
110+
store.add(uri, ClientCookieDecoder.LAX.decode("BETA=BV; Domain=www.foo.com; path=/bar"));
111+
112+
assertEquals(setOf("ALPHA=AV", "BETA=BV"), namesValues(store.get(uri)));
113+
}
114+
115+
@Test
116+
public void returnsEmptyForUnknownDomain() {
117+
ThreadSafeCookieStore store = new ThreadSafeCookieStore();
118+
store.add(Uri.create("http://www.foo.com/"),
119+
ClientCookieDecoder.LAX.decode("ALPHA=VALUE1; Domain=www.foo.com; Path=/"));
120+
121+
List<Cookie> result = store.get(Uri.create("http://www.bar.com/"));
122+
assertTrue(result.isEmpty(), "no cookies should match an unrelated domain");
123+
}
124+
}

0 commit comments

Comments
 (0)