RFR [9] 8139706: JarFile.getBytes could use InputStream.readNBytes (original) (raw)

Claes Redestad claes.redestad at oracle.com
Fri Oct 16 11:49:43 UTC 2015

On 2015-10-16 04:09, Xueming Shen wrote:

On 10/15/15 3:08 PM, Claes Redestad wrote:

On 2015-10-15 23:21, Chris Hegarty wrote: On 15 Oct 2015, at 21:59, ecki at zusammenkunft.net wrote:

Hello, This does change a bit the semantic of the length check. If the stream would return more bytes than the zipentry says the new version would ignore them, the old version was consuming then and then fail the check. However I am not sure if this is relevant. Right, there are certainly some subtle differences resulting from the proposed change. When working on JDK-8138978 I thought about using readNBytes, but played it safe as IOUtils was growing the bye[] lazily too, so no real perf difference. In fact, I think I seen a test failure with using readNBytes here. I’ll have to check. Seeing no jtreg test failures in java/util/zip nor java/util/jar (apart from 2 ignored tests), but I can see a reason for the current implementation being conservative: Corrupt/malicious jar files might lie about the entry length and report very large values, which could bring a down with OOME. I believe we could be both safe and faster than baseline by adding a reasonable limit for when to use readNBytes, e.g., 32k would deal with the majority of .class files. getBytes should be used to read the meta-inf files only, not the .class files.

Correct, but this is still enough to cause statistically significant increases on our footprint measures.

With a reasonable trust limit this change should be safe while avoiding most temporary byte[] allocations when reading meta-inf files. I've verified the startup and footprint numbers and ran it through all java/util/jar and /zip tests without error.

New webrev: http://cr.openjdk.java.net/~redestad/8139706/webrev.02/

/Claes

-sherman

More information about the core-libs-dev mailing list