On Thu, 10 Feb 2022 20:35:19 GMT, Sean Mullan <[email protected]> wrote:
>> 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.
>
> Agree on returning null to maintain current behavior. I would also lean
> towards amending the specification to specify what has been long-standing
> behavior.
If we had to do it over again, I do think throwing IAE is more appropriate
because this case would probably be due to a bug in the application code. Now
code has to defensively check for a null return value. I don't know, maybe we
don't want to modify the specification at this point and leave this as
undefined behavior.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7348
