Jetty 10.0.x 7801 duplicate set session cookie (#7808)

* Issue #7801 Duplicate set session cookie after id change. Signed-off-by: Jan Bartel <janb@webtide.com>
jetty · Jun 8, 2022 · 3caee26 · 3caee26
1 parent c34483e
commit 3caee26
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
@@ -1008,7 +1008,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
@@ -531,6 +531,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
@@ -35,6 +35,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;
 
 /**
@@ -94,6 +95,7 @@ public void verify(WebAppContext context, String oldSessionId, String newSession
                 //verify the contents of the cache changed
                 assertTrue(context.getSessionHandler().getSessionCache().contains(newSessionId));
                 assertFalse(context.getSessionHandler().getSessionCache().contains(oldSessionId));
+                assertFalse(((AbstractSessionCache)context.getSessionHandler().getSessionCache()).doGet(newSessionId).isIdChanged());
                 super.verify(context, oldSessionId, newSessionId);
             }
         });
@@ -178,8 +180,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();
@@ -194,32 +194,27 @@ 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());
+
+            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));
@@ -310,10 +305,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);
             }
         }
     }