Bug 56536

Summary: HttpSessionBindingListener.valueUnbound uses wrong classloader when SingleSignOn valve is used
Product: Tomcat 7 Reporter: Maarten van Hulsentop <maarten>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: 7.0.52   
Target Milestone: ---   
Hardware: PC   
OS: All   
Attachments: Reproduction war (including sources inside)
Patch where the restoring of the old classloader is deferred.

Description Maarten van Hulsentop 2014-05-16 13:27:30 UTC
Created attachment 31630 [details]
Reproduction war (including sources inside)

We are encountering an issue with the call to the valueUnbound listener of our application. We rely on the SingleSignOn valve (org.apache.catalina.authenticator.SingleSignOn) to invalidate all user sessions for all web applications when the user chooses to logout (session invalidate) on one webapp.

It seems that the valueUnboud is always called with the WebappClassLoader of the application where the original Session.invalidate was called. In the SingleSignOn scenario this is not always the webappclassloader.

I have added reproduction steps and .wars below.

It seems that the HttpSessionListener methods _are_ being called with the correct classloader from org.apache.catalina.session.StandardSession.expire(boolean). The expire method holds functionality to set the classloader to the webapp classloader, and restore it after calling. In the patch i have moved the classloader restore code down. This makes that also the valueUnbound calls are now done using the right webappclassloader. But i am not sure if this is valid as also a number of internal calls are being executed in the process. 
I will add the patch in the comments as i can only add a single attachment it seems.

= Reproduction =
I have created a very small demo project (code to be found in the war).

== Preparation ==
- Use a Tomcat 7 runtime.
- Make sure you can login with a user that gets role 'test' by editing <tomcat>/conf/tomcat-users.xml.
- Make sure SingleSignOn valve is enabled in server.xml
- Place SingleSignOut.war in <tomcat>/webapps/
- Make a copy of this <tomcat>/webapps/SingleSignOut.war to <tomcat>/webapps/SingleSignOut2.war
(now you have two web applications that expect a user with role test, and answer to a request on / and on /logout)

== Running the repro ==
- Go to http://localhost:8080/SingleSignOut/
- login: test/test
- Go to http://localhost:8080/SingleSignOut2/
- No login needed
- Go to http://localhost:8080/SingleSignOut2/logout
- See the following log on stdout:
<begin stdout snippet>
Calling session invalidate from /SingleSignOut2 using classloader WebappClassLoader
  context: /SingleSignOut2
  delegate: false
  repositories:
    /WEB-INF/classes/
----------> Parent Classloader:
org.apache.catalina.loader.StandardClassLoader@7a1f0683
/SingleSignOut VALUE UNBOUND using classloader WebappClassLoader
  context: /SingleSignOut2
  delegate: false
  repositories:
    /WEB-INF/classes/
----------> Parent Classloader:
org.apache.catalina.loader.StandardClassLoader@7a1f0683
/SingleSignOut2 VALUE UNBOUND using classloader WebappClassLoader
  context: /SingleSignOut2
  delegate: false
  repositories:
    /WEB-INF/classes/
----------> Parent Classloader:
org.apache.catalina.loader.StandardClassLoader@7a1f0683
</end stdout snippet>
- Observe that the value unboud for /SingleSignOut is being called with the classloader for /SingleSignOut2!
Comment 1 Maarten van Hulsentop 2014-05-16 13:29:29 UTC
Created attachment 31631 [details]
Patch where the restoring of the old classloader is deferred.
Comment 2 Mark Thomas 2014-05-16 18:06:59 UTC
Hmm. The problem is wider than that. There are a number of places where the application listener is called with the wrong class loader.

Simply extending the use of the webapp class loader will cause container listeners to be fired with the wrong listener. A more fine-grained approach is required. I'm taking a look now.
Comment 3 Mark Thomas 2014-05-16 18:20:06 UTC
Scratch that. It isn't that bad. Most of these methods are triggered by web app code so the class loader is already correct. I still think extending the coverage will have undesirable side-effects.
Comment 4 Mark Thomas 2014-05-16 18:32:48 UTC
This has been fixed in trunk for 8.0.8 onwards and in 7.0.x for 7.0.54 onwards.