Skip to content

Commit

Permalink
fix: set clusterKey cookie at first request
Browse files Browse the repository at this point in the history
Currently clusterKey cookie is set only when an HTTP session exists.
In addition to the cookie, the key is also set as a session attribute
so that SessionSerializer can fetch it later.
However, if two or more concurrent requests are sent from the client,
all of them create a new cookie, but the session attribute gets the value
of the first one. This causes the distributed session to be written with
a wrong key and when there a server switch the session cannot be restored.
This change sets the clusterKey cookie even if there is not yet an HTTP
session, so the key is only defined during first request and remains stable
with subsequent calls; once the HTTP session is create, the key is stored
as an attribute.

Potentially fixes #111

See https://github.com/vaadin/kubernetes-kit/issues/111\#issuecomment-2074909790
  • Loading branch information
mcollovati committed Oct 14, 2024
1 parent f87fdb1 commit 3aa8377
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpSession;

import java.util.Optional;
import java.util.UUID;
import java.util.function.Consumer;
Expand Down Expand Up @@ -45,11 +46,14 @@ public static void setIfNeeded(HttpSession session,
Optional<Cookie> clusterKeyCookie = getCookie(request);
if (clusterKeyCookie.isEmpty()) {
String clusterKey = UUID.randomUUID().toString();
session.setAttribute(CurrentKey.COOKIE_NAME, clusterKey);
if (session != null) {
session.setAttribute(CurrentKey.COOKIE_NAME, clusterKey);
}
Cookie cookie = new Cookie(CurrentKey.COOKIE_NAME, clusterKey);
cookieConsumer.accept(cookie);
response.addCookie(cookie);
} else if (session.getAttribute(CurrentKey.COOKIE_NAME) == null) {
} else if (session != null
&& session.getAttribute(CurrentKey.COOKIE_NAME) == null) {
String clusterKey = clusterKeyCookie.get().getValue();
session.setAttribute(CurrentKey.COOKIE_NAME, clusterKey);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpSession;

import java.io.IOException;
import java.util.function.Consumer;

Expand Down Expand Up @@ -72,10 +73,8 @@ protected void doFilter(HttpServletRequest request,
try {
HttpSession session = request.getSession(false);

if (session != null) {
SessionTrackerCookie.setIfNeeded(session, request, response,
cookieConsumer(request));
}
SessionTrackerCookie.setIfNeeded(session, request, response,
cookieConsumer(request));
super.doFilter(request, response, chain);

if (session != null && request.isRequestedSessionIdValid()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,4 +144,23 @@ void getValue_valueIsReturned() {
assertTrue(value.isPresent());
assertEquals(clusterKey, value.get());
}


@Test
void setIfNeeded_nullCookiesAndSession_cookieIsConfigured() {
HttpServletRequest request = mock(HttpServletRequest.class);
when(request.getCookies()).thenReturn(null);
HttpServletResponse response = mock(HttpServletResponse.class);
@SuppressWarnings("unchecked")
Consumer<Cookie> cookieConsumer = (Consumer<Cookie>) mock(
Consumer.class);

SessionTrackerCookie.setIfNeeded(null, request, response,
cookieConsumer);

verify(cookieConsumer).accept(any());
verify(response).addCookie(any());
}


}

0 comments on commit 3aa8377

Please sign in to comment.