[RFC][icedtea-web] Plugin doesn't halt when required elements from JNLP files are missing.

[Danesh Dadachanji]

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?
>>>>
>>>> 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. =)

Regards,
Danesh
-------------- next part --------------
A non-text attachment was scrubbed...
Name: title-vendor-required-02.patch
Type: text/x-patch
Size: 1458 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120111/17663700/title-vendor-required-02.patch