Skip to content

Commit 6f3006b

Browse files
committed
Add handling for same name cookies
1 parent f1fe7a4 commit 6f3006b

File tree

2 files changed

+49
-9
lines changed

2 files changed

+49
-9
lines changed

core/src/main/java/com/linecorp/armeria/client/cookie/DefaultCookieJar.java

+30-9
Original file line numberDiff line numberDiff line change
@@ -90,16 +90,31 @@ public void set(URI uri, Iterable<? extends Cookie> cookies, long createdTimeMil
9090
lock.lock();
9191
try {
9292
for (Cookie cookie : cookies) {
93-
cookie = ensureDomainAndPath(cookie, uri);
93+
final Cookie ensuredCookie = ensureDomainAndPath(cookie, uri);
9494
// remove similar cookie if present
95-
store.removeLong(cookie);
96-
if ((cookie.maxAge() == Cookie.UNDEFINED_MAX_AGE || cookie.maxAge() > 0) &&
97-
cookiePolicy.accept(uri, cookie)) {
98-
store.put(cookie, createdTimeMillis);
99-
final Set<Cookie> cookieSet = filter.computeIfAbsent(cookie.domain(), s -> new HashSet<>());
100-
// remove similar cookie if present
101-
cookieSet.remove(cookie);
102-
cookieSet.add(cookie);
95+
store.removeLong(ensuredCookie);
96+
if ((ensuredCookie.maxAge() == Cookie.UNDEFINED_MAX_AGE || ensuredCookie.maxAge() > 0) &&
97+
cookiePolicy.accept(uri, ensuredCookie)) {
98+
store.put(ensuredCookie, createdTimeMillis);
99+
final Set<Cookie> cookieSet = filter.computeIfAbsent(ensuredCookie.domain(),
100+
s -> new HashSet<>());
101+
102+
// remove the cookie with the same name, domain, and path if present
103+
// https://datatracker.ietf.org/doc/html/rfc6265#page-24
104+
final Cookie oldCookie = cookieSet.stream()
105+
.filter(c -> isSameNameAndPath(c, ensuredCookie))
106+
.findFirst()
107+
.orElse(null);
108+
if (oldCookie == null) {
109+
cookieSet.add(ensuredCookie);
110+
continue;
111+
}
112+
if (!uri.getScheme().startsWith("http") && oldCookie.isHttpOnly()) {
113+
// if the new cookie is received from a non-HTTP and the old cookie is http-only, skip
114+
continue;
115+
}
116+
cookieSet.remove(oldCookie);
117+
cookieSet.add(ensuredCookie);
103118
}
104119
}
105120
} finally {
@@ -213,4 +228,10 @@ private static boolean cookieMatches(Cookie cookie, String host, String path, bo
213228
final boolean pathMatched = path.startsWith(cookiePath);
214229
return satisfiedHostOnly && satisfiedSecure && pathMatched;
215230
}
231+
232+
private static boolean isSameNameAndPath(Cookie oldCookie, Cookie newCookie) {
233+
final String oldCookiePath = oldCookie.path();
234+
assert oldCookiePath != null;
235+
return oldCookie.name().equals(newCookie.name()) && oldCookiePath.equals(newCookie.path());
236+
}
216237
}

core/src/test/java/com/linecorp/armeria/client/cookie/DefaultCookieJarTest.java

+19
Original file line numberDiff line numberDiff line change
@@ -200,4 +200,23 @@ void cookieState() {
200200
assertThat(cookieJar.state(expectCookie, currentTimeMillis + 1000)).isEqualTo(CookieState.EXISTENT);
201201
assertThat(cookieJar.state(expectCookie, currentTimeMillis + 1001)).isEqualTo(CookieState.EXPIRED);
202202
}
203+
204+
@Test
205+
void overwrite() {
206+
final CookieJar cookieJar = new DefaultCookieJar();
207+
final URI foo = URI.create("https://foo.com");
208+
cookieJar.set(foo, Cookies.of(Cookie.ofSecure("name", "value1")));
209+
cookieJar.set(foo, Cookies.of(Cookie.ofSecure("name", "value2")));
210+
211+
assertThat(cookieJar.get(foo)).hasSize(1).contains(
212+
Cookie.secureBuilder("name", "value2").domain("foo.com").path("/").build()
213+
);
214+
215+
final URI nonHttp = URI.create("foo://foo.com");
216+
cookieJar.set(nonHttp, Cookies.of(Cookie.ofSecure("name", "value3")));
217+
218+
assertThat(cookieJar.get(foo)).hasSize(1).contains(
219+
Cookie.secureBuilder("name", "value2").domain("foo.com").path("/").build()
220+
);
221+
}
203222
}

0 commit comments

Comments
 (0)