[icedtea-web] RFC: find jars based on Class-Path entry in manifest
Omair Majid
omajid at redhat.com
Thu Apr 14 16:09:08 PDT 2011
On 04/14/2011 06:38 PM, Dr Andrew John Hughes wrote:
> On 16:34 Thu 14 Apr , Omair Majid wrote:
>> > Hi,
>> >
>> > The attached patch adds support in icedtea-web for finding other jars
>> > using the Class-Path entry in a jar's manifest file.
>> >
>> > An applet the demonstrates the bug is located at
>> > http://jung.sourceforge.net/applet/showlayouts2.html
>> >
>> > The logic of the patch is quite similar to the already-implemented case
>> > for jar indices: when a class cannot be found, we will download jars
>> > specified in the Class-Path and search there too.
>> >
>> > ChangeLog:
>> > 2011-04-14 Omair Majid<omajid at redhat.com>
>> >
>> > * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java: Add new
>> > private variable classpathsInManifest.
>> > (activateJars): When adding jar index, also add Class-Path entries
>> > from the Manifest.mf file in the jar.
>> > (loadClass): Search for jars specified in classpaths before looking
>> > for entries in jar index.
>> > (addNewJar): New method refactored from loadClass.
>> >
>> > Any thoughts or comments?
>> >
>> > Cheers,
>> > Omair
> Inline.
Thanks for the comments!
>> > @@ -765,8 +771,28 @@
>> > // there is currently no mechanism to cache files per
>> > // instance.. so only index cached files
>> > if (localFile != null) {
>> > - JarIndex index = JarIndex.getJarIndex(new JarFile(localFile.getAbsolutePath()),
>> > - null);
>> > + JarFile jarFile = new JarFile(localFile.getAbsolutePath());
>> > + Manifest mf = jarFile.getManifest();
>> > + if (mf != null) {
>> > + String classpath = mf.getMainAttributes().getValue("Class-Path");
>> > + String[] paths = classpath.split(" +");
>> > + for (String path : paths) {
>> > + if (path.trim().length() == 0) {
>> > + continue;
>> > + }
>> > + // we want to search for jars in the same subdir on the server
>> > + // as this jar, so find out that subdirectory
>> > + String dir = jar.getLocation().getPath();
>> > + int lastSlash = dir.lastIndexOf("/");
>> > + if (lastSlash != -1) {
>> > + dir = dir.substring(0, lastSlash+1);
>> > + } else {
>> > + dir = "";
>> > + }
>> > + classpathsInManfest.add(dir + path);
>> > + }
>> > + }
>> > + JarIndex index = JarIndex.getJarIndex(jarFile, null);
> Classpaths are joined by '+'?
>
Each String in the Map is a classpath entry. The '+' is used to join the
jar's directory with the actual jar name. As an example, consider the
applet linked in the original email. The applet's jars are located at
/applet/jar/*. The 'Class-Path' entries contain just the names of the
jar - like foo.jar, bar.jar. The page (and hence the codebase) is
/applet/. If we just joined the name of the jar with the codebase, the
result is /applet/foo.jar, which is not the right path. This bit of code
above joins the original jar's path to the Class-Path jars to create
paths like /applet/jar/foo.jar and /applet/jar/bar.jar.
On second thought, perhaps I should have documented this a little more.
I thought my comment:
// we want to search for jars in the same subdir on the server
// as this jar, so find out that subdirectory
was sufficient, but it may not be.
>> > if (index != null)
>> > jarIndexes.add(index);
>> > }
>> > @@ -1007,8 +1033,32 @@
>> > try {
>> > result = loadClassExt(name);
>> > } catch (ClassNotFoundException cnfe) {
>> > + // Not found in external loader either
>> >
>> > - // Not found in external loader either. As a last resort, look in any available indexes
>> > + // Look in 'Class-Path' as specified in the manifest file
>> > + try {
>> > + for (String classpath: classpathsInManfest) {
>> > + JARDesc desc;
>> > + try {
>> > + URL jarUrl = new URL(file.getCodeBase(), classpath);
>> > + desc = new JARDesc(jarUrl, null, null, false, true, false, true);
>> > + } catch (MalformedURLException mfe) {
>> > + throw new ClassNotFoundException(name);
>> > + }
>> > + try {
>> > + addNewJar(desc);
>> > + } catch (Exception e) {
>> > + throw new ClassNotFoundException(name);
>> > + }
>> > + }
>> > +
>> > + result = loadClassExt(name);
>> > + return result;
>> > + } catch (ClassNotFoundException cnfe1) {
>> > + // continue below
>> > + }
>> > +
>> > + // As a last resort, look in any available indexes
> Exceptions are getting eaten. The CNFE should use the thrown exceptions as the initCause. I'm especially
> worried about just catching Exception; can we not be more specific?
>
I believe we want exceptions to be eaten here. This method, loadClass,
can only throw ClassNotFoundExceptions. It has to search in a few
locations and attempt a number of things before it realizes that a class
can not be found. So the cause that it may throw (and that user code may
see) is very likely to be meaningless anyway.
As for catching Exception, you do have a point. I had left it the way it
was originally (see the code below). I know that it can throw
IllegalArgumentException if the Resource is not being tracked (which
should not happen here). I suppose I can remove the catch block completely.
What I want to do, essentially, is to try loading a class from the newly
added jars, but map any Exceptions thrown to ClassNotFoundException,
which I ignore. This ensures that nothing here stops the
JNLPClassLoader. The JNLPClassLoader can then try other methods for
loading the class.
>> >
>> > // Currently this loads jars directly from the site. We cannot cache it because this
>> > // call is initiated from within the applet, which does not have disk read/write permissions
>> > @@ -1026,33 +1076,11 @@
>> > } catch (MalformedURLException mfe) {
>> > throw new ClassNotFoundException(name);
>> > }
>> > -
>> > - available.add(desc);
>> > -
>> > - tracker.addResource(desc.getLocation(),
>> > - desc.getVersion(),
>> > - null,
>> > - JNLPRuntime.getDefaultUpdatePolicy()
>> > - );
>> > -
>> > - URL remoteURL;
>> > try {
>> > - remoteURL = new URL(file.getCodeBase() + jarName);
>> > - } catch (MalformedURLException mfe) {
>> > - throw new ClassNotFoundException(name);
>> > - }
>> > -
>> > - URL u;
>> > -
>> > - try {
>> > - u = tracker.getCacheURL(remoteURL);
>> > + addNewJar(desc);
>> > } catch (Exception e) {
>> > throw new ClassNotFoundException(name);
>> > }
>> > -
>> > - if (u != null)
>> > - addURL(u);
>> > -
> What's happening here?
No functional changes. This bit of code here would have been duplicated
in the part that downloads jars from the Class-Path. To eliminate the
duplication, I moved it to the new method addNewJar and made both places
call that.
Thanks,
Omair
More information about the distro-pkg-dev
mailing list