You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by fs...@apache.org on 2018/03/15 20:58:03 UTC
svn commit: r1826869 - in /tomcat/trunk:
java/org/apache/catalina/session/PersistentManagerBase.java
test/org/apache/catalina/session/TestPersistentManager.java
webapps/docs/changelog.xml
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org