> On Mar 3, 2016, at 3:26 AM, Paul Sandoz <paul.san...@oracle.com> wrote:
>
>
>> On 2 Mar 2016, at 20:12, Steve Drach <steve.dr...@oracle.com> wrote:
>>
>> Please review the following fix for JDK-8150679
>>
>> webrev: http://cr.openjdk.java.net/~sdrach/8150679/webrev/
>> <http://cr.openjdk.java.net/~sdrach/8150679/webrev/>
>> issue: https://bugs.openjdk.java.net/browse/JDK-8150679
>> <https://bugs.openjdk.java.net/browse/JDK-8150679>
>>
>> The test was modified to demonstrate the problem.
>
> You are essentially bombing out of MR-JAR functionality if the JarEntry is
> not an instance of JarFileEntry.
If it’s not a JarFileEntry, none of the MR functionality has been invoked.
> That might be ok for a short-term solution, but it might require some further
> deeper investigation on things that extend JarEntry and how it is is used by
> VerifierStream [*].
>
> JarFile:
>
> 895 private JarEntry verifiableEntry(ZipEntry ze) {
> 896 if (ze == null) return null;
>
> You don’t need this. The code will anyway throw an NPE elsewhere, and the
> original code threw an NPE when obtaining the name:
Ok. I’ll take this out. Feels a bit uncomfortable though.
>
> return new JarVerifier.VerifierStream(
> getManifestFromReference(),
> ze instanceof JarFileEntry ?
> (JarEntry) ze : getJarEntry(ze.getName()),
> super.getInputStream(ze),
> jv);
>
>
> 897 if (ze instanceof JarFileEntry) {
> 898 // assure the name and entry match for verification
> 899 return ((JarFileEntry)ze).reifiedEntry();
> 900 }
> 901 ze = getJarEntry(ze.getName());
> 902 assert ze instanceof JarEntry;
>
> This assertion is redundant as the method signature of getJarEntry returns
> JarEntry.
I know it’s redundant. It was a statement of fact. but the method signature
does the same thing.
>
>
> 903 if (ze instanceof JarFileEntry) {
> 904 return ((JarFileEntry)ze).reifiedEntry();
> 905 }
> 906 return (JarEntry)ze;
> 907 }
>
>
> MultiReleaseJarURLConnection
> —
>
> Given your changes above i am confused how your test passes for instances of
> URLJarFileEntry since they cannot be reified.
I suspect that it works for regular jar files but not for MR jar files. That’s
another bug in URLJarFile — it gets a versioned entry that can’t be verified.
I mentioned this yesterday. I’ll write a test and if warranted submit a bug on
this.
>
> Paul.
>
> [*] AFAICT JarVerifier directly accesses the fields JarEntry.signers/certs.
Yes, but the overriden fields are the ones of interest.
>
