Skip to content

Commit

Permalink
Issue #6085 Fix duplicate valid session cookies to pick first valid.
Browse files Browse the repository at this point in the history 
Signed-off-by: Jan Bartel <janb@webtide.com>
  • Loading branch information
@janbartel
janbartel committed Mar 25, 2021
1 parent e0111e5 commit c1cb9fe
Show file tree
Hide file tree
Showing 3 changed files with 285 additions and 31 deletions.
72 changes: 44 additions & 28 deletions jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java
View file
Expand Up @@ -1667,41 +1667,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
}
}

Expand Down
178 changes: 175 additions & 3 deletions ...t-sessions-common/src/test/java/org/eclipse/jetty/server/session/DuplicateCookieTest.java
View file
Expand Up @@ -72,13 +72,16 @@ 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.header("Cookie", "JSESSIONID=123"); //doesn't exist
request.header("Cookie", "JSESSIONID=4422"); //does exist
ContentResponse response = request.send();
assertEquals(HttpServletResponse.SC_OK, response.getStatus());
assertEquals("4422", response.getContentAsString());

assertEquals(0, s4422.getRequests());
}
finally
Expand All @@ -89,7 +92,7 @@ public void testMultipleSessionCookiesOnlyOneExists() throws Exception
}

@Test
public void testMultipleSessionCookiesOnlyOneValid() throws Exception
public void testMultipleSessionCookiesValidFirst() throws Exception
{
String contextPath = "";
String servletMapping = "/server";
Expand All @@ -113,20 +116,146 @@ 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.header("Cookie", "JSESSIONID=1122"); //is valid
request.header("Cookie", "JSESSIONID=2233"); //is invalid
request.header("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();
}
}

@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 stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session")))
{
//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.header("Cookie", "JSESSIONID=2233"); //is invalid
request.header("Cookie", "JSESSIONID=2255"); //is invalid
request.header("Cookie", "JSESSIONID=1122"); //is valid
ContentResponse response = request.send();
assertEquals(HttpServletResponse.SC_OK, response.getStatus());
assertEquals("1122", response.getContentAsString());

assertEquals(0, s1122.getRequests());
}
finally
{
server1.stop();
client.stop();
}
}

@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 stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session")))
{
//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.header("Cookie", "JSESSIONID=2233"); //is invalid
request.header("Cookie", "JSESSIONID=1122"); //is valid
request.header("Cookie", "JSESSIONID=2255"); //is invalid
ContentResponse response = request.send();
assertEquals(HttpServletResponse.SC_OK, response.getStatus());
assertEquals("1122", response.getContentAsString());

assertEquals(0, s1122.getRequests());
}
finally
Expand Down Expand Up @@ -194,6 +323,49 @@ public void testMultipleSessionCookiesMultipleExists() throws Exception
}
}

@Test
public void testMulitpleIdenticalSessionCookies() 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 stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session")))
{
//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.header("Cookie", "JSESSIONID=1234");
request.header("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());
}
}

public Session createUnExpiredSession(SessionCache cache, SessionDataStore store, String id) throws Exception
{
long now = System.currentTimeMillis();
Expand Down
66 changes: 66 additions & 0 deletions ...ssions-common/src/test/java/org/eclipse/jetty/server/session/SessionInvalidationTest.java
View file
Expand Up @@ -30,8 +30,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;
Expand Down Expand Up @@ -96,6 +99,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;
Expand Down Expand Up @@ -131,6 +191,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()));
}
}
}
}

0 comments on commit c1cb9fe

Please sign in to comment.