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 fcd5a4efafee..8a68ac008d32 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,41 +1614,57 @@ 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) + if (session == null) { - //no previous id, always accept this one - requestedSessionId = id; - session = s; - } - else if (requestedSessionId.equals(id)) - { - //really a bad request, but will forgive the duplication - } - else if (session == null || !isValid(session)) - { - //no previous session or invalid, accept this one - requestedSessionId = id; - session = s; + //we currently do not have a session selected, use this one if it is valid + HttpSession s = getHttpSession(id); + if (s != null && isValid(s)) + { + //associate it with the request so its reference count is decremented as the + //session exits + //try this session id + requestedSessionId = id; + session = s; + baseRequest.enterSession(session); + baseRequest.setSession(session); + + if (LOG.isDebugEnabled()) + LOG.debug("Selected session {}", session); + } + else + { + if (LOG.isDebugEnabled()) + LOG.debug("No session found for session cookie id {}", id); + + //if we don't have a valid session id yet, just choose the current id + if (requestedSessionId == null) + requestedSessionId = id; + } } else { - //previous session is valid, use it unless both valid - if (s != null && isValid(s)) - throw new BadMessageException("Duplicate valid session cookies: " + requestedSessionId + "," + id); + //we currently have a valid session selected. We will throw an error + //if there is a _different_ valid session id cookie. Duplicate ids, or + //invalid session ids are ignored + if (!session.getId().equals(getSessionIdManager().getId(id))) + { + //load the session to see if it is valid or not + HttpSession s = getHttpSession(id); + if (s != null && isValid(s)) + { + //associate it with the request so its reference count is decremented as the + //request exits + baseRequest.enterSession(s); + if (LOG.isDebugEnabled()) + LOG.debug("Multiple different valid session ids: {}, {}", requestedSessionId, id); + throw new BadMessageException("Duplicate valid session cookies: " + requestedSessionId + " ," + id); + } + } + else if (LOG.isDebugEnabled()) + LOG.debug("Duplicate valid session cookie id: {}", id); } } } - - //if we wound up with a single valid session - if (session != null && isValid(session)) - baseRequest.setSession(session); //associate the session with the request } } 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 bf877069d404..54c040f1b82f 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 @@ -28,6 +28,7 @@ import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.component.LifeCycle; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -66,6 +67,8 @@ public void testMultipleSessionCookiesOnlyOneExists() throws Exception client = new HttpClient(); client.start(); + assertEquals(0, s4422.getRequests()); + //make a request with another session cookie in there that does not exist Request request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping + "?action=check"); request.headers(headers -> headers.add("Cookie", "JSESSIONID=123")); //doesn't exist @@ -73,17 +76,18 @@ public void testMultipleSessionCookiesOnlyOneExists() throws Exception ContentResponse response = request.send(); assertEquals(HttpServletResponse.SC_OK, response.getStatus()); assertEquals("4422", response.getContentAsString()); + assertEquals(0, s4422.getRequests()); } finally { - server1.stop(); - client.stop(); + LifeCycle.stop(server1); + LifeCycle.stop(client); } } @Test - public void testMultipleSessionCookiesOnlyOneValid() throws Exception + public void testMultipleSessionCookiesValidFirst() throws Exception { String contextPath = ""; String servletMapping = "/server"; @@ -107,26 +111,152 @@ public void testMultipleSessionCookiesOnlyOneValid() throws Exception contextHandler.getSessionHandler().getSessionCache().getSessionDataStore(), "1122"); //create an invalid session - createInvalidSession(contextHandler.getSessionHandler().getSessionCache(), + Session s2233 = createInvalidSession(contextHandler.getSessionHandler().getSessionCache(), contextHandler.getSessionHandler().getSessionCache().getSessionDataStore(), "2233"); + //create another invalid session + Session s2255 = createInvalidSession(contextHandler.getSessionHandler().getSessionCache(), + contextHandler.getSessionHandler().getSessionCache().getSessionDataStore(), + "2255"); client = new HttpClient(); client.start(); - //make a request with another session cookie in there that is not valid + assertEquals(0, s1122.getRequests()); + assertEquals(0, s2233.getRequests()); + assertEquals(0, s2255.getRequests()); + + //make a request where the valid session cookie is first Request request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping + "?action=check"); request.headers(headers -> headers.add("Cookie", "JSESSIONID=1122")); //is valid request.headers(headers -> headers.add("Cookie", "JSESSIONID=2233")); //is invalid + request.headers(headers -> headers.add("Cookie", "JSESSIONID=2255")); //is invalid ContentResponse response = request.send(); assertEquals(HttpServletResponse.SC_OK, response.getStatus()); assertEquals("1122", response.getContentAsString()); + assertEquals(0, s1122.getRequests()); } finally { - server1.stop(); - client.stop(); + LifeCycle.stop(server1); + LifeCycle.stop(client); + } + } + + @Test + public void testMultipleSessionCookiesInvalidFirst() throws Exception + { + String contextPath = ""; + String servletMapping = "/server"; + HttpClient client = null; + + DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); + SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory(); + + TestServer server1 = new TestServer(0, -1, -1, cacheFactory, storeFactory); + TestServlet servlet = new TestServlet(); + ServletHolder holder = new ServletHolder(servlet); + ServletContextHandler contextHandler = server1.addContext(contextPath); + contextHandler.addServlet(holder, servletMapping); + server1.start(); + int port1 = server1.getPort(); + + try (StacklessLogging ignored = new StacklessLogging(DuplicateCookieTest.class.getPackage())) + { + //create a valid session + Session s1122 = createUnExpiredSession(contextHandler.getSessionHandler().getSessionCache(), + contextHandler.getSessionHandler().getSessionCache().getSessionDataStore(), + "1122"); + //create an invalid session + Session s2233 = createInvalidSession(contextHandler.getSessionHandler().getSessionCache(), + contextHandler.getSessionHandler().getSessionCache().getSessionDataStore(), + "2233"); + //create another invalid session + Session s2255 = createInvalidSession(contextHandler.getSessionHandler().getSessionCache(), + contextHandler.getSessionHandler().getSessionCache().getSessionDataStore(), + "2255"); + + client = new HttpClient(); + client.start(); + + assertEquals(0, s1122.getRequests()); + assertEquals(0, s2233.getRequests()); + assertEquals(0, s2255.getRequests()); + + //make a request with the valid session cookie last + Request request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping + "?action=check"); + request.headers(headers -> headers.add("Cookie", "JSESSIONID=2233")); //is invalid + request.headers(headers -> headers.add("Cookie", "JSESSIONID=2255")); //is invalid + request.headers(headers -> headers.add("Cookie", "JSESSIONID=1122")); //is valid + ContentResponse response = request.send(); + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + assertEquals("1122", response.getContentAsString()); + + assertEquals(0, s1122.getRequests()); + } + finally + { + LifeCycle.stop(server1); + LifeCycle.stop(client); + } + } + + @Test + public void testMultipleSessionCookiesInvalidValidInvalid() throws Exception + { + String contextPath = ""; + String servletMapping = "/server"; + HttpClient client = null; + + DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); + SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory(); + + TestServer server1 = new TestServer(0, -1, -1, cacheFactory, storeFactory); + TestServlet servlet = new TestServlet(); + ServletHolder holder = new ServletHolder(servlet); + ServletContextHandler contextHandler = server1.addContext(contextPath); + contextHandler.addServlet(holder, servletMapping); + server1.start(); + int port1 = server1.getPort(); + + try (StacklessLogging ignored = new StacklessLogging(DuplicateCookieTest.class.getPackage())) + { + //create a valid session + Session s1122 = createUnExpiredSession(contextHandler.getSessionHandler().getSessionCache(), + contextHandler.getSessionHandler().getSessionCache().getSessionDataStore(), + "1122"); + //create an invalid session + Session s2233 = createInvalidSession(contextHandler.getSessionHandler().getSessionCache(), + contextHandler.getSessionHandler().getSessionCache().getSessionDataStore(), + "2233"); + //create another invalid session + Session s2255 = createInvalidSession(contextHandler.getSessionHandler().getSessionCache(), + contextHandler.getSessionHandler().getSessionCache().getSessionDataStore(), + "2255"); + + client = new HttpClient(); + client.start(); + + assertEquals(0, s1122.getRequests()); + assertEquals(0, s2233.getRequests()); + assertEquals(0, s2255.getRequests()); + + //make a request with another session cookie with the valid session surrounded by invalids + Request request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping + "?action=check"); + request.headers(headers -> headers.add("Cookie", "JSESSIONID=2233")); //is invalid + request.headers(headers -> headers.add("Cookie", "JSESSIONID=1122")); //is valid + request.headers(headers -> headers.add("Cookie", "JSESSIONID=2255")); //is invalid + ContentResponse response = request.send(); + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + assertEquals("1122", response.getContentAsString()); + + assertEquals(0, s1122.getRequests()); + } + finally + { + LifeCycle.stop(server1); + LifeCycle.stop(client); } } @@ -183,8 +313,56 @@ public void testMultipleSessionCookiesMultipleExists() throws Exception } finally { - server1.stop(); - client.stop(); + LifeCycle.stop(server1); + LifeCycle.stop(client); + } + } + + @Test + public void testMultipleIdenticalSessionCookies() throws Exception + { + String contextPath = ""; + String servletMapping = "/server"; + HttpClient client = null; + + DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); + SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory(); + + TestServer server1 = new TestServer(0, -1, -1, cacheFactory, storeFactory); + TestServlet servlet = new TestServlet(); + ServletHolder holder = new ServletHolder(servlet); + ServletContextHandler contextHandler = server1.addContext(contextPath); + contextHandler.addServlet(holder, servletMapping); + server1.start(); + int port1 = server1.getPort(); + + try (StacklessLogging ignored = new StacklessLogging(DuplicateCookieTest.class.getPackage())) + { + //create a valid unexpired session + Session s1234 = createUnExpiredSession(contextHandler.getSessionHandler().getSessionCache(), + contextHandler.getSessionHandler().getSessionCache().getSessionDataStore(), + "1234"); + + client = new HttpClient(); + client.start(); + + //check that the request count is 0 + assertEquals(0, s1234.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=1234")); + ContentResponse response = request.send(); + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + + //check that all valid sessions have their request counts decremented correctly after the request, back to 0 + assertEquals(0, s1234.getRequests()); + } + finally + { + LifeCycle.stop(server1); + LifeCycle.stop(client); } } diff --git a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionInvalidationTest.java b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionInvalidationTest.java index 362c2319777a..525b81d20670 100644 --- a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionInvalidationTest.java +++ b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionInvalidationTest.java @@ -25,8 +25,11 @@ import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jetty.util.StringUtil; import org.junit.jupiter.api.Test; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsStringIgnoringCase; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -91,6 +94,63 @@ public void testInvalidation() throws Exception } } + @Test + public void testCreateInvalidateCheckWithNullCache() throws Exception + { + String contextPath = ""; + String servletMapping = "/server"; + int scavengePeriod = -1; + + NullSessionCacheFactory cacheFactory = new NullSessionCacheFactory(); + SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory(); + ((AbstractSessionDataStoreFactory)storeFactory).setGracePeriodSec(scavengePeriod); + + TestServer server = new TestServer(0, 0, scavengePeriod, + cacheFactory, storeFactory); + ServletContextHandler context = server.addContext(contextPath); + TestServlet servlet = new TestServlet(); + ServletHolder holder = new ServletHolder(servlet); + context.addServlet(holder, servletMapping); + + try + { + server.start(); + int port1 = server.getPort(); + + HttpClient client = new HttpClient(); + client.start(); + try + { + String url = "http://localhost:" + port1 + contextPath + servletMapping; + // Create the session + ContentResponse response1 = client.GET(url + "?action=init"); + assertEquals(HttpServletResponse.SC_OK, response1.getStatus()); + String sessionCookie = response1.getHeaders().get("Set-Cookie"); + assertTrue(sessionCookie != null); + + // Make a request which will invalidate the existing session + Request request2 = client.newRequest(url + "?action=test"); + ContentResponse response2 = request2.send(); + assertEquals(HttpServletResponse.SC_OK, response2.getStatus()); + + //Make a request to get the session - should not exist + Request request3 = client.newRequest(url + "?action=get"); + ContentResponse response3 = request3.send(); + assertEquals(HttpServletResponse.SC_OK, response3.getStatus()); + assertThat(response3.getContentAsString(), containsStringIgnoringCase("session=null")); + + } + finally + { + client.stop(); + } + } + finally + { + server.stop(); + } + } + public static class TestServlet extends HttpServlet { private static final long serialVersionUID = 1L; @@ -126,6 +186,12 @@ else if ("test".equals(action)) assertThrows(IllegalStateException.class, () -> session.setAttribute("a", "b")); assertDoesNotThrow(() -> session.getId()); } + else if ("get".equals(action)) + { + HttpSession session = request.getSession(false); + + httpServletResponse.getWriter().println("SESSION=" + (session == null ? "null" : session.getId())); + } } } }