2 files changed, 89 insertions, 43 deletions

diff --git a/install.cpp b/install.cpp
index 95794ce07..73ddf5e92 100644
--- a/install.cpp
+++ b/install.cpp
@@ -56,8 +56,6 @@
 
 using namespace std::chrono_literals;
 
-static constexpr const char* METADATA_PATH = "META-INF/com/android/metadata";
-
 // Default allocation of progress bar segments to operations
 static constexpr int VERIFICATION_PROGRESS_TIME = 60;
 static constexpr float VERIFICATION_PROGRESS_FRACTION = 0.25;
@@ -79,53 +77,51 @@ static int parse_build_number(const std::string& str) {
     return -1;
 }
 
-bool read_metadata_from_package(ZipArchiveHandle zip, std::string* meta_data) {
-    ZipString metadata_path(METADATA_PATH);
-    ZipEntry meta_entry;
-    if (meta_data == nullptr) {
-        LOG(ERROR) << "string* meta_data can't be nullptr";
-        return false;
-    }
-    if (FindEntry(zip, metadata_path, &meta_entry) != 0) {
-        LOG(ERROR) << "Failed to find " << METADATA_PATH << " in update package";
-        return false;
-    }
+bool read_metadata_from_package(ZipArchiveHandle zip, std::string* metadata) {
+  CHECK(metadata != nullptr);
 
-    meta_data->resize(meta_entry.uncompressed_length, '\0');
-    if (ExtractToMemory(zip, &meta_entry, reinterpret_cast<uint8_t*>(&(*meta_data)[0]),
-                        meta_entry.uncompressed_length) != 0) {
-        LOG(ERROR) << "Failed to read metadata in update package";
-        return false;
-    }
-    return true;
+  static constexpr const char* METADATA_PATH = "META-INF/com/android/metadata";
+  ZipString path(METADATA_PATH);
+  ZipEntry entry;
+  if (FindEntry(zip, path, &entry) != 0) {
+    LOG(ERROR) << "Failed to find " << METADATA_PATH;
+    return false;
+  }
+
+  uint32_t length = entry.uncompressed_length;
+  metadata->resize(length, '\0');
+  int32_t err = ExtractToMemory(zip, &entry, reinterpret_cast<uint8_t*>(&(*metadata)[0]), length);
+  if (err != 0) {
+    LOG(ERROR) << "Failed to extract " << METADATA_PATH << ": " << ErrorCodeString(err);
+    return false;
+  }
+  return true;
 }
 
 // Read the build.version.incremental of src/tgt from the metadata and log it to last_install.
 static void read_source_target_build(ZipArchiveHandle zip, std::vector<std::string>& log_buffer) {
-    std::string meta_data;
-    if (!read_metadata_from_package(zip, &meta_data)) {
-        return;
-    }
-    // Examples of the pre-build and post-build strings in metadata:
-    // pre-build-incremental=2943039
-    // post-build-incremental=2951741
-    std::vector<std::string> lines = android::base::Split(meta_data, "\n");
-    for (const std::string& line : lines) {
-        std::string str = android::base::Trim(line);
-        if (android::base::StartsWith(str, "pre-build-incremental")){
-            int source_build = parse_build_number(str);
-            if (source_build != -1) {
-                log_buffer.push_back(android::base::StringPrintf("source_build: %d",
-                        source_build));
-            }
-        } else if (android::base::StartsWith(str, "post-build-incremental")) {
-            int target_build = parse_build_number(str);
-            if (target_build != -1) {
-                log_buffer.push_back(android::base::StringPrintf("target_build: %d",
-                        target_build));
-            }
-        }
+  std::string metadata;
+  if (!read_metadata_from_package(zip, &metadata)) {
+    return;
+  }
+  // Examples of the pre-build and post-build strings in metadata:
+  //   pre-build-incremental=2943039
+  //   post-build-incremental=2951741
+  std::vector<std::string> lines = android::base::Split(metadata, "\n");
+  for (const std::string& line : lines) {
+    std::string str = android::base::Trim(line);
+    if (android::base::StartsWith(str, "pre-build-incremental")) {
+      int source_build = parse_build_number(str);
+      if (source_build != -1) {
+        log_buffer.push_back(android::base::StringPrintf("source_build: %d", source_build));
+      }
+    } else if (android::base::StartsWith(str, "post-build-incremental")) {
+      int target_build = parse_build_number(str);
+      if (target_build != -1) {
+        log_buffer.push_back(android::base::StringPrintf("target_build: %d", target_build));
+      }
     }
+  }
 }
 
 // Extract the update binary from the open zip archive |zip| located at |path| and store into |cmd|
diff --git a/tests/component/install_test.cpp b/tests/component/install_test.cpp
index fd3b28b07..2143dd7bb 100644
--- a/tests/component/install_test.cpp
+++ b/tests/component/install_test.cpp
@@ -62,6 +62,56 @@ TEST(InstallTest, verify_package_compatibility_invalid_entry) {
   CloseArchive(zip);
 }
 
+TEST(InstallTest, read_metadata_from_package_smoke) {
+  TemporaryFile temp_file;
+  FILE* zip_file = fdopen(temp_file.fd, "w");
+  ZipWriter writer(zip_file);
+  ASSERT_EQ(0, writer.StartEntry("META-INF/com/android/metadata", kCompressStored));
+  const std::string content("abcdefg");
+  ASSERT_EQ(0, writer.WriteBytes(content.data(), content.size()));
+  ASSERT_EQ(0, writer.FinishEntry());
+  ASSERT_EQ(0, writer.Finish());
+  ASSERT_EQ(0, fclose(zip_file));
+
+  ZipArchiveHandle zip;
+  ASSERT_EQ(0, OpenArchive(temp_file.path, &zip));
+  std::string metadata;
+  ASSERT_TRUE(read_metadata_from_package(zip, &metadata));
+  ASSERT_EQ(content, metadata);
+  CloseArchive(zip);
+
+  TemporaryFile temp_file2;
+  FILE* zip_file2 = fdopen(temp_file2.fd, "w");
+  ZipWriter writer2(zip_file2);
+  ASSERT_EQ(0, writer2.StartEntry("META-INF/com/android/metadata", kCompressDeflated));
+  ASSERT_EQ(0, writer2.WriteBytes(content.data(), content.size()));
+  ASSERT_EQ(0, writer2.FinishEntry());
+  ASSERT_EQ(0, writer2.Finish());
+  ASSERT_EQ(0, fclose(zip_file2));
+
+  ASSERT_EQ(0, OpenArchive(temp_file2.path, &zip));
+  metadata.clear();
+  ASSERT_TRUE(read_metadata_from_package(zip, &metadata));
+  ASSERT_EQ(content, metadata);
+  CloseArchive(zip);
+}
+
+TEST(InstallTest, read_metadata_from_package_no_entry) {
+  TemporaryFile temp_file;
+  FILE* zip_file = fdopen(temp_file.fd, "w");
+  ZipWriter writer(zip_file);
+  ASSERT_EQ(0, writer.StartEntry("dummy_entry", kCompressStored));
+  ASSERT_EQ(0, writer.FinishEntry());
+  ASSERT_EQ(0, writer.Finish());
+  ASSERT_EQ(0, fclose(zip_file));
+
+  ZipArchiveHandle zip;
+  ASSERT_EQ(0, OpenArchive(temp_file.path, &zip));
+  std::string metadata;
+  ASSERT_FALSE(read_metadata_from_package(zip, &metadata));
+  CloseArchive(zip);
+}
+
 TEST(InstallTest, update_binary_command_smoke) {
 #ifdef AB_OTA_UPDATER
   TemporaryFile temp_file;