Author: fschumacher Date: Thu Mar 15 20:58:03 2018 New Revision: 1826869 URL: http://svn.apache.org/viewvc?rev=1826869&view=rev Log: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62175
Avoid infinite recursion, when trying to validate a session while loading it with PersistentManager. Modified: tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java tomcat/trunk/test/org/apache/catalina/session/TestPersistentManager.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java?rev=1826869&r1=1826868&r2=1826869&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java (original) +++ tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java Thu Mar 15 20:58:03 2018 @@ -183,6 +183,12 @@ public abstract class PersistentManagerB */ private final Map<String,Object> sessionSwapInLocks = new HashMap<>(); + /* + * Session that is currently getting swapped in to prevent loading it more + * than once concurrently + */ + private final ThreadLocal<Session> sessionToSwapIn = new ThreadLocal<>(); + // ------------------------------------------------------------- Properties @@ -707,18 +713,25 @@ public abstract class PersistentManagerB session = sessions.get(id); if (session == null) { - session = loadSessionFromStore(id); - - if (session != null && !session.isValid()) { - log.error(sm.getString( - "persistentManager.swapInInvalid", id)); - session.expire(); - removeSession(id); - session = null; - } - - if (session != null) { - reactivateLoadedSession(id, session); + Session currentSwapInSession = sessionToSwapIn.get(); + try { + if (currentSwapInSession == null || !id.equals(currentSwapInSession.getId())) { + session = loadSessionFromStore(id); + sessionToSwapIn.set(session); + + if (session != null && !session.isValid()) { + log.error(sm.getString("persistentManager.swapInInvalid", id)); + session.expire(); + removeSession(id); + session = null; + } + + if (session != null) { + reactivateLoadedSession(id, session); + } + } + } finally { + sessionToSwapIn.remove(); } } } Modified: tomcat/trunk/test/org/apache/catalina/session/TestPersistentManager.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/session/TestPersistentManager.java?rev=1826869&r1=1826868&r2=1826869&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/catalina/session/TestPersistentManager.java (original) +++ tomcat/trunk/test/org/apache/catalina/session/TestPersistentManager.java Thu Mar 15 20:58:03 2018 @@ -16,13 +16,27 @@ */ package org.apache.catalina.session; +import java.util.concurrent.atomic.AtomicInteger; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpSessionEvent; +import javax.servlet.http.HttpSessionListener; + import org.junit.Assert; import org.junit.Test; import org.apache.catalina.Context; import org.apache.catalina.Host; +import org.apache.catalina.Manager; +import org.apache.catalina.Session; +import org.apache.catalina.Store; +import org.apache.catalina.connector.Connector; +import org.apache.catalina.connector.Request; +import org.apache.catalina.connector.RequestFacade; import org.apache.tomcat.unittest.TesterContext; import org.apache.tomcat.unittest.TesterHost; +import org.easymock.EasyMock; +import org.easymock.IAnswer; public class TestPersistentManager { @@ -49,11 +63,97 @@ public class TestPersistentManager { // Given the minIdleSwap settings, this should swap one out to get below // the limit manager.processPersistenceChecks(); - Assert.assertEquals(1, manager.getActiveSessions()); - Assert.assertEquals(2, manager.getActiveSessionsFull()); + Assert.assertEquals(1, manager.getActiveSessions()); + Assert.assertEquals(2, manager.getActiveSessionsFull()); manager.createSession(null); - Assert.assertEquals(2, manager.getActiveSessions()); - Assert.assertEquals(3, manager.getActiveSessionsFull()); + Assert.assertEquals(2, manager.getActiveSessions()); + Assert.assertEquals(3, manager.getActiveSessionsFull()); + } + + @Test + public void testBug62175() throws Exception { + PersistentManager manager = new PersistentManager(); + AtomicInteger sessionExpireCounter = new AtomicInteger(); + + Store mockStore = EasyMock.createNiceMock(Store.class); + EasyMock.expect(mockStore.load(EasyMock.anyString())).andAnswer(new IAnswer<Session>() { + + @Override + public Session answer() throws Throwable { + return timedOutSession(manager, sessionExpireCounter); + } + }).anyTimes(); + + EasyMock.replay(mockStore); + + manager.setStore(mockStore); + + Host host = new TesterHost(); + + RequestCachingSessionListener requestCachingSessionListener = new RequestCachingSessionListener(); + + Context context = new TesterContext() { + + @Override + public Object[] getApplicationLifecycleListeners() { + return new Object[] { requestCachingSessionListener }; + } + + @Override + public Manager getManager() { + return manager; + } + }; + context.setParent(host); + + Connector connector = EasyMock.createNiceMock(Connector.class); + Request req = new Request(connector) { + @Override + public Context getContext() { + return context; + } + }; + req.setRequestedSessionId("invalidSession"); + HttpServletRequest request = new RequestFacade(req); + EasyMock.replay(connector); + requestCachingSessionListener.request = request; + + manager.setContext(context); + + manager.start(); + + Assert.assertNull(request.getSession(false)); + Assert.assertEquals(1, sessionExpireCounter.get()); + + } + + private static class RequestCachingSessionListener implements HttpSessionListener { + + private HttpServletRequest request; + + @Override + public void sessionDestroyed(HttpSessionEvent se) { + request.getSession(false); + } + } + + private StandardSession timedOutSession(PersistentManager manager, AtomicInteger counter) { + StandardSession timedOutSession = new StandardSession(manager) { + private static final long serialVersionUID = -5910605558747844210L; + + @Override + public void expire() { + counter.incrementAndGet(); + super.expire(); + } + }; + timedOutSession.isValid = true; + timedOutSession.expiring = false; + timedOutSession.maxInactiveInterval = 1; + timedOutSession.lastAccessedTime = 0; + timedOutSession.id = "invalidSession"; + return timedOutSession; } } + Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1826869&r1=1826868&r2=1826869&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Thu Mar 15 20:58:03 2018 @@ -64,6 +64,11 @@ out sessions to keep the number of active sessions under <code>maxActive</code>. Patch provided by Holger Sunke. (markt) </fix> + <fix> + <bug>62175</bug>: Avoid infinite recursion, when trying to validate + a session while loading it with <code>PersistentManager</code>. + (fschumacher) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org