Skip to content

Commit

Permalink
8190753: (zipfs): Accessing a large entry (> 2^31 bytes) leads to a n…
Browse files Browse the repository at this point in the history
…egative initial size for ByteArrayOutputStream

Reviewed-by: mdoerr
Backport-of: c3d8e92
  • Loading branch information
Alexey Pavlyutkin authored and Yuri Nesterenko committed Oct 28, 2021
1 parent 352672f commit 8a9cda2
Show file tree
Hide file tree
Showing 3 changed files with 368 additions and 6 deletions.
130 changes: 124 additions & 6 deletions src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ class ZipFileSystem extends FileSystem {
private static final boolean isWindows = AccessController.doPrivileged(
(PrivilegedAction<Boolean>)() -> 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

private final boolean forceEnd64;
private final int defaultMethod; // METHOD_STORED if "noCompression=true"
// METHOD_DEFLATED otherwise
Expand Down Expand Up @@ -1450,11 +1455,11 @@ private OutputStream getOutputStream(Entry e) throws IOException {
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);
Expand Down Expand Up @@ -1494,8 +1499,12 @@ public synchronized void close() throws IOException {
}
isClosed = true;
e.size = written;
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();
}
}
super.close();
update(e);
}
Expand Down Expand Up @@ -1530,8 +1539,12 @@ public synchronized void close() throws IOException {
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) {
FileRolloverOutputStream fros = (FileRolloverOutputStream) out;
if (fros.tmpFileOS == null) {
e.bytes = fros.toByteArray();
}
}
super.close();
update(e);
releaseDeflater(def);
Expand Down Expand Up @@ -1614,6 +1627,111 @@ public void close() throws IOException {
}
}

// 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
{
Expand Down
101 changes: 101 additions & 0 deletions test/jdk/jdk/nio/zipfs/LargeCompressedEntrySizeTest.java
Original file line number Diff line number Diff line change
@@ -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.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(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 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);
}
}
}

}
Loading

1 comment on commit 8a9cda2

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.