Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f54c9a1d authored by Winson's avatar Winson
Browse files

De-duplicate entries written with AAPT2 convert 

There was no check for whether we had already written a specific file path to the APK when using the convert command.

If the resources table points 2 resource IDs at the same file on disk, the convert command would write the file twice, creating two entries.

This holds a set of file paths already written and ignores duplicates.

Bug: 123271593

Test: Ran convert on linked bug's weird.apk
Test: aapt2_tests case for duplicate entries

Change-Id: Ia22515bf8e8297624aaadbf6a9e47159026c63e5
parent 3b943e77

tools/aapt2/Android.bp

+2 −1
Original line number Original line Diff line number Diff line
@@ -182,7 +182,8 @@ cc_test_host { 
    defaults: ["aapt2_defaults"],

    defaults: ["aapt2_defaults"],

    data: [

    data: [

         "integration-tests/CompileTest/**/*",

         "integration-tests/CompileTest/**/*",

         "integration-tests/CommandTests/**/*"

         "integration-tests/CommandTests/**/*",

         "integration-tests/ConvertTest/**/*"

    ],

    ],

}

}

tools/aapt2/cmd/Convert.cpp

+10 −4
Original line number Original line Diff line number Diff line
@@ -284,6 +284,8 @@ int Convert(IAaptContext* context, LoadedApk* apk, IArchiveWriter* output_writer 
    // The table might be modified by below code.

    // The table might be modified by below code.

    auto converted_table = apk->GetResourceTable();

    auto converted_table = apk->GetResourceTable();





    std::unordered_set<std::string> files_written;



    // Resources

    // Resources

    for (const auto& package : converted_table->packages) {

    for (const auto& package : converted_table->packages) {

      for (const auto& type : package->types) {

      for (const auto& type : package->types) {
@@ -297,11 +299,15 @@ int Convert(IAaptContext* context, LoadedApk* apk, IArchiveWriter* output_writer 
                return 1;

                return 1;

              }

              }





              // Only serialize if we haven't seen this file before

              if (files_written.insert(*file->path).second) {

                if (!serializer->SerializeFile(file, output_writer)) {

                if (!serializer->SerializeFile(file, output_writer)) {

                  context->GetDiagnostics()->Error(DiagMessage(apk->GetSource())

                  context->GetDiagnostics()->Error(DiagMessage(apk->GetSource())

                                                 << "failed to serialize file " << *file->path);

                                                       << "failed to serialize file "

                                                       << *file->path);

                  return 1;

                  return 1;

                }

                }

              }

            } // file

            } // file

          } // config_value

          } // config_value

        } // entry

        } // entry

tools/aapt2/cmd/Convert_test.cpp

+42 −0
Original line number Original line Diff line number Diff line
@@ -18,6 +18,7 @@ 




#include "LoadedApk.h"

#include "LoadedApk.h"

#include "test/Test.h"

#include "test/Test.h"

#include "ziparchive/zip_archive.h"





using testing::Eq;

using testing::Eq;

using testing::Ne;

using testing::Ne;
@@ -95,4 +96,45 @@ TEST_F(ConvertTest, KeepRawXmlStrings) { 
  EXPECT_THAT(util::GetString(tree.getStrings(), static_cast<size_t>(raw_index)), Eq("007"));

  EXPECT_THAT(util::GetString(tree.getStrings(), static_cast<size_t>(raw_index)), Eq("007"));

}

}





TEST_F(ConvertTest, DuplicateEntriesWrittenOnce) {

  StdErrDiagnostics diag;

  const std::string apk_path =

      file::BuildPath({android::base::GetExecutableDirectory(),

                       "integration-tests", "ConvertTest", "duplicate_entries.apk"});



  const std::string out_convert_apk = GetTestPath("out_convert.apk");

  std::vector<android::StringPiece> convert_args = {

      "-o", out_convert_apk,

      "--output-format", "proto",

      apk_path

  };

  ASSERT_THAT(ConvertCommand().Execute(convert_args, &std::cerr), Eq(0));



  ZipArchiveHandle handle;

  ASSERT_THAT(OpenArchive(out_convert_apk.c_str(), &handle), Eq(0));



  void* cookie = nullptr;



  ZipString prefix("res/theme/10");

  int32_t result = StartIteration(handle, &cookie, &prefix, nullptr);



  // If this is -5, that means we've found a duplicate entry and this test has failed

  EXPECT_THAT(result, Eq(0));



  // But if read succeeds, verify only one res/theme/10 entry

  int count = 0;



  // Can't pass nullptrs into Next()

  ZipString zip_name;

  ZipEntry zip_data;



  while ((result = Next(cookie, &zip_data, &zip_name)) == 0) {

    count++;

  }



  EndIteration(cookie);



  EXPECT_THAT(count, Eq(1));

}



}  // namespace aapt
 
}  // namespace aapt
 
 No newline at end of file

tools/aapt2/integration-tests/ConvertTest/duplicate_entries.apk

0 → 100644
+14.4 MiB

File added.

No diff preview for this file type.

View file