On Mon, 7 Feb 2022 18:44:10 GMT, Sean Mullan <mul...@openjdk.org> wrote:

>> Looking at this a bit more,  it looks like `JariFile::initializeVerifier` is 
>> the only place currently in `JarFile` that could throw a `JarException` and 
>> that method could be called from `JarFile::getInputStream`
>> 
>> As `verifiableEntry` is only called from `JarFile::getInputStream `and this 
>> change validates the `ZipEntry` for being null, which is should,  the only 
>> other possible error would be in the unlikely event. `ZipEntry` was 
>> subclassed passed into `JarFile::getInputStream` resulting in the call to 
>> `ZipEntry::getName` returning null(in which case `ZipFile::getEntry` will 
>> return a `NullPointerException`) or the name being invalid resulting once 
>> again in a possible null value for the returned `ZipEntry` from 
>> `JarFile::getJarEntry` (which calls `ZipFile::getEntry`)
>> 
>> 
>> I am still leaning towards a `ZipException`, not a `JarException`, thrown 
>> from `verifiableEntry`.
>> 
>> That being said, I will go with whatever the consensus is.
>
> If you are pretty sure the only other case are as above, I wonder if a 
> simpler fix would be to change `verifiableEntry()` to check for these null 
> cases and throw a `ZipException` which will get directly propagated by 
> `getInputStream()`?

I can certainly throw a ZipException from `verifiableEntry`. I am a bit 
reluctant to not catch any other `Exception` and then throw a `ZipException` 
from `getInputStream()` as it is certainly possible of encountering some other 
issue due to some stray value in the CEN.

So I will update `verifiableEntry` to validate `ZipEntry` and` 
ZipEntry::getName()` potential issues

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

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

Reply via email to