RFC: 8132359: JarURLConnection.getJarFile() resource leak when file is not found

[mark sheppard]

Hi Michael et al.,
    wrt the actually bug itself and its title should it be changed to something more appropriately
aligned with the fix?
something like
Amend JarURLConnection::getJarFile() to return JarFile object reference for non existent jar file entry URL.

rather than
JarURLConnection.getJarFile() resource leak when file not found.

A couple of observations, and questions  arising from some basic tests I ran.
These were based based on adaptations of the original reproducer test code,
for each of the API method. The results are recorded  in the table below below.


method  JDK15 master build
with Proposed change    JDK15 master build without proposed change (also JDK13)
getJarFile *    return JarFile reference        FileNotFoundException thrown
getJarEntry **  FileNotFoundException thrown
FileNotFoundException thrown
getAttributes
FileNotFoundException thrown
FileNotFoundException thrown
getCertificates FileNotFounfException thrown    FileNotFoundException thrown
getManifest     Mainfest reference returned     FileNotFoundException thrown
getMainAttributes       Attributes reference returned   FileNotFoundException thrown
getInputStream ***      FileNotFoundException thrown    FileNotFoundException thrown





*.    abstract java.net.JarURLConnection method overridden in sun.net.www.protocol.jar.JarURLConnection
**   java.net.JarURLConnection method overridden in sun.net.www.protocol.jar.JarURLConnection
***  java.net.URLConnection method overridden in sun.net.www.protocol.jar.JarURLConnection

So looking at the API docs

the getJarEntry method references getJarFile, - @see #getJarFile (and itself getJarEntry - the @see #getJarEntry should probably be removed),
indicating some dependency association with getJarFile method. However, as it is overridden in the implementation subclass, any anticipated getJarFile().getJarEntry(entryName) is never invoked.

However,  the @throws says the following:
IOException if getting the Jar file for this connection causes an IOException to be thrown

and as "getting the Jar file" doesn't cause an IOException, this should probably be amended to

IOException if getting the Jar file for this connection causes an IOException to be thrown,
or the referenced entry does not exist in the containing Jar file.


should the overridden methods in sun.net.www.protocol.jar.JarURLConnection be qualified with the
@Override annotation ?


regards
Mark
________________________________
From: net-dev <net-dev-bounces at openjdk.java.net> on behalf of Michael McMahon <michael.x.mcmahon at oracle.com>
Sent: Monday 11 May 2020 13:28
To: Alex Kashchenko <akashche at redhat.com>; Daniel Fuchs <daniel.fuchs at oracle.com>; OpenJDK Network Dev list <net-dev at openjdk.java.net>
Cc: Mark Sheppard <mark.sheppard at oracle.com>
Subject: Re: RFC: 8132359: JarURLConnection.getJarFile() resource leak when file is not found


On 10/05/2020 21:51, Alex Kashchenko wrote:
> Thanks for your comments!
>
> On 05/08/2020 06:38 PM, Michael McMahon wrote:
>>
>> [...]
>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8244650
>>>
>>> I believe some sort of spec change will be needed, if only to justify
>>> challenging the JCK. Currently, the proposed change trips the following
>>> test:
>>>
>>> TestCase: [JarURLConnectionT216] FAIL->getManifest() did not throw
>>> expected IOException for
>>> jar:file:/JCK-runtime-15/tests/api/java_net/JarURLConnection//jardir/T11.jar!/NOJARENTRY.class
>>
>
> I've added a reference to this failure to "Compatibility Risk
> Description".
>
I think for the avoidance of doubt we should make the same change to the
getManifest() @Throws spec also,
to cover the above. There is another method getMainAttributes() whose
behavior will also change,
but I think its existing spec covers it, since it depends on getManifest()

- Michael

>
>>>
>>> [...]
>>>
>> I agree. Even though the @throws appears to say an IOException is
>> only thrown
>>
>> if the connection to the JAR file fails, I think we should make it
>> more explicit.
>
> Updated "Specification" part including the changes to the API spec.
>
>
>>
>> [...]
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/net-dev/attachments/20200511/61d00283/attachment.htm>