From cc61cf6a9f74400956886c7f91efef581f4184e2 Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Wed, 23 May 2018 22:23:31 -0700 Subject: [PATCH] Convert deflate image chunks to raw if the raw data is smaller The imgpatch will fail on empty deflates because the bspatch won't call the sink function if the target length is zero. Instead of compressing an empty string, it's cleaner to not generate such empty deflate chunks in the patch. Therefore, we can just convert the chunk type to raw if the target length is smaller than the patch data. Also adjust some unit tests and add the testdata gzipped_source & gzipped_target. These two files are ~1K each and are generated by gzipping two slightly different regular files. Bug: 79265132 Test: unit tests pass, imgpatch applys successfully on the given src/tgt Change-Id: I6bfff3251918137f6762a6f9e9551642371a1124 --- applypatch/imgdiff.cpp | 4 +- applypatch/imgpatch.cpp | 1 + applypatch/include/applypatch/imgdiff_image.h | 3 +- tests/component/imgdiff_test.cpp | 107 ++++++++++++------ tests/testdata/gzipped_source | Bin 0 -> 1436 bytes tests/testdata/gzipped_target | Bin 0 -> 1502 bytes 6 files changed, 80 insertions(+), 35 deletions(-) create mode 100644 tests/testdata/gzipped_source create mode 100644 tests/testdata/gzipped_target diff --git a/applypatch/imgdiff.cpp b/applypatch/imgdiff.cpp index 674cc2b1..415d95f1 100644 --- a/applypatch/imgdiff.cpp +++ b/applypatch/imgdiff.cpp @@ -462,12 +462,12 @@ PatchChunk::PatchChunk(const ImageChunk& tgt) target_len_(tgt.GetRawDataLength()), target_uncompressed_len_(tgt.DataLengthForPatch()), target_compress_level_(tgt.GetCompressLevel()), - data_(tgt.DataForPatch(), tgt.DataForPatch() + tgt.DataLengthForPatch()) {} + data_(tgt.GetRawData(), tgt.GetRawData() + tgt.GetRawDataLength()) {} // Return true if raw data is smaller than the patch size. bool PatchChunk::RawDataIsSmaller(const ImageChunk& tgt, size_t patch_size) { size_t target_len = tgt.GetRawDataLength(); - return (tgt.GetType() == CHUNK_NORMAL && (target_len <= 160 || target_len < patch_size)); + return target_len < patch_size || (tgt.GetType() == CHUNK_NORMAL && target_len <= 160); } void PatchChunk::UpdateSourceOffset(const SortedRangeSet& src_range) { diff --git a/applypatch/imgpatch.cpp b/applypatch/imgpatch.cpp index c4c2707f..2f8f4851 100644 --- a/applypatch/imgpatch.cpp +++ b/applypatch/imgpatch.cpp @@ -54,6 +54,7 @@ static bool ApplyBSDiffPatchAndStreamOutput(const uint8_t* src_data, size_t src_ const Value& patch, size_t patch_offset, const char* deflate_header, SinkFn sink) { size_t expected_target_length = static_cast(Read8(deflate_header + 32)); + CHECK_GT(expected_target_length, 0); int level = Read4(deflate_header + 40); int method = Read4(deflate_header + 44); int window_bits = Read4(deflate_header + 48); diff --git a/applypatch/include/applypatch/imgdiff_image.h b/applypatch/include/applypatch/imgdiff_image.h index 08480723..67160516 100644 --- a/applypatch/include/applypatch/imgdiff_image.h +++ b/applypatch/include/applypatch/imgdiff_image.h @@ -44,6 +44,8 @@ class ImageChunk { int GetType() const { return type_; } + + const uint8_t* GetRawData() const; size_t GetRawDataLength() const { return raw_data_len_; } @@ -99,7 +101,6 @@ class ImageChunk { bsdiff::SuffixArrayIndexInterface** bsdiff_cache); private: - const uint8_t* GetRawData() const; bool TryReconstruction(int level); int type_; // CHUNK_NORMAL, CHUNK_DEFLATE, CHUNK_RAW diff --git a/tests/component/imgdiff_test.cpp b/tests/component/imgdiff_test.cpp index 6c23def0..cb4868a4 100644 --- a/tests/component/imgdiff_test.cpp +++ b/tests/component/imgdiff_test.cpp @@ -197,12 +197,17 @@ TEST(ImgdiffTest, zip_mode_smoke_store) { } TEST(ImgdiffTest, zip_mode_smoke_compressed) { + // Generate 1 block of random data. + std::string random_data; + random_data.reserve(4096); + generate_n(back_inserter(random_data), 4096, []() { return rand() % 256; }); + // Construct src and tgt zip files. TemporaryFile src_file; FILE* src_file_ptr = fdopen(src_file.release(), "wb"); ZipWriter src_writer(src_file_ptr); ASSERT_EQ(0, src_writer.StartEntry("file1.txt", ZipWriter::kCompress)); - const std::string src_content("abcdefg"); + const std::string src_content = random_data; ASSERT_EQ(0, src_writer.WriteBytes(src_content.data(), src_content.size())); ASSERT_EQ(0, src_writer.FinishEntry()); ASSERT_EQ(0, src_writer.Finish()); @@ -212,7 +217,7 @@ TEST(ImgdiffTest, zip_mode_smoke_compressed) { FILE* tgt_file_ptr = fdopen(tgt_file.release(), "wb"); ZipWriter tgt_writer(tgt_file_ptr); ASSERT_EQ(0, tgt_writer.StartEntry("file1.txt", ZipWriter::kCompress)); - const std::string tgt_content("abcdefgxyz"); + const std::string tgt_content = random_data + "extra contents"; ASSERT_EQ(0, tgt_writer.WriteBytes(tgt_content.data(), tgt_content.size())); ASSERT_EQ(0, tgt_writer.FinishEntry()); ASSERT_EQ(0, tgt_writer.Finish()); @@ -245,13 +250,57 @@ TEST(ImgdiffTest, zip_mode_smoke_compressed) { verify_patched_image(src, patch, tgt); } +TEST(ImgdiffTest, zip_mode_empty_target) { + TemporaryFile src_file; + FILE* src_file_ptr = fdopen(src_file.release(), "wb"); + ZipWriter src_writer(src_file_ptr); + ASSERT_EQ(0, src_writer.StartEntry("file1.txt", ZipWriter::kCompress)); + const std::string src_content = "abcdefg"; + ASSERT_EQ(0, src_writer.WriteBytes(src_content.data(), src_content.size())); + ASSERT_EQ(0, src_writer.FinishEntry()); + ASSERT_EQ(0, src_writer.Finish()); + ASSERT_EQ(0, fclose(src_file_ptr)); + + // Construct a empty entry in the target zip. + TemporaryFile tgt_file; + FILE* tgt_file_ptr = fdopen(tgt_file.release(), "wb"); + ZipWriter tgt_writer(tgt_file_ptr); + ASSERT_EQ(0, tgt_writer.StartEntry("file1.txt", ZipWriter::kCompress)); + const std::string tgt_content; + ASSERT_EQ(0, tgt_writer.WriteBytes(tgt_content.data(), tgt_content.size())); + ASSERT_EQ(0, tgt_writer.FinishEntry()); + ASSERT_EQ(0, tgt_writer.Finish()); + + // Compute patch. + TemporaryFile patch_file; + std::vector args = { + "imgdiff", "-z", src_file.path, tgt_file.path, patch_file.path, + }; + ASSERT_EQ(0, imgdiff(args.size(), args.data())); + + // Verify. + std::string tgt; + ASSERT_TRUE(android::base::ReadFileToString(tgt_file.path, &tgt)); + std::string src; + ASSERT_TRUE(android::base::ReadFileToString(src_file.path, &src)); + std::string patch; + ASSERT_TRUE(android::base::ReadFileToString(patch_file.path, &patch)); + + verify_patched_image(src, patch, tgt); +} + TEST(ImgdiffTest, zip_mode_smoke_trailer_zeros) { + // Generate 1 block of random data. + std::string random_data; + random_data.reserve(4096); + generate_n(back_inserter(random_data), 4096, []() { return rand() % 256; }); + // Construct src and tgt zip files. TemporaryFile src_file; FILE* src_file_ptr = fdopen(src_file.release(), "wb"); ZipWriter src_writer(src_file_ptr); ASSERT_EQ(0, src_writer.StartEntry("file1.txt", ZipWriter::kCompress)); - const std::string src_content("abcdefg"); + const std::string src_content = random_data; ASSERT_EQ(0, src_writer.WriteBytes(src_content.data(), src_content.size())); ASSERT_EQ(0, src_writer.FinishEntry()); ASSERT_EQ(0, src_writer.Finish()); @@ -261,7 +310,7 @@ TEST(ImgdiffTest, zip_mode_smoke_trailer_zeros) { FILE* tgt_file_ptr = fdopen(tgt_file.release(), "wb"); ZipWriter tgt_writer(tgt_file_ptr); ASSERT_EQ(0, tgt_writer.StartEntry("file1.txt", ZipWriter::kCompress)); - const std::string tgt_content("abcdefgxyz"); + const std::string tgt_content = random_data + "abcdefg"; ASSERT_EQ(0, tgt_writer.WriteBytes(tgt_content.data(), tgt_content.size())); ASSERT_EQ(0, tgt_writer.FinishEntry()); ASSERT_EQ(0, tgt_writer.Finish()); @@ -298,23 +347,19 @@ TEST(ImgdiffTest, zip_mode_smoke_trailer_zeros) { } TEST(ImgdiffTest, image_mode_simple) { - // src: "abcdefgh" + gzipped "xyz" (echo -n "xyz" | gzip -f | hd). - const std::vector src_data = { 'a', 'b', 'c', 'd', 'e', 'f', 'g', - 'h', '\x1f', '\x8b', '\x08', '\x00', '\xc4', '\x1e', - '\x53', '\x58', '\x00', '\x03', '\xab', '\xa8', '\xac', - '\x02', '\x00', '\x67', '\xba', '\x8e', '\xeb', '\x03', - '\x00', '\x00', '\x00' }; - const std::string src(src_data.cbegin(), src_data.cend()); + std::string gzipped_source_path = from_testdata_base("gzipped_source"); + std::string gzipped_source; + ASSERT_TRUE(android::base::ReadFileToString(gzipped_source_path, &gzipped_source)); + + const std::string src = "abcdefg" + gzipped_source; TemporaryFile src_file; ASSERT_TRUE(android::base::WriteStringToFile(src, src_file.path)); - // tgt: "abcdefgxyz" + gzipped "xxyyzz". - const std::vector tgt_data = { - 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'x', 'y', 'z', '\x1f', '\x8b', - '\x08', '\x00', '\x62', '\x1f', '\x53', '\x58', '\x00', '\x03', '\xab', '\xa8', '\xa8', '\xac', - '\xac', '\xaa', '\x02', '\x00', '\x96', '\x30', '\x06', '\xb7', '\x06', '\x00', '\x00', '\x00' - }; - const std::string tgt(tgt_data.cbegin(), tgt_data.cend()); + std::string gzipped_target_path = from_testdata_base("gzipped_target"); + std::string gzipped_target; + ASSERT_TRUE(android::base::ReadFileToString(gzipped_target_path, &gzipped_target)); + const std::string tgt = "abcdefgxyz" + gzipped_target; + TemporaryFile tgt_file; ASSERT_TRUE(android::base::WriteStringToFile(tgt, tgt_file.path)); @@ -404,23 +449,21 @@ TEST(ImgdiffTest, image_mode_different_num_chunks) { } TEST(ImgdiffTest, image_mode_merge_chunks) { - // src: "abcdefgh" + gzipped "xyz" (echo -n "xyz" | gzip -f | hd). - const std::vector src_data = { 'a', 'b', 'c', 'd', 'e', 'f', 'g', - 'h', '\x1f', '\x8b', '\x08', '\x00', '\xc4', '\x1e', - '\x53', '\x58', '\x00', '\x03', '\xab', '\xa8', '\xac', - '\x02', '\x00', '\x67', '\xba', '\x8e', '\xeb', '\x03', - '\x00', '\x00', '\x00' }; - const std::string src(src_data.cbegin(), src_data.cend()); + // src: "abcdefg" + gzipped_source. + std::string gzipped_source_path = from_testdata_base("gzipped_source"); + std::string gzipped_source; + ASSERT_TRUE(android::base::ReadFileToString(gzipped_source_path, &gzipped_source)); + + const std::string src = "abcdefg" + gzipped_source; TemporaryFile src_file; ASSERT_TRUE(android::base::WriteStringToFile(src, src_file.path)); - // tgt: gzipped "xyz" + "abcdefgh". - const std::vector tgt_data = { - '\x1f', '\x8b', '\x08', '\x00', '\x62', '\x1f', '\x53', '\x58', '\x00', '\x03', '\xab', '\xa8', - '\xa8', '\xac', '\xac', '\xaa', '\x02', '\x00', '\x96', '\x30', '\x06', '\xb7', '\x06', '\x00', - '\x00', '\x00', 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'x', 'y', 'z' - }; - const std::string tgt(tgt_data.cbegin(), tgt_data.cend()); + // tgt: gzipped_target + "abcdefgxyz". + std::string gzipped_target_path = from_testdata_base("gzipped_target"); + std::string gzipped_target; + ASSERT_TRUE(android::base::ReadFileToString(gzipped_target_path, &gzipped_target)); + + const std::string tgt = gzipped_target + "abcdefgxyz"; TemporaryFile tgt_file; ASSERT_TRUE(android::base::WriteStringToFile(tgt, tgt_file.path)); diff --git a/tests/testdata/gzipped_source b/tests/testdata/gzipped_source new file mode 100644 index 0000000000000000000000000000000000000000..6d425d059bb21c6d543ca7a7129c363528340741 GIT binary patch literal 1436 zcmV;N1!MXjiwFQ%0|r|F1MOFBZ`(Ey{;pqfRlH^1YCA7Y8zhkn4DE&jD+UznfT1l6 zT0GfYC{iVi4J-HJ_qlN~3rcFL?4oCydZg+bn|yV=N&dzq+% zEk#iIM)s(e`MP2^+4BAAZ+4!y+?;&jW)f7neTc{R$8U2pX>v2BolG6xD^2{E?h}n(P;ODW4JO$p=)YmSV$LS4YUldUCRajuG+Sk&y4(59_a+6UPE%6@0ifD@6?182hallZ#3?;iE* zp!&+Nj9K#FZ6j^#E7m|?VzB6{tP=Spx=<$Tvb>3%WODd&3TM+(@n~7(Cr8tds_OaH z)g}#^{rYobR~+Z!G%v)iU0=Ebb_eZ#T=1zpJUQOA>*{*QthYOTe0=cu(UV=fw(NVe z_%?~ka(SGeiYNKDaz{Ycp0^Sf*oPL-HsPL-GHuO@U98Z`x&V}yH^_kk=($r>i1{lN zzG5Y$u`14HGjsuGm7L~Uu?c>*WovnZWM>ch+fVQGm~?GY1U76NuemMf`b4KH;7L6)&RIkcNH+V08-5?Z~l#gtwL zVxvlQH!4+pxWcO_AJY3;#gzKsYKf9#m#ZLRoK$Nq%G}b0nm5jsznG~x85}Wg*mwqe z_ye(^*rYAa*hGjO!xT_Fi@wpDkp)-dNG%-7U|>{^*RiQPV1tD#tLrL6Ft1|B;35*3 zE3JDF5m`+|y;35Oi6`5^&#|0W0^I|^cF;`;2YCJ==PGa=a{y(V=Yn&1$m>cWbdRK^E%x`nTE)V^Po z_==q*{Uu(_x&ml~j0rvciig;HK!RZpQ8;W~+9gB_!{OSxOBA9O0P65$KviP6YAfUd zxRmLP5l2s zW+}Sf4kW$7{e^PTEeu%1YjW~ApB(x`*d$#KQ0OyCY-e*Pci+g*H^3hz6@48L`bPLY qzDc~*Ze0?}8f~z@+U;AjPw4ae0I0jl!KAreB*{PG^3YBD4FCYYUCl=T literal 0 HcmV?d00001 diff --git a/tests/testdata/gzipped_target b/tests/testdata/gzipped_target new file mode 100644 index 0000000000000000000000000000000000000000..562126286188b6c1bc554b6bda430af186aaa6df GIT binary patch literal 1502 zcmV<41tIz$iwFQ|0|r|F1MOFBZ`(Ey{;pqfQ@mx~YCDdTG;?DQ=$Z`yT6AdF0Yh6D zv~;q$NTf-~jsxlQ0-&aEBVwp26+n+7VEUQhgrCX;)T$6NB^z5%E z7cXXU@B365B?=}i4U4=KZjJRp+{d5j`Jd-6UR*pmpY?8QgS(NwVq;{=gOz1-B@?z$ zWaS5**+S%bROpmvQDs)FlAGBiN-HDPYGK(`rQgqI5gaVFg_cN{3t?7v7Dd-C@&wdi1Ck6r9Sc2$JTb73DaCgQQ-!f()RA zeZTbE^}beps^ieARRx6YUY2bXODDAwaMA`mZVe14x+qwsoNlCt_qP=adDG@*Bb&4` zQ8`@*CzFlrsFm5OWHZ?E{qb)$mbcs-f97W3WU+mTll$v8xf#^C>C;Z8#`j7C+o$_P z#RE&d!}t-|wanbTPRMA#U2K`JohyIvhv&22W*0l0tTn}zx>WjF5xh^4W_afJ(ouks ztw``seOQ!g#Zd(9893`n|Kqo|A4M{w*Wl|dPPmI{HlsKukjJgrGv<Qu|3=LA+e`EIh@Y0^)y@f+i1lmkN3Bx!`D3FFJ zWnT~@E~?yTcl#m~0&bmbZP}6&nceuJW+O$eno6a8fZguw2ZI_Mk4)ZiCmcWUpS$Po zQM(VSuN0>~OBTGXrA>YL8fZ%l7NN>YkzAq+&C;qUZhR*hjGqnRY%qoA+ zrL;0ZXF3~s>D*HvW)=vinpjpAF%f1VU`J$$Ol4ZZOXWb2r7ur*&7q98yHbKgF5gvR zNUt2Rk_EaO71A%R@F|K1^sbUVrPi4$Mai*Aq!T_)s`6YEiKa6-uboSKHj;BvxS~I> z_6+v$dtyPpNo(A(ju0ohA)t8TeWN!c3#P)A8aSlFz$gu`eN#8U1{+s8uS(~^yz(J~ zi%4K1^SlKSkyVscD=8c)f3qF@45#x_pnCw=0Ns#qf%EqXmyRo+11Rex5uCt7Qk4>^ z^S%&h>3IoG0%#8=u9H-F8hB{@EJQ*C$dB&irG?J@1%<~@0x=t)uZm&Z!Jjw~sGbR- zZ@-D!XUn~?nO1QxtU7cIP?d-?%&fS#J;FNl>q#Hi&8|c0q|5m;CU+=GWgR-*!d3}t z-!2OL@|`69)j!Qb0n|dqgdTpugYP{cLAM7l>^3j-5~77-cdgAO3NaS|YJA+Gnqs)- zTBHk*D`T_p8HQ?@CT&~$7x~$#{y%)5!{0A-f&WwdpQN}RIkcI581BIg6<&uHhRTAK z|Nlcei_!IVAZZQmAC!x*(P81A$yc1YXoKz3ZeOc?LZ9CUK;bC|llpZLMZfsTyf>cGBrFf7bP$%4DP27K z{rSt67muHwhrOeYz2hkzPL9LJNr%G2jnC7~Nk4UHjt{1EbTDqe#ycwi0H={AB&QAl E0Dy+#Q~&?~ literal 0 HcmV?d00001