Issue #6085 Fix reference counts for multiple valid cookies for sessions (#6088) (#6096)

* Issue #6085 Fix reference counts for multiple valid cookies for sesssions

Signed-off-by: Jan Bartel <janb@webtide.com>

jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java

@@ -1614,7 +1614,12 @@ public class SessionHandler extends ScopedHandler
                         if (LOG.isDebugEnabled())
                             LOG.debug("Got Session ID {} from cookie {}", id, sessionCookie);
 
+                        //retrieve the session, which increments the reference count
                         HttpSession s = getHttpSession(id);
+                        //associate it with the request so its reference count
+                        //will be decremented as the request completes
+                        if (s != null && isValid(s))
+                            baseRequest.enterSession(s);
 
                         if (requestedSessionId == null)
                         {
@@ -1640,6 +1645,10 @@ public class SessionHandler extends ScopedHandler
                         }
                     }
                 }
+
+                //if we wound up with a single valid session
+                if (session != null && isValid(session))
+                    baseRequest.setSession(session);  //associate the session with the request
             }
         }
 
@@ -1665,24 +1674,22 @@ public class SessionHandler extends ScopedHandler
 
                     requestedSessionId = uri.substring(s, i);
                     requestedSessionIdFromCookie = false;
+
                     if (LOG.isDebugEnabled())
                         LOG.debug("Got Session ID {} from URL", requestedSessionId);
+
                     session = getHttpSession(requestedSessionId);
+                    if (session != null && isValid(session))
+                    {
+                        baseRequest.enterSession(session); //request enters this session for first time
+                        baseRequest.setSession(session);  //associate the session with the request
+                    }
                 }
             }
         }
 
         baseRequest.setRequestedSessionId(requestedSessionId);
         baseRequest.setRequestedSessionIdFromCookie(requestedSessionId != null && requestedSessionIdFromCookie);
-
-        if (requestedSessionId != null)
-        {
-            if (session != null && isValid(session))
-            {
-                baseRequest.enterSession(session); //request enters this session for first time
-                baseRequest.setSession(session);  //associate the session with the request
-            }
-        }
     }
 
     @Override

tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/DuplicateCookieTest.java

@@ -59,7 +59,7 @@ public class DuplicateCookieTest
         try (StacklessLogging ignored = new StacklessLogging(DuplicateCookieTest.class.getPackage()))
         {
             //create a valid session
-            createUnExpiredSession(contextHandler.getSessionHandler().getSessionCache(),
+            Session s4422 = createUnExpiredSession(contextHandler.getSessionHandler().getSessionCache(),
                 contextHandler.getSessionHandler().getSessionCache().getSessionDataStore(),
                 "4422");
 
@@ -73,6 +73,7 @@ public class DuplicateCookieTest
             ContentResponse response = request.send();
             assertEquals(HttpServletResponse.SC_OK, response.getStatus());
             assertEquals("4422", response.getContentAsString());
+            assertEquals(0, s4422.getRequests());
         }
         finally
         {
@@ -102,7 +103,7 @@ public class DuplicateCookieTest
         try (StacklessLogging ignored = new StacklessLogging(DuplicateCookieTest.class.getPackage()))
         {
             //create a valid session
-            createUnExpiredSession(contextHandler.getSessionHandler().getSessionCache(),
+            Session s1122 = createUnExpiredSession(contextHandler.getSessionHandler().getSessionCache(),
                 contextHandler.getSessionHandler().getSessionCache().getSessionDataStore(),
                 "1122");
             //create an invalid session
@@ -120,6 +121,7 @@ public class DuplicateCookieTest
             ContentResponse response = request.send();
             assertEquals(HttpServletResponse.SC_OK, response.getStatus());
             assertEquals("1122", response.getContentAsString());
+            assertEquals(0, s1122.getRequests());
         }
         finally
         {
@@ -149,25 +151,35 @@ public class DuplicateCookieTest
         try (StacklessLogging ignored = new StacklessLogging(DuplicateCookieTest.class.getPackage()))
         {
             //create some of unexpired sessions
-            createUnExpiredSession(contextHandler.getSessionHandler().getSessionCache(),
+            Session s1234 = createUnExpiredSession(contextHandler.getSessionHandler().getSessionCache(),
                 contextHandler.getSessionHandler().getSessionCache().getSessionDataStore(),
                 "1234");
-            createUnExpiredSession(contextHandler.getSessionHandler().getSessionCache(),
+            Session s5678 = createUnExpiredSession(contextHandler.getSessionHandler().getSessionCache(),
                 contextHandler.getSessionHandler().getSessionCache().getSessionDataStore(),
                 "5678");
-            createUnExpiredSession(contextHandler.getSessionHandler().getSessionCache(),
+            Session s9111 = createUnExpiredSession(contextHandler.getSessionHandler().getSessionCache(),
                 contextHandler.getSessionHandler().getSessionCache().getSessionDataStore(),
                 "9111");
 
             client = new HttpClient();
             client.start();
 
+            //check that the request count is 0
+            assertEquals(0, s1234.getRequests());
+            assertEquals(0, s5678.getRequests());
+            assertEquals(0, s9111.getRequests());
+
             //make a request with multiple valid session ids
             Request request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping + "?action=check");
             request.headers(headers -> headers.add("Cookie", "JSESSIONID=1234"));
             request.headers(headers -> headers.add("Cookie", "JSESSIONID=5678"));
             ContentResponse response = request.send();
             assertEquals(HttpServletResponse.SC_BAD_REQUEST, response.getStatus());
+
+            //check that all valid sessions have their request counts decremented correctly after the request, back to 0
+            assertEquals(0, s1234.getRequests());
+            assertEquals(0, s5678.getRequests());
+            assertEquals(0, s9111.getRequests());
         }
         finally
         {
@@ -183,6 +195,7 @@ public class DuplicateCookieTest
         data.setExpiry(now + TimeUnit.DAYS.toMillis(1));
         Session s = cache.newSession(data);
         cache.add(id, s);
+        s.complete(); //pretend a request that created the session is finished
         return s;
     }