From fd5741ea0fff98294378cd53e8e5405a109e3459 Mon Sep 17 00:00:00 2001 From: markt Date: Sat, 16 Apr 2011 22:25:28 +0000 Subject: [PATCH] Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51042 Don't trigger session creation listeners when changing the session ID during authentication. git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1094069 13f79535-47bb-0310-9956-ffa450edef68 --- java/org/apache/catalina/Session.java | 14 +++++++++++++- java/org/apache/catalina/ha/session/DeltaManager.java | 14 ++------------ java/org/apache/catalina/ha/session/DeltaSession.java | 12 ++++++------ .../apache/catalina/ha/session/JvmRouteBinderValve.java | 3 +-- java/org/apache/catalina/manager/DummyProxySession.java | 6 ++++++ java/org/apache/catalina/session/ManagerBase.java | 2 +- java/org/apache/catalina/session/StandardSession.java | 14 +++++++++++++- webapps/docs/changelog.xml | 4 ++++ 8 files changed, 46 insertions(+), 23 deletions(-) diff --git a/java/org/apache/catalina/Session.java b/java/org/apache/catalina/Session.java index eed50ba82..6ab40c010 100644 --- a/java/org/apache/catalina/Session.java +++ b/java/org/apache/catalina/Session.java @@ -118,7 +118,8 @@ public interface Session { /** - * Set the session identifier for this session. + * Set the session identifier for this session and notifies any associated + * listeners that a new session has been created. * * @param id The new session identifier */ @@ -126,6 +127,17 @@ public interface Session { /** + * Set the session identifier for this session and optionally notifies any + * associated listeners that a new session has been created. + * + * @param id The new session identifier + * @param notify Should any associated listeners be notified that a new + * session has been created? + */ + public void setId(String id, boolean notify); + + + /** * Return descriptive information about this Session implementation and * the corresponding version number, in the format * <description>/<version>. diff --git a/java/org/apache/catalina/ha/session/DeltaManager.java b/java/org/apache/catalina/ha/session/DeltaManager.java index 28d7dd9b0..42e59a16b 100644 --- a/java/org/apache/catalina/ha/session/DeltaManager.java +++ b/java/org/apache/catalina/ha/session/DeltaManager.java @@ -1387,12 +1387,7 @@ public CatalinaCluster getCluster() { // use container maxInactiveInterval so that session will expire correctly in case of primary transfer session.setMaxInactiveInterval(getMaxInactiveInterval()); session.access(); - if(notifySessionListenersOnReplication) { - session.setId(msg.getSessionID()); - } else { - session.setIdInternal(msg.getSessionID()); - add(session); - } + session.setId(msg.getSessionID(), notifySessionListenersOnReplication); session.resetDeltaRequest(); session.endAccess(); @@ -1468,12 +1463,7 @@ public CatalinaCluster getCluster() { if (session != null) { String newSessionID = deserializeSessionId(msg.getSession()); session.setPrimarySession(false); - if (notifySessionListenersOnReplication) { - session.setId(newSessionID); - } else { - session.setIdInternal(newSessionID); - add(session); - } + session.setId(newSessionID, notifyListenersOnReplication); } } diff --git a/java/org/apache/catalina/ha/session/DeltaSession.java b/java/org/apache/catalina/ha/session/DeltaSession.java index 6ec35eeb7..34cc7df7f 100644 --- a/java/org/apache/catalina/ha/session/DeltaSession.java +++ b/java/org/apache/catalina/ha/session/DeltaSession.java @@ -244,17 +244,17 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus this.isPrimarySession = primarySession; } + /** - * Set the session identifier for this session without notify listeners. - * - * @param id - * The new session identifier + * {@inheritDoc} */ - public void setIdInternal(String id) { - this.id = id; + @Override + public void setId(String id, boolean notify) { + super.setId(id, notify); resetDeltaRequest(); } + /** * Set the session identifier for this session. * diff --git a/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java b/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java index c6fecc45f..96aa7b410 100644 --- a/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java +++ b/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java @@ -359,8 +359,7 @@ public class JvmRouteBinderValve extends ValveBase implements ClusterValve { protected void changeSessionID(Request request, String sessionId, String newSessionID, Session catalinaSession) { fireLifecycleEvent("Before session migration", catalinaSession); - // FIXME: setId trigger session Listener, but only chance to register manager with correct id! - catalinaSession.setId(newSessionID); + catalinaSession.setId(newSessionID, false); // FIXME: Why we remove change data from other running request? // setId also trigger resetDeltaRequest!! if (catalinaSession instanceof DeltaSession) diff --git a/java/org/apache/catalina/manager/DummyProxySession.java b/java/org/apache/catalina/manager/DummyProxySession.java index 3bd7baceb..5e66e6890 100644 --- a/java/org/apache/catalina/manager/DummyProxySession.java +++ b/java/org/apache/catalina/manager/DummyProxySession.java @@ -169,6 +169,12 @@ public class DummyProxySession implements Session { } @Override + public void setId(String id, boolean notify) { + this.sessionId = id; + // Ignore notify + } + + @Override public void setManager(Manager manager) { // NOOP } diff --git a/java/org/apache/catalina/session/ManagerBase.java b/java/org/apache/catalina/session/ManagerBase.java index 2a193da43..f9604d179 100644 --- a/java/org/apache/catalina/session/ManagerBase.java +++ b/java/org/apache/catalina/session/ManagerBase.java @@ -768,7 +768,7 @@ public abstract class ManagerBase extends LifecycleMBeanBase */ @Override public void changeSessionId(Session session) { - session.setId(generateSessionId()); + session.setId(generateSessionId(), false); } diff --git a/java/org/apache/catalina/session/StandardSession.java b/java/org/apache/catalina/session/StandardSession.java index 626013ff4..2b99fd478 100644 --- a/java/org/apache/catalina/session/StandardSession.java +++ b/java/org/apache/catalina/session/StandardSession.java @@ -374,6 +374,15 @@ public class StandardSession implements HttpSession, Session, Serializable { */ @Override public void setId(String id) { + setId(id, true); + } + + + /** + * {@inheritDoc} + */ + @Override + public void setId(String id, boolean notify) { if ((this.id != null) && (manager != null)) manager.remove(this); @@ -382,7 +391,10 @@ public class StandardSession implements HttpSession, Session, Serializable { if (manager != null) manager.add(this); - tellNew(); + + if (notify) { + tellNew(); + } } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 20985ffa6..cf8795d3b 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -65,6 +65,10 @@ 51038: Ensure that asynchronous requests are included in access logs. (markt) + + 51042: Don't trigger session creation listeners when a + session ID is changed as part of the authentication process. (markt) + -- 2.11.0