From fd05a3d19c3b24235b994f72106ac811ce7b557f Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Thu, 25 Mar 2021 09:37:17 +1100 Subject: [PATCH] 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 --- .../jetty/server/session/SessionHandler.java | 25 ++++++++++++------- .../server/session/DuplicateCookieTest.java | 23 +++++++++++++---- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java index 4eedb906082e..fcd5a4efafee 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java @@ -1614,7 +1614,12 @@ else if (!DispatcherType.REQUEST.equals(baseRequest.getDispatcherType())) 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 @@ else if (session == null || !isValid(session)) } } } + + //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 @@ else if (session == null || !isValid(session)) 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 diff --git a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/DuplicateCookieTest.java b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/DuplicateCookieTest.java index 0d370a60d4c0..bf877069d404 100644 --- a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/DuplicateCookieTest.java +++ b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/DuplicateCookieTest.java @@ -59,7 +59,7 @@ public void testMultipleSessionCookiesOnlyOneExists() throws Exception 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 void testMultipleSessionCookiesOnlyOneExists() throws Exception ContentResponse response = request.send(); assertEquals(HttpServletResponse.SC_OK, response.getStatus()); assertEquals("4422", response.getContentAsString()); + assertEquals(0, s4422.getRequests()); } finally { @@ -102,7 +103,7 @@ public void testMultipleSessionCookiesOnlyOneValid() throws Exception 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 void testMultipleSessionCookiesOnlyOneValid() throws Exception ContentResponse response = request.send(); assertEquals(HttpServletResponse.SC_OK, response.getStatus()); assertEquals("1122", response.getContentAsString()); + assertEquals(0, s1122.getRequests()); } finally { @@ -149,25 +151,35 @@ public void testMultipleSessionCookiesMultipleExists() throws Exception 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 Session createUnExpiredSession(SessionCache cache, SessionDataStore store 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; }