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

Reviewed-by: phh, andrew
Backport-of: 8a9cda2d84513ab49a54e1d2a7b530f0bae05c61

Author: Alexey Pavlyutkin

jdk/src/share/demo/nio/zipfs/src/com/sun/nio/zipfs/ZipFileSystem.java

@@ -37,7 +37,6 @@
  * this sample code.
  */
 
-
 package com.sun.nio.zipfs;
 
 import java.io.BufferedOutputStream;
@@ -94,6 +93,10 @@ public class ZipFileSystem extends FileSystem {
     private static final boolean isWindows =
         System.getProperty("os.name").startsWith("Windows");
 
+    // a threshold, in bytes, to decide whether to create a temp file
+    // for outputstream of a zip entry
+    private final int tempFileCreationThreshold = 10 * 1024 * 1024; // 10 MB
+
     ZipFileSystem(ZipFileSystemProvider provider,
                   Path zfpath,
                   Map<String, ?> env)
@@ -1417,15 +1420,120 @@ private OutputStream getOutputStream(Entry e) throws IOException {
         if (zc.isUTF8())
             e.flag |= FLAG_EFS;
         OutputStream os;
-        if (useTempFile) {
+        if (useTempFile || e.size >= tempFileCreationThreshold) {
             e.file = getTempPathForEntry(null);
             os = Files.newOutputStream(e.file, WRITE);
         } else {
-            os = new ByteArrayOutputStream((e.size > 0)? (int)e.size : 8192);
+            os = new FileRolloverOutputStream(e);
         }
         return new EntryOutputStream(e, os);
     }
 
+    // A wrapper around the ByteArrayOutputStream. This FileRolloverOutputStream
+    // uses a threshold size to decide if the contents being written need to be
+    // rolled over into a temporary file. Until the threshold is reached, writes
+    // on this outputstream just write it to the internal in-memory byte array
+    // held by the ByteArrayOutputStream. Once the threshold is reached, the
+    // write operation on this outputstream first (and only once) creates a temporary file
+    // and transfers the data that has so far been written in the internal
+    // byte array, to that newly created file. The temp file is then opened
+    // in append mode and any subsequent writes, including the one which triggered
+    // the temporary file creation, will be written to the file.
+    // Implementation note: the "write" and the "close" methods of this implementation
+    // aren't "synchronized" because this FileRolloverOutputStream gets called
+    // only from either DeflatingEntryOutputStream or EntryOutputStream, both of which
+    // already have the necessary "synchronized" before calling these methods.
+    private class FileRolloverOutputStream extends OutputStream {
+        private ByteArrayOutputStream baos = new ByteArrayOutputStream(8192);
+        private final Entry entry;
+        private OutputStream tmpFileOS;
+        private long totalWritten = 0;
+
+        private FileRolloverOutputStream(final Entry e) {
+            this.entry = e;
+        }
+
+        @Override
+        public void write(final int b) throws IOException {
+            if (tmpFileOS != null) {
+                // already rolled over, write to the file that has been created previously
+                writeToFile(b);
+                return;
+            }
+            if (totalWritten + 1 < tempFileCreationThreshold) {
+                // write to our in-memory byte array
+                baos.write(b);
+                totalWritten++;
+                return;
+            }
+            // rollover into a file
+            transferToFile();
+            writeToFile(b);
+        }
+
+        @Override
+        public void write(final byte[] b) throws IOException {
+            write(b, 0, b.length);
+        }
+
+        @Override
+        public void write(final byte[] b, final int off, final int len) throws IOException {
+            if (tmpFileOS != null) {
+                // already rolled over, write to the file that has been created previously
+                writeToFile(b, off, len);
+                return;
+            }
+            if (totalWritten + len < tempFileCreationThreshold) {
+                // write to our in-memory byte array
+                baos.write(b, off, len);
+                totalWritten += len;
+                return;
+            }
+            // rollover into a file
+            transferToFile();
+            writeToFile(b, off, len);
+        }
+
+        @Override
+        public void flush() throws IOException {
+            if (tmpFileOS != null) {
+                tmpFileOS.flush();
+            }
+        }
+
+        @Override
+        public void close() throws IOException {
+            baos = null;
+            if (tmpFileOS != null) {
+                tmpFileOS.close();
+            }
+        }
+
+        private void writeToFile(int b) throws IOException {
+            tmpFileOS.write(b);
+            totalWritten++;
+        }
+
+        private void writeToFile(byte[] b, int off, int len) throws IOException {
+            tmpFileOS.write(b, off, len);
+            totalWritten += len;
+        }
+
+        private void transferToFile() throws IOException {
+            // create a tempfile
+            entry.file = getTempPathForEntry(null);
+            tmpFileOS = new BufferedOutputStream(Files.newOutputStream(entry.file));
+            // transfer the already written data from the byte array buffer into this tempfile
+            baos.writeTo(tmpFileOS);
+            // release the underlying byte array
+            baos = null;
+        }
+
+        private byte[] toByteArray() {
+            return baos == null ? null : baos.toByteArray();
+        }
+    }
+
     private InputStream getInputStream(Entry e)
         throws IOException
     {
@@ -1644,8 +1752,12 @@ public synchronized void close() throws IOException {
                 throw new ZipException("invalid compression method");
             }
             //crc.reset();
-            if (out instanceof ByteArrayOutputStream)
-                e.bytes = ((ByteArrayOutputStream)out).toByteArray();
+            if (out instanceof FileRolloverOutputStream) {
+                FileRolloverOutputStream fros = (FileRolloverOutputStream) out;
+                if (fros.tmpFileOS == null) {
+                    e.bytes = fros.toByteArray();
+                }
+            }
 
             if (e.type == Entry.FILECH) {
                 releaseDeflater(def);

jdk/test/demo/zipfs/LargeCompressedEntrySizeTest.java

@@ -0,0 +1,101 @@
+/*
+ * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.nio.file.FileSystem;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Collections;
+import java.util.Random;
+import java.util.concurrent.TimeUnit;
+import java.net.URI;
+
+
+/**
+ * @test
+ * @bug 8190753 8011146
+ * @summary Verify that using zip filesystem for opening an outputstream for a zip entry whose
+ * compressed size is large, doesn't run into "Negative initial size" exception
+ * @run testng/manual/othervm LargeCompressedEntrySizeTest
+ */
+public class LargeCompressedEntrySizeTest {
+
+    private static final String LARGE_FILE_NAME = "LargeZipEntry.txt";
+    private static final String ZIP_FILE_NAME = "8190753-test-compressed-size.zip";
+
+    @BeforeMethod
+    public void setUp() throws IOException {
+        deleteFiles();
+    }
+
+    @AfterMethod
+    public void tearDown() throws IOException {
+        deleteFiles();
+    }
+
+    /**
+     * Delete the files created for use by the test
+     *
+     * @throws IOException if an error occurs deleting the files
+     */
+    private static void deleteFiles() throws IOException {
+        Files.deleteIfExists(Paths.get(ZIP_FILE_NAME));
+    }
+
+
+    /**
+     * Using zip filesystem, creates a zip file and writes out a zip entry whose compressed size is
+     * expected to be greater than 2gb.
+     */
+    @Test
+    public void testLargeCompressedSizeWithZipFS() throws Exception {
+        final Path zipFile = Paths.get(ZIP_FILE_NAME);
+        final URI uri = URI.create("jar:" + zipFile.toUri());
+        final long largeEntrySize = 6L * 1024L * 1024L * 1024L; // large value which exceeds Integer.MAX_VALUE
+        try (FileSystem fs = FileSystems.newFileSystem(uri, Collections.singletonMap("create", "true"))) {
+            try (OutputStream os = Files.newOutputStream(fs.getPath(LARGE_FILE_NAME))) {
+                long remaining = largeEntrySize;
+                // create a chunk of random bytes which we keep writing out
+                final int chunkSize = 102400;
+                final byte[] chunk = new byte[chunkSize];
+                new Random().nextBytes(chunk);
+                final long start = System.currentTimeMillis();
+                for (long l = 0; l < largeEntrySize; l += chunkSize) {
+                    final int numToWrite = (int) Math.min(remaining, chunkSize);
+                    os.write(chunk, 0, numToWrite);
+                    remaining -= numToWrite;
+                }
+                System.out.println("Took " + TimeUnit.SECONDS.toSeconds(System.currentTimeMillis() - start)
+                        + " seconds to generate entry of size " + largeEntrySize);
+            }
+        }
+    }
+}