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

[Lance Andersen]

On Tue, 8 Feb 2022 18:06:04 GMT, Lance Andersen <lancea at openjdk.org> wrote:

>>> ze can't be null here.
>> 
>> Actually it can be:  Consider the following:
>> 
>> 
>>         try (JarFile jf = new JarFile(SIGNED_VALID_ENTRY_NAME_JAR.toFile(), true)) {
>>             var ze = new ZipEntry("org/gotham/Batcave.class");
>>             var ex= expectThrows(ZipException.class,
>>                     () -> jf.getInputStream(ze) );
>>             // Validate that we receive the expected message from
>>             // JarFile::verifiableEntry when ZipEntry::getName returns null
>>             assertTrue( ex != null && ex.getMessage().equals("Error: ZipEntry should not be null!"));
>>         }
>> 
>> 
>> The above code does generate the error.
>
>> Nit, add space after "if"
> 
> will fix

So a bit more on this.  If the ZipEntry passed to `ZipFile::getInputStream` does not represent an entry within the current Zip/Jar,  `ZipFile::getInputStream` will return a null for the InputStream:


    @Test
    public static void ZipFileZipEntryNullGetInputStreamTest() throws Exception {
        try (ZipFile zf = new ZipFile(VALID_ENTRY_NAME_JAR.toFile())) {
            var ze = new ZipEntry("org/gotham/Batcave.class");
            var is = zf.getInputStream(ze);
            // As the ZipEntry cannot be found, the returned InpuStream is null
            assertNull(is);
        }
    }


  JarFile::getInputStream will also return null when the jar is not signed or we are not verifying the jar:


 @Test
    public static void JarFileZipEntryNullGetInputStreamTest() throws Exception {
        try (JarFile jf = new JarFile(VALID_ENTRY_NAME_JAR.toFile())) {
            var ze = new ZipEntry("org/gotham/Batcave.class");
            var is = jf.getInputStream(ze);
            // As the ZipEntry cannot be found, the returned InpuStream is null
            assertNull(is);
        }

        try (JarFile jf = new JarFile(SIGNED_INVALID_ENTRY_NAME_JAR.toFile(), false)) {
            var ze = new ZipEntry("org/gotham/Batcave.class");
            var is = jf.getInputStream(ze);
            // As the ZipEntry cannot be found, the returned InpuStream is null
            assertNull(is);
        }
    }



This behavior dates back to at least JDK 1.3

So I think we should return null  instead of throwing an Exception when the resulting ZipEntry is null that is returned from the call to`JarFile::getJarEntry` (which calls `ZipFile::getEntry`) for consistency with ZipFile and when the Jar is not signed or not verified.

I also noticed that `ZipFile::getInputStream` and `JarFile::getInputStream` do not mention that a null will be returned if the specified ZipEntry is not found within the Jar/Zip.  I guess I could open a CSR as part of this fix to clarify this 20+ year behavior.

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

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