Cap cookies retained per domain (RFC 6265 5.3)

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

@@ -24,21 +24,34 @@
 import java.util.AbstractMap;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 import static java.util.Objects.requireNonNull;
 
 public final class ThreadSafeCookieStore implements CookieStore {
 
+ // RFC 6265 §5.5 (Implementation Limits) lets a user agent bound the cookies it retains per domain (its
+ // floor is "at least 50 per domain"). Capping this 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. See
+ // evictExcessCookies for the eviction order. Package-private for tests.
+ static final int MAX_COOKIES_PER_DOMAIN = 200;
+
  private final Map<String, Map<CookieKey, StoredCookie>> cookieJar = new ConcurrentHashMap<>();
  private final AtomicInteger counter = new AtomicInteger();
+ // Monotonic per-store stamp giving each stored cookie a strict, tie-free, clock-independent insertion
+ // order for eviction (see evictExcessCookies). Preferred over creation time, which is millisecond-
+ // granular (so it ties under a flood) and wall-clock based (an NTP step backward would reorder it).
+ private final AtomicLong cookieSequence = new AtomicLong();
 
  @Override
  public void add(Uri uri, Cookie cookie) {
@@ -195,7 +208,46 @@ private void add(String requestDomain, String requestPath, Cookie cookie) {
  cookieJar.getOrDefault(keyDomain, Collections.emptyMap()).remove(key);
  } else {
  final Map<CookieKey, StoredCookie> innerMap = cookieJar.computeIfAbsent(keyDomain, domain -> new ConcurrentHashMap<>());
- innerMap.put(key, new StoredCookie(cookie, hostOnly, cookie.maxAge() != Cookie.UNDEFINED_MAX_AGE));
+ innerMap.put(key, new StoredCookie(cookie, hostOnly, cookie.maxAge() != Cookie.UNDEFINED_MAX_AGE, cookieSequence.getAndIncrement()));
+ 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.5 permits a
+ * per-domain cap; §5.3's "remove excess cookies" step evicts expired cookies first, then removes more
+ * until under the limit. The RFC breaks that second tie by least-recently-accessed; we do not track
+ * access time, so we deliberately deviate and evict in insertion order via the strict, tie-free
+ * {@link StoredCookie#seq} stamp.
+ *
+ * <p>Called from {@link #add} right after an insert pushes the bucket over the cap, so it normally
+ * removes a single entry. A single pass drops expired entries and collects the survivors; only if those
+ * still exceed the cap are they ordered by {@code seq} and the oldest excess removed. Victims are dropped
+ * with the two-arg {@code remove(key, value)}, which is identity-based (StoredCookie has no
+ * {@code equals()}): a cookie another thread just re-put under the same key is never collaterally
+ * removed. Two adders evicting concurrently pick the same seq-ordered victims, so their redundant removes
+ * no-op — the bucket may still briefly sit a little below the cap until the next add, but never grows
+ * unbounded.
+ */
+ private static void evictExcessCookies(Map<CookieKey, StoredCookie> innerMap) {
+ List<Map.Entry<CookieKey, StoredCookie>> live = new ArrayList<>(innerMap.size());
+ for (Map.Entry<CookieKey, StoredCookie> entry : innerMap.entrySet()) {
+ if (hasCookieExpired(entry.getValue().cookie, entry.getValue().createdAt)) {
+ innerMap.remove(entry.getKey(), entry.getValue());
+ } else {
+ live.add(entry);
+ }
+ }
+ int excess = live.size() - MAX_COOKIES_PER_DOMAIN;
+ if (excess <= 0) {
+ return;
+ }
+ live.sort(Comparator.comparingLong(entry -> entry.getValue().seq));
+ for (int i = 0; i < excess; i++) {
+ Map.Entry<CookieKey, StoredCookie> victim = live.get(i);
+ innerMap.remove(victim.getKey(), victim.getValue());
  }
  }
 
@@ -292,11 +344,14 @@ private static class StoredCookie {
  final boolean hostOnly;
  final boolean persistent;
  final long createdAt = System.currentTimeMillis();
+ // Strict, tie-free insertion order for eviction; see ThreadSafeCookieStore.cookieSequence.
+ final long seq;
 
- StoredCookie(Cookie cookie, boolean hostOnly, boolean persistent) {
+ StoredCookie(Cookie cookie, boolean hostOnly, boolean persistent, long seq) {
  this.cookie = cookie;
  this.hostOnly = hostOnly;
  this.persistent = persistent;
+ this.seq = seq;
  }
 
  @Override

client/src/test/java/org/asynchttpclient/cookie/ThreadSafeCookieStoreGetTest.java

@@ -112,6 +112,73 @@ 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 evictionDropsExpiredCookiesBeforeLiveOnes() throws InterruptedException {
+ ThreadSafeCookieStore store = new ThreadSafeCookieStore();
+ Uri uri = Uri.create("http://www.foo.com/");
+ int cap = ThreadSafeCookieStore.MAX_COOKIES_PER_DOMAIN;
+ int live = cap - 5;
+
+ // Fill the bucket exactly to the cap: (cap - 5) session cookies that never expire ...
+ for (int i = 0; i < live; i++) {
+ store.add(uri, ClientCookieDecoder.LAX.decode("live" + i + "=v; Domain=www.foo.com; Path=/"));
+ }
+ // ... plus 5 short-lived cookies that will expire before the next add.
+ for (int i = 0; i < 5; i++) {
+ store.add(uri, ClientCookieDecoder.LAX.decode("exp" + i + "=v; Domain=www.foo.com; Path=/; Max-Age=1"));
+ }
+ assertEquals(cap, store.getUnderlying().get("www.foo.com").size(), "precondition: bucket filled to the cap");
+
+ // Max-Age is second-granular, so let > 1s pass for the five short-lived cookies to expire.
+ Thread.sleep(2100);
+
+ // This add pushes the bucket over the cap and triggers eviction. RFC 6265 §5.3 drops expired
+ // cookies first, so all five expired ones go and no live cookie is evicted.
+ store.add(uri, ClientCookieDecoder.LAX.decode("trigger=v; Domain=www.foo.com; Path=/"));
+
+ assertEquals(live + 1, store.getUnderlying().get("www.foo.com").size(),
+ "eviction must drop the expired cookies first, leaving every live cookie in place");
+ assertEquals(live + 1, store.get(uri).size(), "all live cookies (and only those) remain retrievable");
+ }
+
  @Test
  public void returnsEmptyForUnknownDomain() {
  ThreadSafeCookieStore store = new ThreadSafeCookieStore();