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

[Jaikiran Pai]

On Fri, 23 Jul 2021 10:28:02 GMT, Alan Bateman <alanb at openjdk.org> wrote:

> So I think the approach looks good but I think the synchronization needs to be re-checked it is not obvious that is correct or needed. Are there any cases where FileRolloverOutputStream is returned to user code? I don't think so, instead users of the zip file system will get an EntryOutputStream that wraps the FileRolloverOutputStream. The EntryOutputStream methods are synchronized so I assume that FileRolloverOutputStream does not need to it and that would avoid the inconsistency between the write methods and the flush/close methods.

I hadn't paid any thoughts on the "synchronized" part. You are right - the new `FileRolloverOutputStream` doesn't get sent back to the callers directly and instead either the `EntryOutputStream` or the `DeflatingEntryOutputStream` get returned. Both of them have the necessary syncrhonizations in place for `write` and `close` operations. The `flush` of the `FileRolloverOutputStream` calls the `flush` on the `BufferedOutputStream` which already has the necessary synchronization. I've updated the PR to remove the use of `synchronized` from this new class and added a brief note about this for future maintainers, just like the existing `EntryOutputStreamDef` has.

> One other thing to point out is that transferToFile shouldn't need to open the file twice, instead it should be able to open the tmp file for writing once.

The updated version of this PR now fixes this part to open it just once. I had reviewed this `transferTo` multiple times before, but clearly I overlooked this part of the implementation. 

Thank you for these inputs. The updated PR continues to pass the new tests and the existing ones in `test/jdk/jdk/nio/zipfs/`.

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

PR: https://git.openjdk.java.net/jdk/pull/4607