8150469: unpack200 fails to compare crc correctly.
authorksrini
Thu, 07 Apr 2016 12:54:23 -0700
changeset 36960 d7731fdfe7c3
parent 36959 d839463825a2
child 36961 b4a60621d9f9
8150469: unpack200 fails to compare crc correctly. Reviewed-by: jrose
jdk/src/jdk.pack200/share/native/common-unpack/defines.h
jdk/src/jdk.pack200/share/native/common-unpack/unpack.h
jdk/src/jdk.pack200/share/native/common-unpack/zip.cpp
jdk/src/jdk.pack200/share/native/common-unpack/zip.h
jdk/src/jdk.pack200/share/native/unpack200/main.cpp
jdk/test/tools/pack200/PackChecksum.java
--- a/jdk/src/jdk.pack200/share/native/common-unpack/defines.h	Thu Apr 07 11:03:59 2016 -0700
+++ b/jdk/src/jdk.pack200/share/native/common-unpack/defines.h	Thu Apr 07 12:54:23 2016 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2016, 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
@@ -90,6 +90,12 @@
 #define U_NEW(T, n)  (T*) u->alloc(scale_size(n, sizeof(T)))
 #define T_NEW(T, n)  (T*) u->temp_alloc(scale_size(n, sizeof(T)))
 
+// Dealing with big-endian arch
+#ifdef _BIG_ENDIAN
+#define SWAP_INT(a) (((a>>24)&0xff) | ((a<<8)&0xff0000) | ((a>>8)&0xff00) | ((a<<24)&0xff000000))
+#else
+#define SWAP_INT(a) (a)
+#endif
 
 // bytes and byte arrays
 
--- a/jdk/src/jdk.pack200/share/native/common-unpack/unpack.h	Thu Apr 07 11:03:59 2016 -0700
+++ b/jdk/src/jdk.pack200/share/native/common-unpack/unpack.h	Thu Apr 07 12:54:23 2016 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2002, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2002, 2016, 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
@@ -171,7 +171,6 @@
   bytes inbytes;    // direct
   gunzip* gzin;     // gunzip filter, if any
   jar*  jarout;     // output JAR file
-  uint  gzcrc;      // CRC gathered from gzip content
 
 #ifndef PRODUCT
   int   nowrite;
--- a/jdk/src/jdk.pack200/share/native/common-unpack/zip.cpp	Thu Apr 07 11:03:59 2016 -0700
+++ b/jdk/src/jdk.pack200/share/native/common-unpack/zip.cpp	Thu Apr 07 12:54:23 2016 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2016, 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
@@ -341,6 +341,7 @@
 void jar::openJarFile(const char* fname) {
   if (!jarfp) {
     PRINTCR((1, "jar::openJarFile: opening %s\n",fname));
+    jarname = fname;
     jarfp = fopen(fname, "wb");
     if (!jarfp) {
       fprintf(u->errstrm, "Error: Could not open jar file: %s\n",fname);
@@ -551,7 +552,8 @@
       break;
     }
     int nr = readlen - zs.avail_out;
-    u->gzcrc = crc32(u->gzcrc, (const unsigned char *)bufptr, nr);
+    u->gzin->gzlen += nr;
+    u->gzin->gzcrc = crc32(u->gzin->gzcrc, (const unsigned char *)bufptr, nr);
     numread += nr;
     bufptr += nr;
     assert(numread <= maxlen);
@@ -562,15 +564,44 @@
         zs.avail_in -= TRAILER_LEN;
       } else {
         // Bug: 5023768,we read past the TRAILER_LEN to see if there is
-        // any extraneous data, as we don't support concatenated .gz
-        // files just yet.
+        // any extraneous data, as we don't support concatenated .gz files.
         int extra = (int) read_gzin_fn(u, inbuf, 1, inbuflen);
         zs.avail_in += extra - TRAILER_LEN;
       }
-      // %%% should check final CRC and length here
       // %%% should check for concatenated *.gz files here
       if (zs.avail_in > 0)
         u->abort("garbage after end of deflated input stream");
+
+      // at this point we know there are no trailing bytes,
+      // we are safe to get the crc and len.
+      if (u->gzin->gzcrc != 0) {
+        // Read the CRC information from the gzip container
+        fseek(u->infileptr, -TRAILER_LEN, SEEK_END);
+        uint filecrc;
+        uint filelen;
+        fread(&filecrc, sizeof(filecrc), 1, u->infileptr);
+        fread(&filelen, sizeof(filelen), 1, u->infileptr);
+        filecrc = SWAP_INT(filecrc);
+        filelen = SWAP_INT(filelen);
+        if (u->gzin->gzcrc != filecrc ||
+                // rfc1952; ISIZE is the input size modulo 2^32
+                u->gzin->gzlen != (filelen & 0xffffffff)) { // CRC error
+
+          PRINTCR((1, "crc: 0x%x 0x%x\n", u->gzin->gzcrc,  filecrc));
+          PRINTCR((1, "len: 0x%x 0x%x\n", u->gzin->gzlen,  filelen));
+
+          if (u->jarout != null) {
+            // save the file name first, if any
+            const char* outfile = u->jarout->jarname;
+            u->jarout->closeJarFile(false);
+            if (outfile != null) {
+              remove(outfile);
+            }
+          }
+          // Print out the error and exit with return code != 0
+          u->abort("CRC error, invalid compressed data.");
+        }
+      }
       // pop this filter off:
       u->gzin->free();
       break;
@@ -590,7 +621,8 @@
   zstream = NEW(z_stream, 1);
   u->gzin = this;
   u->read_input_fn = read_input_via_gzip;
-  u->gzcrc = crc32(0, Z_NULL, 0);
+  u->gzin->gzcrc = crc32(0, Z_NULL, 0);
+  u->gzin->gzlen = 0;
 }
 
 void gunzip::start(int magic) {
--- a/jdk/src/jdk.pack200/share/native/common-unpack/zip.h	Thu Apr 07 11:03:59 2016 -0700
+++ b/jdk/src/jdk.pack200/share/native/common-unpack/zip.h	Thu Apr 07 12:54:23 2016 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2016, 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
@@ -34,6 +34,8 @@
   FILE*       jarfp;
   int         default_modtime;
 
+  const char* jarname;
+
   // Used by unix2dostime:
   int         modtime_cache;
   uLong       dostime_cache;
@@ -98,6 +100,9 @@
   void* zstream;        // inflater state
   char inbuf[1 << 14];   // input buffer
 
+  uint  gzcrc;      // CRC gathered from gzip *container* content
+  uint  gzlen;      // CRC gathered length
+
   void init(unpacker* u_);  // pushes new value on u->read_input_fn
 
   void free();
--- a/jdk/src/jdk.pack200/share/native/unpack200/main.cpp	Thu Apr 07 11:03:59 2016 -0700
+++ b/jdk/src/jdk.pack200/share/native/unpack200/main.cpp	Thu Apr 07 12:54:23 2016 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2016, 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
@@ -62,13 +62,6 @@
     return unpacker::run(argc, argv);
 }
 
-// Dealing with big-endian arch
-#ifdef _BIG_ENDIAN
-#define SWAP_INT(a) (((a>>24)&0xff) | ((a<<8)&0xff0000) | ((a>>8)&0xff00) | ((a<<24)&0xff000000))
-#else
-#define SWAP_INT(a) (a)
-#endif
-
 // Single-threaded, implementation, not reentrant.
 // Includes a weak error check against MT access.
 #ifndef THREAD_SELF
@@ -366,6 +359,7 @@
 
   if (strcmp(destination_file, "-") == 0) {
     jarout.jarfp = stdout;
+    jarout.jarname = null;
     if (u.errstrm == stdout) // do not mix output
       u.set_option(UNPACK_LOG_FILE, LOGFILE_STDERR);
   } else {
@@ -385,11 +379,12 @@
     // Oops; must slap an input filter on this data.
     setup_gzin(&u);
     u.gzin->start(magic);
+    u.gzin->gzcrc = 0;
+    u.gzin->gzlen = 0;
     if (!u.aborting()) {
       u.start();
     }
   } else {
-    u.gzcrc = 0;
     u.start(peek, sizeof(peek));
   }
 
@@ -422,31 +417,13 @@
     u.start(peek, sizeof(peek));
   }
 
-
-
   int status = 0;
   if (u.aborting()) {
     fprintf(u.errstrm, "Error: %s\n", u.get_abort_message());
     status = 1;
   }
 
-  if (!u.aborting() && u.infileptr != null) {
-    if (u.gzcrc != 0) {
-      // Read the CRC information from the gzip container
-      fseek(u.infileptr, -8, SEEK_END);
-      uint filecrc;
-      fread(&filecrc, sizeof(filecrc), 1, u.infileptr);
-      if (u.gzcrc != SWAP_INT(filecrc)) { // CRC error
-        if (strcmp(destination_file, "-") != 0) {
-          // Output is not stdout, remove it, it's broken
-          if (u.jarout != null)
-            u.jarout->closeJarFile(false);
-          remove(destination_file);
-        }
-        // Print out the error and exit with return code != 0
-        u.abort("CRC error, invalid compressed data.");
-      }
-    }
+  if (u.infileptr != null) {
     fclose(u.infileptr);
     u.infileptr = null;
   }
--- a/jdk/test/tools/pack200/PackChecksum.java	Thu Apr 07 11:03:59 2016 -0700
+++ b/jdk/test/tools/pack200/PackChecksum.java	Thu Apr 07 12:54:23 2016 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2016, 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
@@ -31,65 +31,136 @@
 
 /*
  * @test
- * @bug 8000650
+ * @bug 8000650 8150469
  * @summary unpack200.exe should check gzip crc
  * @compile -XDignore.symbol.file Utils.java PackChecksum.java
  * @run main PackChecksum
  * @author kizune
  */
 public class PackChecksum {
+    final int TRAILER_LEN = 8;
+    final List<String> cmdsList = new ArrayList<>();
+    static enum Case {
+        CRC32,
+        ISIZE,
+        BOTH;
 
+    };
     public static void main(String... args) throws Exception {
-        testChecksum();
+        new PackChecksum().run();
+    }
+    void run() throws Exception {
+        testBrokenTrailer(Case.CRC32); // negative
+        testBrokenTrailer(Case.ISIZE); // negative
+        testBrokenTrailer(Case.BOTH);  // negative
+        testMultipleSegments();
     }
 
-    static void testChecksum() throws Exception {
+    void testMultipleSegments() throws Exception {
+        File inputJar = new File("input.jar");
+        Utils.copyFile(Utils.getGoldenJar(), inputJar);
+        cmdsList.clear();
+
+        File testPack = new File("out.jar.pack.gz");
+
+        cmdsList.clear();
+        cmdsList.add(Utils.getPack200Cmd());
+        // force multiple segments
+        cmdsList.add("--segment-limit=100");
+        cmdsList.add(testPack.getName());
+        cmdsList.add(inputJar.getName());
+        Utils.runExec(cmdsList);
 
+        File destFile = new File("dst.jar");
+        cmdsList.clear();
+        cmdsList.add(Utils.getUnpack200Cmd());
+        cmdsList.add(testPack.getName());
+        cmdsList.add(destFile.getName());
+        try {
+            Utils.runExec(cmdsList);
+            if (!destFile.exists()) {
+                throw new Exception("file not created: " + destFile);
+            }
+        } finally {
+            if (inputJar.exists())
+                inputJar.delete();
+            if (testPack.exists())
+                testPack.delete();
+            if (destFile.exists())
+                destFile.delete();
+        }
+    }
+
+    void testBrokenTrailer(Case type) throws Exception {
+        System.out.println("Testing: case " + type);
         // Create a fresh .jar file
         File testFile = new File("src_tools.jar");
         File testPack = new File("src_tools.pack.gz");
         generateJar(testFile);
-        List<String> cmdsList = new ArrayList<>();
 
+        cmdsList.clear();
         // Create .pack file
         cmdsList.add(Utils.getPack200Cmd());
         cmdsList.add(testPack.getName());
         cmdsList.add(testFile.getName());
         Utils.runExec(cmdsList);
 
-        // Mess up with the checksum of the packed file
+        // mutate the checksum of the packed file
         RandomAccessFile raf = new RandomAccessFile(testPack, "rw");
-        raf.seek(raf.length() - 8);
-        int val = raf.readInt();
-        val = Integer.MAX_VALUE - val;
-        raf.seek(raf.length() - 8);
-        raf.writeInt(val);
+
+        switch (type) {
+            case CRC32:
+                raf.seek(raf.length() - TRAILER_LEN);
+                raf.writeInt(0x0dea0a0d);
+                break;
+            case ISIZE:
+                raf.seek(raf.length() - (TRAILER_LEN/2));
+                raf.writeInt(0x0b0e0e0f);
+                break;
+            default:
+                raf.seek(raf.length() - (TRAILER_LEN));
+                raf.writeLong(0x0dea0a0d0b0e0e0fL);
+                break;
+        }
+
         raf.close();
 
         File dstFile = new File("dst_tools.jar");
+        if (dstFile.exists()) {
+            dstFile.delete();
+        }
         cmdsList.clear();
         cmdsList.add(Utils.getUnpack200Cmd());
         cmdsList.add(testPack.getName());
         cmdsList.add(dstFile.getName());
 
-        boolean passed = false;
+        boolean processFailed = false;
         try {
             Utils.runExec(cmdsList);
         } catch (RuntimeException re) {
             // unpack200 should exit with non-zero exit code
-            passed = true;
-        }
+            processFailed = true;
+        } finally {
+            // tidy up
+            if (testFile.exists())
+                testFile.delete();
+
+            if (testPack.exists())
+                testPack.delete();
 
-        // tidy up
-        if (testFile.exists()) testFile.delete();
-        if (testPack.exists()) testPack.delete();
-        if (dstFile.exists()) dstFile.delete();
-        if (!passed) {
-            throw new Exception("File with incorrect CRC unpacked without the error.");
+            if (!processFailed) {
+                throw new Exception("case " + type +
+                        ": file with incorrect CRC, unpacked without the error.");
+            }
+            if (dstFile.exists()) {
+                dstFile.delete();
+                throw new Exception("case " + type +
+                        ":  file exists: " + dstFile);
+            }
         }
     }
 
-    static void generateJar(File result) throws IOException {
+    void generateJar(File result) throws IOException {
         if (result.exists()) {
             result.delete();
         }