diff --git a/jdk/src/share/demo/nio/zipfs/src/com/sun/nio/zipfs/ZipFileSystem.java b/jdk/src/share/demo/nio/zipfs/src/com/sun/nio/zipfs/ZipFileSystem.java index a3107f559b6..26e2a5bf9e9 100644 --- a/jdk/src/share/demo/nio/zipfs/src/com/sun/nio/zipfs/ZipFileSystem.java +++ b/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 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); diff --git a/jdk/test/demo/zipfs/LargeCompressedEntrySizeTest.java b/jdk/test/demo/zipfs/LargeCompressedEntrySizeTest.java new file mode 100644 index 00000000000..ba339648bc0 --- /dev/null +++ b/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); + } + } + } +} diff --git a/jdk/test/demo/zipfs/ZipFSOutputStreamTest.java b/jdk/test/demo/zipfs/ZipFSOutputStreamTest.java new file mode 100644 index 00000000000..825a4449055 --- /dev/null +++ b/jdk/test/demo/zipfs/ZipFSOutputStreamTest.java @@ -0,0 +1,146 @@ +/* + * 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.io.File; +import java.net.URI; +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.Map; +import java.util.HashMap; +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=3600 ZipFSOutputStreamTest + */ +public class ZipFSOutputStreamTest { + // List of files to be added to the ZIP file along with their sizes in bytes + private static final Map ZIP_ENTRIES = Collections.unmodifiableMap(new HashMap() {{ + put("f1", Integer.MAX_VALUE + 1L); // a value which when cast to an integer, becomes a negative value + put("f2", 25L * 1024L * 1024L); // 25 MB + put("d1/f3", 1234L); + put("d1/d2/f4", 0L); + }}); + + private static final Path ZIP_FILE = Paths.get("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[][] { + { Collections.unmodifiableMap(new HashMap() {{ put("create", "true"); put("noCompression", "true"); }}) }, + { Collections.unmodifiableMap(new HashMap() {{ put("create", "true"); put("noCompression", "false"); }}) } + }; + } + + /** + * 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 env) throws Exception { + final URI uri = URI.create("jar:" + ZIP_FILE.toUri()); + final byte[] chunk = new byte[1024]; + new Random().nextBytes(chunk); + try (final FileSystem zipfs = FileSystems.newFileSystem(uri, env)) { + // create the zip with varying sized entries + for (final Map.Entry 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(uri, Collections.emptyMap())) { + for (final Map.Entry 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; + } + } +}