[RFC][PATCH][icedtea-web]: Added support for signed JNLP file- Updated Patch

[Saad Mohammad]

On 07/25/2011 09:54 AM, Dr Andrew John Hughes wrote:
> On 09:15 Thu 21 Jul     , Jiri Vanek wrote:
>> On 07/20/2011 10:26 PM, Deepak Bhole wrote:
>>> * Saad Mohammad<smohammad at redhat.com>   [2011-07-19 11:04]:
>>>>>   I have attached the updated copy of Patch1.
>>>>>
>>>>>   On 07/19/2011 02:47 AM, Jiri Vanek wrote:
>>>>>>>   >>
>>>>>>>   >>I hope this is the 110% for Patch1! ;)
>>>>>>   >99% ;)
>>>>>   This is for sure the 110% ;)
>>>>>
>>>>>   --
>>>>>   Cheers,
>>>>>   Saad Mohammad
>>>>>
>>> Hi Saad,
>>>
>>> I have a few minor corrections for this one...
>>>
>>>>>   +
>>>>>   +            if (appTemplate == null&&   launchJNLP == null)
>>>>>   +                throw new NullPointerException(
>>>>>   +                        "Template JNLP file and Launching JNLP file are both null.");
>>>>>   +            else if (appTemplate == null)
>>>>>   +                throw new NullPointerException("Template JNLP file is null.");
>>>>>   +            else if (launchJNLP == null)
>>>>>   +                throw new NullPointerException("Launching JNLP file is null.");
>>>>>   +
>>> Throwing RuntimeExceptions is generally not a good idea. Consider making
>>> the above a checked exception.
>> I believe nullpointer exception is correct here. Or it will make mess later.
>> Although to repalce it with JNLMatch exception can do no harm.
>>
> I don't see the point of throwing an NPE, catching it and throwing it attached to something else.

The reason why I am doing this ^, is so the class that is creating an 
instance of JNLPMatcher would not have to handle catching more than one 
exceptions. Generally if any exception is thrown from this constructor, 
it would results in the same outcome (JNLPMatcher failed to be 
initialized). Attaching the exception to the new thrown exception would 
help identify the cause.

An alternative can be to remove the try/catch block completely and let 
the compiler know that the contructor throws NullPointerException and 
IOException(while parsing). In this case, the class that initializes 
JNLPMatcher would have to catch all exceptions, or the two exceptions 
individually.

I think both methods of handing exceptions will work great, but please 
let me know what you think?

>>>>>   +            XMLElement appTemplateXML = new XMLElement();
>>>>>   +            XMLElement launchJNLPXML = new XMLElement();
>>>>>   +
>>>>>   +            // Remove the comments and CDATA from the JNLP file
>>>>>   +            final PipedInputStream pinTemplate = new PipedInputStream();
>>>>>   +            final PipedOutputStream poutTemplate = new PipedOutputStream(pinTemplate);
>>>>>   +            appTemplateXML.sanitizeInput(appTemplate, poutTemplate);
>>>>>   +
>>>>>   +            final PipedInputStream pinJNLPFile = new PipedInputStream();
>>>>>   +            final PipedOutputStream poutJNLPFile = new PipedOutputStream(pinJNLPFile);
>>>>>   +            launchJNLPXML.sanitizeInput(launchJNLP, poutJNLPFile);
>>>>>   +
>>> The above streams are never closed.
>> I' guess, ...
>>
>> ...see previous email..
>>
>> ...l) s.close();
>> }
>>
>>
>> around s.close() yo can possibly clsoe io exception into JNLPMatcherException.
>>
> You need a finally block.

Yes, I will add the finally block. Thanks :)

>>
>> Regards J.
>>


-- 
Cheers,
Saad Mohammad