6907252: ZipFileInputStream Not Thread-Safe
Reviewed-by: sherman
diff --git a/jdk/src/share/classes/java/util/zip/ZStreamRef.java b/jdk/src/share/classes/java/util/zip/ZStreamRef.java
index 980d47f..09b2f30 100644
--- a/jdk/src/share/classes/java/util/zip/ZStreamRef.java
+++ b/jdk/src/share/classes/java/util/zip/ZStreamRef.java
@@ -31,7 +31,7 @@
class ZStreamRef {
- private long address;
+ private volatile long address;
ZStreamRef (long address) {
this.address = address;
}
diff --git a/jdk/src/share/classes/java/util/zip/ZipFile.java b/jdk/src/share/classes/java/util/zip/ZipFile.java
index e6b9f01..adf8057 100644
--- a/jdk/src/share/classes/java/util/zip/ZipFile.java
+++ b/jdk/src/share/classes/java/util/zip/ZipFile.java
@@ -58,7 +58,7 @@
*/
public
class ZipFile implements ZipConstants, Closeable {
- private long jzfile; // address of jzfile data
+ private long jzfile; // address of jzfile data
private final String name; // zip file name
private final int total; // total number of entries
private final boolean locsig; // if zip file starts with LOCSIG (usually true)
@@ -685,7 +685,7 @@
* (possibly compressed) zip file entry.
*/
private class ZipFileInputStream extends InputStream {
- private volatile boolean closeRequested = false;
+ private volatile boolean zfisCloseRequested = false;
protected long jzentry; // address of jzentry data
private long pos; // current position within entry data
protected long rem; // number of remaining bytes within entry
@@ -712,6 +712,7 @@
len = (int) rem;
}
+ // Check if ZipFile open
ensureOpenOrZipException();
len = ZipFile.read(ZipFile.this.jzfile, jzentry, pos, b,
off, len);
@@ -755,9 +756,9 @@
}
public void close() {
- if (closeRequested)
+ if (zfisCloseRequested)
return;
- closeRequested = true;
+ zfisCloseRequested = true;
rem = 0;
synchronized (ZipFile.this) {
diff --git a/jdk/src/share/native/java/util/zip/zip_util.c b/jdk/src/share/native/java/util/zip/zip_util.c
index 05ef55d..3245dea 100644
--- a/jdk/src/share/native/java/util/zip/zip_util.c
+++ b/jdk/src/share/native/java/util/zip/zip_util.c
@@ -1314,12 +1314,23 @@
jint
ZIP_Read(jzfile *zip, jzentry *entry, jlong pos, void *buf, jint len)
{
- jlong entry_size = (entry->csize != 0) ? entry->csize : entry->size;
+ jlong entry_size;
jlong start;
+ if (zip == 0) {
+ return -1;
+ }
+
/* Clear previous zip error */
zip->msg = NULL;
+ if (entry == 0) {
+ zip->msg = "ZIP_Read: jzentry is NULL";
+ return -1;
+ }
+
+ entry_size = (entry->csize != 0) ? entry->csize : entry->size;
+
/* Check specified position */
if (pos < 0 || pos > entry_size - 1) {
zip->msg = "ZIP_Read: specified offset out of range";
@@ -1451,6 +1462,11 @@
char *msg;
char tmpbuf[1024];
+ if (entry == 0) {
+ jio_fprintf(stderr, "jzentry was invalid");
+ return JNI_FALSE;
+ }
+
strcpy(entryname, entry->name);
if (entry->csize == 0) {
/* Entry is stored */
diff --git a/jdk/test/java/util/zip/ZipFile/ZipEntryFreeTest.java b/jdk/test/java/util/zip/ZipFile/ZipEntryFreeTest.java
new file mode 100644
index 0000000..7695a33
--- /dev/null
+++ b/jdk/test/java/util/zip/ZipFile/ZipEntryFreeTest.java
@@ -0,0 +1,130 @@
+/*
+ * Copyright (c) 2015, 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.
+ */
+
+/* @test
+ * @bug 6907252
+ * @summary ZipFileInputStream Not Thread-Safe
+ * @library /lib/testlibrary
+ * @build jdk.testlibrary.*
+ * @run main ZipEntryFreeTest
+ */
+
+import java.io.*;
+import java.nio.file.Paths;
+import java.util.Random;
+import java.util.Timer;
+import java.util.TimerTask;
+import java.util.zip.*;
+import jdk.testlibrary.FileUtils;
+
+public class ZipEntryFreeTest extends Thread {
+
+ private static final int NUM_THREADS = 5;
+ private static final int TEST_ITERATIONS = 5;
+ private static final String ZIPFILE_NAME = "large.zip";
+ private static final String ZIPENTRY_NAME = "random.txt";
+ private static InputStream is = null;
+ final Timer timer = new Timer();
+
+ public static void main(String args[]) throws Exception {
+ createZipFile();
+ try {
+ for (int i = 0; i < TEST_ITERATIONS; i++) {
+ runTest();
+ }
+ } finally {
+ FileUtils.deleteFileIfExistsWithRetry(Paths.get(ZIPFILE_NAME));
+ }
+ }
+
+ private static void runTest() throws Exception {
+ try (ZipFile zf = new ZipFile(new File(ZIPFILE_NAME))) {
+ is = zf.getInputStream(zf.getEntry(ZIPENTRY_NAME + "_0"));
+ Thread[] threadArray = new Thread[NUM_THREADS];
+ for (int i = 0; i < threadArray.length; i++) {
+ threadArray[i] = new ZipEntryFreeTest();
+ }
+ for (int i = 0; i < threadArray.length; i++) {
+ threadArray[i].start();
+ }
+ for (int i = 0; i < threadArray.length; i++) {
+ threadArray[i].join();
+ }
+ }
+ }
+
+ private static void createZipFile() throws Exception {
+ Random rnd = new Random(1000L);
+ byte[] contents = new byte[2_000_000];
+ ZipEntry ze = null;
+
+ try (ZipOutputStream zos =
+ new ZipOutputStream(new FileOutputStream(ZIPFILE_NAME))) {
+ // uncompressed mode seemed to tickle the crash
+ zos.setMethod(ZipOutputStream.STORED);
+ for (int ze_count = 0; ze_count < 10; ze_count++) {
+ rnd.nextBytes(contents);
+ ze = createZipEntry(contents, ze_count);
+ zos.putNextEntry(ze);
+ zos.write(contents, 0, contents.length);
+ }
+ zos.flush();
+ }
+ }
+
+ private static ZipEntry createZipEntry(byte[] b, int i) {
+ ZipEntry ze = new ZipEntry(ZIPENTRY_NAME + "_" + i);
+ ze.setCompressedSize(b.length);
+ ze.setSize(b.length);
+ CRC32 crc = new CRC32();
+ crc.update(b);
+ ze.setCrc(crc.getValue());
+ return ze;
+ }
+
+ @Override
+ public void run() {
+ try {
+ int iteration = 0;
+ TimerTask tt = (new TimerTask() {
+ @Override
+ public void run() {
+ try {
+ is.close();
+ } catch (Exception ex) {
+ ex.printStackTrace(System.out);
+ }
+ }
+ });
+ timer.schedule(tt, 50);
+ while (is.read() != -1 && iteration++ < 1_000) { }
+ } catch (ZipException ze) {
+ // ZipException now expected instead of ZIP_Read crash
+ System.out.println(ze);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ } finally {
+ timer.cancel();
+ }
+ }
+}
