Skip to content

Commit

Permalink
Issue jetty#4888 - Request getSession() now returns existing cached s…
Browse files Browse the repository at this point in the history
…ession

if found

Instead of logging and exception and returning null, if an existing
Session is found in the session cache when attempting to add a new
session then the existing session is used instead and returned
  • Loading branch information
Christopher L. Shannon committed May 19, 2020
1 parent 3215e11 commit 5680f0f
Show file tree
Hide file tree
Showing 5 changed files with 210 additions and 10 deletions.
Expand Up @@ -1604,7 +1604,7 @@ public HttpSession getSession(boolean create)
if (_sessionHandler == null)
throw new IllegalStateException("No SessionManager");

_session = _sessionHandler.newHttpSession(this);
_session = _sessionHandler.newHttpSession(this, true);
HttpCookie cookie = _sessionHandler.getSessionCookie(_session, getContextPath(), isSecure());
if (cookie != null)
_channel.getResponse().replaceCookie(cookie);
Expand Down
Expand Up @@ -440,10 +440,31 @@ private Session loadSession(String id)
*/
@Override
public void add(String id, Session session) throws Exception
{
if (addIfAbsent(id, session) != null)
{
throw new IllegalStateException("Session " + id + " already in cache");
}
}

/**
* Add an entirely new session (created by the application calling Request.getSession(true))
* to the cache if absent. The usage count of the fresh session is incremented if added.
* If session already exists the existing session will be returned else null if it was added
*
* @param id the id
* @param session
* @return the existing Session object if one already exists for the given id, or null if no existing
* Session exists and the new one was added
*/
@Override
public Session addIfAbsent(String id, Session session) throws Exception
{
if (id == null || session == null)
throw new IllegalArgumentException("Add key=" + id + " session=" + (session == null ? "null" : session.getId()));

Session existingSession;

try (Lock lock = session.lock())
{
if (session.getSessionHandler() == null)
Expand All @@ -452,14 +473,15 @@ public void add(String id, Session session) throws Exception
if (!session.isValid())
throw new IllegalStateException("Session " + id + " is not valid");

if (doPutIfAbsent(id, session) == null)
//Check if there is already an existing session for this id
if ((existingSession = doPutIfAbsent(id, session)) == null)
{
session.setResident(true); //its in the cache
session.use(); //the request is using it
}
else
throw new IllegalStateException("Session " + id + " already in cache");
}

return existingSession;
}

/**
Expand Down
Expand Up @@ -129,6 +129,18 @@ default Session renewSessionId(String oldId, String newId, String oldExtendedId,
*/
void add(String id, Session session) throws Exception;

/**
* Adds a new Session to the cache if absent.
* If a session already exists in the cache for the given id then the existing Session is returned
* and the new Session is not added
*
* @param id
* @param session
* @throws Exception
* @return the existing Session object or null if one already exists for the given id
*/
Session addIfAbsent(String id, Session session) throws Exception;

/**
* Get an existing Session. If necessary, the cache will load the data for
* the session from the configured SessionDataStore.
Expand Down
Expand Up @@ -757,6 +757,20 @@ public String getExtendedId(HttpSession session)
* @return the new <code>HttpSession</code>
*/
public HttpSession newHttpSession(HttpServletRequest request)
{
return newHttpSession(request, false);
}

/**
* Creates a new <code>HttpSession</code> or returns an existing <code>HttpSession</code>
* if useExisting is true
*
* @param request the HttpServletRequest containing the requested session id
* @param useExisting <code>true</code> to re-use an existing cached session if exists;
* <code>false</code> to throw an exception on existing cached session
* @return the new <code>HttpSession</code> or existing <code>HttpSession</code> if exists
*/
public HttpSession newHttpSession(HttpServletRequest request, boolean useExisting)
{
long created = System.currentTimeMillis();
String id = _sessionIdManager.newSessionId(request, created);
Expand All @@ -766,15 +780,36 @@ public HttpSession newHttpSession(HttpServletRequest request)

try
{
_sessionCache.add(id, session);
Request.getBaseRequest(request).enterSession(session);
_sessionsCreatedStats.increment();
final Session existingSession;
//If it is ok to use an existing cached session then use addIfAbsent
if (useExisting)
{
existingSession = _sessionCache.addIfAbsent(id, session);
}
else
{
existingSession = null;
//This will throw an exception if an existing Session is already in the cache
_sessionCache.add(id, session);
}

if (request != null && request.isSecure())
session.setAttribute(Session.SESSION_CREATED_SECURE, Boolean.TRUE);
//Check if the returned session is null
//If it is then that means the new session was added
if (existingSession == null)
{
Request.getBaseRequest(request).enterSession(session);
_sessionsCreatedStats.increment();

callSessionCreatedListeners(session);
if (request != null && request.isSecure())
session.setAttribute(Session.SESSION_CREATED_SECURE, Boolean.TRUE);

callSessionCreatedListeners(session);
}
else
{
//We found an existing session for this id so return that instead
session = existingSession;
}
return session;
}
catch (Exception e)
Expand Down
@@ -0,0 +1,131 @@
//
// ========================================================================
// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
// ------------------------------------------------------------------------
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
//
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
//
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
//
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
//

package org.eclipse.jetty.server.session;

import javax.servlet.http.HttpSession;

import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.handler.DefaultHandler;
import org.eclipse.jetty.server.handler.HandlerList;
import org.eclipse.jetty.servlet.DefaultServlet;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.util.component.LifeCycle;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class DuplicateSessionCacheTest
{
private Server server;
private HttpClient client;

ServletContextHandler contextHandler;

@BeforeEach
public void startServer() throws Exception
{
server = new Server();
ServerConnector connector = new ServerConnector(server);
connector.setPort(0);
server.addConnector(connector);

// Default session behavior
contextHandler = new ServletContextHandler(ServletContextHandler.SESSIONS);
contextHandler.setContextPath("/");
contextHandler.addServlet(DefaultServlet.class, "/");

HandlerList handlers = new HandlerList();
handlers.addHandler(contextHandler);
handlers.addHandler(new DefaultHandler());

server.setHandler(handlers);
server.start();
}

@AfterEach
public void stopServerAndClient()
{
LifeCycle.stop(server);
LifeCycle.stop(client);
}

@BeforeEach
public void startClient() throws Exception
{
client = new HttpClient();
client.start();
}

@Test
public void testNewSessionUseExistingTrue() throws Exception
{

//Create a new session and get the Id
Request request1 = new Request(null, null);
HttpSession session1 = contextHandler.getSessionHandler().newHttpSession(request1);
String id = session1.getId();

//Re-use the same session id and try and create a new session
//With version 9.4.20 this would work but return a new session which is not ideal if one exists
//After version 9.4.21 this would throw an exception
//With the commit this test is included in now the existing session will be returned
Request request2 = new Request(null, null);
request2.setRequestedSessionId(id);
HttpSession session2 = contextHandler.getSessionHandler().newHttpSession(request2,true);

//Verify the two returned sessions are the same
assertTrue(session1 == session2);
assertEquals(id, session1.getId());
}

@Test
public void testUseExistingCachedFalse() throws Exception
{
//Create a new session and get the Id
Request request1 = new Request(null, null);
HttpSession session1 = contextHandler.getSessionHandler().newHttpSession(request1);
String id = session1.getId();

//Should return null because a session exists and useExisting is false
Request request2 = new Request(null, null);
request2.setRequestedSessionId(id);
assertNull(contextHandler.getSessionHandler().newHttpSession(request2, false));
}

@Test
public void testNewHttpSessionOriginalMethod() throws Exception
{
//Create a new session and get the Id
Request request1 = new Request(null, null);
HttpSession session1 = contextHandler.getSessionHandler().newHttpSession(request1);
String id = session1.getId();

//Should return null because a session exists
Request request2 = new Request(null, null);
request2.setRequestedSessionId(id);
assertNull(contextHandler.getSessionHandler().newHttpSession(request2));
}
}

0 comments on commit 5680f0f

Please sign in to comment.