diff --git a/client/src/main/java/org/asynchttpclient/cookie/ThreadSafeCookieStore.java b/client/src/main/java/org/asynchttpclient/cookie/ThreadSafeCookieStore.java index ab0e63235..e1e69b5a9 100644 --- a/client/src/main/java/org/asynchttpclient/cookie/ThreadSafeCookieStore.java +++ b/client/src/main/java/org/asynchttpclient/cookie/ThreadSafeCookieStore.java @@ -37,6 +37,12 @@ public final class ThreadSafeCookieStore implements CookieStore { + // RFC 6265 §5.3 (step 12) lets a user agent cap the cookies it retains. Bounding cookies per domain + // keeps a server from growing the jar — and the per-request retrieval scan in get(Uri) — without bound. + // Chosen generously (well above browser per-domain limits of ~50–180) so it only trips under abuse, + // never for realistic usage. Package-private for tests. + static final int MAX_COOKIES_PER_DOMAIN = 200; + private final Map> cookieJar = new ConcurrentHashMap<>(); private final AtomicInteger counter = new AtomicInteger(); @@ -196,6 +202,33 @@ private void add(String requestDomain, String requestPath, Cookie cookie) { } else { final Map innerMap = cookieJar.computeIfAbsent(keyDomain, domain -> new ConcurrentHashMap<>()); innerMap.put(key, new StoredCookie(cookie, hostOnly, cookie.maxAge() != Cookie.UNDEFINED_MAX_AGE)); + if (innerMap.size() > MAX_COOKIES_PER_DOMAIN) { + evictExcessCookies(innerMap); + } + } + } + + /** + * Bounds a single domain's cookie bucket at {@link #MAX_COOKIES_PER_DOMAIN} (RFC 6265 §5.3 step 12). + * Evicts expired entries first, then the oldest by creation time until back at the cap. Called from + * {@link #add} right after an insert pushes the bucket over the cap, so it normally removes a single + * entry. Concurrency-safe: entries are dropped with the two-arg {@code remove(key, value)} so a cookie + * another thread just replaced under the same key is never collaterally removed; a benign race between + * concurrent adders may evict one extra, which is self-correcting. + */ + private static void evictExcessCookies(Map innerMap) { + innerMap.entrySet().removeIf(entry -> hasCookieExpired(entry.getValue().cookie, entry.getValue().createdAt)); + while (innerMap.size() > MAX_COOKIES_PER_DOMAIN) { + Map.Entry oldest = null; + for (Map.Entry entry : innerMap.entrySet()) { + if (oldest == null || entry.getValue().createdAt < oldest.getValue().createdAt) { + oldest = entry; + } + } + if (oldest == null) { + break; + } + innerMap.remove(oldest.getKey(), oldest.getValue()); } } diff --git a/client/src/test/java/org/asynchttpclient/cookie/ThreadSafeCookieStoreGetTest.java b/client/src/test/java/org/asynchttpclient/cookie/ThreadSafeCookieStoreGetTest.java index 6234549fa..1e70957e4 100644 --- a/client/src/test/java/org/asynchttpclient/cookie/ThreadSafeCookieStoreGetTest.java +++ b/client/src/test/java/org/asynchttpclient/cookie/ThreadSafeCookieStoreGetTest.java @@ -112,6 +112,44 @@ public void returnsMultipleDistinctCookiesAtSameDomainPath() { assertEquals(setOf("ALPHA=AV", "BETA=BV"), namesValues(store.get(uri))); } + @Test + public void perDomainCookieCountIsCappedUnderFlood() { + ThreadSafeCookieStore store = new ThreadSafeCookieStore(); + Uri uri = Uri.create("http://www.foo.com/"); + int flood = ThreadSafeCookieStore.MAX_COOKIES_PER_DOMAIN + 50; + for (int i = 0; i < flood; i++) { + store.add(uri, ClientCookieDecoder.LAX.decode("c" + i + "=v" + i + "; Domain=www.foo.com; Path=/")); + } + assertEquals(ThreadSafeCookieStore.MAX_COOKIES_PER_DOMAIN, store.getUnderlying().get("www.foo.com").size(), + "a single domain's cookies must be capped at MAX_COOKIES_PER_DOMAIN"); + } + + @Test + public void cookiesUnderTheCapAreAllRetained() { + ThreadSafeCookieStore store = new ThreadSafeCookieStore(); + Uri uri = Uri.create("http://www.foo.com/"); + for (int i = 0; i < 20; i++) { + store.add(uri, ClientCookieDecoder.LAX.decode("c" + i + "=v" + i + "; Domain=www.foo.com; Path=/")); + } + assertEquals(20, store.getUnderlying().get("www.foo.com").size(), "nothing is evicted below the cap"); + assertEquals(20, store.get(uri).size()); + } + + @Test + public void capIsPerDomainNotGlobal() { + ThreadSafeCookieStore store = new ThreadSafeCookieStore(); + Uri foo = Uri.create("http://www.foo.com/"); + for (int i = 0; i < ThreadSafeCookieStore.MAX_COOKIES_PER_DOMAIN + 20; i++) { + store.add(foo, ClientCookieDecoder.LAX.decode("c" + i + "=v" + i + "; Domain=www.foo.com; Path=/")); + } + store.add(Uri.create("http://www.bar.com/"), + ClientCookieDecoder.LAX.decode("only=1; Domain=www.bar.com; Path=/")); + + assertEquals(ThreadSafeCookieStore.MAX_COOKIES_PER_DOMAIN, store.getUnderlying().get("www.foo.com").size()); + assertEquals(1, store.getUnderlying().get("www.bar.com").size(), + "flooding one domain must not evict another domain's cookies"); + } + @Test public void returnsEmptyForUnknownDomain() { ThreadSafeCookieStore store = new ThreadSafeCookieStore();