diff --git a/client/src/main/java/org/asynchttpclient/cookie/ThreadSafeCookieStore.java b/client/src/main/java/org/asynchttpclient/cookie/ThreadSafeCookieStore.java index 311e70a7c..31c22ae84 100644 --- a/client/src/main/java/org/asynchttpclient/cookie/ThreadSafeCookieStore.java +++ b/client/src/main/java/org/asynchttpclient/cookie/ThreadSafeCookieStore.java @@ -32,6 +32,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import java.util.function.LongSupplier; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -52,6 +53,16 @@ public final class ThreadSafeCookieStore implements CookieStore { // 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(); + private final LongSupplier clock; + + public ThreadSafeCookieStore() { + this(System::currentTimeMillis); + } + + // For tests: expiry can be checked without waiting for it. + ThreadSafeCookieStore(LongSupplier clock) { + this.clock = clock; + } @Override public void add(Uri uri, Cookie cookie) { @@ -160,7 +171,7 @@ private static String cookiePath(@Nullable String rawCookiePath, String requestP } } - private static boolean hasCookieExpired(Cookie cookie, long whenCreated) { + private boolean hasCookieExpired(Cookie cookie, long whenCreated) { // if not specify max-age, this cookie should be discarded when user agent is to be closed, but it is not expired. if (cookie.maxAge() == Cookie.UNDEFINED_MAX_AGE) { return false; @@ -171,7 +182,7 @@ private static boolean hasCookieExpired(Cookie cookie, long whenCreated) { } if (whenCreated > 0) { - long deltaSecond = (System.currentTimeMillis() - whenCreated) / 1000; + long deltaSecond = (clock.getAsLong() - whenCreated) / 1000; return deltaSecond > cookie.maxAge(); } else { return false; @@ -221,7 +232,8 @@ private void add(String requestDomain, String requestPath, Cookie cookie) { cookieJar.getOrDefault(keyDomain, Collections.emptyMap()).remove(key); } else { final Map innerMap = cookieJar.computeIfAbsent(keyDomain, domain -> new ConcurrentHashMap<>()); - innerMap.put(key, new StoredCookie(cookie, hostOnly, cookie.maxAge() != Cookie.UNDEFINED_MAX_AGE, cookieSequence.getAndIncrement())); + innerMap.put(key, new StoredCookie(cookie, hostOnly, cookie.maxAge() != Cookie.UNDEFINED_MAX_AGE, clock.getAsLong(), + cookieSequence.getAndIncrement())); if (innerMap.size() > MAX_COOKIES_PER_DOMAIN) { evictExcessCookies(innerMap); } @@ -244,7 +256,7 @@ private void add(String requestDomain, String requestPath, Cookie cookie) { * 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 innerMap) { + private void evictExcessCookies(Map innerMap) { List> live = new ArrayList<>(innerMap.size()); for (Map.Entry entry : innerMap.entrySet()) { if (hasCookieExpired(entry.getValue().cookie, entry.getValue().createdAt)) { @@ -356,14 +368,15 @@ private static class StoredCookie { final Cookie cookie; final boolean hostOnly; final boolean persistent; - final long createdAt = System.currentTimeMillis(); + final long createdAt; // Strict, tie-free insertion order for eviction; see ThreadSafeCookieStore.cookieSequence. final long seq; - StoredCookie(Cookie cookie, boolean hostOnly, boolean persistent, long seq) { + StoredCookie(Cookie cookie, boolean hostOnly, boolean persistent, long createdAt, long seq) { this.cookie = cookie; this.hostOnly = hostOnly; this.persistent = persistent; + this.createdAt = createdAt; this.seq = seq; } diff --git a/client/src/test/java/org/asynchttpclient/CookieStoreTest.java b/client/src/test/java/org/asynchttpclient/CookieStoreTest.java index ff6a777fc..532f1029c 100644 --- a/client/src/test/java/org/asynchttpclient/CookieStoreTest.java +++ b/client/src/test/java/org/asynchttpclient/CookieStoreTest.java @@ -18,7 +18,6 @@ import io.netty.handler.codec.http.cookie.ClientCookieDecoder; import io.netty.handler.codec.http.cookie.ClientCookieEncoder; import io.netty.handler.codec.http.cookie.Cookie; -import io.netty.handler.codec.http.cookie.DefaultCookie; import org.asynchttpclient.cookie.CookieStore; import org.asynchttpclient.cookie.ThreadSafeCookieStore; import org.asynchttpclient.uri.Uri; @@ -28,9 +27,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.Collection; import java.util.List; -import java.util.Set; import java.util.stream.Collectors; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -85,7 +82,6 @@ public void runAllSequentiallyBecauseNotThreadSafe() throws Exception { shouldAlsoServeNonSecureCookiesBasedOnTheUriScheme(); shouldNotServeSecureCookiesForDefaultRetrievedHttpUriScheme(); shouldServeSecureCookiesForSpecificallyRetrievedHttpUriScheme(); - shouldCleanExpiredCookieFromUnderlyingDataStructure(); } private static void addCookieWithEmptyPath() { @@ -355,26 +351,4 @@ private static void shouldServeSecureCookiesForSpecificallyRetrievedHttpUriSchem assertEquals("VALUE3", store.get(uri).get(0).value()); assertTrue(store.get(uri).get(0).isSecure()); } - - private static void shouldCleanExpiredCookieFromUnderlyingDataStructure() throws Exception { - ThreadSafeCookieStore store = new ThreadSafeCookieStore(); - store.add(Uri.create("https://foo.org/moodle/"), getCookie("JSESSIONID", "FOO", 1)); - store.add(Uri.create("https://bar.org/moodle/"), getCookie("JSESSIONID", "BAR", 1)); - store.add(Uri.create("https://bar.org/moodle/"), new DefaultCookie("UNEXPIRED_BAR", "BAR")); - store.add(Uri.create("https://foobar.org/moodle/"), new DefaultCookie("UNEXPIRED_FOOBAR", "FOOBAR")); - - - assertEquals(4, store.getAll().size()); - Thread.sleep(2000); - store.evictExpired(); - assertEquals(2, store.getUnderlying().size()); - Collection unexpiredCookieNames = store.getAll().stream().map(Cookie::name).collect(Collectors.toList()); - assertTrue(unexpiredCookieNames.containsAll(Set.of("UNEXPIRED_BAR", "UNEXPIRED_FOOBAR"))); - } - - private static Cookie getCookie(String key, String value, int maxAge) { - DefaultCookie cookie = new DefaultCookie(key, value); - cookie.setMaxAge(maxAge); - return cookie; - } } diff --git a/client/src/test/java/org/asynchttpclient/cookie/ThreadSafeCookieStoreExpiryTest.java b/client/src/test/java/org/asynchttpclient/cookie/ThreadSafeCookieStoreExpiryTest.java new file mode 100644 index 000000000..ffeef13f4 --- /dev/null +++ b/client/src/test/java/org/asynchttpclient/cookie/ThreadSafeCookieStoreExpiryTest.java @@ -0,0 +1,59 @@ +/* + * 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.Cookie; +import io.netty.handler.codec.http.cookie.DefaultCookie; +import org.asynchttpclient.uri.Uri; +import org.junit.jupiter.api.Test; + +import java.util.Set; +import java.util.concurrent.atomic.AtomicLong; +import java.util.stream.Collectors; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +public class ThreadSafeCookieStoreExpiryTest { + + private final AtomicLong now = new AtomicLong(1_000_000); + private final ThreadSafeCookieStore store = new ThreadSafeCookieStore(now::get); + + @Test + public void evictExpiredRemovesExpiredCookiesAndEmptyDomains() { + store.add(Uri.create("https://foo.org/moodle/"), cookie("JSESSIONID", "FOO", 1)); + store.add(Uri.create("https://bar.org/moodle/"), cookie("JSESSIONID", "BAR", 1)); + store.add(Uri.create("https://bar.org/moodle/"), new DefaultCookie("UNEXPIRED_BAR", "BAR")); + store.add(Uri.create("https://foobar.org/moodle/"), new DefaultCookie("UNEXPIRED_FOOBAR", "FOOBAR")); + assertEquals(4, store.getAll().size()); + + now.addAndGet(1000); + store.evictExpired(); + assertEquals(3, store.getUnderlying().size()); + assertEquals(4, store.getAll().size()); + + now.addAndGet(1000); + store.evictExpired(); + assertEquals(2, store.getUnderlying().size()); + assertEquals(Set.of("UNEXPIRED_BAR", "UNEXPIRED_FOOBAR"), + store.getAll().stream().map(Cookie::name).collect(Collectors.toSet())); + } + + private static Cookie cookie(String name, String value, long maxAge) { + DefaultCookie cookie = new DefaultCookie(name, value); + cookie.setMaxAge(maxAge); + return cookie; + } +}