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

2022-02-17 Thread Lance Andersen
On Fri, 11 Feb 2022 17:43:47 GMT, Lance Andersen wrote: >> src/java.base/share/classes/java/util/jar/JarFile.java line 881: >> >>> 879: ze = getJarEntry(entryName); >>> 880: } else { >>> 881: throw new ZipException("ZipEntry::getName returned null"); >> >> Throwi

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

2022-02-17 Thread Lance Andersen
> 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

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

2022-02-11 Thread Lance Andersen
On Fri, 11 Feb 2022 13:45:47 GMT, Alan Bateman wrote: >> Lance Andersen has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Return a null InputStream when the ZipEntry is not found in the Jar >> - Address formatting and message feedback >

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

2022-02-11 Thread Alan Bateman
On Thu, 10 Feb 2022 21:35:56 GMT, Lance Andersen 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

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

2022-02-10 Thread Lance Andersen
On Thu, 10 Feb 2022 20:37:50 GMT, Sean Mullan wrote: >> 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 appropriat

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

2022-02-10 Thread Lance Andersen
> 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

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

2022-02-10 Thread Sean Mullan
On Thu, 10 Feb 2022 20:35:19 GMT, Sean Mullan 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

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

2022-02-10 Thread Sean Mullan
On Wed, 9 Feb 2022 21:16:08 GMT, Lance Andersen wrote: >>> 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

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

2022-02-09 Thread Lance Andersen
On Tue, 8 Feb 2022 18:06:04 GMT, Lance Andersen 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"

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

2022-02-08 Thread Lance Andersen
On Tue, 8 Feb 2022 18:56:02 GMT, Alan Bateman wrote: >>> I'm almost tempted to have getInputStream(ZipEntry) be re-specified to >>> throw IAE if the zip entry name is null. >> >> I personally think it is best to continue throw the NPE as that provides >> symmetry with ZipFile::getInputStream,

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

2022-02-08 Thread Alan Bateman
On Tue, 8 Feb 2022 18:11:38 GMT, Lance Andersen wrote: > I personally think it is best to continue throw the NPE as that provides > symmetry with ZipFile::getInputStream, aligns with the current javadoc where > a null parameter will throw an NPE unless specified elsewhere, there are > existing

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

2022-02-08 Thread Lance Andersen
On Tue, 8 Feb 2022 15:55:50 GMT, Alan Bateman 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= ex

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

2022-02-08 Thread Lance Andersen
On Tue, 8 Feb 2022 18:05:25 GMT, Lance Andersen wrote: >> ze can't be null here. > >> 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/g

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

2022-02-08 Thread Lance Andersen
On Tue, 8 Feb 2022 15:31:51 GMT, Sean Mullan wrote: >> Lance Andersen has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Reduce Exception checking to JarFile::verifiableEntry > > src/java.base/share/classes/java/util/jar/JarFile.java line 8

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

2022-02-08 Thread Lance Andersen
On Tue, 8 Feb 2022 18:06:52 GMT, Lance Andersen wrote: >> Ah, yes - good catch! > > Will do. > I'm almost tempted to have getInputStream(ZipEntry) be re-specified to throw > IAE if the zip entry name is null. I personally think it is best to continue throw the NPE as that provides symmetry wi

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

2022-02-08 Thread Lance Andersen
On Tue, 8 Feb 2022 16:15:20 GMT, Sean Mullan wrote: >> if ZipEntry is extended and getName() overridden then you can't trust the >> name. So I think you'll have extract the name rather than calling >> ZipEntry::getName twice. I'm almost tempted to have getInputStream(ZipEntry) >> be re-specifi

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

2022-02-08 Thread Sean Mullan
On Tue, 8 Feb 2022 15:57:00 GMT, Alan Bateman wrote: >> src/java.base/share/classes/java/util/jar/JarFile.java line 871: >> >>> 869: } >>> 870: // ZipEntry::getName should not return null >>> 871: if(ze.getName() != null) { >> >> Nit, add space after "if" > > if ZipEntry

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

2022-02-08 Thread Alan Bateman
On Tue, 8 Feb 2022 15:27:46 GMT, Sean Mullan wrote: >> Lance Andersen has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Reduce Exception checking to JarFile::verifiableEntry > > src/java.base/share/classes/java/util/jar/JarFile.java line 8

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

2022-02-08 Thread Sean Mullan
On Mon, 7 Feb 2022 23:02:54 GMT, Lance Andersen 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 >

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

2022-02-07 Thread Lance Andersen
On Mon, 7 Feb 2022 20:16:43 GMT, Lance Andersen wrote: >> 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 >> `getInputS

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

2022-02-07 Thread Lance Andersen
> 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

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

2022-02-07 Thread Lance Andersen
On Mon, 7 Feb 2022 18:44:10 GMT, Sean Mullan 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 `verifiableE

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

2022-02-07 Thread Sean Mullan
On Mon, 7 Feb 2022 16:52:07 GMT, Lance Andersen wrote: >> `JarException` is a subclass of `ZipException` though, so I think this would >> be ok to throw and still be compliant with the specification. > > Looking at this a bit more, it looks like `JariFile::initializeVerifier` is > the only pla

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

2022-02-07 Thread Lance Andersen
On Mon, 7 Feb 2022 15:16:43 GMT, Sean Mullan wrote: >> 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 > > `Jar

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

2022-02-07 Thread Sean Mullan
On Fri, 4 Feb 2022 15:19:11 GMT, Lance Andersen wrote: >> 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(

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

2022-02-04 Thread Lance Andersen
On Fri, 4 Feb 2022 16:06:34 GMT, Lance Andersen wrote: > > Could these unexpected exceptions also occur when using the > > `JarInputStream` API? > > It's a different code path as Zip/JarFile leverage the CEN where > Zip/JarInputStream leverage the LOC. I can give it a go and if there is an >

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

2022-02-04 Thread Lance Andersen
On Fri, 4 Feb 2022 15:55:33 GMT, Sean Mullan wrote: > Could these unexpected exceptions also occur when using the `JarInputStream` > API? It's a different code path as Zip/JarFile leverage the CEN where Zip/JarInputStream leverage the LOC. I can give it a go and if there is an issue will cr

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

2022-02-04 Thread Sean Mullan
On Fri, 4 Feb 2022 12:42:39 GMT, Lance Andersen 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 > unexpe

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

2022-02-04 Thread Lance Andersen
On Fri, 4 Feb 2022 15:06:59 GMT, Alan Bateman 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 >>

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

2022-02-04 Thread Alan Bateman
On Fri, 4 Feb 2022 12:42:39 GMT, Lance Andersen 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 > unexpe

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

2022-02-04 Thread Lance Andersen
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.