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

Signed-off-by: Jan Bartel <janb@webtide.com>
This commit is contained in:
Jan Bartel 2021-03-25 20:49:43 +11:00 committed by Simone Bordet
parent e0111e581a
commit af042ba297
3 changed files with 285 additions and 31 deletions

View File

@ -1667,41 +1667,57 @@ public class SessionHandler extends ScopedHandler
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 (session == null)
{
//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 (requestedSessionId == 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;
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
}
}

View File

@ -72,6 +72,8 @@ public class DuplicateCookieTest
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
@ -79,6 +81,7 @@ public class DuplicateCookieTest
ContentResponse response = request.send();
assertEquals(HttpServletResponse.SC_OK, response.getStatus());
assertEquals("4422", response.getContentAsString());
assertEquals(0, s4422.getRequests());
}
finally
@ -89,7 +92,7 @@ public class DuplicateCookieTest
}
@Test
public void testMultipleSessionCookiesOnlyOneValid() throws Exception
public void testMultipleSessionCookiesValidFirst() throws Exception
{
String contextPath = "";
String servletMapping = "/server";
@ -113,20 +116,146 @@ public class DuplicateCookieTest
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
@ -194,6 +323,49 @@ public class DuplicateCookieTest
}
}
@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();

View File

@ -30,8 +30,11 @@ import org.eclipse.jetty.client.api.ContentResponse;
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;
@ -96,6 +99,63 @@ public class SessionInvalidationTest
}
}
@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;
@ -131,6 +191,12 @@ public class SessionInvalidationTest
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()));
}
}
}
}