Remove data descriptor entries from zip on write am: ba31559ca0

am: ff135c8db3

Change-Id: I17f11eee4a462e4bd8a41f55cc9b8a3765e696de

3 files changed, 127 insertions, 0 deletions

diff --git a/src/main/java/com/android/apkzlib/zip/StoredEntry.java b/src/main/java/com/android/apkzlib/zip/StoredEntry.java
index 485a45a..664734e 100644
--- a/src/main/java/com/android/apkzlib/zip/StoredEntry.java
+++ b/src/main/java/com/android/apkzlib/zip/StoredEntry.java
@@ -655,6 +655,22 @@ public class StoredEntry {
     }
 
     /**
+     * Removes the data descriptor, if it has one and resets the data descriptor bit in the
+     * central directory header.
+     *
+     * @return was the data descriptor remove?
+     */
+    boolean removeDataDescriptor() {
+        if (dataDescriptorType == DataDescriptorType.NO_DATA_DESCRIPTOR) {
+            return false;
+        }
+
+        dataDescriptorType = DataDescriptorType.NO_DATA_DESCRIPTOR;
+        cdh.resetDeferredCrc();
+        return true;
+    }
+
+    /**
      * Obtains the local header data.
      *
      * @return the header data
diff --git a/src/main/java/com/android/apkzlib/zip/ZFile.java b/src/main/java/com/android/apkzlib/zip/ZFile.java
index 7ef84cf..69eee12 100644
--- a/src/main/java/com/android/apkzlib/zip/ZFile.java
+++ b/src/main/java/com/android/apkzlib/zip/ZFile.java
@@ -1447,6 +1447,14 @@ public class ZFile implements Closeable {
         raf = new RandomAccessFile(file, "rw");
         state = ZipFileState.OPEN_RW;
 
+        /*
+         * Now that we've open the zip and are ready to write, clear out any data descriptors
+         * in the zip since we don't need them and they take space in the archive.
+         */
+        for (StoredEntry entry : entries()) {
+            dirty |= entry.removeDataDescriptor();
+        }
+
         if (wasClosed) {
             notify(ZFileExtension::open);
         }
@@ -1909,6 +1917,10 @@ public class ZFile implements Closeable {
             anyChanges |= entry.realign();
         }
 
+        if (anyChanges) {
+            dirty = true;
+        }
+
         return anyChanges;
     }
 
@@ -2421,6 +2433,7 @@ public class ZFile implements Closeable {
             String name = entry.getCentralDirectoryHeader().getName();
             FileUseMapEntry<StoredEntry> positioned =
                     positionInFile(entry, PositionHint.LOWEST_OFFSET);
+
             entries.put(name, positioned);
         }
 
diff --git a/src/test/java/com/android/apkzlib/zip/ZFileTest.java b/src/test/java/com/android/apkzlib/zip/ZFileTest.java
index af74fab..080197d 100644
--- a/src/test/java/com/android/apkzlib/zip/ZFileTest.java
+++ b/src/test/java/com/android/apkzlib/zip/ZFileTest.java
@@ -22,6 +22,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
@@ -1476,4 +1477,101 @@ public class ZFileTest {
             assertTrue(vl.getLogs().get(0).toLowerCase(Locale.US).contains("extract"));
         }
     }
+
+    @Test
+    public void sortZipContentsWithDeferredCrc32() throws Exception {
+        /*
+         * Create a zip file with deferred CRC32 and files in non-alphabetical order.
+         * ZipOutputStream always creates deferred CRC32 entries.
+         */
+        File zipFile = new File(mTemporaryFolder.getRoot(), "a.zip");
+        try (ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(zipFile))) {
+            zos.putNextEntry(new ZipEntry("b"));
+            zos.write(new byte[1000]);
+            zos.putNextEntry(new ZipEntry("a"));
+            zos.write(new byte[1000]);
+        }
+
+        /*
+         * Now open the zip using a ZFile and sort the contents and check that the deferred CRC32
+         * bits were reset.
+         */
+        try (ZFile zf = new ZFile(zipFile)) {
+            StoredEntry a = zf.get("a");
+            assertNotNull(a);
+            assertNotSame(DataDescriptorType.NO_DATA_DESCRIPTOR, a.getDataDescriptorType());
+            StoredEntry b = zf.get("b");
+            assertNotNull(b);
+            assertNotSame(DataDescriptorType.NO_DATA_DESCRIPTOR, b.getDataDescriptorType());
+            assertTrue(
+                    a.getCentralDirectoryHeader().getOffset()
+                            > b.getCentralDirectoryHeader().getOffset());
+
+            zf.sortZipContents();
+            zf.update();
+
+            a = zf.get("a");
+            assertNotNull(a);
+            assertSame(DataDescriptorType.NO_DATA_DESCRIPTOR, a.getDataDescriptorType());
+            b = zf.get("b");
+            assertNotNull(b);
+            assertSame(DataDescriptorType.NO_DATA_DESCRIPTOR, b.getDataDescriptorType());
+
+            assertTrue(
+                    a.getCentralDirectoryHeader().getOffset()
+                            < b.getCentralDirectoryHeader().getOffset());
+        }
+    }
+
+    @Test
+    public void alignZipContentsWithDeferredCrc32() throws Exception {
+        /*
+         * Create an unaligned zip file with deferred CRC32 and files in non-alphabetical order.
+         * We need an uncompressed file to make realigning have any effect.
+         */
+        File zipFile = new File(mTemporaryFolder.getRoot(), "a.zip");
+        try (ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(zipFile))) {
+            zos.putNextEntry(new ZipEntry("x"));
+            zos.write(new byte[1000]);
+            zos.putNextEntry(new ZipEntry("y"));
+            zos.write(new byte[1000]);
+            ZipEntry zEntry = new ZipEntry("z");
+            zEntry.setSize(1000);
+            zEntry.setMethod(ZipEntry.STORED);
+            zEntry.setCrc(Hashing.crc32().hashBytes(new byte[1000]).asInt());
+            zos.putNextEntry(zEntry);
+            zos.write(new byte[1000]);
+        }
+
+        /*
+         * Now open the zip using a ZFile and realign the contents and check that the deferred CRC32
+         * bits were reset.
+         */
+        ZFileOptions options = new ZFileOptions();
+        options.setAlignmentRule(AlignmentRules.constant(2000));
+        try (ZFile zf = new ZFile(zipFile, options)) {
+            StoredEntry x = zf.get("x");
+            assertNotNull(x);
+            assertNotSame(DataDescriptorType.NO_DATA_DESCRIPTOR, x.getDataDescriptorType());
+            StoredEntry y = zf.get("y");
+            assertNotNull(y);
+            assertNotSame(DataDescriptorType.NO_DATA_DESCRIPTOR, y.getDataDescriptorType());
+            StoredEntry z = zf.get("z");
+            assertNotNull(z);
+            assertSame(DataDescriptorType.NO_DATA_DESCRIPTOR, z.getDataDescriptorType());
+
+            zf.realign();
+            zf.update();
+
+            x = zf.get("x");
+            assertNotNull(x);
+            assertSame(DataDescriptorType.NO_DATA_DESCRIPTOR, x.getDataDescriptorType());
+            y = zf.get("y");
+            assertNotNull(y);
+            assertSame(DataDescriptorType.NO_DATA_DESCRIPTOR, y.getDataDescriptorType());
+            z = zf.get("z");
+            assertNotNull(z);
+            assertSame(DataDescriptorType.NO_DATA_DESCRIPTOR, z.getDataDescriptorType());
+        }
+    }
 }