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>
Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
  • Loading branch information
@sbordet
sbordet committed Mar 25, 2021
1 parent 30633a4 commit eb7e588
Show file tree
Hide file tree
Showing 3 changed files with 297 additions and 37 deletions.
72 changes: 44 additions & 28 deletions jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java
View file
Expand Up @@ -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
}
}

Expand Down
196 changes: 187 additions & 9 deletions ...t-sessions-common/src/test/java/org/eclipse/jetty/server/session/DuplicateCookieTest.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -66,24 +67,27 @@ 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
request.headers(headers -> headers.add("Cookie", "JSESSIONID=4422")); //does exist
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";
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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);
}
}

Expand Down

0 comments on commit eb7e588

Please sign in to comment.