Merge "Fix zipalign alignment error"

tools/zipalign/Android.bp

@@ -63,6 +63,8 @@ cc_test_host {
         "libgmock",
     ],
     data: [
+         "tests/data/diffOrders.zip",
+         "tests/data/holes.zip",
          "tests/data/unaligned.zip",
     ],
     defaults: ["zipalign_defaults"],

tools/zipalign/ZipAlign.cpp

@@ -47,7 +47,6 @@ static int copyAndAlign(ZipFile* pZin, ZipFile* pZout, int alignment, bool zopfl
 {
     int numEntries = pZin->getNumEntries();
     ZipEntry* pEntry;
-    int bias = 0;
     status_t status;
 
     for (int i = 0; i < numEntries; i++) {
@@ -68,30 +67,20 @@ static int copyAndAlign(ZipFile* pZin, ZipFile* pZout, int alignment, bool zopfl
 
             if (zopfli) {
                 status = pZout->addRecompress(pZin, pEntry, &pNewEntry);
-                bias += pNewEntry->getCompressedLen() - pEntry->getCompressedLen();
             } else {
                 status = pZout->add(pZin, pEntry, padding, &pNewEntry);
             }
         } else {
             const int alignTo = getAlignment(pageAlignSharedLibs, alignment, pEntry);
 
-            /*
-             * Copy the entry, adjusting as required.  We assume that the
-             * file position in the new file will be equal to the file
-             * position in the original.
-             */
-            off_t newOffset = pEntry->getFileOffset() + bias;
-            padding = (alignTo - (newOffset % alignTo)) % alignTo;
-
             //printf("--- %s: orig at %ld(+%d) len=%ld, adding pad=%d\n",
             //    pEntry->getFileName(), (long) pEntry->getFileOffset(),
             //    bias, (long) pEntry->getUncompressedLen(), padding);
-            status = pZout->add(pZin, pEntry, padding, &pNewEntry);
+            status = pZout->add(pZin, pEntry, alignTo, &pNewEntry);
         }
 
         if (status != OK)
             return 1;
-        bias += padding;
         //printf(" added '%s' at %ld (pad=%d)\n",
         //    pNewEntry->getFileName(), (long) pNewEntry->getFileOffset(),
         //    padding);

tools/zipalign/ZipFile.cpp

@@ -502,6 +502,32 @@ status_t ZipFile::addCommon(const char* fileName, const void* data, size_t size,
     return result;
 }
 
+/*
+ * Based on the current position in the output zip, assess where the entry
+ * payload will end up if written as-is. If alignment is not satisfactory,
+ * add some padding in the extra field.
+ *
+ */
+status_t ZipFile::alignEntry(android::ZipEntry* pEntry, uint32_t alignTo){
+    if (alignTo == 0 || alignTo == 1)
+        return OK;
+
+    // Calculate where the entry payload offset will end up if we were to write
+    // it as-is.
+    uint64_t expectedPayloadOffset = ftell(mZipFp) +
+        android::ZipEntry::LocalFileHeader::kLFHLen +
+        pEntry->mLFH.mFileNameLength +
+        pEntry->mLFH.mExtraFieldLength;
+
+    // If the alignment is not what was requested, add some padding in the extra
+    // so the payload ends up where is requested.
+    uint64_t alignDiff = alignTo - (expectedPayloadOffset % alignTo);
+    if (alignDiff == 0)
+        return OK;
+
+    return pEntry->addPadding(alignDiff);
+}
+
 /*
  * Add an entry by copying it from another zip file.  If "padding" is
  * nonzero, the specified number of bytes will be added to the "extra"
@@ -510,7 +536,7 @@ status_t ZipFile::addCommon(const char* fileName, const void* data, size_t size,
  * If "ppEntry" is non-NULL, a pointer to the new entry will be returned.
  */
 status_t ZipFile::add(const ZipFile* pSourceZip, const ZipEntry* pSourceEntry,
-    int padding, ZipEntry** ppEntry)
+    int alignTo, ZipEntry** ppEntry)
 {
     ZipEntry* pEntry = NULL;
     status_t result;
@@ -537,11 +563,10 @@ status_t ZipFile::add(const ZipFile* pSourceZip, const ZipEntry* pSourceEntry,
     result = pEntry->initFromExternal(pSourceEntry);
     if (result != OK)
         goto bail;
-    if (padding != 0) {
-        result = pEntry->addPadding(padding);
-        if (result != OK)
-            goto bail;
-    }
+
+    result = alignEntry(pEntry, alignTo);
+    if (result != OK)
+      goto bail;
 
     /*
      * From here on out, failures are more interesting.

tools/zipalign/ZipFile.h

@@ -102,14 +102,14 @@ class ZipFile {
     }
 
     /*
-     * Add an entry by copying it from another zip file.  If "padding" is
-     * nonzero, the specified number of bytes will be added to the "extra"
-     * field in the header.
+     * Add an entry by copying it from another zip file.  If "alignment" is
+     * nonzero, an appropriate number of bytes will be added to the "extra"
+     * field in the header so the entry payload is aligned.
      *
      * If "ppEntry" is non-NULL, a pointer to the new entry will be returned.
      */
     status_t add(const ZipFile* pSourceZip, const ZipEntry* pSourceEntry,
-        int padding, ZipEntry** ppEntry);
+        int alignment, ZipEntry** ppEntry);
 
     /*
      * Add an entry by copying it from another zip file, recompressing with
@@ -163,6 +163,8 @@ class ZipFile {
     ZipFile(const ZipFile& src);
     ZipFile& operator=(const ZipFile& src);
 
+    status_t alignEntry(android::ZipEntry* pEntry, uint32_t alignTo);
+
     class EndOfCentralDir {
     public:
         EndOfCentralDir(void) :

tools/zipalign/tests/src/align_test.cpp

@@ -22,3 +22,29 @@ TEST(Align, Unaligned) {
   int result = process(src.c_str(), dst.c_str(), 4, true, false, 4096);
   ASSERT_EQ(0, result);
 }
+
+// Align a zip featuring a hole at the beginning. The
+// hole in the archive is a delete entry in the Central
+// Directory.
+TEST(Align, Holes) {
+  const std::string src = GetTestPath("holes.zip");
+  const std::string dst = GetTestPath("holes_out.zip");
+
+  int processed = process(src.c_str(), dst.c_str(), 4, true, false, 4096);
+  ASSERT_EQ(0, processed);
+
+  int verified = verify(dst.c_str(), 4, false, true);
+  ASSERT_EQ(0, verified);
+}
+
+// Align a zip where LFH order and CD entries differ.
+TEST(Align, DifferenteOrders) {
+  const std::string src = GetTestPath("diffOrders.zip");
+  const std::string dst = GetTestPath("diffOrders_out.zip");
+
+  int processed = process(src.c_str(), dst.c_str(), 4, true, false, 4096);
+  ASSERT_EQ(0, processed);
+
+  int verified = verify(dst.c_str(), 4, false, true);
+  ASSERT_EQ(0, verified);
+}