Re: [Vote] Modify classloading in RequestUtils

[Robert Leland]

See comments enclosed

James Mitchell wrote:

Not sure if you caught the thread on the users list, or the bug recently
posted by Kjeld Froberg:

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


Here's the proposed code change:

RequestUtils.java
-----------------------------------------------
From:
   public static Class applicationClass(String className)
       throws ClassNotFoundException {

       // Look up the class loader to be used
       ClassLoader classLoader =
           Thread.currentThread().getContextClassLoader();
       if (classLoader == null) {
           classLoader = RequestUtils.class.getClassLoader();
       }

       // Attempt to load the specified class
       return (classLoader.loadClass(className));
   }
-----------------------------------------------
To:
   public static Class applicationClass(String className)
       throws ClassNotFoundException {
		ClassLoader ctxLoader = null;
		try {
			ctxLoader = Thread.currentThread().getContextClassLoader();
			return Class.forName(className, true, ctxLoader);

		} catch(ClassNotFoundException ex) {
			if(ctxLoader == null) { throw ex; }
		} catch(SecurityException ex) {

		}
		return Class.forName(className);
	}

By changing these few lines, I am able to run the full test suite without
error. (And that includes the new test.tomcat.33)

I'm not sure if this should be a [Vote] or a [commit and wait till they
complain], so I opted for the former.

I would be +0,--ONLY-- after some more digging.

Lets walk through this, here is my guess.
The patch would use the containers class loader first then the JVM's class
loader, the original code uses
the containers class loader then if that fails looks for the class in the struts Jar.

About April 2001, I recall the 'Class.forName' method being frowned upon,

I would suggest
1) search the archives, if you can't find the reference about Class.forName()
then I'll see if I can dig it up.
2) Take a look at the log of applicationClass() and determine why/when

RequestUtils.class.getClassLoader()

was added it could have been to work around a bug in another container.

-Rob

Even though I'm not truly sure of the consequences of this change, I'm +1.


James Mitchell

--
To unsubscribe, e-mail:   <mailto:struts-dev-unsubscribe@;jakarta.apache.org>
For additional commands, e-mail: <mailto:struts-dev-help@;jakarta.apache.org>