From 3aa83775682e7385144edfca17653146f6da4783 Mon Sep 17 00:00:00 2001 From: Marco Collovati Date: Mon, 14 Oct 2024 08:14:37 +0200 Subject: [PATCH] fix: set clusterKey cookie at first request 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 --- .../sessiontracker/SessionTrackerCookie.java | 8 ++++++-- .../sessiontracker/SessionTrackerFilter.java | 7 +++---- .../SessionTrackerCookieTest.java | 19 +++++++++++++++++++ 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/kubernetes-kit-starter/src/main/java/com/vaadin/kubernetes/starter/sessiontracker/SessionTrackerCookie.java b/kubernetes-kit-starter/src/main/java/com/vaadin/kubernetes/starter/sessiontracker/SessionTrackerCookie.java index 23f1dfb..baf4576 100644 --- a/kubernetes-kit-starter/src/main/java/com/vaadin/kubernetes/starter/sessiontracker/SessionTrackerCookie.java +++ b/kubernetes-kit-starter/src/main/java/com/vaadin/kubernetes/starter/sessiontracker/SessionTrackerCookie.java @@ -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; @@ -45,11 +46,14 @@ public static void setIfNeeded(HttpSession session, Optional 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); } diff --git a/kubernetes-kit-starter/src/main/java/com/vaadin/kubernetes/starter/sessiontracker/SessionTrackerFilter.java b/kubernetes-kit-starter/src/main/java/com/vaadin/kubernetes/starter/sessiontracker/SessionTrackerFilter.java index f921e6e..193adcd 100644 --- a/kubernetes-kit-starter/src/main/java/com/vaadin/kubernetes/starter/sessiontracker/SessionTrackerFilter.java +++ b/kubernetes-kit-starter/src/main/java/com/vaadin/kubernetes/starter/sessiontracker/SessionTrackerFilter.java @@ -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; @@ -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() diff --git a/kubernetes-kit-starter/src/test/java/com/vaadin/kubernetes/starter/sessiontracker/SessionTrackerCookieTest.java b/kubernetes-kit-starter/src/test/java/com/vaadin/kubernetes/starter/sessiontracker/SessionTrackerCookieTest.java index d5752b1..d595c3e 100644 --- a/kubernetes-kit-starter/src/test/java/com/vaadin/kubernetes/starter/sessiontracker/SessionTrackerCookieTest.java +++ b/kubernetes-kit-starter/src/test/java/com/vaadin/kubernetes/starter/sessiontracker/SessionTrackerCookieTest.java @@ -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 cookieConsumer = (Consumer) mock( + Consumer.class); + + SessionTrackerCookie.setIfNeeded(null, request, response, + cookieConsumer); + + verify(cookieConsumer).accept(any()); + verify(response).addCookie(any()); + } + + }