You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2011/04/17 00:25:29 UTC

svn commit: r1094069 - in /tomcat/trunk: java/org/apache/catalina/ java/org/apache/catalina/ha/session/ java/org/apache/catalina/manager/ java/org/apache/catalina/session/ webapps/docs/

Author: markt
Date: Sat Apr 16 22:25:28 2011
New Revision: 1094069

URL: http://svn.apache.org/viewvc?rev=1094069&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51042
Don't trigger session creation listeners when changing the session ID during authentication.

Modified:
    tomcat/trunk/java/org/apache/catalina/Session.java
    tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java
    tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java
    tomcat/trunk/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java
    tomcat/trunk/java/org/apache/catalina/manager/DummyProxySession.java
    tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java
    tomcat/trunk/java/org/apache/catalina/session/StandardSession.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/catalina/Session.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/Session.java?rev=1094069&r1=1094068&r2=1094069&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/Session.java (original)
+++ tomcat/trunk/java/org/apache/catalina/Session.java Sat Apr 16 22:25:28 2011
@@ -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>.

Modified: tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java?rev=1094069&r1=1094068&r2=1094069&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java (original)
+++ tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java Sat Apr 16 22:25:28 2011
@@ -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);
         }
     }
 

Modified: tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java?rev=1094069&r1=1094068&r2=1094069&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java (original)
+++ tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java Sat Apr 16 22:25:28 2011
@@ -244,17 +244,17 @@ public class DeltaSession extends Standa
         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.
      *

Modified: tomcat/trunk/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java?rev=1094069&r1=1094068&r2=1094069&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java (original)
+++ tomcat/trunk/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java Sat Apr 16 22:25:28 2011
@@ -359,8 +359,7 @@ public class JvmRouteBinderValve extends
     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)

Modified: tomcat/trunk/java/org/apache/catalina/manager/DummyProxySession.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/manager/DummyProxySession.java?rev=1094069&r1=1094068&r2=1094069&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/manager/DummyProxySession.java (original)
+++ tomcat/trunk/java/org/apache/catalina/manager/DummyProxySession.java Sat Apr 16 22:25:28 2011
@@ -169,6 +169,12 @@ public class DummyProxySession implement
     }
 
     @Override
+    public void setId(String id, boolean notify) {
+        this.sessionId = id;
+        // Ignore notify
+    }
+
+    @Override
     public void setManager(Manager manager) {
         // NOOP
     }

Modified: tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java?rev=1094069&r1=1094068&r2=1094069&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java (original)
+++ tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java Sat Apr 16 22:25:28 2011
@@ -768,7 +768,7 @@ public abstract class ManagerBase extend
      */
     @Override
     public void changeSessionId(Session session) {
-        session.setId(generateSessionId());
+        session.setId(generateSessionId(), false);
     }
     
     

Modified: tomcat/trunk/java/org/apache/catalina/session/StandardSession.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/StandardSession.java?rev=1094069&r1=1094068&r2=1094069&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/session/StandardSession.java (original)
+++ tomcat/trunk/java/org/apache/catalina/session/StandardSession.java Sat Apr 16 22:25:28 2011
@@ -374,6 +374,15 @@ public class StandardSession implements 
      */
     @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 
 
         if (manager != null)
             manager.add(this);
-        tellNew();
+        
+        if (notify) {
+            tellNew();
+        }
     }
 
 

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1094069&r1=1094068&r2=1094069&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Sat Apr 16 22:25:28 2011
@@ -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">



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1094069 - in /tomcat/trunk: java/org/apache/catalina/ java/org/apache/catalina/ha/session/ java/org/apache/catalina/manager/ java/org/apache/catalina/session/ webapps/docs/

Posted by Mark Thomas <ma...@apache.org>.
On 26/04/2011 23:24, Filip Hanik - Dev Lists wrote:
> On 4/21/2011 8:00 AM, Mark Thomas wrote:
>> On 19/04/2011 16:27, Filip Hanik - Dev Lists wrote:
>>>
>>> On 4/18/2011 4:39 AM, Mark Thomas wrote:
>>>> On 18/04/2011 10:13, Remy Maucherat wrote:
>>>>> On Sat, 2011-04-16 at 22:25 +0000, markt@apache.org wrote:
>>>>>> Author: markt
>>>>>> Date: Sat Apr 16 22:25:28 2011
>>>>>> New Revision: 1094069
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=1094069&view=rev
>>>>>> Log:
>>>>>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51042
>>>>>> Don't trigger session creation listeners when changing the session
>>>>>> ID during authentication.
>>>>> But the listeners have to be aware that the id changed.
>>>> Why? I have checked the Servlet spec and I don't see any event defined
>>>> for "session ID changed". I also don't see anything (although I may
>>>> have
>>>> missed it) that says the ID must be constant.
>>> Every logical application that uses the ID as a key, would like to know
>>> that the ID has changed since the key is no longer valid. Those apps
>>> would rely on some sort event that the key is no longer there.
>>> regardless of what the servlet spec says, it's seems logical.
>> That raises the question of what event to fire.
> There is nothing that keeps us from defining events. While adhering to
> the spec, it doesn't mean to limit us to it.
> This is a pretty important event, the change of the ID (key) to existing
> data.
> I'd be considering a ID_CHANGE_EVENT here

+1. That works for me.

Mark

>> The session lifecycle
>> events are for when<spec-quote>An HttpSession has been created,
>> invalidated, or timed out.</spec-quote>. None of those apply in this
>> case. Hence my leaning towards the view that no event should be fired.
>> If this causes an issue for an application, it can always disable the
>> session fixation protection and provide their own alternative protection.
>>
>> I assume that you are suggesting that the session invalidated + session
>> created events are used, but as I said before, if those events are fired
>> then the object binding and attribute change events should also be
>> fired. I can see firing all these additional events being more likely to
>> cause problems for applications that just a change of the session ID.
>>
>> Mark
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>>
>>
>> -----
>> No virus found in this message.
>> Checked by AVG - www.avg.com
>> Version: 10.0.1209 / Virus Database: 1500/3593 - Release Date: 04/23/11
>>
>>
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
> 




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1094069 - in /tomcat/trunk: java/org/apache/catalina/ java/org/apache/catalina/ha/session/ java/org/apache/catalina/manager/ java/org/apache/catalina/session/ webapps/docs/

Posted by Filip Hanik - Dev Lists <de...@hanik.com>.
On 4/21/2011 8:00 AM, Mark Thomas wrote:
> On 19/04/2011 16:27, Filip Hanik - Dev Lists wrote:
>>
>> On 4/18/2011 4:39 AM, Mark Thomas wrote:
>>> On 18/04/2011 10:13, Remy Maucherat wrote:
>>>> On Sat, 2011-04-16 at 22:25 +0000, markt@apache.org wrote:
>>>>> Author: markt
>>>>> Date: Sat Apr 16 22:25:28 2011
>>>>> New Revision: 1094069
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1094069&view=rev
>>>>> Log:
>>>>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51042
>>>>> Don't trigger session creation listeners when changing the session
>>>>> ID during authentication.
>>>> But the listeners have to be aware that the id changed.
>>> Why? I have checked the Servlet spec and I don't see any event defined
>>> for "session ID changed". I also don't see anything (although I may have
>>> missed it) that says the ID must be constant.
>> Every logical application that uses the ID as a key, would like to know
>> that the ID has changed since the key is no longer valid. Those apps
>> would rely on some sort event that the key is no longer there.
>> regardless of what the servlet spec says, it's seems logical.
> That raises the question of what event to fire.
There is nothing that keeps us from defining events. While adhering to the spec, it doesn't mean to limit us to it.
This is a pretty important event, the change of the ID (key) to existing data.
I'd be considering a ID_CHANGE_EVENT here
> The session lifecycle
> events are for when<spec-quote>An HttpSession has been created,
> invalidated, or timed out.</spec-quote>. None of those apply in this
> case. Hence my leaning towards the view that no event should be fired.
> If this causes an issue for an application, it can always disable the
> session fixation protection and provide their own alternative protection.
>
> I assume that you are suggesting that the session invalidated + session
> created events are used, but as I said before, if those events are fired
> then the object binding and attribute change events should also be
> fired. I can see firing all these additional events being more likely to
> cause problems for applications that just a change of the session ID.
>
> Mark
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 10.0.1209 / Virus Database: 1500/3593 - Release Date: 04/23/11
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1094069 - in /tomcat/trunk: java/org/apache/catalina/ java/org/apache/catalina/ha/session/ java/org/apache/catalina/manager/ java/org/apache/catalina/session/ webapps/docs/

Posted by Mark Thomas <ma...@apache.org>.
On 19/04/2011 16:27, Filip Hanik - Dev Lists wrote:
> 
> 
> On 4/18/2011 4:39 AM, Mark Thomas wrote:
>> On 18/04/2011 10:13, Remy Maucherat wrote:
>>> On Sat, 2011-04-16 at 22:25 +0000, markt@apache.org wrote:
>>>> Author: markt
>>>> Date: Sat Apr 16 22:25:28 2011
>>>> New Revision: 1094069
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1094069&view=rev
>>>> Log:
>>>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51042
>>>> Don't trigger session creation listeners when changing the session
>>>> ID during authentication.
>>> But the listeners have to be aware that the id changed.
>> Why? I have checked the Servlet spec and I don't see any event defined
>> for "session ID changed". I also don't see anything (although I may have
>> missed it) that says the ID must be constant.
> 
> Every logical application that uses the ID as a key, would like to know
> that the ID has changed since the key is no longer valid. Those apps
> would rely on some sort event that the key is no longer there.
> regardless of what the servlet spec says, it's seems logical.

That raises the question of what event to fire. The session lifecycle
events are for when <spec-quote>An HttpSession has been created,
invalidated, or timed out.</spec-quote>. None of those apply in this
case. Hence my leaning towards the view that no event should be fired.
If this causes an issue for an application, it can always disable the
session fixation protection and provide their own alternative protection.

I assume that you are suggesting that the session invalidated + session
created events are used, but as I said before, if those events are fired
then the object binding and attribute change events should also be
fired. I can see firing all these additional events being more likely to
cause problems for applications that just a change of the session ID.

Mark



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1094069 - in /tomcat/trunk: java/org/apache/catalina/ java/org/apache/catalina/ha/session/ java/org/apache/catalina/manager/ java/org/apache/catalina/session/ webapps/docs/

Posted by Filip Hanik - Dev Lists <de...@hanik.com>.

On 4/18/2011 4:39 AM, Mark Thomas wrote:
> On 18/04/2011 10:13, Remy Maucherat wrote:
>> On Sat, 2011-04-16 at 22:25 +0000, markt@apache.org wrote:
>>> Author: markt
>>> Date: Sat Apr 16 22:25:28 2011
>>> New Revision: 1094069
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1094069&view=rev
>>> Log:
>>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51042
>>> Don't trigger session creation listeners when changing the session ID during authentication.
>> But the listeners have to be aware that the id changed.
> Why? I have checked the Servlet spec and I don't see any event defined
> for "session ID changed". I also don't see anything (although I may have
> missed it) that says the ID must be constant.

Every logical application that uses the ID as a key, would like to know that the ID has changed since the key is no longer valid. Those apps 
would rely on some sort event that the key is no longer there. regardless of what the servlet spec says, it's seems logical.

Filip

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1094069 - in /tomcat/trunk: java/org/apache/catalina/ java/org/apache/catalina/ha/session/ java/org/apache/catalina/manager/ java/org/apache/catalina/session/ webapps/docs/

Posted by Mark Thomas <ma...@apache.org>.
On 18/04/2011 10:13, Remy Maucherat wrote:
> On Sat, 2011-04-16 at 22:25 +0000, markt@apache.org wrote:
>> Author: markt
>> Date: Sat Apr 16 22:25:28 2011
>> New Revision: 1094069
>>
>> URL: http://svn.apache.org/viewvc?rev=1094069&view=rev
>> Log:
>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51042
>> Don't trigger session creation listeners when changing the session ID during authentication.
> 
> But the listeners have to be aware that the id changed.

Why? I have checked the Servlet spec and I don't see any event defined
for "session ID changed". I also don't see anything (although I may have
missed it) that says the ID must be constant.

It Tomcat treats changing the ID as "session invalidated" followed by
"session created" then the listeners would need to be notified (for both
events) but that raises additional questions about events for object
binding and attribute changes - all those events should probably be
triggered as well since the attributes are essentially being removed
from the old session and added to the new. That could be an expensive
and unnecessary process for some applications.

The previous behaviour only triggered a session created event for the
new session ID. There was no corresponding session destroyed event for
the old session ID. Regardless of whether we decide that events are
required or not, the previous behaviour was wrong.

My preference is to keep things simple and not trigger any events when
the ID changes. Sessions aren't being invalidated or created so I think
firing those events is more likely to cause problems for applications
than not firing them.

To date (AFAIR), there has been one issue reported on the users list
(changing ID broke a Java applet - fixed by not changing the ID on
authentication) and one bug (that session created is fired but not
session destroyed - that triggered this fix).

Once we agree on how to handle this, we need to make sure all session id
changes (authentication, jvmRoute changes) are handled the same way.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1094069 - in /tomcat/trunk: java/org/apache/catalina/ java/org/apache/catalina/ha/session/ java/org/apache/catalina/manager/ java/org/apache/catalina/session/ webapps/docs/

Posted by Remy Maucherat <re...@apache.org>.
On Sat, 2011-04-16 at 22:25 +0000, markt@apache.org wrote:
> Author: markt
> Date: Sat Apr 16 22:25:28 2011
> New Revision: 1094069
> 
> URL: http://svn.apache.org/viewvc?rev=1094069&view=rev
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51042
> Don't trigger session creation listeners when changing the session ID during authentication.

But the listeners have to be aware that the id changed.

Rémy



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1094069 - in /tomcat/trunk: java/org/apache/catalina/ java/org/apache/catalina/ha/session/ java/org/apache/catalina/manager/ java/org/apache/catalina/session/ webapps/docs/

Posted by Mark Thomas <ma...@apache.org>.
On 18/04/2011 02:43, Keiichi Fujino wrote:
> A primary node is never notified to the listener when changing the
> session ID during authentication.
> Should I not notify the listener if non-primary node receives
> EVT_CHANGE_SESSION_ID?

My own view is that a session ID change (from authentication, from a
jvmRoute change or from anything else) should not trigger a notification
to any session listeners. However, that may not be the community view.
Remy started a discussion on this elsewhere in this thread. I'll respond
to that and I think we should see what the conclusion is before making
any more changes.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1094069 - in /tomcat/trunk: java/org/apache/catalina/ java/org/apache/catalina/ha/session/ java/org/apache/catalina/manager/ java/org/apache/catalina/session/ webapps/docs/

Posted by Keiichi Fujino <kf...@apache.org>.
A primary node is never notified to the listener when changing the
session ID during authentication.
Should I not notify the listener if non-primary node receives
EVT_CHANGE_SESSION_ID?

Index: java/org/apache/catalina/ha/session/DeltaManager.java
===================================================================
--- java/org/apache/catalina/ha/session/DeltaManager.java	(revision 1094215)
+++ java/org/apache/catalina/ha/session/DeltaManager.java	(working copy)
@@ -1463,7 +1463,7 @@
         if (session != null) {
             String newSessionID = deserializeSessionId(msg.getSession());
             session.setPrimarySession(false);
-            session.setId(newSessionID, notifyListenersOnReplication);
+            session.setId(newSessionID, false);
         }
     }



Is JvmRouteSessionIDBinderListener also the same?

Index: java/org/apache/catalina/ha/session/JvmRouteSessionIDBinderListener.java
===================================================================
--- java/org/apache/catalina/ha/session/JvmRouteSessionIDBinderListener.java	(revision
1037582)
+++ java/org/apache/catalina/ha/session/JvmRouteSessionIDBinderListener.java	(working
copy)
@@ -144,7 +144,7 @@
                         Session session = context.getManager().findSession(
                                 sessionmsg.getOrignalSessionID());
                         if (session != null) {
-                            session.setId(sessionmsg.getBackupSessionID());
+
session.setId(sessionmsg.getBackupSessionID(), false);
                         } else if (log.isInfoEnabled())
                             log.info(sm.getString("jvmRoute.lostSession",
                                     sessionmsg.getOrignalSessionID(),


2011/4/17  <ma...@apache.org>:
> Author: markt
> Date: Sat Apr 16 22:25:28 2011
> New Revision: 1094069
>
> URL: http://svn.apache.org/viewvc?rev=1094069&view=rev
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51042
> Don't trigger session creation listeners when changing the session ID during authentication.
>
> Modified:
>    tomcat/trunk/java/org/apache/catalina/Session.java
>    tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java
>    tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java
>    tomcat/trunk/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java
>    tomcat/trunk/java/org/apache/catalina/manager/DummyProxySession.java
>    tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java
>    tomcat/trunk/java/org/apache/catalina/session/StandardSession.java
>    tomcat/trunk/webapps/docs/changelog.xml
>
> Modified: tomcat/trunk/java/org/apache/catalina/Session.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/Session.java?rev=1094069&r1=1094068&r2=1094069&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/Session.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/Session.java Sat Apr 16 22:25:28 2011
> @@ -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>.
>
> Modified: tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java?rev=1094069&r1=1094068&r2=1094069&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java Sat Apr 16 22:25:28 2011
> @@ -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);
>         }
>     }
>
>
> Modified: tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java?rev=1094069&r1=1094068&r2=1094069&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java Sat Apr 16 22:25:28 2011
> @@ -244,17 +244,17 @@ public class DeltaSession extends Standa
>         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.
>      *
>
> Modified: tomcat/trunk/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java?rev=1094069&r1=1094068&r2=1094069&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java Sat Apr 16 22:25:28 2011
> @@ -359,8 +359,7 @@ public class JvmRouteBinderValve extends
>     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)
>
> Modified: tomcat/trunk/java/org/apache/catalina/manager/DummyProxySession.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/manager/DummyProxySession.java?rev=1094069&r1=1094068&r2=1094069&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/manager/DummyProxySession.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/manager/DummyProxySession.java Sat Apr 16 22:25:28 2011
> @@ -169,6 +169,12 @@ public class DummyProxySession implement
>     }
>
>     @Override
> +    public void setId(String id, boolean notify) {
> +        this.sessionId = id;
> +        // Ignore notify
> +    }
> +
> +    @Override
>     public void setManager(Manager manager) {
>         // NOOP
>     }
>
> Modified: tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java?rev=1094069&r1=1094068&r2=1094069&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java Sat Apr 16 22:25:28 2011
> @@ -768,7 +768,7 @@ public abstract class ManagerBase extend
>      */
>     @Override
>     public void changeSessionId(Session session) {
> -        session.setId(generateSessionId());
> +        session.setId(generateSessionId(), false);
>     }
>
>
>
> Modified: tomcat/trunk/java/org/apache/catalina/session/StandardSession.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/StandardSession.java?rev=1094069&r1=1094068&r2=1094069&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/session/StandardSession.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/session/StandardSession.java Sat Apr 16 22:25:28 2011
> @@ -374,6 +374,15 @@ public class StandardSession implements
>      */
>     @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
>
>         if (manager != null)
>             manager.add(this);
> -        tellNew();
> +
> +        if (notify) {
> +            tellNew();
> +        }
>     }
>
>
>
> Modified: tomcat/trunk/webapps/docs/changelog.xml
> URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1094069&r1=1094068&r2=1094069&view=diff
> ==============================================================================
> --- tomcat/trunk/webapps/docs/changelog.xml (original)
> +++ tomcat/trunk/webapps/docs/changelog.xml Sat Apr 16 22:25:28 2011
> @@ -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">
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>



-- 
Keiichi.Fujino

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org