Skip to content

Commit 8a9cda2

Browse files
Alexey PavlyutkinYuri Nesterenko
Alexey Pavlyutkin
authored and
Yuri Nesterenko
committedOct 28, 2021
8190753: (zipfs): Accessing a large entry (> 2^31 bytes) leads to a negative initial size for ByteArrayOutputStream
Reviewed-by: mdoerr Backport-of: c3d8e92
·
jdk-11.0.28+0jdk-11.0.14-ga
1 parent 352672f commit 8a9cda2

File tree

3 files changed

+368
-6
lines changed

3 files changed

+368
-6
lines changed
 

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

+124-6
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,11 @@ class ZipFileSystem extends FileSystem {
9393
private static final boolean isWindows = AccessController.doPrivileged(
9494
(PrivilegedAction<Boolean>)() -> System.getProperty("os.name")
9595
.startsWith("Windows"));
96+
97+
// a threshold, in bytes, to decide whether to create a temp file
98+
// for outputstream of a zip entry
99+
private final int tempFileCreationThreshold = 10 * 1024 * 1024; // 10 MB
100+
96101
private final boolean forceEnd64;
97102
private final int defaultMethod; // METHOD_STORED if "noCompression=true"
98103
// METHOD_DEFLATED otherwise
@@ -1450,11 +1455,11 @@ private OutputStream getOutputStream(Entry e) throws IOException {
14501455
if (zc.isUTF8())
14511456
e.flag |= FLAG_USE_UTF8;
14521457
OutputStream os;
1453-
if (useTempFile) {
1458+
if (useTempFile || e.size >= tempFileCreationThreshold) {
14541459
e.file = getTempPathForEntry(null);
14551460
os = Files.newOutputStream(e.file, WRITE);
14561461
} else {
1457-
os = new ByteArrayOutputStream((e.size > 0)? (int)e.size : 8192);
1462+
os = new FileRolloverOutputStream(e);
14581463
}
14591464
if (e.method == METHOD_DEFLATED) {
14601465
return new DeflatingEntryOutputStream(e, os);
@@ -1494,8 +1499,12 @@ public synchronized void close() throws IOException {
14941499
}
14951500
isClosed = true;
14961501
e.size = written;
1497-
if (out instanceof ByteArrayOutputStream)
1498-
e.bytes = ((ByteArrayOutputStream)out).toByteArray();
1502+
if (out instanceof FileRolloverOutputStream) {
1503+
FileRolloverOutputStream fros = (FileRolloverOutputStream) out;
1504+
if (fros.tmpFileOS == null) {
1505+
e.bytes = fros.toByteArray();
1506+
}
1507+
}
14991508
super.close();
15001509
update(e);
15011510
}
@@ -1530,8 +1539,12 @@ public synchronized void close() throws IOException {
15301539
e.size = def.getBytesRead();
15311540
e.csize = def.getBytesWritten();
15321541
e.crc = crc.getValue();
1533-
if (out instanceof ByteArrayOutputStream)
1534-
e.bytes = ((ByteArrayOutputStream)out).toByteArray();
1542+
if (out instanceof FileRolloverOutputStream) {
1543+
FileRolloverOutputStream fros = (FileRolloverOutputStream) out;
1544+
if (fros.tmpFileOS == null) {
1545+
e.bytes = fros.toByteArray();
1546+
}
1547+
}
15351548
super.close();
15361549
update(e);
15371550
releaseDeflater(def);
@@ -1614,6 +1627,111 @@ public void close() throws IOException {
16141627
}
16151628
}
16161629

1630+
// A wrapper around the ByteArrayOutputStream. This FileRolloverOutputStream
1631+
// uses a threshold size to decide if the contents being written need to be
1632+
// rolled over into a temporary file. Until the threshold is reached, writes
1633+
// on this outputstream just write it to the internal in-memory byte array
1634+
// held by the ByteArrayOutputStream. Once the threshold is reached, the
1635+
// write operation on this outputstream first (and only once) creates a temporary file
1636+
// and transfers the data that has so far been written in the internal
1637+
// byte array, to that newly created file. The temp file is then opened
1638+
// in append mode and any subsequent writes, including the one which triggered
1639+
// the temporary file creation, will be written to the file.
1640+
// Implementation note: the "write" and the "close" methods of this implementation
1641+
// aren't "synchronized" because this FileRolloverOutputStream gets called
1642+
// only from either DeflatingEntryOutputStream or EntryOutputStream, both of which
1643+
// already have the necessary "synchronized" before calling these methods.
1644+
private class FileRolloverOutputStream extends OutputStream {
1645+
private ByteArrayOutputStream baos = new ByteArrayOutputStream(8192);
1646+
private final Entry entry;
1647+
private OutputStream tmpFileOS;
1648+
private long totalWritten = 0;
1649+
1650+
private FileRolloverOutputStream(final Entry e) {
1651+
this.entry = e;
1652+
}
1653+
1654+
@Override
1655+
public void write(final int b) throws IOException {
1656+
if (tmpFileOS != null) {
1657+
// already rolled over, write to the file that has been created previously
1658+
writeToFile(b);
1659+
return;
1660+
}
1661+
if (totalWritten + 1 < tempFileCreationThreshold) {
1662+
// write to our in-memory byte array
1663+
baos.write(b);
1664+
totalWritten++;
1665+
return;
1666+
}
1667+
// rollover into a file
1668+
transferToFile();
1669+
writeToFile(b);
1670+
}
1671+
1672+
@Override
1673+
public void write(final byte[] b) throws IOException {
1674+
write(b, 0, b.length);
1675+
}
1676+
1677+
@Override
1678+
public void write(final byte[] b, final int off, final int len) throws IOException {
1679+
if (tmpFileOS != null) {
1680+
// already rolled over, write to the file that has been created previously
1681+
writeToFile(b, off, len);
1682+
return;
1683+
}
1684+
if (totalWritten + len < tempFileCreationThreshold) {
1685+
// write to our in-memory byte array
1686+
baos.write(b, off, len);
1687+
totalWritten += len;
1688+
return;
1689+
}
1690+
// rollover into a file
1691+
transferToFile();
1692+
writeToFile(b, off, len);
1693+
}
1694+
1695+
@Override
1696+
public void flush() throws IOException {
1697+
if (tmpFileOS != null) {
1698+
tmpFileOS.flush();
1699+
}
1700+
}
1701+
1702+
@Override
1703+
public void close() throws IOException {
1704+
baos = null;
1705+
if (tmpFileOS != null) {
1706+
tmpFileOS.close();
1707+
}
1708+
}
1709+
1710+
private void writeToFile(int b) throws IOException {
1711+
tmpFileOS.write(b);
1712+
totalWritten++;
1713+
}
1714+
1715+
private void writeToFile(byte[] b, int off, int len) throws IOException {
1716+
tmpFileOS.write(b, off, len);
1717+
totalWritten += len;
1718+
}
1719+
1720+
private void transferToFile() throws IOException {
1721+
// create a tempfile
1722+
entry.file = getTempPathForEntry(null);
1723+
tmpFileOS = new BufferedOutputStream(Files.newOutputStream(entry.file));
1724+
// transfer the already written data from the byte array buffer into this tempfile
1725+
baos.writeTo(tmpFileOS);
1726+
// release the underlying byte array
1727+
baos = null;
1728+
}
1729+
1730+
private byte[] toByteArray() {
1731+
return baos == null ? null : baos.toByteArray();
1732+
}
1733+
}
1734+
16171735
private InputStream getInputStream(Entry e)
16181736
throws IOException
16191737
{

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

+101
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
/*
2+
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*
23+
*/
24+
25+
import org.testng.annotations.AfterMethod;
26+
import org.testng.annotations.BeforeMethod;
27+
import org.testng.annotations.Test;
28+
29+
import java.io.IOException;
30+
import java.io.OutputStream;
31+
import java.nio.file.FileSystem;
32+
import java.nio.file.FileSystems;
33+
import java.nio.file.Files;
34+
import java.nio.file.Path;
35+
import java.util.Collections;
36+
import java.util.Random;
37+
import java.util.concurrent.TimeUnit;
38+
import java.net.URI;
39+
40+
41+
/**
42+
* @test
43+
* @bug 8190753 8011146
44+
* @summary Verify that using zip filesystem for opening an outputstream for a zip entry whose
45+
* compressed size is large, doesn't run into "Negative initial size" exception
46+
* @run testng/manual/othervm LargeCompressedEntrySizeTest
47+
*/
48+
public class LargeCompressedEntrySizeTest {
49+
50+
private static final String LARGE_FILE_NAME = "LargeZipEntry.txt";
51+
private static final String ZIP_FILE_NAME = "8190753-test-compressed-size.zip";
52+
53+
@BeforeMethod
54+
public void setUp() throws IOException {
55+
deleteFiles();
56+
}
57+
58+
@AfterMethod
59+
public void tearDown() throws IOException {
60+
deleteFiles();
61+
}
62+
63+
/**
64+
* Delete the files created for use by the test
65+
*
66+
* @throws IOException if an error occurs deleting the files
67+
*/
68+
private static void deleteFiles() throws IOException {
69+
Files.deleteIfExists(Path.of(ZIP_FILE_NAME));
70+
}
71+
72+
73+
/**
74+
* Using zip filesystem, creates a zip file and writes out a zip entry whose compressed size is
75+
* expected to be greater than 2gb.
76+
*/
77+
@Test
78+
public void testLargeCompressedSizeWithZipFS() throws Exception {
79+
final Path zipFile = Path.of(ZIP_FILE_NAME);
80+
final URI uri = URI.create("jar:" + zipFile.toUri());
81+
final long largeEntrySize = 6L * 1024L * 1024L * 1024L; // large value which exceeds Integer.MAX_VALUE
82+
try (FileSystem fs = FileSystems.newFileSystem(uri, Collections.singletonMap("create", "true"))) {
83+
try (OutputStream os = Files.newOutputStream(fs.getPath(LARGE_FILE_NAME))) {
84+
long remaining = largeEntrySize;
85+
// create a chunk of random bytes which we keep writing out
86+
final int chunkSize = 102400;
87+
final byte[] chunk = new byte[chunkSize];
88+
new Random().nextBytes(chunk);
89+
final long start = System.currentTimeMillis();
90+
for (long l = 0; l < largeEntrySize; l += chunkSize) {
91+
final int numToWrite = (int) Math.min(remaining, chunkSize);
92+
os.write(chunk, 0, numToWrite);
93+
remaining -= numToWrite;
94+
}
95+
System.out.println("Took " + TimeUnit.SECONDS.toSeconds(System.currentTimeMillis() - start)
96+
+ " seconds to generate entry of size " + largeEntrySize);
97+
}
98+
}
99+
}
100+
101+
}

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

+143
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
/*
2+
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*
23+
*/
24+
25+
import org.testng.Assert;
26+
import org.testng.annotations.AfterMethod;
27+
import org.testng.annotations.BeforeMethod;
28+
import org.testng.annotations.DataProvider;
29+
import org.testng.annotations.Test;
30+
31+
import java.io.IOException;
32+
import java.io.InputStream;
33+
import java.io.OutputStream;
34+
import java.io.File;
35+
import java.net.URI;
36+
import java.nio.file.FileSystem;
37+
import java.nio.file.FileSystems;
38+
import java.nio.file.Files;
39+
import java.nio.file.Path;
40+
import java.util.Map;
41+
import java.util.Random;
42+
43+
44+
/**
45+
* @test
46+
* @summary Verify that the outputstream created for zip file entries, through the ZipFileSystem
47+
* works fine for varying sizes of the zip file entries
48+
* @bug 8190753 8011146
49+
* @run testng/timeout=300 ZipFSOutputStreamTest
50+
*/
51+
public class ZipFSOutputStreamTest {
52+
// List of files to be added to the ZIP file along with their sizes in bytes
53+
private static final Map<String, Long> ZIP_ENTRIES = Map.of(
54+
"f1", Integer.MAX_VALUE + 1L, // a value which when cast to an integer, becomes a negative value
55+
"f2", 25L * 1024L * 1024L, // 25 MB
56+
"d1/f3", 1234L,
57+
"d1/d2/f4", 0L);
58+
59+
private static final Path ZIP_FILE = Path.of("zipfs-outputstream-test.zip");
60+
61+
@BeforeMethod
62+
public void setUp() throws IOException {
63+
deleteFiles();
64+
}
65+
66+
@AfterMethod
67+
public void tearDown() throws IOException {
68+
deleteFiles();
69+
}
70+
71+
private static void deleteFiles() throws IOException {
72+
Files.deleteIfExists(ZIP_FILE);
73+
}
74+
75+
@DataProvider(name = "zipFSCreationEnv")
76+
private Object[][] zipFSCreationEnv() {
77+
return new Object[][]{
78+
{Map.of("create", "true", "noCompression", "true")}, // STORED
79+
{Map.of("create", "true", "noCompression", "false")} // DEFLATED
80+
81+
};
82+
}
83+
84+
/**
85+
* Create a zip filesystem and write out entries of varying sizes using the outputstream returned
86+
* by the ZipFileSystem. Then verify that the generated zip file entries are as expected,
87+
* both in size and content
88+
*/
89+
@Test(dataProvider = "zipFSCreationEnv")
90+
public void testOutputStream(final Map<String, ?> env) throws Exception {
91+
final URI uri = URI.create("jar:" + ZIP_FILE.toUri() );
92+
final byte[] chunk = new byte[1024];
93+
new Random().nextBytes(chunk);
94+
try (final FileSystem zipfs = FileSystems.newFileSystem(uri, env)) {
95+
// create the zip with varying sized entries
96+
for (final Map.Entry<String, Long> entry : ZIP_ENTRIES.entrySet()) {
97+
final Path entryPath = zipfs.getPath(entry.getKey());
98+
if (entryPath.getParent() != null) {
99+
Files.createDirectories(entryPath.getParent());
100+
}
101+
try (final OutputStream os = Files.newOutputStream(entryPath)) {
102+
writeAsChunks(os, chunk, entry.getValue());
103+
}
104+
}
105+
}
106+
// now verify the written content
107+
try (final FileSystem zipfs = FileSystems.newFileSystem(uri, Map.of())) {
108+
for (final Map.Entry<String, Long> entry : ZIP_ENTRIES.entrySet()) {
109+
final Path entryPath = zipfs.getPath(entry.getKey());
110+
try (final InputStream is = Files.newInputStream(entryPath)) {
111+
final byte[] buf = new byte[chunk.length];
112+
int numRead;
113+
long totalRead = 0;
114+
while ((numRead = is.read(buf)) != -1) {
115+
totalRead += numRead;
116+
// verify the content
117+
for (int i = 0, chunkoffset = (int) ((totalRead - numRead) % chunk.length);
118+
i < numRead; i++, chunkoffset++) {
119+
Assert.assertEquals(buf[i], chunk[chunkoffset % chunk.length],
120+
"Unexpected content in " + entryPath);
121+
}
122+
}
123+
Assert.assertEquals(totalRead, (long) entry.getValue(),
124+
"Unexpected number of bytes read from zip entry " + entryPath);
125+
}
126+
}
127+
}
128+
}
129+
130+
/**
131+
* Repeatedly writes out to the outputstream, the chunk of data, till the number of bytes
132+
* written to the stream equals the totalSize
133+
*/
134+
private static void writeAsChunks(final OutputStream os, final byte[] chunk,
135+
final long totalSize) throws IOException {
136+
long remaining = totalSize;
137+
for (long l = 0; l < totalSize; l += chunk.length) {
138+
final int numToWrite = (int) Math.min(remaining, chunk.length);
139+
os.write(chunk, 0, numToWrite);
140+
remaining -= numToWrite;
141+
}
142+
}
143+
}

1 commit comments

Comments
 (1)

openjdk-notifier[bot] commented on Oct 28, 2021

@openjdk-notifier[bot]
openjdk-notifier[bot]
on Oct 28, 2021

Review

Issues

Please sign in to comment.