6907252: ZipFileInputStream Not Thread-Safe

Reviewed-by: sherman

src/share/classes/java/util/zip/ZStreamRef.java

@@ -31,7 +31,7 @@ package java.util.zip;
 
 class ZStreamRef {
 
-    private long address;
+    private volatile long address;
     ZStreamRef (long address) {
         this.address = address;
     }

src/share/classes/java/util/zip/ZipFile.java

@@ -58,7 +58,7 @@ import static java.util.zip.ZipConstants64.*;
  */
 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 @@ class ZipFile implements ZipConstants, Closeable {
      * (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 @@ class ZipFile implements ZipConstants, Closeable {
                     len = (int) rem;
                 }
 
+                // Check if ZipFile open
                 ensureOpenOrZipException();
                 len = ZipFile.read(ZipFile.this.jzfile, jzentry, pos, b,
                                    off, len);
@@ -755,9 +756,9 @@ class ZipFile implements ZipConstants, Closeable {
         }
 
         public void close() {
-            if (closeRequested)
+            if (zfisCloseRequested)
                 return;
-            closeRequested = true;
+            zfisCloseRequested = true;
 
             rem = 0;
             synchronized (ZipFile.this) {

src/share/native/java/util/zip/zip_util.c

@@ -1314,12 +1314,23 @@ ZIP_GetEntryDataOffset(jzfile *zip, jzentry *entry)
 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 @@ ZIP_ReadEntry(jzfile *zip, jzentry *entry, unsigned char *buf, char *entryname)
     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 */

test/java/util/zip/ZipFile/ZipEntryFreeTest.java

+/*
+ * 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();
+        }
+    }
+}