RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName()

[Lance Andersen]

On Fri, 4 Feb 2022 15:06:59 GMT, Alan Bateman <alanb at openjdk.org> 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