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 reference counts for multiple valid cookies for sessions #6096

Merged
merged 1 commit into from Mar 24, 2021
Merged

Issue #6085 Fix reference counts for multiple valid cookies for sessions #6096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 16 additions & 9 deletions jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java
View file
Open in desktop
Expand Up @@ -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);
sbordet marked this conversation as resolved.
Show resolved Hide resolved

if (requestedSessionId == null)
{
Expand All @@ -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
}
}

Expand All @@ -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
Expand Down
23 changes: 18 additions & 5 deletions ...t-sessions-common/src/test/java/org/eclipse/jetty/server/session/DuplicateCookieTest.java
View file
Open in desktop
Expand Up @@ -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");

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

Expand Down