[RFC][icedtea-web] Plugin doesn't halt when required elements from JNLP files are missing.
Pavel Tisnovsky
ptisnovs at redhat.com
Mon Mar 12 03:50:29 PDT 2012
Jiri Vanek wrote:
> On 03/09/2012 09:32 PM, Danesh Dadachanji wrote:
>> On 11/01/12 02:18 PM, Danesh Dadachanji wrote:
>>> On 11/01/12 11:31 AM, Jiri Vanek wrote:
>>>> On 01/06/2012 08:08 PM, Danesh Dadachanji wrote:
>>>>> On 06/01/12 12:37 PM, Jiri Vanek wrote:
>>>>>> On 01/06/2012 05:44 PM, Danesh Dadachanji wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Looking at the plugin docs[1], <title> and <vendor> are required
>>>>>>> elements of <information>, without them the plugin should stop
>>>>>>> with a
>>>>>>> fatal error. Currently, icedtea-web just returns null from their
>>>>>>> respective getters and handles null accordingly. However it
>>>>>>> should not
>>>>>>> even reach that far. Here's a simple patch that throws a
>>>>>>> ParseException when not found.
>>>>>>>
>>>>>>> ChangeLog
>>>>>>> +2012-01-06 Danesh Dadachanji <ddadacha at redhat.com>
>>>>>>> +
>>>>>>> + Applications using JNLP files without a title or vendor section
>>>>>>> + still run, despite them being required elements.
>>>>>>> + * netx/net/sourceforge/jnlp/Parser.java:
>>>>>>> + (getInformationDesc): If title or vendor are not found in info,
>>>>>>> + a new ParseException is thrown.
>>>>>>> + * netx/net/sourceforge/jnlp/resources/Messages.properties: Add
>>>>>>> + PNoTitleElement and PNoVendorElement
>>>>>>> +
>>>>>>>
>>>>>>> Any comments? Is this okay for HEAD?
> In title of this email you write "Plugin" but it seems to me this is fix
> IN java web start with impact to plugin.
> If it is also for applets, can you add javaws someAppelt.jnlp tests?
> (as AppeltTest is dooing now) and test the same exceptions as in case of
> javaws some-application.jnlp? You can use already translated jar-ed
> applet and just call jnlp fiels upon him (same as I'm suggesting for
> your current javaws tests-set)
> Soon there will be intorudcued in-browser applet testing, do you think
> to remind me to add html tests for this case later? (Or you can do
> this, if you will want to check the in-browser "engine" ;)
>
> Few more comment in-line:
>>>>>>>
>>>>>>> Regards,
>>>>>>> Danesh
>>>>>>>
>>>>>>> [1]
>>>>>>> http://docs.oracle.com/javase/7/docs/technotes/guides/javaws/developersguide/syntax.html
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> I do not like the idea of forcing somebody to have elements, when
>>>>>> they
>>>>>> can be still empty. But if documentation say so.... How proprietary
>>>>>> plugin is dealing with it?
>>>>>
>>>>> It throws an exception, stopping further execution - the same as what
>>>>> this patch does for icedtea-web. =)
>>>>>
>>>>> If you'd like to test it, remove a title/vendor tag from a JNLP file
>>>>> and run it. You may need to with the java console being set to "Show
>>>>> console" - especially if it's an applet.
>>>>>
>>>>> Regards,
>>>>> Danesh
>>>>
>>>> Jsut from couriosity - how does proprietary plugin is dealing when
>>>> vendor or title is empty?
>>>
>>> Nice catch, I did not think about this. The proprietary plugin behaves
>>> the same way if either are empty - it throws a fatal exception. I've
>>> attached an updated patch to take this into consideration. Note that if
>>> the tags are only white spaces, this is also considered empty but the
>>> parser handles that case already. (e.g. <title> </title>)
>>>
>>>> I still don't like the idea of exiting when element is not present, but
>>>> continue when element is empty :(
>>>>
>>>
>>> IMO, it helps end-users of applets/webstarts. It forces devs to add more
>>> information to their jnlp files and therefore gives end-users more
>>> detailed warning dialogs when the app is asking for more access. It also
>>> acts as a reminder for any dev that's forgotten or does not know about
>>> this attribute. Obviously this can be used to spoof end-users but that's
>>> what my other patch is for =)
>>>
>>>> But specification is clear - they are required and existence of content
>>>> is not mentioned.
>>>>
>>>> In that case I think it is ok for head. But please wait until branching
>>>> is done.
>>>>
>>>
>>> Once the updated patch is good with you, I'll push to HEAD after
>>> branching is finished.
>>>
>>>> Thanx for reading the specification :)
>>>>
>>>
>>> Thanks for the comments! They are much appreciated. =)
>>
>> Realized a bunch of our tests do not include <vendor> so the
>> attachment is a new patch adding these.
>>
>> I've also added a regression test that has a JNLP without <title> and
>> another without <vendor>. The testcase checks for the exception
>> message thrown. Right now, the tests will not pass without the launch
>> error patch that's in review[1] but I figured I'd post to get some
>> comments.
>>
>> Any thoughts? Can I push to HEAD once [1] is in there?
>>
>> ChangeLog:
>> +2012-03-09 Danesh Dadachanji <ddadacha at redhat.com>
>> +
>> + Applications using JNLP files without a title or vendor section
>> + still run, despite them being required elements.
>> + * netx/net/sourceforge/jnlp/Parser.java:
>> + (getInformationDesc): If title or vendor are not found in info,
>> + a new ParseException is thrown.
>> + * netx/net/sourceforge/jnlp/resources/Messages.properties:
>> + Added PNoTitleElement and PNoVendorElement
>> + *
>> tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer1.jnlp,
>> + *
>> tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer1_1.jnlp,
>>
>> + *
>> tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer2.jnlp,
>> + *
>> tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer2_1.jnlp,
>>
>> + * tests/jnlp_tests/signed/MissingJar/resources/MissingJar.jnlp,
>> + * tests/jnlp_tests/signed/MissingJar/resources/MissingJar2.jnlp,
>> + * tests/jnlp_tests/signed/MissingJar/resources/MissingJar3.jnlp,
>> + * tests/jnlp_tests/signed/MissingJar/resources/MissingJar4.jnlp,
>> + *
>> tests/jnlp_tests/signed/ReadPropertiesBySignedHack/resources/ReadPropertiesBySignedHack.jnlp,
>>
>> + *
>> tests/jnlp_tests/signed/ReadPropertiesSigned/resources/ReadPropertiesSigned1.jnlp,
>>
>> + *
>> tests/jnlp_tests/signed/ReadPropertiesSigned/resources/ReadPropertiesSigned2.jnlp,
>>
>> + *
>> tests/jnlp_tests/simple/AccessClassInPackage/resources/AccessClassInPackage.jnlp,
>>
>> + *
>> tests/jnlp_tests/simple/AddShutdownHook/resources/AddShutdownHook.jnlp,
>> + *
>> tests/jnlp_tests/simple/AllStackTraces/resources/AllStackTraces.jnlp
>> + *
>> tests/jnlp_tests/simple/CreateClassLoader/resources/CreateClassLoader.jnlp,
>>
>> + *
>> tests/jnlp_tests/simple/ReadEnvironment/resources/ReadEnvironment.jnlp,
>> + *
>> tests/jnlp_tests/simple/ReadProperties/resources/ReadProperties1.jnlp,
>> + *
>> tests/jnlp_tests/simple/ReadProperties/resources/ReadProperties2.jnlp,
>> + *
>> tests/jnlp_tests/simple/RedirectStreams/resources/RedirectStreams.jnlp,
>> + *
>> tests/jnlp_tests/simple/ReplaceSecurityManager/resources/ReplaceSecurityManager.jnlp,
>>
>> + *
>> tests/jnlp_tests/simple/SetContextClassLoader/resources/SetContextClassLoader.jnlp,
>>
>> + * tests/netx/unit/net/sourceforge/jnlp/templates/template8.jnlp:
>> + Added missing title/vendor tags that make them fail with this
>> changeset.
>> + *
>> tests/jnlp_tests/simple/TitleVendorParser/resources/TitleParser.jnlp,
>> + *
>> tests/jnlp_tests/simple/TitleVendorParser/resources/VendorParser.jnlp,
>> + *
>> tests/jnlp_tests/simple/TitleVendorParser/srcs/TitleVendorParser.java,
>> + *
>> tests/jnlp_tests/simple/TitleVendorParser/testcases/TitleVendorParserTest.java:
>>
>> + New test that runs two JNLPs, one without the title tag and the
>> other
>> + without the vendor tag, checking that the correct eception is
>> thrown.
>> +
>>
>>
>> Cheers,
>> Danesh
>>
>> [1]
>> http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2012-March/017620.html
>>
>>
>>
>> title-vendor-required-03.patch
>>
>>
>> diff --git a/netx/net/sourceforge/jnlp/Parser.java
>> b/netx/net/sourceforge/jnlp/Parser.java
>> --- a/netx/net/sourceforge/jnlp/Parser.java
>> +++ b/netx/net/sourceforge/jnlp/Parser.java
>> @@ -504,6 +504,11 @@ class Parser {
>> child = child.getNextSibling();
>> }
>>
>> + if (info.getTitle() == null || info.getTitle().equals(""))
>> + throw new ParseException(R("PNoTitleElement"));
>> + if (info.getVendor() == null || info.getVendor().equals(""))
>> + throw new ParseException(R("PNoVendorElement"));
>> +
>> return info;
>> }
>>
> I would suggest "".equals(info.get*().trim()) instead of
> info.get*().equals("")
> Usage of trim i straight forward and I think it is necessary.
> The "".equals instead of string.equals("") really just minor nitpick
> :) "".equals(null) returns false instead of NUllPointerException. I do
> not believe trim() will ever return null, but it is good habit (IMHO O:) )
> If no need for trim will be here, then just "".equals(info.get*()) will
> be enough instead of both conditions.
String.trim() does not return null so you don't need to worry
Then you could use predicate isEmpty() which is IMHO more readable than equals("") (since 1.6)
>
>> diff --git a/netx/net/sourceforge/jnlp/resources/Messages.properties
>> b/netx/net/sourceforge/jnlp/resources/Messages.properties
>> --- a/netx/net/sourceforge/jnlp/resources/Messages.properties
>> +++ b/netx/net/sourceforge/jnlp/resources/Messages.properties
>> @@ -105,6 +105,8 @@ PInnerJ2SE=j2se element cannot be specif
>> PTwoMains=Duplicate main JAR defined in a resources element (there
>> can be only one)
>> PNativeHasMain=Cannot specify main attribute on native JARs.
>> PNoInfoElement=No information section defined
>> +PNoTitleElement=The title section has not been defined in the JNLP file.
>> +PNoVendorElement=The vendor section has not been defined in the JNLP
>> file.
>> PTwoDescriptions=Duplicate description of kind {0}
>> PSharing=Element "sharing-allowed" is illegal in a standard JNLP file
>> PTwoSecurity=Only one security element allowed per JNLPFile.
>> diff --git
>> a/tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer1.jnlp
>> b/tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer1.jnlp
>> ---
>> a/tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer1.jnlp
>> +++
>> b/tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer1.jnlp
>> @@ -4,6 +4,7 @@
>> href="CacheReproducer1.jnlp">
>> <information>
>> <title>Just prints out "Good simple javaws exapmle" using
>> reflection call from CacheReproducer.jar SimpletestSigned1.jar</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="CacheReproducer.jar" main="true"/>
>> diff --git
>> a/tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer1_1.jnlp
>> b/tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer1_1.jnlp
>>
>> ---
>> a/tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer1_1.jnlp
>>
>> +++
>> b/tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer1_1.jnlp
>>
>> @@ -4,6 +4,7 @@
>> href="CacheReproducer1_1.jnlp">
>> <information>
>> <title>Just prints out "Good simple javaws exapmle" using
>> reflection call from CacheReproducer.jar SimpletestSigned1.jar</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="CacheReproducer.jar" main="true"/>
>> diff --git
>> a/tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer2.jnlp
>> b/tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer2.jnlp
>> ---
>> a/tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer2.jnlp
>> +++
>> b/tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer2.jnlp
>> @@ -4,6 +4,7 @@
>> href="CacheReproducer2.jnlp">
>> <information>
>> <title>Just prints out "Good simple javaws exapmle" using
>> reflection call from CacheReproducer.jar SimpletestSigned1.jar</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="SimpletestSigned1.jar" main="false"
>> download="eager"/>
>> diff --git
>> a/tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer2_1.jnlp
>> b/tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer2_1.jnlp
>>
>> ---
>> a/tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer2_1.jnlp
>>
>> +++
>> b/tests/jnlp_tests/signed/CacheReproducer/resources/CacheReproducer2_1.jnlp
>>
>> @@ -3,7 +3,8 @@
>> codebase="./"
>> href="CacheReproducer2_1.jnlp">
>> <information>
>> -<title><title>Just prints out "Good simple javaws exapmle" using
>> reflection call from CacheReproducer.jar
>> SimpletestSigned1.jar</title></title>
>> +<title>Just prints out "Good simple javaws exapmle" using reflection
>> call from CacheReproducer.jar SimpletestSigned1.jar</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="SimpletestSigned1.jar" main="false"
>> download="lazy"/>
>> diff --git
>> a/tests/jnlp_tests/signed/MissingJar/resources/MissingJar.jnlp
>> b/tests/jnlp_tests/signed/MissingJar/resources/MissingJar.jnlp
>> --- a/tests/jnlp_tests/signed/MissingJar/resources/MissingJar.jnlp
>> +++ b/tests/jnlp_tests/signed/MissingJar/resources/MissingJar.jnlp
>> @@ -4,6 +4,7 @@
>> href="MissingJar.jnlp">
>> <information>
>> <title>test MissingJar</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <!-- MissingJar is name of reproducer and of main jar,
>> diff --git
>> a/tests/jnlp_tests/signed/MissingJar/resources/MissingJar2.jnlp
>> b/tests/jnlp_tests/signed/MissingJar/resources/MissingJar2.jnlp
>> --- a/tests/jnlp_tests/signed/MissingJar/resources/MissingJar2.jnlp
>> +++ b/tests/jnlp_tests/signed/MissingJar/resources/MissingJar2.jnlp
>> @@ -4,6 +4,7 @@
>> href="MissingJar2.jnlp">
>> <information>
>> <title>test MissingJar</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="ThisOneIsMissing.jar" />
>> diff --git
>> a/tests/jnlp_tests/signed/MissingJar/resources/MissingJar3.jnlp
>> b/tests/jnlp_tests/signed/MissingJar/resources/MissingJar3.jnlp
>> --- a/tests/jnlp_tests/signed/MissingJar/resources/MissingJar3.jnlp
>> +++ b/tests/jnlp_tests/signed/MissingJar/resources/MissingJar3.jnlp
>> @@ -4,6 +4,7 @@
>> href="MissingJar3.jnlp">
>> <information>
>> <title>test MissingJar</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <!-- MissingJar is name of reproducer and of main jar,
>> diff --git
>> a/tests/jnlp_tests/signed/MissingJar/resources/MissingJar4.jnlp
>> b/tests/jnlp_tests/signed/MissingJar/resources/MissingJar4.jnlp
>> --- a/tests/jnlp_tests/signed/MissingJar/resources/MissingJar4.jnlp
>> +++ b/tests/jnlp_tests/signed/MissingJar/resources/MissingJar4.jnlp
>> @@ -4,6 +4,7 @@
>> href="MissingJar4.jnlp">
>> <information>
>> <title>test MissingJar</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="ThisOneIsMissing.jar" />
>> diff --git
>> a/tests/jnlp_tests/signed/ReadPropertiesBySignedHack/resources/ReadPropertiesBySignedHack.jnlp
>> b/tests/jnlp_tests/signed/ReadPropertiesBySignedHack/resources/ReadPropertiesBySignedHack.jnlp
>>
>> ---
>> a/tests/jnlp_tests/signed/ReadPropertiesBySignedHack/resources/ReadPropertiesBySignedHack.jnlp
>>
>> +++
>> b/tests/jnlp_tests/signed/ReadPropertiesBySignedHack/resources/ReadPropertiesBySignedHack.jnlp
>>
>> @@ -4,6 +4,7 @@
>> href="ReadPropertiesBySignedHack.jnlp">
>> <information>
>> <title>read properties using System.getenv()</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="ReadPropertiesBySignedHack.jar" main="true"/>
>> diff --git
>> a/tests/jnlp_tests/signed/ReadPropertiesSigned/resources/ReadPropertiesSigned1.jnlp
>> b/tests/jnlp_tests/signed/ReadPropertiesSigned/resources/ReadPropertiesSigned1.jnlp
>>
>> ---
>> a/tests/jnlp_tests/signed/ReadPropertiesSigned/resources/ReadPropertiesSigned1.jnlp
>>
>> +++
>> b/tests/jnlp_tests/signed/ReadPropertiesSigned/resources/ReadPropertiesSigned1.jnlp
>>
>> @@ -4,6 +4,7 @@
>> href="ReadPropertiesSigned1.jnlp">
>> <information>
>> <title>read properties using System.getenv()</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="ReadPropertiesSigned.jar" main="true"/>
>> diff --git
>> a/tests/jnlp_tests/signed/ReadPropertiesSigned/resources/ReadPropertiesSigned2.jnlp
>> b/tests/jnlp_tests/signed/ReadPropertiesSigned/resources/ReadPropertiesSigned2.jnlp
>>
>> ---
>> a/tests/jnlp_tests/signed/ReadPropertiesSigned/resources/ReadPropertiesSigned2.jnlp
>>
>> +++
>> b/tests/jnlp_tests/signed/ReadPropertiesSigned/resources/ReadPropertiesSigned2.jnlp
>>
>> @@ -4,6 +4,7 @@
>> href="ReadPropertiesSigned2.jnlp">
>> <information>
>> <title>read properties using System.getenv()</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="ReadPropertiesSigned.jar" main="true"/>
>> diff --git
>> a/tests/jnlp_tests/simple/AccessClassInPackage/resources/AccessClassInPackage.jnlp
>> b/tests/jnlp_tests/simple/AccessClassInPackage/resources/AccessClassInPackage.jnlp
>>
>> ---
>> a/tests/jnlp_tests/simple/AccessClassInPackage/resources/AccessClassInPackage.jnlp
>>
>> +++
>> b/tests/jnlp_tests/simple/AccessClassInPackage/resources/AccessClassInPackage.jnlp
>>
>> @@ -4,6 +4,7 @@
>> href="AccessClassInPackage.jnlp">
>> <information>
>> <title>Test accessClassInPackage</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="AccessClassInPackage.jar" main="true"/>
>> diff --git
>> a/tests/jnlp_tests/simple/AddShutdownHook/resources/AddShutdownHook.jnlp
>> b/tests/jnlp_tests/simple/AddShutdownHook/resources/AddShutdownHook.jnlp
>> ---
>> a/tests/jnlp_tests/simple/AddShutdownHook/resources/AddShutdownHook.jnlp
>> +++
>> b/tests/jnlp_tests/simple/AddShutdownHook/resources/AddShutdownHook.jnlp
>> @@ -4,6 +4,7 @@
>> href="AddShutdownHook.jnlp">
>> <information>
>> <title>test adding shutdown hooks</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="AddShutdownHook.jar" main="true" download="eager"/>
>> diff --git
>> a/tests/jnlp_tests/simple/AllStackTraces/resources/AllStackTraces.jnlp
>> b/tests/jnlp_tests/simple/AllStackTraces/resources/AllStackTraces.jnlp
>> ---
>> a/tests/jnlp_tests/simple/AllStackTraces/resources/AllStackTraces.jnlp
>> +++
>> b/tests/jnlp_tests/simple/AllStackTraces/resources/AllStackTraces.jnlp
>> @@ -4,6 +4,7 @@
>> href="AllStackTraces.jnlp">
>> <information>
>> <title>Test Thread.getAllStackTraces</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="AllStackTraces.jar" main="true" download="eager"/>
>> diff --git
>> a/tests/jnlp_tests/simple/CreateClassLoader/resources/CreateClassLoader.jnlp
>> b/tests/jnlp_tests/simple/CreateClassLoader/resources/CreateClassLoader.jnlp
>>
>> ---
>> a/tests/jnlp_tests/simple/CreateClassLoader/resources/CreateClassLoader.jnlp
>>
>> +++
>> b/tests/jnlp_tests/simple/CreateClassLoader/resources/CreateClassLoader.jnlp
>>
>> @@ -4,6 +4,7 @@
>> href="CreateClassLoader.jnlp">
>> <information>
>> <title>set context classloader</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="CreateClassLoader.jar" main="true"
>> download="eager"/>
>> diff --git
>> a/tests/jnlp_tests/simple/ReadEnvironment/resources/ReadEnvironment.jnlp
>> b/tests/jnlp_tests/simple/ReadEnvironment/resources/ReadEnvironment.jnlp
>> ---
>> a/tests/jnlp_tests/simple/ReadEnvironment/resources/ReadEnvironment.jnlp
>> +++
>> b/tests/jnlp_tests/simple/ReadEnvironment/resources/ReadEnvironment.jnlp
>> @@ -4,6 +4,7 @@
>> href="ReadEnvironment.jnlp">
>> <information>
>> <title>ReadEnvironment using System.getenv()</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="ReadEnvironment.jar" main="true" />
>> diff --git
>> a/tests/jnlp_tests/simple/ReadProperties/resources/ReadProperties1.jnlp
>> b/tests/jnlp_tests/simple/ReadProperties/resources/ReadProperties1.jnlp
>> ---
>> a/tests/jnlp_tests/simple/ReadProperties/resources/ReadProperties1.jnlp
>> +++
>> b/tests/jnlp_tests/simple/ReadProperties/resources/ReadProperties1.jnlp
>> @@ -4,6 +4,7 @@
>> href="ReadProperties1.jnlp">
>> <information>
>> <title>read properties using System.getenv()</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="ReadProperties.jar" main="true"/>
>> diff --git
>> a/tests/jnlp_tests/simple/ReadProperties/resources/ReadProperties2.jnlp
>> b/tests/jnlp_tests/simple/ReadProperties/resources/ReadProperties2.jnlp
>> ---
>> a/tests/jnlp_tests/simple/ReadProperties/resources/ReadProperties2.jnlp
>> +++
>> b/tests/jnlp_tests/simple/ReadProperties/resources/ReadProperties2.jnlp
>> @@ -4,6 +4,7 @@
>> href="ReadProperties2.jnlp">
>> <information>
>> <title>read properties using System.getenv()</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="ReadProperties.jar" main="true"/>
>> diff --git
>> a/tests/jnlp_tests/simple/RedirectStreams/resources/RedirectStreams.jnlp
>> b/tests/jnlp_tests/simple/RedirectStreams/resources/RedirectStreams.jnlp
>> ---
>> a/tests/jnlp_tests/simple/RedirectStreams/resources/RedirectStreams.jnlp
>> +++
>> b/tests/jnlp_tests/simple/RedirectStreams/resources/RedirectStreams.jnlp
>> @@ -4,6 +4,7 @@
>> href="RedirectStreams.jnlp">
>> <information>
>> <title>redirect stdin/stdout streams</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="RedirectStreams.jar" main="true"/>
>> diff --git
>> a/tests/jnlp_tests/simple/ReplaceSecurityManager/resources/ReplaceSecurityManager.jnlp
>> b/tests/jnlp_tests/simple/ReplaceSecurityManager/resources/ReplaceSecurityManager.jnlp
>>
>> ---
>> a/tests/jnlp_tests/simple/ReplaceSecurityManager/resources/ReplaceSecurityManager.jnlp
>>
>> +++
>> b/tests/jnlp_tests/simple/ReplaceSecurityManager/resources/ReplaceSecurityManager.jnlp
>>
>> @@ -4,6 +4,7 @@
>> href="ReplaceSecurityManager.jnlp">
>> <information>
>> <title>Test replacing security manager</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="ReplaceSecurityManager.jar" main="true"/>
>> diff --git
>> a/tests/jnlp_tests/simple/SetContextClassLoader/resources/SetContextClassLoader.jnlp
>> b/tests/jnlp_tests/simple/SetContextClassLoader/resources/SetContextClassLoader.jnlp
>>
>> ---
>> a/tests/jnlp_tests/simple/SetContextClassLoader/resources/SetContextClassLoader.jnlp
>>
>> +++
>> b/tests/jnlp_tests/simple/SetContextClassLoader/resources/SetContextClassLoader.jnlp
>>
>> @@ -4,6 +4,7 @@
>> href="SetContextClassLoader.jnlp">
>> <information>
>> <title>set context classloader</title>
>> +<vendor>Red Hat</vendor>
>> </information>
>> <resources>
>> <jar href="SetContextClassLoader.jar" main="true"
>> download="eager"/>
>
> Yap, tahnx for this. I think you can push them asap - independently on
> rest of the fix.
>
>> diff --git
>> a/tests/jnlp_tests/simple/TitleVendorParser/resources/TitleParser.jnlp
>> b/tests/jnlp_tests/simple/TitleVendorParser/resources/TitleParser.jnlp
>> new file mode 100644
>> --- /dev/null
>> +++
>> b/tests/jnlp_tests/simple/TitleVendorParser/resources/TitleParser.jnlp
>> @@ -0,0 +1,52 @@
>> +<!--
>> +
>> +This file is part of IcedTea.
>> +
>> +IcedTea is free software; you can redistribute it and/or modify
>> +it under the terms of the GNU General Public License as published by
>> +the Free Software Foundation; either version 2, or (at your option)
>> +any later version.
>> +
>> +IcedTea is distributed in the hope that it will be useful, but
>> +WITHOUT ANY WARRANTY; without even the implied warranty of
>> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> +General Public License for more details.
>> +
>> +You should have received a copy of the GNU General Public License
>> +along with IcedTea; see the file COPYING. If not, write to the
>> +Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
>> Boston, MA
>> +02110-1301 USA.
>> +
>> +Linking this library statically or dynamically with other modules is
>> +making a combined work based on this library. Thus, the terms and
>> +conditions of the GNU General Public License cover the whole
>> +combination.
>> +
>> +As a special exception, the copyright holders of this library give you
>> +permission to link this library with independent modules to produce an
>> +executable, regardless of the license terms of these independent
>> +modules, and to copy and distribute the resulting executable under
>> +terms of your choice, provided that you also meet, for each linked
>> +independent module, the terms and conditions of the license of that
>> +module. An independent module is a module which is not derived from
>> +or based on this library. If you modify this library, you may extend
>> +this exception to your version of the library, but you are not
>> +obligated to do so. If you do not wish to do so, delete this
>> +exception statement from your version.
>> +
>> + -->
>> +<?xml version="1.0" encoding="utf-8"?>
>> +<jnlp spec="1.0" href="TitleParser.jnlp" codebase=".">
>> +<information>
>> +<vendor>NetX</vendor>
>> +<homepage href="http://jnlp.sourceforge.net/netx/"/>
>> +<description>TitleParser</description>
>> +<offline/>
>> +</information>
>> +<resources>
>> +<j2se version="1.4+"/>
>> +<jar href="TitleVendorParser.jar"/>
>> +</resources>
>> +<application-desc main-class="TitleVendorParser">
>> +</application-desc>
>> +</jnlp>
>> diff --git
>> a/tests/jnlp_tests/simple/TitleVendorParser/resources/VendorParser.jnlp
>> b/tests/jnlp_tests/simple/TitleVendorParser/resources/VendorParser.jnlp
>> new file mode 100644
>> --- /dev/null
>> +++
>> b/tests/jnlp_tests/simple/TitleVendorParser/resources/VendorParser.jnlp
>> @@ -0,0 +1,52 @@
>> +<!--
>> +
>> +This file is part of IcedTea.
>> +
>> +IcedTea is free software; you can redistribute it and/or modify
>> +it under the terms of the GNU General Public License as published by
>> +the Free Software Foundation; either version 2, or (at your option)
>> +any later version.
>> +
>> +IcedTea is distributed in the hope that it will be useful, but
>> +WITHOUT ANY WARRANTY; without even the implied warranty of
>> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> +General Public License for more details.
>> +
>> +You should have received a copy of the GNU General Public License
>> +along with IcedTea; see the file COPYING. If not, write to the
>> +Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
>> Boston, MA
>> +02110-1301 USA.
>> +
>> +Linking this library statically or dynamically with other modules is
>> +making a combined work based on this library. Thus, the terms and
>> +conditions of the GNU General Public License cover the whole
>> +combination.
>> +
>> +As a special exception, the copyright holders of this library give you
>> +permission to link this library with independent modules to produce an
>> +executable, regardless of the license terms of these independent
>> +modules, and to copy and distribute the resulting executable under
>> +terms of your choice, provided that you also meet, for each linked
>> +independent module, the terms and conditions of the license of that
>> +module. An independent module is a module which is not derived from
>> +or based on this library. If you modify this library, you may extend
>> +this exception to your version of the library, but you are not
>> +obligated to do so. If you do not wish to do so, delete this
>> +exception statement from your version.
>> +
>> + -->
>> +<?xml version="1.0" encoding="utf-8"?>
>> +<jnlp spec="1.0" href="VendorParser.jnlp" codebase=".">
>> +<information>
>> +<title>VendorParser</title>
>> +<homepage href="http://jnlp.sourceforge.net/netx/"/>
>> +<description>VendorParser</description>
>> +<offline/>
>> +</information>
>> +<resources>
>> +<j2se version="1.4+"/>
>> +<jar href="TitleVendorParser.jar"/>
>> +</resources>
>> +<application-desc main-class="TitleVendorParser">
>> +</application-desc>
>> +</jnlp>
>> diff --git
>> a/tests/jnlp_tests/simple/TitleVendorParser/srcs/TitleVendorParser.java
>> b/tests/jnlp_tests/simple/TitleVendorParser/srcs/TitleVendorParser.java
>> new file mode 100644
>> --- /dev/null
>> +++
>> b/tests/jnlp_tests/simple/TitleVendorParser/srcs/TitleVendorParser.java
>> @@ -0,0 +1,43 @@
>> +/* TitleVendorParser.java
>> +Copyright (C) 2011 Red Hat, Inc.
>> +
>> +This file is part of IcedTea.
>> +
>> +IcedTea is free software; you can redistribute it and/or
>> +modify it under the terms of the GNU General Public License as
>> published by
>> +the Free Software Foundation, version 2.
>> +
>> +IcedTea is distributed in the hope that it will be useful,
>> +but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> +General Public License for more details.
>> +
>> +You should have received a copy of the GNU General Public License
>> +along with IcedTea; see the file COPYING. If not, write to
>> +the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
>> Boston, MA
>> +02110-1301 USA.
>> +
>> +Linking this library statically or dynamically with other modules is
>> +making a combined work based on this library. Thus, the terms and
>> +conditions of the GNU General Public License cover the whole
>> +combination.
>> +
>> +As a special exception, the copyright holders of this library give you
>> +permission to link this library with independent modules to produce an
>> +executable, regardless of the license terms of these independent
>> +modules, and to copy and distribute the resulting executable under
>> +terms of your choice, provided that you also meet, for each linked
>> +independent module, the terms and conditions of the license of that
>> +module. An independent module is a module which is not derived from
>> +or based on this library. If you modify this library, you may extend
>> +this exception to your version of the library, but you are not
>> +obligated to do so. If you do not wish to do so, delete this
>> +exception statement from your version.
>> + */
>> +
>> +public class TitleVendorParser {
>> +
>> + public static void main(String[] args) {
>> + System.out.println("Sould not be reached but JNLP was parsed
>> fine.");
>> + }
>> +}
>> diff --git
>> a/tests/jnlp_tests/simple/TitleVendorParser/testcases/TitleVendorParserTest.java
>> b/tests/jnlp_tests/simple/TitleVendorParser/testcases/TitleVendorParserTest.java
>>
>> new file mode 100644
>> --- /dev/null
>> +++
>> b/tests/jnlp_tests/simple/TitleVendorParser/testcases/TitleVendorParserTest.java
>>
>> @@ -0,0 +1,76 @@
>> +/* TitleVendorParserTest.java
>> +Copyright (C) 2011 Red Hat, Inc.
>> +
>> +This file is part of IcedTea.
>> +
>> +IcedTea is free software; you can redistribute it and/or
>> +modify it under the terms of the GNU General Public License as
>> published by
>> +the Free Software Foundation, version 2.
>> +
>> +IcedTea is distributed in the hope that it will be useful,
>> +but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> +General Public License for more details.
>> +
>> +You should have received a copy of the GNU General Public License
>> +along with IcedTea; see the file COPYING. If not, write to
>> +the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
>> Boston, MA
>> +02110-1301 USA.
>> +
>> +Linking this library statically or dynamically with other modules is
>> +making a combined work based on this library. Thus, the terms and
>> +conditions of the GNU General Public License cover the whole
>> +combination.
>> +
>> +As a special exception, the copyright holders of this library give you
>> +permission to link this library with independent modules to produce an
>> +executable, regardless of the license terms of these independent
>> +modules, and to copy and distribute the resulting executable under
>> +terms of your choice, provided that you also meet, for each linked
>> +independent module, the terms and conditions of the license of that
>> +module. An independent module is a module which is not derived from
>> +or based on this library. If you modify this library, you may extend
>> +this exception to your version of the library, but you are not
>> +obligated to do so. If you do not wish to do so, delete this
>> +exception statement from your version.
>> + */
>> +
>> +
>> +import net.sourceforge.jnlp.ServerAccess;
>> +import org.junit.Assert;
>> +import org.junit.Test;
>> +
>> +public class TitleVendorParserTest {
>> +
>> + private static ServerAccess server = new ServerAccess();
>> +
>> + @Test
>> + public void testForTitle() throws Exception {
>> + System.out.println("connecting TitleVendorParser request,
>> testing TitleParser");
>> + System.err.println("connecting TitleVendorParser request,
>> testing TitleParser");
>> + ServerAccess.ProcessResult
>> pr=server.executeJavawsHeadless(null,"/TitleParser.jnlp");
>> + System.out.println(pr.stdout);
>> + System.err.println(pr.stderr);
>> + String s1 = "Sould not be reached but JNLP was parsed fine.";
>> + Assert.assertFalse("testForTitle stdout should not contain "
>> + s1 + " but did.", pr.stdout.contains(s1));
>> + String s2 = "net.sourceforge.jnlp.ParseException: The title
>> section has not been defined in the JNLP file.";
>> + Assert.assertTrue("testForTitle stderr should contain " + s2
>> + " but did not.", pr.stderr.contains(s2));
>> + Assert.assertFalse(pr.wasTerminated);
>> + Assert.assertEquals((Integer)0, pr.returnValue);
>> + }
>> +
>> + @Test
>> + public void testForVendor() throws Exception {
>> + System.out.println("connecting TitleVendorParser request,
>> testing VendorParser");
>> + System.err.println("connecting TitleVendorParser request,
>> testing VendorParser");
>> + ServerAccess.ProcessResult
>> pr=server.executeJavawsHeadless(null,"/VendorParser.jnlp");
>> + System.out.println(pr.stdout);
>> + System.err.println(pr.stderr);
>> + String s1 = "Sould not be reached but JNLP was parsed fine.";
>> + Assert.assertFalse("testForVendor stdout should not contain "
>> + s1 + " but did.", pr.stdout.contains(s1));
>> + String s2 = "net.sourceforge.jnlp.ParseException: The vendor
>> section has not been defined in the JNLP file.";
>> + Assert.assertTrue("testForVendor stderr should contain " + s2
>> + " but did not.", pr.stderr.contains(s2));
>> + Assert.assertFalse(pr.wasTerminated);
>> + Assert.assertEquals((Integer)0, pr.returnValue);
>> + }
>> +}
>
> Now you will hate me :
> I think it is worthy to add also one testy with both missing, and few
> tests which will try to paly with count and order of mandatory and
> obligatory elements.
> Also I woud asuggest to introduce
> String s1 = "Sould not be reached but JNLP was parsed fine.";
> Assert.assertFalse("testForVendor stdout should not contain " +
> s1 + " but did.", pr.stdout.contains(s1));
> String s2 = "net.sourceforge.jnlp.ParseException: The vendor
> section has not been defined in the JNLP file.";
> Assert.assertTrue("testForVendor stderr should contain " + s2 +
> " but did not.", pr.stderr.contains(s2));
> Assert.assertFalse(pr.wasTerminated);
> Assert.assertEquals((Integer)0, pr.returnValue);
>
> as separate method (especialy if more tests will be added)
>
>
> Do I understand correctly that when information element is missing, then
> jnlp is processed correctly?
> For both yes and no - can you add test for it?
> If yes - what does getInformationDesc() returns?? null?? I believe jnlp
> should not be processed :-/
> Also I think you do not need to add new jar with new srcs class
> (TitleVendorParser) (==just remove thsi file and no jar will be
> created), but you can yuse eg simpletest1) and just use your new jnlps
> to call this old jar. But this is really up to you, if you want to have
> by-your-test-controlled jar, then as you wish. (Me, personally, should
> probably stay with my own jar ;)
>
> I got used to check for ClassNotFound exception, which is here for
> reason that netx.jar is broken, javaws is broken or the whole test is
> compeltly broken. But when you are checking for stdout of your jar, then
> it is not necessary, it just helps to see imidietly that soemting more
> bad then just test failure have occured. Do you think it is worthy to
> add? By other words - Should I get rid of this "habit" ?
>> diff --git
>> a/tests/netx/unit/net/sourceforge/jnlp/templates/template8.jnlp
>> b/tests/netx/unit/net/sourceforge/jnlp/templates/template8.jnlp
>> --- a/tests/netx/unit/net/sourceforge/jnlp/templates/template8.jnlp
>> +++ b/tests/netx/unit/net/sourceforge/jnlp/templates/template8.jnlp
>> @@ -1,5 +1,9 @@
>> <?xml version="1.0"?>
>> <jnlp spec="1.5+" codebase="file" href="www.redhat.com">
>> +<information>
>> +<title>Sample Test</title>
>> +<vendor>RedHat</vendor>
>> +</information>
>> <resources>
>> <j2se version='1.6+' />
>> <jar href='myResource.jar' main='true' />
> Thsi one should be pushed with rest of in-tests vendor/title fixes
>
>
> TYVM!
>
> J.
>
More information about the distro-pkg-dev
mailing list