2009/12/18 Mark Thomas <ma...@apache.org>: > On 18/12/2009 03:43, kkoli...@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 >>
>> * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=47774 >> Ensure web application class loader is used when calling session listeners >> 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 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. > > 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 would say that it is not so easy to review all places where isValid() is called. I see two more approaches available here: A). Place fix in StandardSession.expire(boolean) That is, wrap the call to listener.sessionDestroyed(event); with setting and then clearing the TCCL. The fireContainerEvent() calls above and below that line probably do not need webapp's classLoader, though I am not sure. The Context reference is available there, so Loader can be obtained. B). Implement a wrapper for HttpSessionListener that sets TCCL before calling the wrapped listener. Add wrappers when the listeners are added to the webapp. The if (!(listeners[j] instanceof HttpSessionListener)) check in StandardSession.notify() code means that it has to be a wrapper that specifically implements the HttpSessionListener interface. Any thoughts? Best regards, Konstantin Kolinko --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
