tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <ma...@apache.org>
Subject Re: svn commit: r892120 - /tomcat/tc6.0.x/trunk/STATUS.txt
Date Fri, 18 Dec 2009 09:26:29 GMT
On 18/12/2009 03:43, kkolinko@apache.org wrote:
> Author: kkolinko
> Date: Fri Dec 18 03:43:42 2009
> New Revision: 892120
> 
> URL: http://svn.apache.org/viewvc?rev=892120&view=rev
> Log:
> votes
> 
> Modified:
>     tomcat/tc6.0.x/trunk/STATUS.txt
> 
> Modified: tomcat/tc6.0.x/trunk/STATUS.txt
> URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=892120&r1=892119&r2=892120&view=diff
> ==============================================================================
> --- tomcat/tc6.0.x/trunk/STATUS.txt (original)
> +++ tomcat/tc6.0.x/trunk/STATUS.txt Fri Dec 18 03:43:42 2009
> @@ -362,7 +362,7 @@
>  
>  * Prevent lost log messages on shutdown
>    http://svn.apache.org/viewvc?rev=888072&view=rev
> -  +1: markt, jim
> +  +1: markt, jim, kkolinko
>    -1: 
>  
>  * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=47537
> @@ -405,6 +405,21 @@
>    http://svn.apache.org/viewvc?rev=890530&view=rev
>    +1: markt
>    -1: 
> +   0: kkolinko:
> +       Re approach:
> +         Request.isRequestedSessionIdValid() serves as implementation for
> +         HttpServletRequest.isRequestedSessionIdValid().
> +         I think that ClassLoader should already be set when calling this
> +         method.

I did a search of places where this is called from. Some set the tccl
set, some don't. I decided to go for the safe option and implement this
in Request.isRequestedSessionIdValid() to ensure the tccl was always
set, regardless of what called the method.

Yes, in some cases the tccl will be set unnecessarily. I thought about
testing if the tccl has already been set to the webapp classloader but
having looked at the implementation for
Thread.setsetContextClassLoader() I decided to go for the simpler approach.

> +         I mean, the problem is elsewhere. Supposedly in CoyoteAdapter.
> +         parseSessionCookiesId(), but may be earlier in the call chain.
> +
> +         I won't oppose the patch. I have to think a bit more about it.
> +
> +       Re implementation:
> +         if (session == null) return false; // return early and do not bother with thread
class loader

Yep. That makes sense. I'll add that now.

Mark



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


Mime
View raw message