On Fri, 4 Feb 2022 15:06:59 GMT, Alan Bateman <[email protected]> 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
>
> src/java.base/share/classes/java/util/jar/JarFile.java line 840:
>
>> 838: throws IOException
>> 839: {
>> 840: Objects.requireNonNull(ze, "ze");
>
> Is the NPE specified?
Yes it is specified in the JarFile main paragraph
`Unless otherwise noted, passing a null argument to a constructor or method in
this class will cause a NullPointerException to be thrown.`
ZipFile::getInputStream validates the argument as soon as it is passed to
getInputStream.
> src/java.base/share/classes/java/util/jar/JarFile.java line 866:
>
>> 864: } catch (Exception e2) {
>> 865: // Any other Exception should be a ZipException
>> 866: throw (ZipException) new ZipException("Zip file format
>> error").initCause(e2);
>
> If there is ZIP format error then I would expect ZipException or the more
> general IOException is already thrown. So I think this is catching other
> cases, maybe broken manifests or signed JAR files, in which case a
> JarException may be better.
JarFile::getInputStream. mentions ZipException but not JarException which is
why I chose this. If we change this to JarException, I would need to update
the javadoc and create a CSR.
Please let me know your preference
-------------
PR: https://git.openjdk.java.net/jdk/pull/7348
