Bug 55943 - Provide a way prevent looking at the System classloader before the webapp classloaders
Provide a way prevent looking at the System classloader before the webapp cl...
Product: Tomcat 7
Classification: Unclassified
Component: Catalina
PC Linux
: P2 enhancement (vote)
: ---
Assigned To: Tomcat Developers Mailing List
Depends on:
  Show dependency tree
Reported: 2014-01-01 10:02 UTC by chris.dow8
Modified: 2014-09-08 13:23 UTC (History)
1 user (show)


Note You need to log in before you can comment on or make changes to this bug.
Description chris.dow8 2014-01-01 10:02:46 UTC
I would like a way to prevent Tomcat from checking the System classloader before the webapp classloaders.  This would be useful when using the embedded Tomact API to produce isolated servlet containers.  

I ran in to some difficulty with this when I was adding Tomcat support to the SBT (Simple Build Tool) plugin: https://github.com/JamesEarlDouglas/xsbt-web-plugin

In particular I had an issue where SBT includes a version of the Scala standard library, on the system classpath, that has been run through Proguard.  This  was conflicting with web applications that were including their own version of the Scala standard library.  I was eventually able to work around this using this hack: https://github.com/JamesEarlDouglas/xsbt-web-plugin/commit/f8a9b149f0c7c87d7b6e8f862c493841d82ad90a However, it would be nice if there was a way to accomplish this that didn't involve such a hack.

I would be happy to submit a patch for this.  However, I would like some guidence on how the API should be changed to accomplish this.  Perhaps a new flag on the WebappLoader class?

Comment 1 Mark Thomas 2014-01-01 11:24:58 UTC
If this change were to be implemented then it should be implemented the same way as the delegate flag since that controls a similar behaviour. I'd lean towards to boolean flag that simply disabled the code block (0.2) that checked the system class loader.

Note that the system class loader is checked first to enforce the specification requirement that web applications must not be allowed to override Java SE platform classes. I'd hesitate before adding an option to disable this check because of the specification requirement.

I wonder if there isn't a better solution to this issue. The system class loader might not be the best class loader to use here. It is really the bootstrap class loader that is required but you can't get a reference to that in some JREs - including Oracle's. The class loader hierarchy for an Oracle JVM is system->ext->bootstrap so using the ext class loader would work in that case.

I'm thinking that rather than using the system class loader in this case the parent of the system class loader should be used (if it has one). That should both fix this issue and still enforce the specification requirement for not allowing the overriding of Java SE platformclasses.
Comment 2 chris.dow8 2014-01-03 16:28:50 UTC
Switching to use a different classloader does sound a lot easier. Should I use the parent of the system classloader? It sounds like we really want the  bootstrap loader. Couldn't I recursively get the parent loaders, until I run out, to get the bootstrap loader?
Comment 3 Mark Thomas 2014-01-09 14:20:04 UTC
Doing that won't get you the bootstrap class loader in an Oracle JRE - you'll get the ext loader (which is good enough for this use case).

For the sake of clarity, my suggestion is start at the system class loader, recursively get the parent and use the last non-null value you find.
Comment 4 Mark Thomas 2014-01-14 12:11:45 UTC

*** This bug has been marked as a duplicate of bug 55945 ***
Comment 5 Mark Thomas 2014-01-14 12:12:18 UTC
Sorry - wrong bug.
Comment 6 Mark Thomas 2014-01-17 15:37:25 UTC
I've fixed this in 8.0.x for 8.0.0 and in 7.0.x for 7.0.51 onwards.

As I was cleaning up the use of the system class loader I also refactored the handling of parent==null which fell back to the system class loader to make it a little (probably not noticeably) faster.
Comment 7 chris.dow8 2014-01-17 16:42:12 UTC
You beat me to it.  Thanks for all of your help.
Comment 8 romain.manni-bucau 2014-02-18 09:56:45 UTC

this totaally breaks tomcat and tomee usage in embeded mode (+ has a lot of side effect in normal mode).

1) the j2seClassLoader is not overridable in children classloader which is a pain since system was
2) system is no more used
3) it breaks compatibility

can you revert it and make it active just with a flag is asked?

Side note: for a minor this is an important change which should have been showed a bit more (7.1 maybe)
Comment 9 hifisoftware 2014-04-08 22:57:25 UTC
This change breaks previous behaviour. We have an app that sets some static variable values and then launches embedded tomcat. Embeded tomcat has a war file that inspects values of these static variables. This no longer seems to work.

We want to use a newer version of tomcat in order to close any security issues. Is there is any way to add a flag to be able to revert to the old behaviour?
Comment 10 Mark Thomas 2014-04-09 00:27:37 UTC
This configurable. See this thread for details:
Comment 11 hifisoftware 2014-04-10 00:34:32 UTC
Thank Mark for the link. I have trouble understanding how to configure to follow the old behaviour.

Are you suggesting to override the WebappClassLoader class?
Comment 12 hifisoftware 2014-04-10 16:19:12 UTC
I was able to figure out the fix. When I added the following line to context.xml file, class loader behaviour was restored:
<Loader delegate="true"/>

Comment 13 Konstantin Kolinko 2014-04-30 00:06:08 UTC
(In reply to Mark Thomas from comment #6)

The essential bit of r1559153 / r1559134 is the following change:

@@ -1186,9 +1200,9 @@ public class WebappClassLoader extends U
         // (0.2) Try loading the class with the system class loader, to prevent
         //       the webapp from overriding J2SE classes
         String resourceName = binaryNameToPath(name, false);
-        if (system.getResource(resourceName) != null) {
+        if (j2seClassLoader.getResource(resourceName) != null) {
             try {
-                clazz = system.loadClass(name);
+                clazz = j2seClassLoader.loadClass(name);

The old code used 'System' classloader - the JVM CLASSPATH.
The new code uses 'Bootstrap' classloader - the topmost non-null parent of System class loader - the one that provides Java SE core classes.

As such, class-loader-howto,html has to be corrected.
The classes lookup order in 7.0.50 and earlier is:

 *  Bootstrap classes of your JVM
 *  System class loader classes (described above)
 *  /WEB-INF/classes of your web application
 *  /WEB-INF/lib/*.jar of your web application
 *  Common class loader classes (described above)

For 8.0.0 and 7.0.52 and later it now is

 *  Bootstrap classes of your JVM
 *  /WEB-INF/classes of your web application
 *  /WEB-INF/lib/*.jar of your web application
 *  System class loader classes (described above)
 *  Common class loader classes (described above)

I am REOPENING this issue to apply this documentation fix. It is worth noting this in migration guide.

It may be worth to add that if one configures <Loader delegate="true"/>,
the above order becomes

 *  Bootstrap classes of your JVM
 *  System class loader classes (described above)
 *  Common class loader classes (described above)
 *  /WEB-INF/classes of your web application
 *  /WEB-INF/lib/*.jar of your web application

One use case when jar is added by Java to the system classloader is using -javaagent option.

A thread:
Of course, such jar files should not be in WEB-INF/lib directory.
Comment 14 Mark Thomas 2014-09-08 13:23:21 UTC
Docs updated, note added to the 7.0.x changelog and th 7.0.x upgrade guide modified to add a section on notable changes with this as the first.