[RFC][IcedTea-Web]: Fix for PR464

[Andrew Su]

----- Original Message -----
> From: "Dr Andrew John Hughes" <ahughes at redhat.com>
> To: "Andrew Su" <asu at redhat.com>
> Cc: "distro-pkg-dev" <distro-pkg-dev at openjdk.java.net>
> Sent: Wednesday, June 8, 2011 10:04:16 AM
> Subject: Re: [RFC][IcedTea-Web]: Fix for PR464
> On 20:02 Tue 07 Jun , Andrew Su wrote:
> > Hello,
> >
> > This patch is to allow plugin to run sites that use jnlp_href to
> > specify parameters for applets through a jnlp file.
> >
> > Reported in PR464:
> > http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=464
> >
> > ChangeLog:
> >
> > 2011-06-07 Andrew Su <asu at redhat.com>
> >
> >         * NEWS: Updated.
> >         * netx/net/sourceforge/jnlp/JNLPFile.java:
> >         (JNLPFile): Calls new constructor.
> >         (JNLPFile): New constructor to take an option for forcing a
> >         codebase.
> >         (JNLPFile): Call parse with extra parameter.
> >         (parse): Use the given codebase passed in if we did not find
> >         one.
> >         * netx/net/sourceforge/jnlp/Parser.java:
> >         (Parser): Calls new constructor.
> >         (Parser): New constructor which takes in a codebase as a
> >         last option.
> >         * netx/net/sourceforge/jnlp/PluginBridge.java:
> >         (jars): Changed to use HashSet instead of String[].
> >         (PluginBridge): Calls new JNLPFile's constructor with
> >         current
> >         codebase, and adds any jars specified in the JNLP file.
> >         (getResources): Updated to work with HashSet of jars.
> >
> >
> > For the change in NEWS, wasn't sure which category to put it under..
> > (placed it under netx).
> > Ok for HEAD?
> >
> > Cheers,
> >   Andrew
> 
> NEWS change looks fine. What I don't understand is the motivation to
> do this String[] to HashSet change in this patch; I think it should be
> changed in a separate patch if really necessary.
> 
> This:
> 
> >
> > - for (int i = 0; i < jars.length; i++)
> > - if (jars[i].length() > 0)
> > - jarDescs.add(new JARDesc(new URL(codeBase, jars[i]),
> > + Iterator<String> it = jars.iterator();
> > + while (it.hasNext()) {
> > + String name = it.next();
> > + if (name.length() > 0)
> > + jarDescs.add(new JARDesc(new URL(codeBase, name),
> >                                          null, null, false, true,
> >                                          false, true));
> > + }
> >
> 
> is identical AFAICS, bar the change to JARDesc constructor call.

The reason for the change is the following snippet:

-                JNLPFile jnlpFile = new JNLPFile(jnlp);
+                JNLPFile jnlpFile = new JNLPFile(jnlp, null, false, JNLPRuntime.getDefaultUpdatePolicy(), this.codeBase);
                 Map<String, String> jnlpParams = jnlpFile.getApplet().getParameters();
 
                 // Change the parameter name to lowercase to follow conventions.
                 for (Map.Entry<String, String> entry : jnlpParams.entrySet()) {
-                    atts.put(entry.getKey().toLowerCase(), entry.getValue());
+                    this.atts.put(entry.getKey().toLowerCase(), entry.getValue());
                 }
+                JARDesc[] jarDescs = jnlpFile.getResources().getJARs();
+                for (JARDesc jarDesc : jarDescs) {
+                     String fileName = jarDesc.getLocation().toExternalForm();
+                     this.jars.add(fileName);
+                 }
             } catch (MalformedURLException e) {

Here we are adding the names to 'jars' as well. Which means that 'jars' if left as String[] will mean we will have to extend the size if there are any items in the snippet you posted. Also we would need to go through the items, to avoid having same items added more than once. (since the site dev, may choose to specify it in both the JNLP file and as a parameter.).

Cheers,
 Andrew