On Mar 3, 2016, at 3:26 AM, Paul Sandoz <paul.sandoz@oracle.com> wrote:

...

On 2 Mar 2016, at 20:12, Steve Drach <steve.drach@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.

On 3/3/16, 10:26 AM, Dave Brosius wrote:

Greetings,

It would be nice if java.util.regex.Matcher had a replacement for

Matcher appendReplacement(StringBuffer sb, String replacement) StringBuffer appendTail(StringBuffer sb)

That took StringBuilder.

we have added that in 9, right?