On Mon, 28 Jun 2021 03:41:20 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> Can I please get a review for this proposed fix for the issue reported in > https://bugs.openjdk.java.net/browse/JDK-8190753? > > The commit here checks for the size of the zip entry before trying to create > a `ByteArrayOutputStream` for that entry's content. A new jtreg test has been > included in this commit to reproduce the issue and verify the fix. > > P.S: It's still a bit arguable whether it's a good idea to create the > `ByteArrayOutputStream` with an initial size potentially as large as the > `MAX_ARRAY_SIZE` or whether it's better to just use some smaller default > value. However, I think that can be addressed separately while dealing with > https://bugs.openjdk.java.net/browse/JDK-8011146 This may be just moving the problem because writing to the BAOS will fail when the deflated size is too large to fit in a byte array. The zip provider can use a temporary file so maybe it should use that when appending to existing zip entries that are larger than some threshold. At some point we may need deeper changes here, e.g. start out with a BAOS and spill over to a temporary file when the deflated size reaches some threshold. I didn't study the test too closely but just to mention that tests with zip entries > 2GB can be problematic to test. The test will probably need the @requires tag to limit it to 64-bit systems and maybe some minimum memory size. It may also need testing on a wide range of systems to get some idea of run time. Test machines with spinning rust (HDDs) come to mind. ------------- PR: https://git.openjdk.java.net/jdk/pull/4607
