RFR  8139706: JarFile.getBytes could use InputStream.readNBytes
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:
>>>> 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/
More information about the core-libs-dev