[rfc][icedtea-web] jnlp_href extensions inside HTML applets - PR974
Andrew Azores
aazores at redhat.com
Thu Aug 15 07:42:45 PDT 2013
On 08/14/2013 09:28 AM, Adam Domurad wrote:
> On 08/01/2013 03:41 PM, Andrew Azores wrote:
>> On 07/30/2013 01:46 AM, Jiri Vanek wrote:
> [ snip ]
>
> Fix comments:
>
>> diff --git a/netx/net/sourceforge/jnlp/ParserSettings.java
>> b/netx/net/sourceforge/jnlp/ParserSettings.java
>> --- a/netx/net/sourceforge/jnlp/ParserSettings.java
>> +++ b/netx/net/sourceforge/jnlp/ParserSettings.java
>> @@ -37,6 +37,9 @@ exception statement from your version.
>>
>> package net.sourceforge.jnlp;
>>
>> +import java.util.ArrayList;
>> +import java.util.List;
>> +
>> /**
>> * Contains settings to be used by the Parser while parsing JNLP files.
>> *
>> @@ -44,9 +47,12 @@ package net.sourceforge.jnlp;
>> */
>> public class ParserSettings {
>>
>> + private static final String doubleArgs = "-basedir -jnlp -arg
>> -param -property -update";
>> +
>
> This seems clearer as an array. As well, consider using the all-caps
> and underscore convention for constants.
I've taken these out completely. They were a leftover from the command
line argument checks Boot.java does, but ParserSettings' needs are much
much simpler, at least at the moment.
>
>> private final boolean isStrict;
>> private final boolean extensionAllowed;
>> private final boolean malformedXmlAllowed;
>> + private static String[] cmdArgs;
>
> I prefer storing 'global parser settings' here. Setting some global
> parser settings is more obvious in intent than
> ParserSettings.setCommandLineArgs(...).
This does make more sense, you're right. No need to save the args when
they're just being used to make a single ParserSettings instance, which
is really just a struct with three booleans... d'oh. :)
>
>>
>> /** Create a new ParserSettings with the defautl parser settings */
>> public ParserSettings() {
>> @@ -75,4 +81,66 @@ public class ParserSettings {
>> return isStrict;
>> }
>>
>> -}
>> \ No newline at end of file
>> + public static void setCommandLineArgs(String[] args) {
>> + cmdArgs = args;
>> + }
>> +
>> + public static ParserSettings getCommandLineParserSettings() {
>
> Consider making this take a string parameter (combined with the
> recommended change above).
Well, it's now a String[] parameter, since that's what the current
single caller of this method has on-hand already.
>
>> + if (cmdArgs == null || cmdArgs.length == 0) {
>> + return new ParserSettings();
>> + }
>> +
>> + boolean strict = false;
>> + boolean malformedXmlAllowed = true;
>> +
>> + if (null != getOption("-strict")) {
>> + strict = true;
>> + }
>> +
>> + if (null != getOption("-xml")) {
>> + malformedXmlAllowed = false;
>> + }
>> +
>> + return new ParserSettings(strict, true, malformedXmlAllowed);
>> + }
>> +
>> + /**
>> + * Return value of the first occurence of the specified
>> + * option, or null if the option is not present. If the
>> + * option is a flag (0-parameter) and is present then the
>> + * option name is returned.
>> + */
>> + private static String getOption(String option) {
>> + String result[] = getOptions(option);
>> +
>> + if (result.length == 0)
>> + return null;
>> + else
>> + return result[0];
>> + }
>> +
>> + /**
>> + * Return all the values of the specified option, or an empty
>> + * array if the option is not present. If the option is a
>> + * flag (0-parameter) and is present then the option name is
>> + * returned once for each occurrence.
>> + */
>> + private static String[] getOptions(String option) {
>> + List<String> result = new ArrayList<String>();
>> +
>> + for (int i = 0; i < cmdArgs.length; i++) {
>> + if (option.equals(cmdArgs[i])) {
>> + if (-1 == doubleArgs.indexOf(option))
>> + result.add(option);
>> + else if (i + 1 < cmdArgs.length)
>> + result.add(cmdArgs[i + 1]);
>> + }
>> +
>> + if (cmdArgs[i].startsWith("-") && -1 !=
>> doubleArgs.indexOf(cmdArgs[i]))
>> + i++;
>> + }
>> +
>> + return result.toArray(new String[result.size()]);
>> + }
>> +
>> +}
>> diff --git a/netx/net/sourceforge/jnlp/PluginBridge.java
>> b/netx/net/sourceforge/jnlp/PluginBridge.java
>> --- a/netx/net/sourceforge/jnlp/PluginBridge.java
>> +++ b/netx/net/sourceforge/jnlp/PluginBridge.java
>> @@ -26,19 +26,19 @@ import java.io.ByteArrayInputStream;
>> import java.io.File;
>> import java.io.IOException;
>> import java.io.InputStream;
>> +import java.net.MalformedURLException;
>> import java.net.URL;
>> -import java.net.MalformedURLException;
>> +import java.util.ArrayList;
>> +import java.util.Arrays;
>> import java.util.HashSet;
>> +import java.util.List;
>> import java.util.Locale;
>> -import java.util.List;
>> -import java.util.ArrayList;
>> import java.util.Map;
>> import java.util.Set;
>>
>> +import net.sourceforge.jnlp.runtime.JNLPRuntime;
>> import sun.misc.BASE64Decoder;
>>
>> -import net.sourceforge.jnlp.runtime.JNLPRuntime;
>> -
>> /**
>> * Allows reuse of code that expects a JNLPFile object,
>> * while overriding behaviour specific to applets.
>> @@ -47,6 +47,7 @@ public class PluginBridge extends JNLPFi
>>
>> private PluginParameters params;
>> private Set<String> jars = new HashSet<String>();
>> + private List<ExtensionDesc> extensionJars = new
>> ArrayList<ExtensionDesc>();
>> //Folders can be added to the code-base through the archive tag
>> private List<String> codeBaseFolders = new ArrayList<String>();
>> private String[] cacheJars = new String[0];
>> @@ -90,6 +91,7 @@ public class PluginBridge extends JNLPFi
>> this.codeBase = codebase;
>> this.sourceLocation = documentBase;
>> this.params = params;
>> + this.parserSettings =
>> ParserSettings.getCommandLineParserSettings();
>
> I prefer to have some reference to 'global settings' versus 'command
> line settings' - eg some 'getGlobalParserSettings' method. I am not
> really fully convinced this should be anything except 'new
> ParserSettings()' as you had before.
I think it makes sense to make ParserSettings which respect the command
line flags available like this, even if maybe the PluginBridge doesn't
particularly need to care about what those settings are beyond not being
a null reference. Then, might as well give it the correct settings if
they're available, right?
> [ snip ]
> Looks good, though.
>
> Thanks,
> -Adam
Here's the Changelog again since I've also added some small unit tests.
Changelog:
* netx/net/sourceforge/jnlp/ParserSettings.java: (globalParserSettings)
static ParserSettings instance to store settings.
(getGlobalParserSettings) two new methods. Determine, store, and return
globalParserSettings
* netx/net/sourceforge/jnlp/PluginBridge.java: (extensionJars) stores
list of JNLP extensions. (getResources) returns this list
* netx/net/sourceforge/jnlp/runtime/Boot.java: minor refactor to use
ParserSettings.getGlobalParserSettings()
* tests/netx/unit/net/sourceforge/jnlp/ParserSettingsTest.java: ensure
that ParserSettings.getGlobalParserSettings() works as intended
*
tests/reproducers/custom/ExtensionJnlpsInApplet/testcases/ExtensionJnlpsInAppletTest.java:
tests browser launch of HTML file with embedded JNLP applet referencing
extension JNLP
*
tests/reproducers/custom/ExtensionJnlpsInApplet/resources/ExtensionJnlpHelper.jnlp:
same
*
tests/reproducers/custom/ExtensionJnlpsInApplet/resources/ExtensionJnlpTest.html:
same
*
tests/reproducers/custom/ExtensionJnlpsInApplet/resources/ExtensionJnlpTestApplet.jnlp:
same
*
tests/reproducers/custom/ExtensionJnlpsInApplet/srcs/ExtensionJnlpHelper.java:
same
*
tests/reproducers/custom/ExtensionJnlpsInApplet/srcs/ExtensionJnlpTestApplet.java:
same
* tests/reproducers/custom/ExtensionJnlpsInApplet/srcs/Makefile: same
Thanks,
--
Andrew A
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix.patch
Type: text/x-patch
Size: 5271 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130815/e647615a/fix.patch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: reproducer.patch
Type: text/x-patch
Size: 17133 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130815/e647615a/reproducer.patch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: unit.patch
Type: text/x-patch
Size: 3457 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130815/e647615a/unit.patch
More information about the distro-pkg-dev
mailing list