diff --git a/tools/zipalign/Android.bp b/tools/zipalign/Android.bp index d88ee740dc..135cd766ea 100644 --- a/tools/zipalign/Android.bp +++ b/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"], diff --git a/tools/zipalign/ZipAlign.cpp b/tools/zipalign/ZipAlign.cpp index 1851ac562c..08f67ff9d0 100644 --- a/tools/zipalign/ZipAlign.cpp +++ b/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); diff --git a/tools/zipalign/ZipFile.cpp b/tools/zipalign/ZipFile.cpp index 29d1bc6849..9938a06088 100644 --- a/tools/zipalign/ZipFile.cpp +++ b/tools/zipalign/ZipFile.cpp @@ -502,6 +502,32 @@ bail: 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 @@ bail: * 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. diff --git a/tools/zipalign/ZipFile.h b/tools/zipalign/ZipFile.h index 11d20c5a2a..854f981a32 100644 --- a/tools/zipalign/ZipFile.h +++ b/tools/zipalign/ZipFile.h @@ -102,14 +102,14 @@ public: } /* - * 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 @@ private: ZipFile(const ZipFile& src); ZipFile& operator=(const ZipFile& src); + status_t alignEntry(android::ZipEntry* pEntry, uint32_t alignTo); + class EndOfCentralDir { public: EndOfCentralDir(void) : diff --git a/tools/zipalign/tests/data/diffOrders.zip b/tools/zipalign/tests/data/diffOrders.zip new file mode 100644 index 0000000000..8f512ed49b Binary files /dev/null and b/tools/zipalign/tests/data/diffOrders.zip differ diff --git a/tools/zipalign/tests/data/holes.zip b/tools/zipalign/tests/data/holes.zip new file mode 100644 index 0000000000..c88f891c70 Binary files /dev/null and b/tools/zipalign/tests/data/holes.zip differ diff --git a/tools/zipalign/tests/src/align_test.cpp b/tools/zipalign/tests/src/align_test.cpp index 073a15678f..cbd92187f3 100644 --- a/tools/zipalign/tests/src/align_test.cpp +++ b/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); +}