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

Reviewed-by: lancea

src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java

@@ -119,6 +119,11 @@ class ZipFileSystem extends FileSystem {
     private final boolean noExtt;        // see readExtra()
     private final boolean useTempFile;   // use a temp file for newOS, default
                                          // is to use BAOS for better performance
+
+    // 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
+
     private final boolean forceEnd64;
     private final int defaultCompressionMethod; // METHOD_STORED if "noCompression=true"
                                                 // METHOD_DEFLATED otherwise
@@ -1944,11 +1949,11 @@ class ZipFileSystem extends FileSystem {
         if (zc.isUTF8())
             e.flag |= FLAG_USE_UTF8;
         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);
         }
         if (e.method == METHOD_DEFLATED) {
             return new DeflatingEntryOutputStream(e, os);
@@ -1988,8 +1993,9 @@ class ZipFileSystem extends FileSystem {
             }
             isClosed = true;
             e.size = written;
-            if (out instanceof ByteArrayOutputStream)
-                e.bytes = ((ByteArrayOutputStream)out).toByteArray();
+            if (out instanceof FileRolloverOutputStream fros && fros.tmpFileOS == null) {
+                e.bytes = fros.toByteArray();
+            }
             super.close();
             update(e);
         }
@@ -2024,8 +2030,9 @@ class ZipFileSystem extends FileSystem {
             e.size  = def.getBytesRead();
             e.csize = def.getBytesWritten();
             e.crc = crc.getValue();
-            if (out instanceof ByteArrayOutputStream)
-                e.bytes = ((ByteArrayOutputStream)out).toByteArray();
+            if (out instanceof FileRolloverOutputStream fros && fros.tmpFileOS == null) {
+                e.bytes = fros.toByteArray();
+            }
             super.close();
             update(e);
             releaseDeflater(def);
@@ -2107,6 +2114,111 @@ class ZipFileSystem extends FileSystem {
         }
     }
 
+    // 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
     {

test/jdk/jdk/nio/zipfs/LargeCompressedEntrySizeTest.java

@@ -0,0 +1,98 @@
+/*
+ * 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.util.Collections;
+import java.util.Random;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * @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(Path.of(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 = Path.of(ZIP_FILE_NAME);
+        final long largeEntrySize = 6L * 1024L * 1024L * 1024L; // large value which exceeds Integer.MAX_VALUE
+        try (FileSystem fs = FileSystems.newFileSystem(zipFile, 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);
+            }
+        }
+    }
+
+}

test/jdk/jdk/nio/zipfs/ZipFSOutputStreamTest.java

@@ -0,0 +1,140 @@
+/*
+ * 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.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import java.io.IOException;
+import java.io.InputStream;
+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.util.Map;
+import java.util.Random;
+
+
+/**
+ * @test
+ * @summary Verify that the outputstream created for zip file entries, through the ZipFileSystem
+ * works fine for varying sizes of the zip file entries
+ * @bug 8190753 8011146
+ * @run testng/timeout=300 ZipFSOutputStreamTest
+ */
+public class ZipFSOutputStreamTest {
+    // List of files to be added to the ZIP file along with their sizes in bytes
+    private static final Map<String, Long> ZIP_ENTRIES = Map.of(
+            "f1", Integer.MAX_VALUE + 1L, // a value which when cast to an integer, becomes a negative value
+            "f2", 25L * 1024L * 1024L, // 25 MB
+            "d1/f3", 1234L,
+            "d1/d2/f4", 0L);
+
+    private static final Path ZIP_FILE = Path.of("zipfs-outputstream-test.zip");
+
+    @BeforeMethod
+    public void setUp() throws IOException {
+        deleteFiles();
+    }
+
+    @AfterMethod
+    public void tearDown() throws IOException {
+        deleteFiles();
+    }
+
+    private static void deleteFiles() throws IOException {
+        Files.deleteIfExists(ZIP_FILE);
+    }
+
+    @DataProvider(name = "zipFSCreationEnv")
+    private Object[][] zipFSCreationEnv() {
+        return new Object[][]{
+                {Map.of("create", "true", "noCompression", "true")}, // STORED
+                {Map.of("create", "true", "noCompression", "false")} // DEFLATED
+
+        };
+    }
+
+    /**
+     * Create a zip filesystem and write out entries of varying sizes using the outputstream returned
+     * by the ZipFileSystem. Then verify that the generated zip file entries are as expected,
+     * both in size and content
+     */
+    @Test(dataProvider = "zipFSCreationEnv")
+    public void testOutputStream(final Map<String, ?> env) throws Exception {
+        final byte[] chunk = new byte[1024];
+        new Random().nextBytes(chunk);
+        try (final FileSystem zipfs = FileSystems.newFileSystem(ZIP_FILE, env)) {
+            // create the zip with varying sized entries
+            for (final Map.Entry<String, Long> entry : ZIP_ENTRIES.entrySet()) {
+                final Path entryPath = zipfs.getPath(entry.getKey());
+                if (entryPath.getParent() != null) {
+                    Files.createDirectories(entryPath.getParent());
+                }
+                try (final OutputStream os = Files.newOutputStream(entryPath)) {
+                    writeAsChunks(os, chunk, entry.getValue());
+                }
+            }
+        }
+        // now verify the written content
+        try (final FileSystem zipfs = FileSystems.newFileSystem(ZIP_FILE)) {
+            for (final Map.Entry<String, Long> entry : ZIP_ENTRIES.entrySet()) {
+                final Path entryPath = zipfs.getPath(entry.getKey());
+                try (final InputStream is = Files.newInputStream(entryPath)) {
+                    final byte[] buf = new byte[chunk.length];
+                    int numRead;
+                    long totalRead = 0;
+                    while ((numRead = is.read(buf)) != -1) {
+                        totalRead += numRead;
+                        // verify the content
+                        for (int i = 0, chunkoffset = (int) ((totalRead - numRead) % chunk.length);
+                             i < numRead; i++, chunkoffset++) {
+                            Assert.assertEquals(buf[i], chunk[chunkoffset % chunk.length],
+                                    "Unexpected content in " + entryPath);
+                        }
+                    }
+                    Assert.assertEquals(totalRead, (long) entry.getValue(),
+                            "Unexpected number of bytes read from zip entry " + entryPath);
+                }
+            }
+        }
+    }
+
+    /**
+     * Repeatedly writes out to the outputstream, the chunk of data, till the number of bytes
+     * written to the stream equals the totalSize
+     */
+    private static void writeAsChunks(final OutputStream os, final byte[] chunk,
+                                      final long totalSize) throws IOException {
+        long remaining = totalSize;
+        for (long l = 0; l < totalSize; l += chunk.length) {
+            final int numToWrite = (int) Math.min(remaining, chunk.length);
+            os.write(chunk, 0, numToWrite);
+            remaining -= numToWrite;
+        }
+    }
+}