From f7eb760fe76cb66c5d5341b03d6a66cc1f06d795 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Mon, 27 Mar 2017 15:12:48 -0700 Subject: applypatch: Change the ssize_t length parameters to size_t. Mostly for applypatch family APIs like ApplyBSDiffPatch() and ApplyImagePatch(). Changing to size_t doesn't indicate they would necessarily work with very large size_t (e.g. > ssize_t), just similar to write(2). But otherwise accepting negative length doesn't make much sense. Also change the return type of SinkFn from ssize_t to size_t. Callers tell a successful sink by comparing the number of written bytes against the desired value. Negative return values like -1 are not needed. This also makes it consistent with bsdiff::bspatch interface. Test: recovery_component_test Test: Apply an incremental with the new updater. Change-Id: I7ff1615203a5c9854134f75d019e266f4ea6e714 --- tests/component/imgdiff_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests/component/imgdiff_test.cpp') diff --git a/tests/component/imgdiff_test.cpp b/tests/component/imgdiff_test.cpp index 2f648501c..64e1f2990 100644 --- a/tests/component/imgdiff_test.cpp +++ b/tests/component/imgdiff_test.cpp @@ -27,7 +27,7 @@ using android::base::get_unaligned; -static ssize_t MemorySink(const unsigned char* data, ssize_t len, void* token) { +static size_t MemorySink(const unsigned char* data, size_t len, void* token) { std::string* s = static_cast(token); s->append(reinterpret_cast(data), len); return len; -- cgit v1.2.3 From c0e1c46a707370952ea1ddeb71b176d04fe71bb9 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 1 Feb 2017 10:20:10 -0800 Subject: applypatch: Let Apply{BSDiff,Image}Patch accept std::function. Test: mmma bootable/recovery system/update_engine Test: recovery_component_test Change-Id: I93c2caa87bf94a53509bb37f98f2c02bcadb6f5c --- tests/component/imgdiff_test.cpp | 80 ++++++++++++---------------------------- 1 file changed, 24 insertions(+), 56 deletions(-) (limited to 'tests/component/imgdiff_test.cpp') diff --git a/tests/component/imgdiff_test.cpp b/tests/component/imgdiff_test.cpp index 64e1f2990..7d00a3d53 100644 --- a/tests/component/imgdiff_test.cpp +++ b/tests/component/imgdiff_test.cpp @@ -14,6 +14,8 @@ * limitations under the License. */ +#include + #include #include @@ -27,12 +29,6 @@ using android::base::get_unaligned; -static size_t MemorySink(const unsigned char* data, size_t len, void* token) { - std::string* s = static_cast(token); - s->append(reinterpret_cast(data), len); - return len; -} - // Sanity check for the given imgdiff patch header. static void verify_patch_header(const std::string& patch, size_t* num_normal, size_t* num_raw, size_t* num_deflate) { @@ -79,6 +75,18 @@ static void verify_patch_header(const std::string& patch, size_t* num_normal, si if (num_deflate != nullptr) *num_deflate = deflate; } +static void verify_patched_image(const std::string& src, const std::string& patch, + const std::string& tgt) { + std::string patched; + ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast(src.data()), src.size(), + reinterpret_cast(patch.data()), patch.size(), + [&patched](const unsigned char* data, size_t len) { + patched.append(reinterpret_cast(data), len); + return len; + })); + ASSERT_EQ(tgt, patched); +} + TEST(ImgdiffTest, invalid_args) { // Insufficient inputs. ASSERT_EQ(2, imgdiff(1, (const char* []){ "imgdiff" })); @@ -124,11 +132,7 @@ TEST(ImgdiffTest, image_mode_smoke) { ASSERT_EQ(0U, num_deflate); ASSERT_EQ(1U, num_raw); - std::string patched; - ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast(src.data()), src.size(), - reinterpret_cast(patch.data()), patch.size(), - MemorySink, &patched)); - ASSERT_EQ(tgt, patched); + verify_patched_image(src, patch, tgt); } TEST(ImgdiffTest, zip_mode_smoke_store) { @@ -177,11 +181,7 @@ TEST(ImgdiffTest, zip_mode_smoke_store) { ASSERT_EQ(0U, num_deflate); ASSERT_EQ(1U, num_raw); - std::string patched; - ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast(src.data()), src.size(), - reinterpret_cast(patch.data()), patch.size(), - MemorySink, &patched)); - ASSERT_EQ(tgt, patched); + verify_patched_image(src, patch, tgt); } TEST(ImgdiffTest, zip_mode_smoke_compressed) { @@ -230,11 +230,7 @@ TEST(ImgdiffTest, zip_mode_smoke_compressed) { ASSERT_EQ(1U, num_deflate); ASSERT_EQ(2U, num_raw); - std::string patched; - ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast(src.data()), src.size(), - reinterpret_cast(patch.data()), patch.size(), - MemorySink, &patched)); - ASSERT_EQ(tgt, patched); + verify_patched_image(src, patch, tgt); } TEST(ImgdiffTest, zip_mode_smoke_trailer_zeros) { @@ -286,11 +282,7 @@ TEST(ImgdiffTest, zip_mode_smoke_trailer_zeros) { ASSERT_EQ(1U, num_deflate); ASSERT_EQ(2U, num_raw); - std::string patched; - ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast(src.data()), src.size(), - reinterpret_cast(patch.data()), patch.size(), - MemorySink, &patched)); - ASSERT_EQ(tgt, patched); + verify_patched_image(src, patch, tgt); } TEST(ImgdiffTest, image_mode_simple) { @@ -333,11 +325,7 @@ TEST(ImgdiffTest, image_mode_simple) { ASSERT_EQ(1U, num_deflate); ASSERT_EQ(2U, num_raw); - std::string patched; - ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast(src.data()), src.size(), - reinterpret_cast(patch.data()), patch.size(), - MemorySink, &patched)); - ASSERT_EQ(tgt, patched); + verify_patched_image(src, patch, tgt); } TEST(ImgdiffTest, image_mode_different_num_chunks) { @@ -413,11 +401,7 @@ TEST(ImgdiffTest, image_mode_merge_chunks) { ASSERT_EQ(1U, num_deflate); ASSERT_EQ(2U, num_raw); - std::string patched; - ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast(src.data()), src.size(), - reinterpret_cast(patch.data()), patch.size(), - MemorySink, &patched)); - ASSERT_EQ(tgt, patched); + verify_patched_image(src, patch, tgt); } TEST(ImgdiffTest, image_mode_spurious_magic) { @@ -454,11 +438,7 @@ TEST(ImgdiffTest, image_mode_spurious_magic) { ASSERT_EQ(0U, num_deflate); ASSERT_EQ(1U, num_raw); - std::string patched; - ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast(src.data()), src.size(), - reinterpret_cast(patch.data()), patch.size(), - MemorySink, &patched)); - ASSERT_EQ(tgt, patched); + verify_patched_image(src, patch, tgt); } TEST(ImgdiffTest, image_mode_short_input1) { @@ -494,11 +474,7 @@ TEST(ImgdiffTest, image_mode_short_input1) { ASSERT_EQ(0U, num_deflate); ASSERT_EQ(1U, num_raw); - std::string patched; - ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast(src.data()), src.size(), - reinterpret_cast(patch.data()), patch.size(), - MemorySink, &patched)); - ASSERT_EQ(tgt, patched); + verify_patched_image(src, patch, tgt); } TEST(ImgdiffTest, image_mode_short_input2) { @@ -534,11 +510,7 @@ TEST(ImgdiffTest, image_mode_short_input2) { ASSERT_EQ(0U, num_deflate); ASSERT_EQ(1U, num_raw); - std::string patched; - ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast(src.data()), src.size(), - reinterpret_cast(patch.data()), patch.size(), - MemorySink, &patched)); - ASSERT_EQ(tgt, patched); + verify_patched_image(src, patch, tgt); } TEST(ImgdiffTest, image_mode_single_entry_long) { @@ -577,9 +549,5 @@ TEST(ImgdiffTest, image_mode_single_entry_long) { ASSERT_EQ(0U, num_deflate); ASSERT_EQ(0U, num_raw); - std::string patched; - ASSERT_EQ(0, ApplyImagePatch(reinterpret_cast(src.data()), src.size(), - reinterpret_cast(patch.data()), patch.size(), - MemorySink, &patched)); - ASSERT_EQ(tgt, patched); + verify_patched_image(src, patch, tgt); } -- cgit v1.2.3 From ce5fa5e5384508655c804519d428a402bb3df1d9 Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Mon, 15 May 2017 12:32:33 -0700 Subject: Print SHA1 of the patch if bsdiff fails with data error This will help us to identify the patch corruption. Meanwhile fix a wrong size parameter passed to bspatch. (patch->data.size() into patch->data.size() - patch_offset). Also remove the only usage of "ApplyBSDiffPatchMem()" and inline its Sink function for simplicity. Bug: 37855643 Test: Prints SHA1 for corrupted patch in imgdiff_test. Change-Id: Ibf2db8c08b0ded1409bb7c91a3547a6bf99c601d --- tests/component/imgdiff_test.cpp | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) (limited to 'tests/component/imgdiff_test.cpp') diff --git a/tests/component/imgdiff_test.cpp b/tests/component/imgdiff_test.cpp index 7d00a3d53..6f5960bbd 100644 --- a/tests/component/imgdiff_test.cpp +++ b/tests/component/imgdiff_test.cpp @@ -551,3 +551,42 @@ TEST(ImgdiffTest, image_mode_single_entry_long) { verify_patched_image(src, patch, tgt); } + +TEST(ImgpatchTest, image_mode_patch_corruption) { + // 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()); + 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()); + TemporaryFile tgt_file; + ASSERT_TRUE(android::base::WriteStringToFile(tgt, tgt_file.path)); + + TemporaryFile patch_file; + std::vector args = { + "imgdiff", src_file.path, tgt_file.path, patch_file.path, + }; + ASSERT_EQ(0, imgdiff(args.size(), args.data())); + + // Verify. + std::string patch; + ASSERT_TRUE(android::base::ReadFileToString(patch_file.path, &patch)); + verify_patched_image(src, patch, tgt); + + // Corrupt the end of the patch and expect the ApplyImagePatch to fail. + patch.insert(patch.end() - 10, 10, '0'); + ASSERT_EQ(-1, ApplyImagePatch(reinterpret_cast(src.data()), src.size(), + reinterpret_cast(patch.data()), patch.size(), + [](const unsigned char* /*data*/, size_t len) { return len; })); +} -- cgit v1.2.3 From 14ebc1e5ae6968424eb242f3b0330f82e475a1e4 Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Wed, 5 Jul 2017 12:04:07 -0700 Subject: Fix a rare failure for imgdiff when random data equals gzip header In a rare case, a random chunk will pass both the gzip header check and the inflation process; but fail the uncompressed length check in the footer. This leads to a imgdiff failure. So, we should treat this chunk as 'normal' instead of 'inflated' while generating the patch. Bug: 63334984 Test: imgdiff generates patch successfully on previous failing images. Change-Id: Ice84f22d3653bce9756bda91e70528c0d2f264a0 --- tests/component/imgdiff_test.cpp | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) (limited to 'tests/component/imgdiff_test.cpp') diff --git a/tests/component/imgdiff_test.cpp b/tests/component/imgdiff_test.cpp index 6f5960bbd..bf25aebb0 100644 --- a/tests/component/imgdiff_test.cpp +++ b/tests/component/imgdiff_test.cpp @@ -328,6 +328,39 @@ TEST(ImgdiffTest, image_mode_simple) { verify_patched_image(src, patch, tgt); } +TEST(ImgdiffTest, image_mode_bad_gzip) { + // Modify the uncompressed length in the gzip footer. + 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', + '\xff', '\xff', '\xff' }; + const std::string src(src_data.cbegin(), src_data.cend()); + TemporaryFile src_file; + ASSERT_TRUE(android::base::WriteStringToFile(src, src_file.path)); + + // Modify the uncompressed length in the gzip footer. + 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', '\xff', '\xff', '\xff' + }; + const std::string tgt(tgt_data.cbegin(), tgt_data.cend()); + TemporaryFile tgt_file; + ASSERT_TRUE(android::base::WriteStringToFile(tgt, tgt_file.path)); + + TemporaryFile patch_file; + std::vector args = { + "imgdiff", src_file.path, tgt_file.path, patch_file.path, + }; + ASSERT_EQ(0, imgdiff(args.size(), args.data())); + + // Verify. + std::string patch; + ASSERT_TRUE(android::base::ReadFileToString(patch_file.path, &patch)); + verify_patched_image(src, patch, tgt); +} + TEST(ImgdiffTest, image_mode_different_num_chunks) { // src: "abcdefgh" + gzipped "xyz" (echo -n "xyz" | gzip -f | hd) + gzipped "test". const std::vector src_data = { -- cgit v1.2.3