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

[Dr Andrew John Hughes]

On 10:17 Wed 08 Jun     , Andrew Su wrote:
> 
> 
> ----- 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

Ok, so let's do the HashSet change in one patch, and then apply the new support on top of it.
It makes the changes clearer.
-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37