[jdk8u-dev] RFR: 8190753: (zipfs): Accessing a large entry (> 2^31 bytes) leads to a negative initial size for ByteArrayOutputStream [v2]

Andrew John Hughes andrew at openjdk.java.net
Thu Apr 7 01:22:41 UTC 2022


On Tue, 5 Apr 2022 08:49:17 GMT, Alexey Pavlyutkin <duke at openjdk.java.net> wrote:

>> Hi!
>> 
>> Please review the backport of JDK-8190753 to 8u. The patch fixes IllegalArgumentException that takes place on accessing large (>2^31 bytes) entries in zipfs.
>> 
>> The original patch
>> 
>> https://github.com/openjdk/jdk11u-dev/pull/545
>> 
>> was applied with the following changes:
>> 
>> - changes to absent DeflatingEntryOutputStream class were skipped (in 8u EntryOutputStream handles both RAW and deflating streaming);
>> - the tests were updated to not use unsupported APIs like Path.of() and Map.of()
>> - timeout value for ZipFSOutputStreamTest was increased because the test is too greedy and may fail by timeout on low performance hosts
>> 
>> The changes were previously reviewed: https://mail.openjdk.java.net/pipermail/jdk8u-dev/2021-December/014479.html
>> 
>> Verification/regression: jdk_other (20.04/amd64)
>
> Alexey Pavlyutkin has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - fixing whitespace difference
>  - fixing whitespace difference

This version looks a lot better. There do still seem to be a few odd differences that have crept in, when compared to the patch in 11u, which I've noted in my review. These should be quick to fix, then we can get this in.

jdk/test/demo/zipfs/LargeCompressedEntrySizeTest.java line 92:

> 90:                 final long start = System.currentTimeMillis();
> 91:                 for (long l = 0; l < largeEntrySize; l += chunkSize) {
> 92:                     final int numToWrite = (int)Math.min(remaining, chunkSize);

11u has a space between `(int)` and `Math`.

jdk/test/demo/zipfs/ZipFSOutputStreamTest.java line 120:

> 118:                         totalRead += numRead;
> 119:                         // verify the content
> 120:                         for (int i = 0, chunkoffset = (int)((totalRead - numRead) % chunk.length);

Again, 11u has a space between `(int)` and `((totalRead`

jdk/test/demo/zipfs/ZipFSOutputStreamTest.java line 126:

> 124:                         }
> 125:                     }
> 126:                     Assert.assertEquals(totalRead, (long)entry.getValue(),

11u has a space between `(long)` and `entry`.

jdk/test/demo/zipfs/ZipFSOutputStreamTest.java line 141:

> 139:         long remaining = totalSize;
> 140:         for (long l = 0; l < totalSize; l += chunk.length) {
> 141:             final int numToWrite = (int)Math.min(remaining, chunk.length);

11u has a space between `(int)` and `Math`

-------------

PR: https://git.openjdk.java.net/jdk8u-dev/pull/17


More information about the jdk8u-dev mailing list