RFR: 6714834: JarFile.getManifest() leaves an open InputStream as an undocumented side effect

Jaikiran Pai jpai at openjdk.java.net
Tue Sep 15 15:05:36 UTC 2020


Can I please get a review and a sponsor for this patch which fixes the issue reported in
https://bugs.openjdk.java.net/browse/JDK-6714834?

As noted by the reporter in that issue, when the `java.util.jar.JarFile.getManifest()` method is called, it internally
calls the private `getManifestFromReference`. This private method implementation opens an `InputStream` for passing it
on to the constructor of the `Manifest`, but never closes it. The commit here fixes that part to use a
`try-with-resources` to close the `InputStream` once the `Manifest` instance is created.

This issue is only applicable when the `JarFile` is created with `verify` as `false`, which isn't the default.

In that issue report, there's also a mention that this can lead to incorrect manifest files ending up in jar files:

> can lead to serious and hard to track bugs (e.g. when replacing the manifest and generating a new JarFile, the old one
> can be unexpectedly taken in some circumstances unless one calls myJar.close() after myJar.getManifest())

I have gone through the code to see how this can happen and have also done some testing to see if this is possible. But
my tests and the code haven't shown this as a possibilty. The `Manifest` doesn't store any `InputStream` once it's
created nor does the `JarFile` store that stream. Although the `ZipFile`, which `JarFile` extends from, does store
these opened `InputStreams` into a `Set`, it only does it to close them as part of the `java.lang.ref.Cleaner` contract
and from what I can see, none of these APIs return or use these stored `InputStream`s for any other purpose. So I don't
see how a leaking `InputStream` can lead to a wrong `Manifest` ending up in a copy of a `JarFile`. So the commit in
this PR only addresses the leaking `InputStream`.

I haven't added any new tests to verify this fix because given the nature of this issue, I couldn't think of a
consistent and determinstic way to verify it. I have however run the `jdk:tier1` tests locally which hasn't shown any
related failures.

-------------

Commit messages:
 - 6714834: JarFile.getManifest() leaves an open InputStream as an undocumented side effect

Changes: https://git.openjdk.java.net/jdk/pull/186/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=186&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-6714834
  Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/186.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/186/head:pull/186

PR: https://git.openjdk.java.net/jdk/pull/186


More information about the core-libs-dev mailing list