Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Issue #6085 Fix duplicate valid session cookies to pick first valid. #6103

Merged
merged 3 commits into from Mar 25, 2021
Merged

Issue #6085 Fix duplicate valid session cookies to pick first valid. #6103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c1cb9fe
Issue #6085 Fix duplicate valid session cookies to pick first valid.
janbartel Mar 25, 2021
2cd5990
Issue #6085 Fix duplicate valid session cookies to pick first valid.
sbordet Mar 25, 2021
1aa1ca4
Issue #6085 Fix duplicate valid session cookies to pick first valid.
sbordet Mar 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
74 changes: 46 additions & 28 deletions jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java
View file
Open in desktop
Expand Up @@ -1667,41 +1667,59 @@ 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
//request exits
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
183 changes: 180 additions & 3 deletions ...t-sessions-common/src/test/java/org/eclipse/jetty/server/session/DuplicateCookieTest.java
View file
Open in desktop
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,54 @@ public void testMultipleSessionCookiesMultipleExists() throws Exception
}
}

@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 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());
}
finally
{
server1.stop();
client.stop();
}
}

public Session createUnExpiredSession(SessionCache cache, SessionDataStore store, String id) throws Exception
{
long now = System.currentTimeMillis();
Expand Down