Re: RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName() [v3]
On Fri, 11 Feb 2022 17:43:47 GMT, Lance Andersen wrote:
>> src/java.base/share/classes/java/util/jar/JarFile.java line 881:
>>
>>> 879: ze = getJarEntry(entryName);
>>> 880: } else {
>>> 881: throw new ZipException("ZipEntry::getName returned null");
>>
>> Throwing ZipException when ZipEntry::getName returns null is still
>> surprising but not terrible. The spec for getInputStream specifies
>> ZipException for when a zip file format occurs so we might have to extend
>> that to add "or the zip entry name is null".
>
> If you would like me to update the ZipException to clarify this I can. The
> original issue was due to a format error in the CEN so the current wording
> covers that. It does not cover the case where ZipEntry is subclassed and
> ZipEntry::getName() returns null which is what your suggested wording would
> address.
>
> Please note the above change addresses the signed jar scenario. I can add an
> additional check in JarFile::getInputStream to check for null from
> ZipEntry::getName so that it covers unsigned jars and signed jars not being
> verified.
>
> The issue will not occur with ZipEFile::getInputStream given its use of
> ZipEntry.name which can never be null.
>
> Another thought would be to return null for the InputStream similar to when
> the ZipEntry does not represent a file within the Jar
>
> Please let me know your preference
Per a discussion with Sean and Alan, we are now returning null for the
InputStream to be consistent with ZipFile::getInputStream and how unsigned jars
are handled
-
PR: https://git.openjdk.java.net/jdk/pull/7348
Re: RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName() [v3]
On Fri, 11 Feb 2022 13:45:47 GMT, Alan Bateman wrote:
>> Lance Andersen has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Return a null InputStream when the ZipEntry is not found in the Jar
>> - Address formatting and message feedback
>
> src/java.base/share/classes/java/util/jar/JarFile.java line 881:
>
>> 879: ze = getJarEntry(entryName);
>> 880: } else {
>> 881: throw new ZipException("ZipEntry::getName returned null");
>
> Throwing ZipException when ZipEntry::getName returns null is still surprising
> but not terrible. The spec for getInputStream specifies ZipException for
> when a zip file format occurs so we might have to extend that to add "or the
> zip entry name is null".
If you would like me to update the ZipException to clarify this I can. The
original issue was due to a format error in the CEN so the current wording
covers that. It does not cover the case where ZipEntry is subclassed and
ZipEntry::getName() returns null which is what your suggested wording would
address.
Please note the above change addresses the signed jar scenario. I can add an
additional check in JarFile::getInputStream to check for null from
ZipEntry::getName so that it covers unsigned jars and signed jars not being
verified.
The issue will not occur with ZipEntry::getInputStream given its use of
ZipEntry.name which can never be null.
Please let me know your preference
-
PR: https://git.openjdk.java.net/jdk/pull/7348
Re: RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName() [v3]
On Thu, 10 Feb 2022 21:35:56 GMT, Lance Andersen wrote:
>> Hi all,
>>
>> Please review the attached patch to address
>>
>> - That JarFile::getInputStream did not check for a null ZipEntry passed as a
>> parameter
>> - Have Zip/JarFile::getInputStream throw a ZipException in the event that an
>> unexpected exception occurs
>>
>> Mach5 tiers1-3 runs are clean as are the TCK java.util.zip and java.util.jar
>> test runs
>>
>> Best
>> Lance
>
> Lance Andersen has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Return a null InputStream when the ZipEntry is not found in the Jar
> - Address formatting and message feedback
src/java.base/share/classes/java/util/jar/JarFile.java line 881:
> 879: ze = getJarEntry(entryName);
> 880: } else {
> 881: throw new ZipException("ZipEntry::getName returned null");
Throwing ZipException when ZipEntry::getName returns null is still surprising
but not terrible. The spec for getInputStream specifies ZipException for when
a zip file format occurs so we might have to extend that to add "or the zip
entry name is null".
-
PR: https://git.openjdk.java.net/jdk/pull/7348
Re: RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName() [v3]
> Hi all, > > Please review the attached patch to address > > - That JarFile::getInputStream did not check for a null ZipEntry passed as a > parameter > - Have Zip/JarFile::getInputStream throw a ZipException in the event that an > unexpected exception occurs > > Mach5 tiers1-3 runs are clean as are the TCK java.util.zip and java.util.jar > test runs > > Best > Lance Lance Andersen has updated the pull request incrementally with two additional commits since the last revision: - Return a null InputStream when the ZipEntry is not found in the Jar - Address formatting and message feedback - Changes: - all: https://git.openjdk.java.net/jdk/pull/7348/files - new: https://git.openjdk.java.net/jdk/pull/7348/files/6c75384a..32f6c284 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7348=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7348=01-02 Stats: 95 lines in 3 files changed: 41 ins; 20 del; 34 mod Patch: https://git.openjdk.java.net/jdk/pull/7348.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7348/head:pull/7348 PR: https://git.openjdk.java.net/jdk/pull/7348
