DO NOT REPLY [Bug 22419] New: - ClasspathResourceLoader should be using the Thread's context ClassLoader

[bu...@apache.org]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=22419>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=22419

ClasspathResourceLoader should be using the Thread's context ClassLoader

           Summary: ClasspathResourceLoader should be using the Thread's
                    context ClassLoader
           Product: Velocity
           Version: 1.3.1
          Platform: Other
        OS/Version: Other
            Status: NEW
          Severity: Minor
          Priority: Other
         Component: Source
        AssignedTo: velocity-dev@jakarta.apache.org
        ReportedBy: headius@headius.com


I see the following problems with ClasspathResourceLoader:

1. It loads templates using the ClassLoader it was loaded in, which in many
cases will be the top-level or a high-level ClassLoader within an appserver
2. Individual applications within the server almost always are given their own
contextual ClassLoader. In Tomcat, for example, each webapp has its own loader.
3. Each webapp should be able to package templates in jars to be put in
WEB-INF/lib and have Velocity load them correctly.
4. Each webapp should not have to put velocity-xxx.jar in their WEB-INF/lib to
be able to use Velocity.
5. Therefore, Velocity should be able to load templates using the contextual
ClassLoader appropriate for the *caller*

ClasspathResourceLoader uses getClass().getClassLoader() to look for templates.
If Velocity is loaded at the system or server level, and a webapp hopes to use
this method to load its own jarred templates, it will fail as it did for me.

The fix is terribly simple, and I think it makes sense to apply it directly to
ClasspathResourceLoader, or at least provide a ThreadContextResourceLoader, as I
have done:

In ClasspathResourceLoader.getResourceStream(), change:

ClassLoader classLoader = this.getClass().getClassLoader();

to:

ClassLoader classLoader = Thread.currentThread().getContextClassLoader();

This will cause the resource loading to look first at the caller's level
(webapp) and if not found, move up the ClassLoader chain. I believe this is the
appropriate way to handle loading templates as resources. From the documentation
for getContextClassLoader():

...
     * Returns the context ClassLoader for this Thread. The context
     * ClassLoader is provided by the creator of the thread for use
     * by code running in this thread when loading classes and resources.
     * If not set, the default is the ClassLoader context of the parent
     * Thread. The context ClassLoader of the primordial thread is
     * typically set to the class loader used to load the application.
...

The resource should be loaded from the ClassLoader that the creator of the
calling thread intended...not from the classloader that loaded Velocity. Also
keep in mind this change would not break loading from the system classpath...it
would just include contextual ClassLoaders in the lookup process.

I will look into creating a patch if it would be useful, but it's a one-line fix.