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

[Dr Andrew John Hughes]

On 10:47 Thu 28 Jul     , Saad Mohammad wrote:
> 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?

I'm not suggesting it should throw additional exceptions, but that
these lines should just directly throw JNLPMatcherException rather
than this throw-catch-throw dance.

> 
> >>>>>   +            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
> 

-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37