Fix asset compression to check name ends with arg

There exists a discrepancy between how aapt1 and aapt2 prevent the
compression of assets using the -0 flag. aapt1 checked if the file name
ended with an argument, but aapt2 is checking if the file extension
exactly matches the argument. This change makes aapt2 behave the same as
aapt1 for asset compression using the -0 flag.

Bug: 132823799
Test: aapt2_tests
Change-Id: I641b3ebce29e4407b543faea373a5ce516b70cda

6 files changed, 140 insertions, 45 deletions

diff --git a/tools/aapt2/cmd/Compile_test.cpp b/tools/aapt2/cmd/Compile_test.cpp
index 5f637bd8d582..fb786a31360e 100644
--- a/tools/aapt2/cmd/Compile_test.cpp
+++ b/tools/aapt2/cmd/Compile_test.cpp
@@ -200,7 +200,7 @@ static void AssertTranslations(CommandTestFixture *ctf, std::string file_name,
   const std::string compiled_files_dir = ctf->GetTestPath("/compiled_" + file_name);
   const std::string out_apk = ctf->GetTestPath("/" + file_name + ".apk");
 
-  CHECK(ctf->WriteFile(source_file, sTranslatableXmlContent));
+  ctf->WriteFile(source_file, sTranslatableXmlContent);
   CHECK(file::mkdirs(compiled_files_dir.data()));
 
   ASSERT_EQ(CompileCommand(&diag).Execute({
diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index f354bb610224..4b977225fd01 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -297,6 +297,25 @@ struct R {
   };
 };
 
+template <typename T>
+uint32_t GetCompressionFlags(const StringPiece& str, T options) {
+  if (options.do_not_compress_anything) {
+    return 0;
+  }
+
+  if (options.regex_to_not_compress
+      && std::regex_search(str.to_string(), options.regex_to_not_compress.value())) {
+    return 0;
+  }
+
+  for (const std::string& extension : options.extensions_to_not_compress) {
+    if (util::EndsWith(str, extension)) {
+      return 0;
+    }
+  }
+  return ArchiveEntry::kCompress;
+}
+
 class ResourceFileFlattener {
  public:
   ResourceFileFlattener(const ResourceFileFlattenerOptions& options, IAaptContext* context,
@@ -321,8 +340,6 @@ class ResourceFileFlattener {
     std::string dst_path;
   };
 
-  uint32_t GetCompressionFlags(const StringPiece& str);
-
   std::vector<std::unique_ptr<xml::XmlResource>> LinkAndVersionXmlFile(ResourceTable* table,
                                                                        FileOperation* file_op);
 
@@ -381,26 +398,6 @@ ResourceFileFlattener::ResourceFileFlattener(const ResourceFileFlattenerOptions&
   }
 }
 
-// TODO(rtmitchell): turn this function into a variable that points to a method that retrieves the
-// compression flag
-uint32_t ResourceFileFlattener::GetCompressionFlags(const StringPiece& str) {
-  if (options_.do_not_compress_anything) {
-    return 0;
-  }
-
-  if (options_.regex_to_not_compress
-      && std::regex_search(str.to_string(), options_.regex_to_not_compress.value())) {
-    return 0;
-  }
-
-  for (const std::string& extension : options_.extensions_to_not_compress) {
-    if (util::EndsWith(str, extension)) {
-      return 0;
-    }
-  }
-  return ArchiveEntry::kCompress;
-}
-
 static bool IsTransitionElement(const std::string& name) {
   return name == "fade" || name == "changeBounds" || name == "slide" || name == "explode" ||
          name == "changeImageTransform" || name == "changeTransform" ||
@@ -640,7 +637,8 @@ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archiv
           }
         } else {
           error |= !io::CopyFileToArchive(context_, file_op.file_to_copy, file_op.dst_path,
-                                          GetCompressionFlags(file_op.dst_path), archive_writer);
+                                          GetCompressionFlags(file_op.dst_path, options_),
+                                          archive_writer);
         }
       }
     }
@@ -1547,16 +1545,7 @@ class Linker {
     }
 
     for (auto& entry : merged_assets) {
-      uint32_t compression_flags = ArchiveEntry::kCompress;
-      std::string extension = file::GetExtension(entry.first).to_string();
-
-      if (options_.do_not_compress_anything
-          || options_.extensions_to_not_compress.count(extension) > 0
-          || (options_.regex_to_not_compress
-              && std::regex_search(extension, options_.regex_to_not_compress.value()))) {
-        compression_flags = 0u;
-      }
-
+      uint32_t compression_flags = GetCompressionFlags(entry.first, options_);
       if (!io::CopyFileToArchive(context_, entry.second.get(), entry.first, compression_flags,
                                  writer)) {
         return false;
diff --git a/tools/aapt2/cmd/Link.h b/tools/aapt2/cmd/Link.h
index 7c583858ee1d..5b0653ed53bd 100644
--- a/tools/aapt2/cmd/Link.h
+++ b/tools/aapt2/cmd/Link.h
@@ -248,7 +248,7 @@ class LinkCommand : public Command {
         "Changes the name of the target package for instrumentation. Most useful\n"
             "when used in conjunction with --rename-manifest-package.",
         &options_.manifest_fixer_options.rename_instrumentation_target_package);
-    AddOptionalFlagList("-0", "File extensions not to compress.",
+    AddOptionalFlagList("-0", "File suffix not to compress.",
         &options_.extensions_to_not_compress);
     AddOptionalSwitch("--no-compress", "Do not compress any resources.",
         &options_.do_not_compress_anything);
diff --git a/tools/aapt2/cmd/Link_test.cpp b/tools/aapt2/cmd/Link_test.cpp
index 9ea93f638aff..32ed1dd81b3f 100644
--- a/tools/aapt2/cmd/Link_test.cpp
+++ b/tools/aapt2/cmd/Link_test.cpp
@@ -43,10 +43,8 @@ TEST_F(LinkTest, RemoveRawXmlStrings) {
   // Load the binary xml tree
   android::ResXMLTree tree;
   std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(out_apk, &diag);
-
   std::unique_ptr<io::IData> data = OpenFileAsData(apk.get(), "res/xml/test.xml");
   ASSERT_THAT(data, Ne(nullptr));
-
   AssertLoadXml(apk.get(), data.get(), &tree);
 
   // Check that the raw string index has not been assigned
@@ -71,10 +69,8 @@ TEST_F(LinkTest, KeepRawXmlStrings) {
   // Load the binary xml tree
   android::ResXMLTree tree;
   std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(out_apk, &diag);
-
   std::unique_ptr<io::IData> data = OpenFileAsData(apk.get(), "res/xml/test.xml");
   ASSERT_THAT(data, Ne(nullptr));
-
   AssertLoadXml(apk.get(), data.get(), &tree);
 
   // Check that the raw string index has been set to the correct string pool entry
@@ -83,4 +79,96 @@ TEST_F(LinkTest, KeepRawXmlStrings) {
   EXPECT_THAT(util::GetString(tree.getStrings(), static_cast<size_t>(raw_index)), Eq("007"));
 }
 
+TEST_F(LinkTest, NoCompressAssets) {
+  StdErrDiagnostics diag;
+  std::string content(500, 'a');
+  WriteFile(GetTestPath("assets/testtxt"), content);
+  WriteFile(GetTestPath("assets/testtxt2"), content);
+  WriteFile(GetTestPath("assets/test.txt"), content);
+  WriteFile(GetTestPath("assets/test.hello.txt"), content);
+  WriteFile(GetTestPath("assets/test.hello.xml"), content);
+
+  const std::string out_apk = GetTestPath("out.apk");
+  std::vector<std::string> link_args = {
+      "--manifest", GetDefaultManifest(),
+      "-o", out_apk,
+      "-0", ".txt",
+      "-0", "txt2",
+      "-0", ".hello.txt",
+      "-0", "hello.xml",
+      "-A", GetTestPath("assets")
+  };
+
+  ASSERT_TRUE(Link(link_args, &diag));
+
+  std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(out_apk, &diag);
+  ASSERT_THAT(apk, Ne(nullptr));
+  io::IFileCollection* zip = apk->GetFileCollection();
+  ASSERT_THAT(zip, Ne(nullptr));
+
+  auto file = zip->FindFile("assets/testtxt");
+  ASSERT_THAT(file, Ne(nullptr));
+  EXPECT_TRUE(file->WasCompressed());
+
+  file = zip->FindFile("assets/testtxt2");
+  ASSERT_THAT(file, Ne(nullptr));
+  EXPECT_FALSE(file->WasCompressed());
+
+  file = zip->FindFile("assets/test.txt");
+  ASSERT_THAT(file, Ne(nullptr));
+  EXPECT_FALSE(file->WasCompressed());
+
+  file = zip->FindFile("assets/test.hello.txt");
+  ASSERT_THAT(file, Ne(nullptr));
+  EXPECT_FALSE(file->WasCompressed());
+
+  file = zip->FindFile("assets/test.hello.xml");
+  ASSERT_THAT(file, Ne(nullptr));
+  EXPECT_FALSE(file->WasCompressed());
+}
+
+TEST_F(LinkTest, NoCompressResources) {
+  StdErrDiagnostics diag;
+  std::string content(500, 'a');
+  const std::string compiled_files_dir = GetTestPath("compiled");
+  ASSERT_TRUE(CompileFile(GetTestPath("res/raw/testtxt"), content, compiled_files_dir, &diag));
+  ASSERT_TRUE(CompileFile(GetTestPath("res/raw/test.txt"), content, compiled_files_dir, &diag));
+  ASSERT_TRUE(CompileFile(GetTestPath("res/raw/test1.hello.txt"), content, compiled_files_dir,
+              &diag));
+  ASSERT_TRUE(CompileFile(GetTestPath("res/raw/test2.goodbye.xml"), content, compiled_files_dir,
+              &diag));
+
+  const std::string out_apk = GetTestPath("out.apk");
+  std::vector<std::string> link_args = {
+      "--manifest", GetDefaultManifest(),
+      "-o", out_apk,
+      "-0", ".txt",
+      "-0", ".hello.txt",
+      "-0", "goodbye.xml",
+  };
+
+  ASSERT_TRUE(Link(link_args, compiled_files_dir, &diag));
+
+  std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(out_apk, &diag);
+  ASSERT_THAT(apk, Ne(nullptr));
+  io::IFileCollection* zip = apk->GetFileCollection();
+  ASSERT_THAT(zip, Ne(nullptr));
+
+  auto file = zip->FindFile("res/raw/testtxt");
+  ASSERT_THAT(file, Ne(nullptr));
+  EXPECT_TRUE(file->WasCompressed());
+
+  file = zip->FindFile("res/raw/test.txt");
+  ASSERT_THAT(file, Ne(nullptr));
+  EXPECT_FALSE(file->WasCompressed());
+
+  file = zip->FindFile("res/raw/test1.hello.hello.txt");
+  ASSERT_THAT(file, Ne(nullptr));
+  EXPECT_FALSE(file->WasCompressed());
+
+  file = zip->FindFile("res/raw/test2.goodbye.goodbye.xml");
+  ASSERT_THAT(file, Ne(nullptr));
+  EXPECT_FALSE(file->WasCompressed());
+}
+
 }  // namespace aapt
\ No newline at end of file
diff --git a/tools/aapt2/test/Fixture.cpp b/tools/aapt2/test/Fixture.cpp
index a51b4a4649f1..5386802dbc8e 100644
--- a/tools/aapt2/test/Fixture.cpp
+++ b/tools/aapt2/test/Fixture.cpp
@@ -80,7 +80,7 @@ void TestDirectoryFixture::TearDown() {
   ClearDirectory(temp_dir_);
 }
 
-bool TestDirectoryFixture::WriteFile(const std::string& path, const std::string& contents) {
+void TestDirectoryFixture::WriteFile(const std::string& path, const std::string& contents) {
   CHECK(util::StartsWith(path, temp_dir_))
       << "Attempting to create a file outside of test temporary directory.";
 
@@ -91,16 +91,31 @@ bool TestDirectoryFixture::WriteFile(const std::string& path, const std::string&
     file::mkdirs(dirs);
   }
 
-  return android::base::WriteStringToFile(contents, path);
+  CHECK(android::base::WriteStringToFile(contents, path));
 }
 
 bool CommandTestFixture::CompileFile(const std::string& path, const std::string& contents,
                                      const android::StringPiece& out_dir, IDiagnostics* diag) {
-  CHECK(WriteFile(path, contents));
+  WriteFile(path, contents);
   CHECK(file::mkdirs(out_dir.data()));
   return CompileCommand(diag).Execute({path, "-o", out_dir, "-v"}, &std::cerr) == 0;
 }
 
+bool CommandTestFixture::Link(const std::vector<std::string>& args, IDiagnostics* diag) {
+  std::vector<android::StringPiece> link_args;
+  for(const std::string& arg : args) {
+    link_args.emplace_back(arg);
+  }
+
+  // Link against the android SDK
+  std::string android_sdk = file::BuildPath({android::base::GetExecutableDirectory(),
+                                             "integration-tests", "CommandTests",
+                                             "android-28.jar"});
+  link_args.insert(link_args.end(), {"-I", android_sdk});
+
+  return LinkCommand(diag).Execute(link_args, &std::cerr) == 0;
+}
+
 bool CommandTestFixture::Link(const std::vector<std::string>& args,
                               const android::StringPiece& flat_dir, IDiagnostics* diag) {
   std::vector<android::StringPiece> link_args;
@@ -128,10 +143,10 @@ bool CommandTestFixture::Link(const std::vector<std::string>& args,
 
 std::string CommandTestFixture::GetDefaultManifest(const char* package_name) {
   const std::string manifest_file = GetTestPath("AndroidManifest.xml");
-  CHECK(WriteFile(manifest_file, android::base::StringPrintf(R"(
+  WriteFile(manifest_file, android::base::StringPrintf(R"(
       <manifest xmlns:android="http://schemas.android.com/apk/res/android"
           package="%s">
-      </manifest>)", package_name)));
+      </manifest>)", package_name));
   return manifest_file;
 }
 
diff --git a/tools/aapt2/test/Fixture.h b/tools/aapt2/test/Fixture.h
index fce2aebfecaa..457d65e30b65 100644
--- a/tools/aapt2/test/Fixture.h
+++ b/tools/aapt2/test/Fixture.h
@@ -58,7 +58,7 @@ class TestDirectoryFixture : public ::testing::Test {
 
   // Creates a file with the specified contents, creates any intermediate directories in the
   // process. The file path must be an absolute path within the test directory.
-  bool WriteFile(const std::string& path, const std::string& contents);
+  void WriteFile(const std::string& path, const std::string& contents);
 
  private:
   std::string temp_dir_;
@@ -75,6 +75,9 @@ class CommandTestFixture : public TestDirectoryFixture {
   bool CompileFile(const std::string& path, const std::string& contents,
                    const android::StringPiece& flat_out_dir, IDiagnostics* diag);
 
+  // Executes the link command with the specified arguments.
+  bool Link(const std::vector<std::string>& args, IDiagnostics* diag);
+
   // Executes the link command with the specified arguments. The flattened files residing in the
   // flat directory will be added to the link command as file arguments.
   bool Link(const std::vector<std::string>& args, const android::StringPiece& flat_dir,