Jetty 9.4.x 7801 duplicate set session cookies (#7809)

* Issue #7801 Duplicate session cookies after session id change. Signed-off-by: Jan Bartel <janb@webtide.com>
jetty · Jun 8, 2022 · ff2cb45 · ff2cb45
1 parent 5b4d1dd
commit ff2cb45
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 17 deletions.
diff --git a/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java b/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java
@@ -1013,7 +1013,6 @@ public void testFormRedirect() throws Exception
             "Cookie: JSESSIONID=" + session + "\r\n" +
             "\r\n");
         assertThat(response, startsWith("HTTP/1.1 200 OK"));
-        assertThat(response, containsString("JSESSIONID=" + session));
 
         response = _connector.getResponse("GET /ctx/admin/info HTTP/1.0\r\n" +
             "Cookie: JSESSIONID=" + session + "\r\n" +

diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java
@@ -548,6 +548,9 @@ public void release(String id, Session session) throws Exception
             //don't do anything with the session until the last request for it has finished
             if ((session.getRequests() <= 0))
             {
+                //reset the idchanged flag
+                session.setIdChanged(false);
+
                 //save the session
                 if (!_sessionDataStore.isPassivating())
                 {

diff --git a/...test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionRenewTest.java b/...test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionRenewTest.java
@@ -40,6 +40,7 @@
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNotSame;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**
@@ -183,8 +184,6 @@ public void doTest(RenewalVerifier verifier) throws Exception
         String contextPath = "";
         String servletMapping = "/server";
         WebAppContext context = _server.addWebAppContext(".", contextPath);
-        TestHttpChannelCompleteListener scopeListener = new TestHttpChannelCompleteListener();
-        _server.getServerConnector().addBean(scopeListener);
         context.setParentLoaderPriority(true);
         context.addServlet(TestServlet.class, servletMapping);
         TestHttpSessionIdListener testListener = new TestHttpSessionIdListener();
@@ -199,33 +198,29 @@ public void doTest(RenewalVerifier verifier) throws Exception
             client.start();
 
             //make a request to create a session
-            CountDownLatch synchronizer = new CountDownLatch(1);
-            scopeListener.setExitSynchronizer(synchronizer);
             ContentResponse response = client.GET("http://localhost:" + port + contextPath + servletMapping + "?action=create");
             assertEquals(HttpServletResponse.SC_OK, response.getStatus());
-
-            //ensure request has finished being handled
-            synchronizer.await(5, TimeUnit.SECONDS);
 
             String sessionCookie = response.getHeaders().get("Set-Cookie");
             assertTrue(sessionCookie != null);
             assertFalse(testListener.isCalled());
 
             //make a request to change the sessionid
-            synchronizer = new CountDownLatch(1);
-            scopeListener.setExitSynchronizer(synchronizer);
             Request request = client.newRequest("http://localhost:" + port + contextPath + servletMapping + "?action=renew");
             ContentResponse renewResponse = request.send();
             assertEquals(HttpServletResponse.SC_OK, renewResponse.getStatus());
-
-            //ensure request has finished being handled
-            synchronizer.await(5, TimeUnit.SECONDS);
 
             String renewSessionCookie = renewResponse.getHeaders().get("Set-Cookie");
             assertNotNull(renewSessionCookie);
             assertNotSame(sessionCookie, renewSessionCookie);
             assertTrue(testListener.isCalled());
 
+            //make another request and check the cookie isn't set again
+            request = client.newRequest("http://localhost:" + port + contextPath + servletMapping + "?action=check");
+            ContentResponse checkResponse = request.send();
+            assertEquals(HttpServletResponse.SC_OK, checkResponse.getStatus());
+            assertNull(checkResponse.getHeaders().get("Set-Cookie"));
+
             if (verifier != null)
                 verifier.verify(context, TestServer.extractSessionId(sessionCookie), TestServer.extractSessionId(renewSessionCookie));
         }
@@ -315,10 +310,10 @@ else if ("renew".equals(action))
 
                 assertTrue(sessionIdManager.isIdInUse(afterSessionId)); //new session id should be in use
                 assertFalse(sessionIdManager.isIdInUse(beforeSessionId));
-
-
-                if (((Session)afterSession).isIdChanged())
-                    ((org.eclipse.jetty.server.Response)response).replaceCookie(sessionManager.getSessionCookie(afterSession, request.getContextPath(), request.isSecure()));
+            }
+            else
+            {
+                request.getSession(false);
             }
         }
     }