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

[Jiri Vanek]

On 07/20/2011 10:10 PM, Deepak Bhole wrote:
> * Saad Mohammad<smohammad at redhat.com>  [2011-07-06 15:38]:
>> >  This is the updated patch for adding support of signed JNLP file. I
>> >  have attached two patches.
>> >
>> >  More Info:
>> >        http://jcp.org/aboutJava/communityprocess/maintenance/jsr056/jnlp-7_0-changes.html
>> >  (Change 3)
>> >
>> >
> Hi Saad,
>
> Generally the patch is good. I have a few minor corrections, please see
> below.
...snip...
>> >  +                                try {
>> >  +                                    JarFile jarFile = new JarFile(localFile);
>> >  +                                    Enumeration<JarEntry>  entries = jarFile.entries();
>> >  +                                    JarEntry je;
>> >  +
>> >  +                                    while (entries.hasMoreElements()) {
>> >  +                                        je = entries.nextElement();
>> >  +                                        String jeName = je.getName().toUpperCase();
>> >  +
>> >  +                                        if (jeName.equals(template) || jeName.equals(application)) {
>> >  +
>> >  +                                            if (JNLPRuntime.isDebug())
>> >  +                                                System.out.println("\tCreating Jar InputStream from Jar Entry");
>> >  +
>> >  +                                            InputStream inStream = jarFile.getInputStream(je);

> There is a resource leak here.. this stream is opened, but who is closing it?

I' guess, that parser is closing the stream, thats why I did not point to this.  When I tried to reuse the strem, "stream already closed" exception was thrown.
Saad, can you check it one more time?

But it si true tht wenexception is thrown, then there should be some finally blok which will close the streams.
it should be reffactored to:

Strams s=null;
try{
}catch(Exception 1 e){...
}catch(Exception 2 e){...
}..
}finally{
if (s!=null) s.close();
}


around s.close() yo can possibly clsoe io exception into JNLPMatcherException.

>
>> >  +                                            InputStreamReader inputReader = new InputStreamReader(
>> >  +                                                    inStream);
>>snip...
>> >  +                                            if (url.getProtocol().equals("file")) // If the file is on the local file system, use original path, otherwise find cache file
>> >  +                                                jn = new File(url.getPath());
>> >  +                                            else
>> >  +                                                jn = CacheUtil.getCacheFile(url, null);
>> >  +
>> >  +                                            FileReader fr = new FileReader(jn);
> Same as above.. resource leak. Stream is opened but not closed.
>
>> >  +                                            InputStreamReader jnlpReader = fr;
.getLocation());

J.