RFR: 6714834: JarFile.getManifest() leaves an open InputStream as an undocumented side effect [v2]
Jaikiran Pai
jpai at openjdk.java.net
Tue Sep 15 15:47:31 UTC 2020
On Tue, 15 Sep 2020 15:33:51 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:
>> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Remove "final"
>
> Hi Jaikiran,
>
> This is not an area I know too well - so I won't review formally, but the proposed changes look reasonable to me.
> Closing the stream from within `JarFile` after creating the manifest looks innocuous and should release any resource
> held by the stream earlier instead of waiting for the `JarFile` to be closed. As long as the input stream `close()`
> method is idem potent this should be safe, and AFAICS that is the case for the two input stream subclasses that can be
> returned by `ZipFile::getInputStream`. WRT to the claims in the JBS issue I see that that was logged against Java 6:
> there was no `Cleanable` at this time and it is possible that the internals of ZipFile/JarFile were quite different.
Thank you for the review Daniel.
> WRT to the claims in the JBS issue I see that that was logged against Java 6: there was no Cleanable at this time and
> it is possible that the internals of ZipFile/JarFile were quite different.
You are right. I hadn't paid attention to that detail. It's likely that it might have been behaving differently at that
time.
As for this:
> As long as the input stream close() method is idem potent this should be safe, and AFAICS that is the case for the two
> input stream subclasses that can be returned by ZipFile::getInputStream.
I'm curious, in the context of this change, why idempotency would be a necessity. Would there be a "double close"
somehow on this `InputStream` instance?
-------------
PR: https://git.openjdk.java.net/jdk/pull/186
More information about the security-dev
mailing list