bug in SimpleSessionStore/ServerSession id

[Mel Martinez <me...@yahoo.com>]

I posted on this earlier (last night), but the
tomcat-dev list is now so slow that I don't know if it
ever really made it to the list.

I'm encountering a bug now in SimpleSessionStore.  In
the inner class (gawd I hate inner classes :-))
SimpleSessionManager's getNewSession() method, a
NullPointerException is thrown when the method tries
to add the new session to the 'sessions' hashtable
because the newly created session ID is null.  This
happens because, as near as I can tell, it is never
set.  I'm not sure who is working on this code or why
it is not getting set, but it is very reproduceable -
every time I try to access any servlet it crashes! 
:-)

My app does not depend on sessions (we use a portable,
non-servlet API dependent session management system)
so it would be nice if this bit of code wasn't
crashing on me.

In my early post, if it ever made it, I proposed
altering the MessageBytes.toString() function to never
return a null (I think it is very bad form for a
non-null object to return a null value from it's
toString()) and instead simply return "null". 
However, I realize now that MessageBytes is a bit
special in that it has a type T_NULL and that this
could have larger impact if other code relies on this.
 Thus, I'd leave this for someone more knowledgeable
with how MessageBytes is used to make that change if
at all.

The only thing I can think of to do right now is to
modify the getNewSession() method to check the
returned String representation of the session's id to
see if it is null.  If so, use "null".  This fixes the
crash and shouldn't, I believe, cause any new problems
since the dang thing is null at this point anyway.

I'm going to go ahead and commit this change, if this
is bad, let me know or go ahead and change it back -
but if so, please fix the underlying problem.

Mel


__________________________________________________
Do You Yahoo!?
Get email at your own domain with Yahoo! Mail. 
http://personal.mail.yahoo.com/

Re: bug in SimpleSessionStore/ServerSession id

[cm...@yahoo.com]

Hi Mel,

I'm working on the SimpleSessionStore, it had problems with recycling the 
ServerSession objects and I tried to fix them while removing some
complexity and spaghetti code.

I'll check that - probably after this weekend I'll be done with the
session and threading changes. 

Regarding MessageBytes - it must be able to store null ( it's a holder for
a String - which can be null ). The session Id can also be null, before it
is set by the session id generator, and is also null after the session
expires. So the fix is not in not returning null, but in making sure the
code that uses the session id will access it when the session is in the
right state. 

Defining the state of a ServerSession object is not yet completed, and
it would certainly help to get some review - as well as for the other
core objects.   


Costin

On Fri, 23 Mar 2001, Mel Martinez wrote:

> I posted on this earlier (last night), but the
> tomcat-dev list is now so slow that I don't know if it
> ever really made it to the list.
> 
> I'm encountering a bug now in SimpleSessionStore.  In
> the inner class (gawd I hate inner classes :-))
> SimpleSessionManager's getNewSession() method, a
> NullPointerException is thrown when the method tries
> to add the new session to the 'sessions' hashtable
> because the newly created session ID is null.  This
> happens because, as near as I can tell, it is never
> set.  I'm not sure who is working on this code or why
> it is not getting set, but it is very reproduceable -
> every time I try to access any servlet it crashes! 
> :-)
> 
> My app does not depend on sessions (we use a portable,
> non-servlet API dependent session management system)
> so it would be nice if this bit of code wasn't
> crashing on me.
> 
> In my early post, if it ever made it, I proposed
> altering the MessageBytes.toString() function to never
> return a null (I think it is very bad form for a
> non-null object to return a null value from it's
> toString()) and instead simply return "null". 
> However, I realize now that MessageBytes is a bit
> special in that it has a type T_NULL and that this
> could have larger impact if other code relies on this.
>  Thus, I'd leave this for someone more knowledgeable
> with how MessageBytes is used to make that change if
> at all.
> 
> The only thing I can think of to do right now is to
> modify the getNewSession() method to check the
> returned String representation of the session's id to
> see if it is null.  If so, use "null".  This fixes the
> crash and shouldn't, I believe, cause any new problems
> since the dang thing is null at this point anyway.
> 
> I'm going to go ahead and commit this change, if this
> is bad, let me know or go ahead and change it back -
> but if so, please fix the underlying problem.
> 
> Mel
> 
> 
> __________________________________________________
> Do You Yahoo!?
> Get email at your own domain with Yahoo! Mail. 
> http://personal.mail.yahoo.com/
>