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

[Danesh Dadachanji]

On 12/03/12 07:26 AM, Jiri Vanek wrote:
> On 03/12/2012 12:22 PM, Pavel Tisnovsky wrote:
>> Jiri Vanek wrote:
>>> On 03/12/2012 11:50 AM, Pavel Tisnovsky wrote:
>>>> 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
>>> snip
>>>>>>
>>>>>> 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.

I would still need 2 conditions because I want the if to pass when 
info.get*() is null. That check is not to look out for a NPE, it's the 
opposite. If it's null, the tags aren't there! So I'd still need both.

Also, I remember thinking about this case before posting the patch. It 
already trims it before saving vendor into the info var. =) However I 
think you're right, it should still be there in case somewhere down the 
line, the trim before saving is removed.

>>>>
>>>> String.trim() does not return null so you don't need to worry
>>> Yy - as I have already said ;) but STILL it is good habit O:)
>>>>
>>>> Then you could use predicate isEmpty() which is IMHO more readable
>>>> than equals("") (since 1.6)
>>> isEmpty do not trim :((( => info.get*() == null ||
>>> info.get*().trim().isEmpty() ... :D (if we can live with method which
>>> appeared in 1.6)
>> Yeah I mean info.get*().trim().isEmpty()
>> (btw I think we could end with this basic stuff ;-)
>>>
>>> But this leads me to question - which javaSE version is icedtea-web
>>> comaptible with and which _should_ be compatible with?
>>
>> I assume it should be compatible with 1.6 and 1.7
> Yap, I guess the same... Poor mainframes with ibm 1.4.2 are out of luck
> again ;)
>> (and I'm pretty sure that
>> the code already contains some 1.6-related code ;-) which could be easy
> Although i do not KNOW about it I BELIEVE to it too !-) ..and I think
> there is no need to keep older javas supported anyway...
>> to check using --source and --target)
> Ah Yah!

I think it's safe to use this here. I will definitely use isEmpty() then!

Updated patch coming shortly. Thanks again for all the comments!

Cheers,
Danesh