Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CookieStore] Only set Cookies if they are not already set #2033

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ public <T> ListenableFuture<T> executeRequest(Request request, AsyncHandler<T> h
if (!cookies.isEmpty()) {
RequestBuilder requestBuilder = request.toBuilder();
for (Cookie cookie : cookies) {
requestBuilder.addOrReplaceCookie(cookie);
requestBuilder.addCookieIfUnset(cookie);
}
request = requestBuilder.build();
}
Expand Down
21 changes: 18 additions & 3 deletions client/src/main/java/org/asynchttpclient/RequestBuilderBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,21 @@ public T addCookie(Cookie cookie) {
* @return this
*/
public T addOrReplaceCookie(Cookie cookie) {
return maybeAddOrReplaceCookie(cookie, true);
}

/**
* Add a cookie based on its name, if it does not exist yet. Cookies that
* are already set will be ignored.
*
* @param cookie the new cookie
* @return this
*/
public T addCookieIfUnset(Cookie cookie) {
return maybeAddOrReplaceCookie(cookie, false);
}

private T maybeAddOrReplaceCookie(Cookie cookie, boolean allowReplace) {
String cookieKey = cookie.name();
boolean replace = false;
int index = 0;
Expand All @@ -335,10 +350,10 @@ public T addOrReplaceCookie(Cookie cookie) {

index++;
}
if (replace) {
cookies.set(index, cookie);
} else {
if (!replace) {
cookies.add(cookie);
} else if (allowReplace) {
cookies.set(index, cookie);
}
return asDerivedType();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,8 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture<?>
CookieStore cookieStore = config.getCookieStore();
if (cookieStore != null) {
// Update request's cookies assuming that cookie store is already updated by Interceptors
List<Cookie> cookies = cookieStore.get(newUri);
if (!cookies.isEmpty()) {
for (Cookie cookie : cookies) {
requestBuilder.addOrReplaceCookie(cookie);
}
for (Cookie cookie : cookieStore.get(newUri)) {
requestBuilder.addCookieIfUnset(cookie);
}
}

Expand Down
34 changes: 34 additions & 0 deletions client/src/test/java/org/asynchttpclient/RequestBuilderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,40 @@ public void testAddOrReplaceCookies() {
assertEquals(requestBuilder.cookies.size(), 2, "cookie size must be 2 after adding 1 more cookie i.e. cookie3");
}

@RepeatedIfExceptionsTest(repeats = 5)
public void testAddIfUnsetCookies() {
RequestBuilder requestBuilder = new RequestBuilder();
Cookie cookie = new DefaultCookie("name", "value");
cookie.setDomain("google.com");
cookie.setPath("/");
cookie.setMaxAge(1000);
cookie.setSecure(true);
cookie.setHttpOnly(true);
requestBuilder.addCookieIfUnset(cookie);
assertEquals(requestBuilder.cookies.size(), 1, "cookies size should be 1 after adding one cookie");
assertEquals(requestBuilder.cookies.get(0), cookie, "cookie does not match");

Cookie cookie2 = new DefaultCookie("name", "value");
cookie2.setDomain("google2.com");
cookie2.setPath("/path");
cookie2.setMaxAge(1001);
cookie2.setSecure(false);
cookie2.setHttpOnly(false);

requestBuilder.addCookieIfUnset(cookie2);
assertEquals(requestBuilder.cookies.size(), 1, "cookies size should remain 1 as we just ignored cookie2 because of a cookie with same name");
assertEquals(requestBuilder.cookies.get(0), cookie, "cookie does not match");

Cookie cookie3 = new DefaultCookie("name2", "value");
cookie3.setDomain("google.com");
cookie3.setPath("/");
cookie3.setMaxAge(1000);
cookie3.setSecure(true);
cookie3.setHttpOnly(true);
requestBuilder.addCookieIfUnset(cookie3);
assertEquals(requestBuilder.cookies.size(), 2, "cookie size must be 2 after adding 1 more cookie i.e. cookie3");
}

@RepeatedIfExceptionsTest(repeats = 5)
public void testSettingQueryParamsBeforeUrlShouldNotProduceNPE() {
RequestBuilder requestBuilder = new RequestBuilder();
Expand Down