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

diff --git a/java/org/apache/catalina/Session.java b/java/org/apache/catalina/Session.java
index eed50ba..6ab40c0 100644 (file)
--- 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
      * <code>&lt;description&gt;/&lt;version&gt;</code>.

diff --git a/java/org/apache/catalina/ha/session/DeltaManager.java b/java/org/apache/catalina/ha/session/DeltaManager.java
index 28d7dd9..42e59a1 100644 (file)
--- 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 6ec35ee..34cc7df 100644 (file)
--- 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 c6fecc4..96aa7b4 100644 (file)
--- 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 3bd7bac..5e66e68 100644 (file)
--- 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 2a193da..f9604d1 100644 (file)
--- 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 626013f..2b99fd4 100644 (file)
--- 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 20985ff..cf8795d 100644 (file)
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -65,6 +65,10 @@
         <bug>51038</bug>: Ensure that asynchronous requests are included in
         access logs. (markt)
       </fix>
+      <fix>
+        <bug>51042</bug>: Don&apos;t trigger session creation listeners when a
+        session ID is changed as part of the authentication process. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">