[rfc][icedtea-web] Ignore invalid .jar files in applets [resubmit]
Adam Domurad
adomurad at redhat.com
Tue Jul 3 12:40:33 PDT 2012
On Tue, 2012-07-03 at 20:59 +0200, Jiri Vanek wrote:
> On 06/18/2012 04:42 PM, Adam Domurad wrote:
> > Hey all, re-submitting my patch, as Jiri requested.
> >
> > ChangeLog:
> > 2012-05-28 Adam Domurad<adomurad at redhat.com>
> >
> > Ignore invalid jar files in applets, like the oracle plugin does.
> > * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java:
> > (isInvalidJar): New, checks for ZipException in new JarFile(...)
> > (shouldFilterInvalidJars): New, checks if we are in an applet
> > (initializeResources): if 'shouldFilterInvalidJars()' is true and a jar
> > is not a valid jar file, the jar is filtered out and normal execution
> > continues.
> >
>
> Looks very eell. Just few question inline
> >
> >
> > On Mon, 2012-05-28 at 15:26 -0400, Adam Domurad wrote:
> >> > Hey all. Second try at a patch to ignore invalid .jar files, this time
> >> > only affecting applets (ie, not pages that use jnlp_href).
> >> >
> >> > The proprietary plug-in seems to just skip over any malformed .jar files
> >> > and carry on loading in, while in applets. This patch emulates that
> >> > behaviour. Pages with jnlp_href still crash with a ZipException on jar
> >> > verification (the proprietary plugin also fatally errors).
> >> >
> >> > This alleviates some of the symptoms of
> >> > http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1011
> >> > Once this patch has been accepted in some form, I hope to make it so
> >> > that the plugin will parse folders differently than jar files and look
> >> > for resources in folders like the proprietary plug-in does.
> >> >
> >> > Le ChangeLog:
> >> > 2012-05-28 Adam Domurad<adomurad at redhat.com>
> >> >
> >> > Ignore invalid jar files in applets, like the oracle plugin does.
> >> > * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java:
> >> > (isValidJar): New, checks for ZipException in new JarFile(...)
> >> > (shouldIgnoreInvalidJars): New, checks if we are in an applet
> >> > (initializeResources): if 'shouldIgnoreInvalidJars()' is true and a jar
> >> > is not a valid jar file, the jar is filtered out and normal execution
> >> > continues.
> >> >
> >
> >
> > patch-attempt-3.patch
> >
> >
> > diff --git a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> > --- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> > +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> > @@ -42,6 +42,7 @@ import java.util.Collections;
> > import java.util.Enumeration;
> > import java.util.HashMap;
> > import java.util.HashSet;
> > +import java.util.Iterator;
> > import java.util.LinkedList;
> > import java.util.List;
> > import java.util.Map;
> > @@ -431,6 +432,39 @@ public class JNLPClassLoader extends URL
> > }
> >
> > /**
> > + * Check if a described jar file is invalid
> > + * @param jar the jar to check
> > + * @return true if file exists AND is an invalid jar, false otherwise
> > + */
> > + private boolean isInvalidJar(JARDesc jar){
> > + File cacheFile = tracker.getCacheFile(jar.getLocation());
> > + if (cacheFile == null)
> > + return false;//File cannot be retrieved, do not claim it is an invalid jar
> > + boolean isInvalid = false;
> > + try {
> > + JarFile jarFile = new JarFile(cacheFile.getAbsolutePath());
> > + jarFile.close();
> > + } catch (IOException ioe){
> > + //Catch a ZipException or any other read failure
> > + isInvalid = true;
> > + }
> > + return isInvalid;
> > + }
> > +
> > + /**
> > + * Determine how invalid jars should be handled
> > + * @return whether to filter invalid jars, or error later on
> > + */
> > + private boolean shouldFilterInvalidJars(){
> > + if (file instanceof PluginBridge){
> > + PluginBridge pluginBridge = (PluginBridge)file;
> > + /*Ignore on applet, ie !useJNLPHref*/
> > + return !pluginBridge.useJNLPHref();
> > + }
> > + return false;//Error is default behaviour
> > + }
> > +
> > + /**
> > * Load all of the JARs used in this JNLP file into the
> > * ResourceTracker for downloading.
> > */
> > @@ -467,10 +501,26 @@ public class JNLPClassLoader extends URL
> > if (strict)
> > fillInPartJars(initialJars); // add in each initial part's lazy jars
> >
> > + waitForJars(initialJars); //download the jars first.
> > +
> > + //A ZipException will propagate later on if the jar is invalid and not checked here
> > + if (shouldFilterInvalidJars()){
> > + //We filter any invalid jars
> > + Iterator<JARDesc> iterator = initialJars.iterator();
> > + while (iterator.hasNext()){
> > + JARDesc jar = iterator.next();
> > + if (isInvalidJar(jar)) {
> > + //Remove this jar as an available jar
> > + iterator.remove();
> > + tracker.removeResource(jar.getLocation());
> > + available.remove(jar);
> > + }
> > + }
> > + }
> > +
> > if (JNLPRuntime.isVerifying()) {
> >
> > JarCertVerifier jcv;
> > - waitForJars(initialJars); //download the jars first.
>
> this removed line scares me a bit....
It was moved to the top. Confirmed OK with Omair.
> >
> > try {
> > jcv = verifyJars(initialJars);
> > @@ -533,6 +583,7 @@ public class JNLPClassLoader extends URL
> >
> > for (JARDesc jarDesc : file.getResources().getJARs()) {
> > try {
> > +
> > File cachedFile = tracker.getCacheFile(jarDesc.getLocation());
> >
> > if (cachedFile == null) {
> > @@ -570,6 +621,10 @@ public class JNLPClassLoader extends URL
> > jarLocationSecurityMap.put(jarDesc.getLocation(), jarSecurity);
> > } catch (MalformedURLException mfe) {
> > System.err.println(mfe.getMessage());
> > + } catch (IllegalArgumentException iae){
>
> Do you think it is possible to caught this exception on better place and rethrow our custom one, which will be then catches here? IllegalaRgument one is pretty common one.
>
> > + //Caused by ignored resource being removed due to not being valid
>
> this should be in debug mode or message should go from Message.properties.
>
> > + System.err.println("JAR " + jarDesc.getLocation() + " is not a valid jar file. Continuing.");
> > + continue;
This seems consistent with the follow message slightly at the
top:
if(cachedFile == null) {
System.err.println("JAR " + jarDesc.getLocation() + " not found.
Continuing.");
continue; // JAR not found. Keep going.
}
If the other message makes sense, this makes sense... Both or none imo
> > }
> > }
> > activateJars(initialJars);
>
I have fixed the locality of the exception. It is now clearly to handle
an exception in getting the resource, which was removed. Good catch.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch-attempt-4.patch
Type: text/x-patch
Size: 3775 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120703/ca952296/patch-attempt-4.patch
More information about the distro-pkg-dev
mailing list