You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Daniel Rall <dl...@finemaltcoding.com> on 2003/02/06 00:33:57 UTC

[TOMCAT 4.1.18] StandardSession.setId() found incohesive

Developers, I'd like your input on why setId() has the side effects
that it does, and your suggestions on what solution I should implement
which would be acceptable to both Tomcat 4 and 5, and cleanly usable
by myself.  The issue I filed includes the Valve implementation which
excercises the setId() method, and hacks around its side effects.



TRACKING ISSUE: http://issues.apache.org/bugzilla/show_bug.cgi?id=16822


OBSERVED BEHAVIOR: ManagerBase.createSession() calls
StandardSession.setId() as the last thing it does before returning a
newly created session.  StandardSession.setId() exhibits the
undocumented side effect of firing a session creation event, iterating
over all HttpSessionListeners registered with the current web context
and notifying them of its "creation":

    public void setId(String id) {

        if ((this.id != null) && (manager != null))
            manager.remove(this);

        this.id = id;

        if (manager != null)
            manager.add(this);

        // Notify interested session event listeners
        fireSessionEvent(Session.SESSION_CREATED_EVENT, null);

        // Notify interested application event listeners
        Context context = (Context) manager.getContainer();
        Object listeners[] = context.getApplicationListeners();
        if (listeners != null) {
            HttpSessionEvent event =
                new HttpSessionEvent(getSession());
            for (int i = 0; i < listeners.length; i++) {
                if (!(listeners[i] instanceof HttpSessionListener))
                    continue;
                HttpSessionListener listener =
                    (HttpSessionListener) listeners[i];
                try {
                    fireContainerEvent(context,
                                       "beforeSessionCreated",
                                       listener);
                    listener.sessionCreated(event);
                    fireContainerEvent(context,
                                       "afterSessionCreated",
                                       listener);
                } catch (Throwable t) {
                    try {
                        fireContainerEvent(context,
                                           "afterSessionCreated",
                                           listener);
                    } catch (Exception e) {
                        ;
                    }
                    // FIXME - should we do anything besides log these?
                    log(sm.getString("standardSession.sessionEvent"), t);
                }
            }
        }

    }

A second call to setId() will again fire a session creation event for
the existing session.  Here's some trace from making a single request
when calling setId() explicitly:

2003-02-03 17:46:43,113 [Ajp13Processor[17025][4]] INFO  default - Adding
session org.apache.catalina.session.StandardSessionFacade@4977e2 with id of
2EA4840C7D4D6C7F3FF76F1F95C575D5
2003-02-03 17:46:43,175 [Ajp13Processor[17025][4]] INFO  default - Adding
session org.apache.catalina.session.StandardSessionFacade@4977e2 with id of
C6C908E127E6230CC81AE70E10D914A4

My web application's list of active sessions (stored as a Map of
StandardSessionFacade objects keyed by session ID) will look as
follows:

Session creation event fired as ManagerBase sets the initial ID by
calling StandardSession.setId() during its createSession() method
              |
              |
              v
.______________________________________.
| Key  | Value                         |
`--------------------------------------'
| ID A | Facade A --> Session A (ID A) |
`--------------------------------------'
              |
              |
Second session creation event fired as SessionIdValve resets the ID,
calling setId() for a second time (this time explicitly).
              |
              |
              v
.______________________________________.
| Key  | Value                         |
`--------------------------------------'
| ID A | Facade A --> Session A (ID A) |
| ID B | Facade A --> Session A (ID B) |
`--------------------------------------'

The entry keyed by session ID A will not be removed from our list of
active sessions when Session A expires, as the session now has an ID
of B.


USE CASE: When creating a new session, I must set the session
identifier (or manipulate it after the initial generation from a
Valve) using request-specific inputs.

Specifically, I must re-use any session identifier supplied by the
client via its JSESSIONID cookie if that session identifier is not
already in use.  Because the Servlet API dictates that the session
cookie is always named JSESSIONID, this is a must have for support of
wildcard cookie domains across hosts which share a common base domain
name.

For instance, if I have host1.domain.com and host2.domain.com, and set
the cookie domain of sessions cookie to the wildcard ".domain.com"
(some pathetic browsers only support two part wildcard domains), the
cookie will apply to both hosts.  This is problematic when a client
which has established a session on host1 tries to establish a second
session on host2 -- host2 will assign the client a new session ID,
wiping the client's memory of its session ID for host1.  Now, if host1
and host2 are gracious enough to use the same session ID, they can
effectively share the JSESSIONID cookie, allowing the client to enjoy
simultaneous sessions on both host1 and host2 while working within the
Servlet API.


PROBLEM: Tomcat's Manager interface has no API for accessing
contextual information specific to a request.  Combining a Valve which
provides this request-specific context via thread-local storage with
my own custom manager is hacky at best, and would require that I
completely re-implement the code in StandardSession (due to its
package-private scoping).


RANT: Catalina should not hide its session created/destroy event
notification code in a package-private class.  It should probably live
in a Manager -- rather than Session -- implementation, either in its
createSession() and expire(Session) methods, or in its add(Session)
and remove(Session) methods.

-- 

Daniel Rall <dl...@apache.org>

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


Re: [TOMCAT 4.1.18] StandardSession.setId() found incohesive

Posted by Remy Maucherat <re...@apache.org>.
jean-frederic clere wrote:
> Daniel Rall wrote:
> 
>> Developers, I'd like your input on why setId() has the side effects
>> that it does, and your suggestions on what solution I should implement
>> which would be acceptable to both Tomcat 4 and 5, and cleanly usable
>> by myself.  The issue I filed includes the Valve implementation which
>> excercises the setId() method, and hacks around its side effects.
>>
>>
>>
>> TRACKING ISSUE: http://issues.apache.org/bugzilla/show_bug.cgi?id=16822
>>
>>
>> OBSERVED BEHAVIOR: ManagerBase.createSession() calls
>> StandardSession.setId() as the last thing it does before returning a
>> newly created session.
> 
> 
> In Tomcat5 I have added a createEmptySession() that should solve the 
> problem.

It's a question of timing. Since I've allowed Glenn's API changes for 
the deployer in 4.1.19, then it's probably ok to backport your fix now, 
as we're not trying to stabilize in preparation for a new stable release 
(yet).

Remy


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


Re: [TOMCAT 4.1.18] StandardSession.setId() found incohesive

Posted by jean-frederic clere <jf...@fujitsu-siemens.com>.
Daniel Rall wrote:
> Developers, I'd like your input on why setId() has the side effects
> that it does, and your suggestions on what solution I should implement
> which would be acceptable to both Tomcat 4 and 5, and cleanly usable
> by myself.  The issue I filed includes the Valve implementation which
> excercises the setId() method, and hacks around its side effects.
> 
> 
> 
> TRACKING ISSUE: http://issues.apache.org/bugzilla/show_bug.cgi?id=16822
> 
> 
> OBSERVED BEHAVIOR: ManagerBase.createSession() calls
> StandardSession.setId() as the last thing it does before returning a
> newly created session.

In Tomcat5 I have added a createEmptySession() that should solve the problem.

>  StandardSession.setId() exhibits the
> undocumented side effect of firing a session creation event, iterating
> over all HttpSessionListeners registered with the current web context
> and notifying them of its "creation":
> 
>     public void setId(String id) {
> 
>         if ((this.id != null) && (manager != null))
>             manager.remove(this);
> 
>         this.id = id;
> 
>         if (manager != null)
>             manager.add(this);
> 
>         // Notify interested session event listeners
>         fireSessionEvent(Session.SESSION_CREATED_EVENT, null);
> 
>         // Notify interested application event listeners
>         Context context = (Context) manager.getContainer();
>         Object listeners[] = context.getApplicationListeners();
>         if (listeners != null) {
>             HttpSessionEvent event =
>                 new HttpSessionEvent(getSession());
>             for (int i = 0; i < listeners.length; i++) {
>                 if (!(listeners[i] instanceof HttpSessionListener))
>                     continue;
>                 HttpSessionListener listener =
>                     (HttpSessionListener) listeners[i];
>                 try {
>                     fireContainerEvent(context,
>                                        "beforeSessionCreated",
>                                        listener);
>                     listener.sessionCreated(event);
>                     fireContainerEvent(context,
>                                        "afterSessionCreated",
>                                        listener);
>                 } catch (Throwable t) {
>                     try {
>                         fireContainerEvent(context,
>                                            "afterSessionCreated",
>                                            listener);
>                     } catch (Exception e) {
>                         ;
>                     }
>                     // FIXME - should we do anything besides log these?
>                     log(sm.getString("standardSession.sessionEvent"), t);
>                 }
>             }
>         }
> 
>     }
> 
> A second call to setId() will again fire a session creation event for
> the existing session.  Here's some trace from making a single request
> when calling setId() explicitly:
> 
> 2003-02-03 17:46:43,113 [Ajp13Processor[17025][4]] INFO  default - Adding
> session org.apache.catalina.session.StandardSessionFacade@4977e2 with id of
> 2EA4840C7D4D6C7F3FF76F1F95C575D5
> 2003-02-03 17:46:43,175 [Ajp13Processor[17025][4]] INFO  default - Adding
> session org.apache.catalina.session.StandardSessionFacade@4977e2 with id of
> C6C908E127E6230CC81AE70E10D914A4
> 
> My web application's list of active sessions (stored as a Map of
> StandardSessionFacade objects keyed by session ID) will look as
> follows:
> 
> Session creation event fired as ManagerBase sets the initial ID by
> calling StandardSession.setId() during its createSession() method
>               |
>               |
>               v
> .______________________________________.
> | Key  | Value                         |
> `--------------------------------------'
> | ID A | Facade A --> Session A (ID A) |
> `--------------------------------------'
>               |
>               |
> Second session creation event fired as SessionIdValve resets the ID,
> calling setId() for a second time (this time explicitly).
>               |
>               |
>               v
> .______________________________________.
> | Key  | Value                         |
> `--------------------------------------'
> | ID A | Facade A --> Session A (ID A) |
> | ID B | Facade A --> Session A (ID B) |
> `--------------------------------------'
> 
> The entry keyed by session ID A will not be removed from our list of
> active sessions when Session A expires, as the session now has an ID
> of B.
> 
> 
> USE CASE: When creating a new session, I must set the session
> identifier (or manipulate it after the initial generation from a
> Valve) using request-specific inputs.
> 
> Specifically, I must re-use any session identifier supplied by the
> client via its JSESSIONID cookie if that session identifier is not
> already in use.  Because the Servlet API dictates that the session
> cookie is always named JSESSIONID, this is a must have for support of
> wildcard cookie domains across hosts which share a common base domain
> name.
> 
> For instance, if I have host1.domain.com and host2.domain.com, and set
> the cookie domain of sessions cookie to the wildcard ".domain.com"
> (some pathetic browsers only support two part wildcard domains), the
> cookie will apply to both hosts.  This is problematic when a client
> which has established a session on host1 tries to establish a second
> session on host2 -- host2 will assign the client a new session ID,
> wiping the client's memory of its session ID for host1.  Now, if host1
> and host2 are gracious enough to use the same session ID, they can
> effectively share the JSESSIONID cookie, allowing the client to enjoy
> simultaneous sessions on both host1 and host2 while working within the
> Servlet API.
> 
> 
> PROBLEM: Tomcat's Manager interface has no API for accessing
> contextual information specific to a request.  Combining a Valve which
> provides this request-specific context via thread-local storage with
> my own custom manager is hacky at best, and would require that I
> completely re-implement the code in StandardSession (due to its
> package-private scoping).
> 
> 
> RANT: Catalina should not hide its session created/destroy event
> notification code in a package-private class.  It should probably live
> in a Manager -- rather than Session -- implementation, either in its
> createSession() and expire(Session) methods, or in its add(Session)
> and remove(Session) methods.
> 



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


Re: [TOMCAT 4.1.18] StandardSession.setId() found incohesive

Posted by Tom Anderson <ta...@infonow.com>.
I'm not quite sure why it does but I am finding the implementation of 
this class in particular to be somewhat flawed.   For example, the 
recycle() method doesn't remove a session from memory (via the Manager) 
but expire() does.   So, when something is recycled, it is actually 
left on the list of sessions too.

What does this have to do with setId().   Well, I only found all of the 
above problems because I am trying to fix my copy of JDBCStore to 
retain the session ID when it calls setId() (the problem I'm trying to 
fix is that I discovered a Session "leak" related to the fact that a 
loaded session has a different ID than what's in the database).   But 
the JDBCStore.load() method calls ManagerBase.createSession() which 
ALSO calls Session.setId()... my head is spinning.   So if I fix 
JDBCStore.load() then setId() gets fired twice with the undesired 
side-effects.

I'm not exactly answering your question but hopefully re-affirming that 
there is a problem here.   My opinion is that the StandardSession class 
should have no knowledge of the Manager that created it.  Then maybe 
these circular calls would be eliminated.   But what do I know?

~Tom


On Wednesday, February 5, 2003, at 04:33 PM, Daniel Rall wrote:

> Developers, I'd like your input on why setId() has the side effects
> that it does, and your suggestions on what solution I should implement
> which would be acceptable to both Tomcat 4 and 5, and cleanly usable
> by myself.  The issue I filed includes the Valve implementation which
> excercises the setId() method, and hacks around its side effects.
>
>
>
> TRACKING ISSUE: http://issues.apache.org/bugzilla/show_bug.cgi?id=16822
>
>
> OBSERVED BEHAVIOR: ManagerBase.createSession() calls
> StandardSession.setId() as the last thing it does before returning a
> newly created session.  StandardSession.setId() exhibits the
> undocumented side effect of firing a session creation event, iterating
> over all HttpSessionListeners registered with the current web context
> and notifying them of its "creation":
>
>     public void setId(String id) {
>
>         if ((this.id != null) && (manager != null))
>             manager.remove(this);
>
>         this.id = id;
>
>         if (manager != null)
>             manager.add(this);
>
>         // Notify interested session event listeners
>         fireSessionEvent(Session.SESSION_CREATED_EVENT, null);
>
>         // Notify interested application event listeners
>         Context context = (Context) manager.getContainer();
>         Object listeners[] = context.getApplicationListeners();
>         if (listeners != null) {
>             HttpSessionEvent event =
>                 new HttpSessionEvent(getSession());
>             for (int i = 0; i < listeners.length; i++) {
>                 if (!(listeners[i] instanceof HttpSessionListener))
>                     continue;
>                 HttpSessionListener listener =
>                     (HttpSessionListener) listeners[i];
>                 try {
>                     fireContainerEvent(context,
>                                        "beforeSessionCreated",
>                                        listener);
>                     listener.sessionCreated(event);
>                     fireContainerEvent(context,
>                                        "afterSessionCreated",
>                                        listener);
>                 } catch (Throwable t) {
>                     try {
>                         fireContainerEvent(context,
>                                            "afterSessionCreated",
>                                            listener);
>                     } catch (Exception e) {
>                         ;
>                     }
>                     // FIXME - should we do anything besides log these?
>                     log(sm.getString("standardSession.sessionEvent"), 
> t);
>                 }
>             }
>         }
>
>     }
>
> A second call to setId() will again fire a session creation event for
> the existing session.  Here's some trace from making a single request
> when calling setId() explicitly:
>
> 2003-02-03 17:46:43,113 [Ajp13Processor[17025][4]] INFO  default - 
> Adding
> session org.apache.catalina.session.StandardSessionFacade@4977e2 with 
> id of
> 2EA4840C7D4D6C7F3FF76F1F95C575D5
> 2003-02-03 17:46:43,175 [Ajp13Processor[17025][4]] INFO  default - 
> Adding
> session org.apache.catalina.session.StandardSessionFacade@4977e2 with 
> id of
> C6C908E127E6230CC81AE70E10D914A4
>
> My web application's list of active sessions (stored as a Map of
> StandardSessionFacade objects keyed by session ID) will look as
> follows:
>
> Session creation event fired as ManagerBase sets the initial ID by
> calling StandardSession.setId() during its createSession() method
>               |
>               |
>               v
> .______________________________________.
> | Key  | Value                         |
> `--------------------------------------'
> | ID A | Facade A --> Session A (ID A) |
> `--------------------------------------'
>               |
>               |
> Second session creation event fired as SessionIdValve resets the ID,
> calling setId() for a second time (this time explicitly).
>               |
>               |
>               v
> .______________________________________.
> | Key  | Value                         |
> `--------------------------------------'
> | ID A | Facade A --> Session A (ID A) |
> | ID B | Facade A --> Session A (ID B) |
> `--------------------------------------'
>
> The entry keyed by session ID A will not be removed from our list of
> active sessions when Session A expires, as the session now has an ID
> of B.
>
>
> USE CASE: When creating a new session, I must set the session
> identifier (or manipulate it after the initial generation from a
> Valve) using request-specific inputs.
>
> Specifically, I must re-use any session identifier supplied by the
> client via its JSESSIONID cookie if that session identifier is not
> already in use.  Because the Servlet API dictates that the session
> cookie is always named JSESSIONID, this is a must have for support of
> wildcard cookie domains across hosts which share a common base domain
> name.
>
> For instance, if I have host1.domain.com and host2.domain.com, and set
> the cookie domain of sessions cookie to the wildcard ".domain.com"
> (some pathetic browsers only support two part wildcard domains), the
> cookie will apply to both hosts.  This is problematic when a client
> which has established a session on host1 tries to establish a second
> session on host2 -- host2 will assign the client a new session ID,
> wiping the client's memory of its session ID for host1.  Now, if host1
> and host2 are gracious enough to use the same session ID, they can
> effectively share the JSESSIONID cookie, allowing the client to enjoy
> simultaneous sessions on both host1 and host2 while working within the
> Servlet API.
>
>
> PROBLEM: Tomcat's Manager interface has no API for accessing
> contextual information specific to a request.  Combining a Valve which
> provides this request-specific context via thread-local storage with
> my own custom manager is hacky at best, and would require that I
> completely re-implement the code in StandardSession (due to its
> package-private scoping).
>
>
> RANT: Catalina should not hide its session created/destroy event
> notification code in a package-private class.  It should probably live
> in a Manager -- rather than Session -- implementation, either in its
> createSession() and expire(Session) methods, or in its add(Session)
> and remove(Session) methods.
>
> -- 
>
> Daniel Rall <dl...@apache.org>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
>


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