6707281: Adler32.update() JavaDoc is wrong

6553961: java.util.zip.{CRC32,Adler32}.update(int) doc errors
6646605: Missing method ZipFile.getComment()
6841232: ZipFile should implement Closeable
4985614: Failure on calls to ZipFile constructor
5032358: "java.util.zip.ZipException: The system cannot find the file specified"
6846616: java/util/zip/ZipFile/ReadAfterClose.java failed after fix for 6735255
Summary: some misc bug/rfe fixes for zipfile
Reviewed-by: alanb

make/java/java/mapfile-vers

@@ -280,6 +280,9 @@ SUNWprivate_1.1 {
                 Java_sun_misc_VM_initialize;
 		Java_sun_misc_VMSupport_initAgentProperties;
 
+                # ZipFile.c needs this one
+		throwFileNotFoundException;
+
 #		Java_sun_misc_VM_getState;			threads.c
 #		Java_sun_misc_VM_threadsSuspended;		threads.c
 #		Java_sun_misc_VM_unsuspendSomeThreads;		threads.c

make/java/zip/mapfile-vers

@@ -51,6 +51,7 @@ SUNWprivate_1.1 {
 		Java_java_util_zip_Inflater_reset;
 		Java_java_util_zip_Inflater_setDictionary;
 		Java_java_util_zip_ZipFile_close;
+		Java_java_util_zip_ZipFile_getCommentBytes;
 		Java_java_util_zip_ZipFile_freeEntry;
 		Java_java_util_zip_ZipFile_getEntry;
 		Java_java_util_zip_ZipFile_getEntryBytes;

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

@@ -43,18 +43,18 @@ class Adler32 implements Checksum {
     public Adler32() {
     }
 
-
     /**
-     * Updates checksum with specified byte.
+     * Updates the checksum with the specified byte (the low eight
+     * bits of the argument b).
      *
-     * @param b an array of bytes
+     * @param b the byte to update the checksum with
      */
     public void update(int b) {
         adler = update(adler, b);
     }
 
     /**
-     * Updates checksum with specified array of bytes.
+     * Updates the checksum with the specified array of bytes.
      */
     public void update(byte[] b, int off, int len) {
         if (b == null) {
@@ -67,21 +67,23 @@ class Adler32 implements Checksum {
     }
 
     /**
-     * Updates checksum with specified array of bytes.
+     * Updates the checksum with the specified array of bytes.
+     *
+     * @param b the byte array to update the checksum with
      */
     public void update(byte[] b) {
         adler = updateBytes(adler, b, 0, b.length);
     }
 
     /**
-     * Resets checksum to initial value.
+     * Resets the checksum to initial value.
      */
     public void reset() {
         adler = 1;
     }
 
     /**
-     * Returns checksum value.
+     * Returns the checksum value.
      */
     public long getValue() {
         return (long)adler & 0xffffffffL;

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

@@ -43,14 +43,17 @@ class CRC32 implements Checksum {
 
 
     /**
-     * Updates CRC-32 with specified byte.
+     * Updates the CRC-32 checksum with the specified byte (the low
+     * eight bits of the argument b).
+     *
+     * @param b the byte to update the checksum with
      */
     public void update(int b) {
         crc = update(crc, b);
     }
 
     /**
-     * Updates CRC-32 with specified array of bytes.
+     * Updates the CRC-32 checksum with the specified array of bytes.
      */
     public void update(byte[] b, int off, int len) {
         if (b == null) {
@@ -63,7 +66,7 @@ class CRC32 implements Checksum {
     }
 
     /**
-     * Updates checksum with specified array of bytes.
+     * Updates the CRC-32 checksum with the specified array of bytes.
      *
      * @param b the array of bytes to update the checksum with
      */

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

@@ -25,6 +25,7 @@
 
 package java.util.zip;
 
+import java.io.Closeable;
 import java.io.InputStream;
 import java.io.IOException;
 import java.io.EOFException;
@@ -47,7 +48,7 @@ import static java.util.zip.ZipConstants64.*;
  * @author      David Connelly
  */
 public
-class ZipFile implements ZipConstants {
+class ZipFile implements ZipConstants, Closeable {
     private long jzfile;  // address of jzfile data
     private String name;  // zip file name
     private int total;    // total number of entries
@@ -248,6 +249,25 @@ class ZipFile implements ZipConstants {
         this(file, OPEN_READ, charset);
     }
 
+    /**
+     * Returns the zip file comment, or null if none.
+     *
+     * @return the comment string for the zip file, or null if none
+     *
+     * @throws IllegalStateException if the zip file has been closed
+     *
+     * Since 1.7
+     */
+    public String getComment() {
+        synchronized (this) {
+            ensureOpen();
+            byte[] bcomm = getCommentBytes(jzfile);
+            if (bcomm == null)
+                return null;
+            return zc.toString(bcomm, bcomm.length);
+        }
+    }
+
     /**
      * Returns the zip file entry for the specified name, or null
      * if not found.
@@ -663,6 +683,7 @@ class ZipFile implements ZipConstants {
     private static native long getEntrySize(long jzentry);
     private static native int getEntryMethod(long jzentry);
     private static native int getEntryFlag(long jzentry);
+    private static native byte[] getCommentBytes(long jzfile);
 
     private static final int JZENTRY_NAME = 0;
     private static final int JZENTRY_EXTRA = 1;

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

@@ -40,6 +40,8 @@
 #include "zip_util.h"
 #ifdef WIN32
 #include "io_util_md.h"
+#else
+#include "io_util.h"
 #endif
 
 #include "java_util_zip_ZipFile.h"
@@ -102,11 +104,12 @@ Java_java_util_zip_ZipFile_open(JNIEnv *env, jclass cls, jstring name,
             }
 #else
             zfd = JVM_Open(path, flag, 0);
-#endif
-
-            if (zfd >= 0) {
-                zip = ZIP_Put_In_Cache(path, zfd, &msg, lastModified);
+            if (zfd < 0) {
+                throwFileNotFoundException(env, name);
+                goto finally;
             }
+#endif
+            zip = ZIP_Put_In_Cache(path, zfd, &msg, lastModified);
         }
 
         if (zip != 0) {
@@ -118,7 +121,6 @@ Java_java_util_zip_ZipFile_open(JNIEnv *env, jclass cls, jstring name,
         } else {
             ThrowZipException(env, "error in opening zip file");
         }
-
 finally:
         JNU_ReleaseStringPlatformChars(env, name, path);
     }
@@ -231,7 +233,25 @@ Java_java_util_zip_ZipFile_getEntryCrc(JNIEnv *env, jclass cls, jlong zentry)
 }
 
 JNIEXPORT jbyteArray JNICALL
-Java_java_util_zip_ZipFile_getEntryBytes(JNIEnv *env, jclass cls, jlong zentry, jint type)
+Java_java_util_zip_ZipFile_getCommentBytes(JNIEnv *env,
+                                           jclass cls,
+                                           jlong zfile)
+{
+    jzfile *zip = jlong_to_ptr(zfile);
+    jbyteArray jba = NULL;
+
+    if (zip->comment != NULL) {
+        if ((jba = (*env)->NewByteArray(env, zip->clen)) == NULL)
+            return NULL;
+        (*env)->SetByteArrayRegion(env, jba, 0, zip->clen, (jbyte*)zip->comment);
+    }
+    return jba;
+}
+
+JNIEXPORT jbyteArray JNICALL
+Java_java_util_zip_ZipFile_getEntryBytes(JNIEnv *env,
+                                         jclass cls,
+                                         jlong zentry, jint type)
 {
     jzentry *ze = jlong_to_ptr(zentry);
     int len = 0;

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

@@ -256,6 +256,8 @@ freeZip(jzfile *zip)
 #else
     free(zip->cencache.data);
 #endif
+    if (zip->comment != NULL)
+        free(zip->comment);
     if (zip->zfd != -1) ZFILE_Close(zip->zfd);
     free(zip);
 }
@@ -265,6 +267,24 @@ static const jlong END_MAXLEN = 0xFFFF + ENDHDR;
 
 #define READBLOCKSZ 128
 
+static jboolean verifyEND(jzfile *zip, jlong endpos, char *endbuf) {
+    /* ENDSIG matched, however the size of file comment in it does not
+       match the real size. One "common" cause for this problem is some
+       "extra" bytes are padded at the end of the zipfile.
+       Let's do some extra verification, we don't care about the performance
+       in this situation.
+     */
+    jlong cenpos = endpos - ENDSIZ(endbuf);
+    jlong locpos = cenpos - ENDOFF(endbuf);
+    char buf[4];
+    return (cenpos >= 0 &&
+            locpos >= 0 &&
+            readFullyAt(zip->zfd, buf, sizeof(buf), cenpos) != -1 &&
+            GETSIG(buf) == CENSIG &&
+            readFullyAt(zip->zfd, buf, sizeof(buf), locpos) != -1 &&
+            GETSIG(buf) == LOCSIG);
+}
+
 /*
  * Searches for end of central directory (END) header. The contents of
  * the END header will be read and placed in endbuf. Returns the file
@@ -280,6 +300,7 @@ findEND(jzfile *zip, void *endbuf)
     const ZFILE zfd = zip->zfd;
     const jlong minHDR = len - END_MAXLEN > 0 ? len - END_MAXLEN : 0;
     const jlong minPos = minHDR - (sizeof(buf)-ENDHDR);
+    jint clen;
 
     for (pos = len - sizeof(buf); pos >= minPos; pos -= (sizeof(buf)-ENDHDR)) {
 
@@ -302,13 +323,31 @@ findEND(jzfile *zip, void *endbuf)
                 buf[i+1] == 'K'    &&
                 buf[i+2] == '\005' &&
                 buf[i+3] == '\006' &&
-                (pos + i + ENDHDR + ENDCOM(buf + i) == len)) {
-                    /* Found END header */
-                    memcpy(endbuf, buf + i, ENDHDR);
-                    return pos + i;
+                ((pos + i + ENDHDR + ENDCOM(buf + i) == len)
+                 || verifyEND(zip, pos + i, buf + i))) {
+                /* Found END header */
+                memcpy(endbuf, buf + i, ENDHDR);
+
+                clen = ENDCOM(endbuf);
+                if (clen != 0) {
+                    zip->comment = malloc(clen + 1);
+                    if (zip->comment == NULL) {
+                        return -1;
+                    }
+                    if (readFullyAt(zfd, zip->comment, clen, pos + i + ENDHDR)
+                        == -1) {
+                        free(zip->comment);
+                        zip->comment = NULL;
+                        return -1;
+                    }
+                    zip->comment[clen] = '\0';
+                    zip->clen = clen;
+                }
+                return pos + i;
             }
         }
     }
+
     return -1; /* END header not found */
 }
 
@@ -654,7 +693,6 @@ readCEN(jzfile *zip, jint knownTotal)
         ZIP_FORMAT_ERROR("invalid CEN header (bad header size)");
 
     zip->total = i;
-
     goto Finally;
 
  Catch:

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

@@ -217,6 +217,7 @@ typedef struct jzfile {   /* Zip file */
     ZFILE zfd;            /* open file descriptor */
     void *lock;           /* read lock */
     char *comment;        /* zip file comment */
+    jint clen;            /* length of the zip file comment */
     char *msg;            /* zip error message */
     jzcell *entries;      /* array of hash cells */
     jint total;           /* total number of entries */

test/java/util/zip/ZipFile/ReadAfterClose.java

@@ -22,7 +22,7 @@
  */
 
 /* @test
-   @bug 4528128
+   @bug 4528128 6846616
    @summary Test if reading InputStream of a closed ZipFile crashes VM
    @author kladko
    */
@@ -40,7 +40,7 @@ public class ReadAfterClose {
         zf.close();
         try {
             in.read();
-        } catch (ZipException e) {
+        } catch (IOException e) {
             return;
         }
         throw new Exception("Test failed.");

test/java/util/zip/ZipFile/ReadZip.java

@@ -22,7 +22,7 @@
  */
 
 /* @test
-   @bug 4241361 4842702
+   @bug 4241361 4842702 4985614 6646605 5032358
    @summary Make sure we can read a zip file.
  */
 
@@ -30,7 +30,7 @@ import java.io.*;
 import java.util.zip.*;
 
 public class ReadZip {
-    private static void Unreached (Object o)
+    private static void unreached (Object o)
         throws Exception
     {
         // Should never get here
@@ -42,10 +42,10 @@ public class ReadZip {
                                           "input.zip"));
 
         // Make sure we throw NPE on null objects
-        try { Unreached (zf.getEntry(null)); }
+        try { unreached (zf.getEntry(null)); }
         catch (NullPointerException e) {}
 
-        try { Unreached (zf.getInputStream(null)); }
+        try { unreached (zf.getInputStream(null)); }
         catch (NullPointerException e) {}
 
         ZipEntry ze = zf.getEntry("ReadZip.java");
@@ -53,5 +53,65 @@ public class ReadZip {
             throw new Exception("cannot read from zip file");
         }
         zf.close();
+
+        // Make sure we can read the zip file that has some garbage
+        // bytes padded at the end.
+        FileInputStream fis = new FileInputStream(
+                                   new File(System.getProperty("test.src", "."),
+                                            "input.zip"));
+        File newZip = new File(System.getProperty("test.src", "."),
+                               "input2.zip");
+        FileOutputStream fos = new FileOutputStream(newZip);
+
+        byte[] buf = new byte[1024];
+        int n = 0;
+        while ((n = fis.read(buf)) != -1) {
+            fos.write(buf, 0, n);
+        }
+        fis.close();
+        // pad some bytes
+        fos.write(1); fos.write(3); fos.write(5); fos.write(7);
+        fos.close();
+        try {
+            zf = new ZipFile(newZip);
+            ze = zf.getEntry("ReadZip.java");
+            if (ze == null) {
+                throw new Exception("cannot read from zip file");
+            }
+        } finally {
+            zf.close();
+            newZip.delete();
+        }
+
+        // Read zip file comment
+        try {
+
+            ZipOutputStream zos = new ZipOutputStream(
+                                      new FileOutputStream(newZip));
+            ze = new ZipEntry("ZipEntry");
+            zos.putNextEntry(ze);
+            zos.write(1); zos.write(2); zos.write(3); zos.write(4);
+            zos.closeEntry();
+            zos.setComment("This is the comment for testing");
+            zos.close();
+
+            zf = new ZipFile(newZip);
+            ze = zf.getEntry("ZipEntry");
+            if (ze == null)
+                throw new Exception("cannot read entry from zip file");
+            if (!"This is the comment for testing".equals(zf.getComment()))
+                throw new Exception("cannot read comment from zip file");
+        } finally {
+            zf.close();
+            newZip.delete();
+        }
+
+        // Throw a FNF exception when read a non-existing zip file
+        try { unreached (new ZipFile(
+                             new File(System.getProperty("test.src", "."),
+                                     "input"
+                                      + String.valueOf(new java.util.Random().nextInt())
+                                      + ".zip")));
+        } catch (FileNotFoundException fnfe) {}
     }
 }