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

@@ -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) {

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<size_t>(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);

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

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<const char*> 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<char> 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<char> 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<char> 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<char> 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));