[rfc][icedtea-web] Ignore invalid .jar files specified for plugin

[Adam Domurad]

[Replying to list this time]
Thanks for the informative reply & discussion !

If still relevant with your changes to JCV in the works, here's just the
refactoring attached.

On Fri, 2012-05-25 at 17:19 -0400, Danesh Dadachanji wrote:
> Hi Adam,
> 
> On 25/05/12 03:17 PM, Adam Domurad wrote:
> > So, first patch that isn't something trivial, I'll definitely need
> > people to weigh in on this one.
> >
> > The proprietary plug-in seems to just skip over any malformed .jar files
> > and carry on loading. This patch emulates that behaviour.
> > This alleviates some of the symptoms of
> > http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1011
> > However, the plugin should still parse folders differently and look for
> > resources in folders like the proprietary plug-in does.
> >
> 
> Just to summarize what we discussed on IRC, these changes do not belong in JCV. IMO, IcedTea-Web should never think of this extra dir 
> as a jar. Going through the stack trace, the verification call happens in JNLPClassLoader#initializeResources. The first line of that 
> method (eventually) points to where the change should be, in grabbing the resources from the HTML page. IMO these changes are more 
> suited for PluginBridge#getResources[1], right before the names passed into the archive attribute are turned into JARDescs. Keep in 
> mind  CodeBaseClassLoader for adding things that are actually in the specified dir. Thanks Omair for pointing this out!
> 
> Remember to watch out for the plugin being called using jnlp_href, these dirs should not be accepted then.
> 
> There are a few more comments below.
> 
> > ChangeLog:
> > 2012-05-25  Adam Domurad<adomurad at redhat.com>
> >
> 
> It went from one space to no spaces! :O 2
> 
> > 	Ignore invalid jar files, like the oracle plugin does.
> > 	* netx/net/sourceforge/jnlp/tools/JarCertVerifier.java: Added extra
> > 	possible verification result, INVALID_JAR. This value occurs if the
> > 	.jar file could not be properly loaded. The .jar file is then ignored.
> >
> >
> >
> > ignore-invalid-jars.patch
> >
> >
> > diff --git a/netx/net/sourceforge/jnlp/tools/JarCertVerifier.java b/netx/net/sourceforge/jnlp/tools/JarCertVerifier.java
> > --- a/netx/net/sourceforge/jnlp/tools/JarCertVerifier.java
> 
> 
> > +++ b/netx/net/sourceforge/jnlp/tools/JarCertVerifier.java
> > @@ -183,9 +185,7 @@ public class JarCertVerifier implements
> >           verifiedJars = new ArrayList<String>();
> >           unverifiedJars = new ArrayList<String>();
> >
> > -        for (int i = 0; i<  jars.size(); i++) {
> > -
> > -            JARDesc jar = jars.get(i);
> > +        for (JARDesc jar : jars) {
> >
> >               try {
> >
> 
> In general, I would prefer things like refactoring that aren't directly related to your changes to be in separate patches. Especially 
> for such a sensitive class like JCV. =) Please feel free to pull these chunks out into a different changeset though!
> 
> Cheers,
> Danesh
> 
> 
> [1] http://icedtea.classpath.org/hg/icedtea-web/file/6df151bb5320/netx/net/sourceforge/jnlp/PluginBridge.java#l209


-------------- next part --------------
A non-text attachment was scrubbed...
Name: refactoring-only.patch
Type: text/x-patch
Size: 2957 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120528/caba4c9b/refactoring-only.patch