Aapt2 Optimize: Exempt particular resources from path shortening

Design: go/no_path_shorten
Bug: b/246845175
Test: Added new atests and verified affected atests pass
Change-Id: I4d5b48ea9f0efd4740823101b9a3d776d151a808

9 files changed, 81 insertions, 15 deletions

diff --git a/tools/aapt2/cmd/Convert.cpp b/tools/aapt2/cmd/Convert.cpp
index 612e3a630013..7381a85f4339 100644
--- a/tools/aapt2/cmd/Convert.cpp
+++ b/tools/aapt2/cmd/Convert.cpp
@@ -364,7 +364,8 @@ bool ExtractResourceConfig(const std::string& path, IAaptContext* context,
   }
   std::unordered_set<ResourceName> resources_exclude_list;
   bool result = ParseResourceConfig(content, context, resources_exclude_list,
-                                    out_options.name_collapse_exemptions);
+                                    out_options.name_collapse_exemptions,
+                                    out_options.path_shorten_exemptions);
   if (!result) {
     return false;
   }
diff --git a/tools/aapt2/cmd/Optimize.cpp b/tools/aapt2/cmd/Optimize.cpp
index d7a39bfb8c0b..dbe79701bf5c 100644
--- a/tools/aapt2/cmd/Optimize.cpp
+++ b/tools/aapt2/cmd/Optimize.cpp
@@ -322,7 +322,8 @@ bool ExtractConfig(const std::string& path, IAaptContext* context, OptimizeOptio
     return false;
   }
   return ParseResourceConfig(content, context, options->resources_exclude_list,
-                             options->table_flattener_options.name_collapse_exemptions);
+                             options->table_flattener_options.name_collapse_exemptions,
+                             options->table_flattener_options.path_shorten_exemptions);
 }
 
 bool ExtractAppDataFromManifest(OptimizeContext* context, const LoadedApk* apk,
diff --git a/tools/aapt2/cmd/Optimize.h b/tools/aapt2/cmd/Optimize.h
index 1879f25bc1b0..ee53af107b17 100644
--- a/tools/aapt2/cmd/Optimize.h
+++ b/tools/aapt2/cmd/Optimize.h
@@ -122,7 +122,8 @@ class OptimizeCommand : public Command {
             "--resources-config-path.",
         &options_.table_flattener_options.collapse_key_stringpool);
     AddOptionalSwitch("--shorten-resource-paths",
-        "Shortens the paths of resources inside the APK.",
+        "Shortens the paths of resources inside the APK. Resources can be exempted using the \n"
+        "\"no_path_shorten\" directive in a file specified by --resources-config-path.",
         &options_.shorten_resource_paths);
     // TODO(b/246489170): keep the old option and format until transform to the new one
     AddOptionalFlag("--resource-path-shortening-map",
diff --git a/tools/aapt2/cmd/Util.cpp b/tools/aapt2/cmd/Util.cpp
index 92849cf02d48..1671e1e1a6a1 100644
--- a/tools/aapt2/cmd/Util.cpp
+++ b/tools/aapt2/cmd/Util.cpp
@@ -448,7 +448,8 @@ std::regex GetRegularExpression(const std::string &input) {
 
 bool ParseResourceConfig(const std::string& content, IAaptContext* context,
                          std::unordered_set<ResourceName>& out_resource_exclude_list,
-                         std::set<ResourceName>& out_name_collapse_exemptions) {
+                         std::set<ResourceName>& out_name_collapse_exemptions,
+                         std::set<ResourceName>& out_path_shorten_exemptions) {
   for (StringPiece line : util::Tokenize(content, '\n')) {
     line = util::TrimWhitespace(line);
     if (line.empty()) {
@@ -477,6 +478,8 @@ bool ParseResourceConfig(const std::string& content, IAaptContext* context,
         out_resource_exclude_list.insert(resource_name.ToResourceName());
       } else if (directive == "no_collapse" || directive == "no_obfuscate") {
         out_name_collapse_exemptions.insert(resource_name.ToResourceName());
+      } else if (directive == "no_path_shorten") {
+        out_path_shorten_exemptions.insert(resource_name.ToResourceName());
       }
     }
   }
diff --git a/tools/aapt2/cmd/Util.h b/tools/aapt2/cmd/Util.h
index 169d5f92f7dd..712c07b71695 100644
--- a/tools/aapt2/cmd/Util.h
+++ b/tools/aapt2/cmd/Util.h
@@ -81,7 +81,8 @@ std::regex GetRegularExpression(const std::string &input);
 
 bool ParseResourceConfig(const std::string& content, IAaptContext* context,
                          std::unordered_set<ResourceName>& out_resource_exclude_list,
-                         std::set<ResourceName>& out_name_collapse_exemptions);
+                         std::set<ResourceName>& out_name_collapse_exemptions,
+                         std::set<ResourceName>& out_path_shorten_exemptions);
 
 }  // namespace aapt
 
diff --git a/tools/aapt2/cmd/Util_test.cpp b/tools/aapt2/cmd/Util_test.cpp
index 28a6de80e7ce..139bfbcd0f41 100644
--- a/tools/aapt2/cmd/Util_test.cpp
+++ b/tools/aapt2/cmd/Util_test.cpp
@@ -416,19 +416,27 @@ TEST(UtilTest, ParseConfigWithDirectives) {
   const std::string& content = R"(
 bool/remove_me#remove
 bool/keep_name#no_collapse
+layout/keep_path#no_path_shorten
 string/foo#no_obfuscate
 dimen/bar#no_obfuscate
+layout/keep_name_and_path#no_collapse,no_path_shorten
 )";
   aapt::test::Context context;
   std::unordered_set<ResourceName> resource_exclusion;
   std::set<ResourceName> name_collapse_exemptions;
+  std::set<ResourceName> path_shorten_exemptions;
 
-  EXPECT_TRUE(ParseResourceConfig(content, &context, resource_exclusion, name_collapse_exemptions));
+  EXPECT_TRUE(ParseResourceConfig(content, &context, resource_exclusion, name_collapse_exemptions,
+                                  path_shorten_exemptions));
 
   EXPECT_THAT(name_collapse_exemptions,
               UnorderedElementsAre(ResourceName({}, ResourceType::kString, "foo"),
                                    ResourceName({}, ResourceType::kDimen, "bar"),
-                                   ResourceName({}, ResourceType::kBool, "keep_name")));
+                                   ResourceName({}, ResourceType::kBool, "keep_name"),
+                                   ResourceName({}, ResourceType::kLayout, "keep_name_and_path")));
+  EXPECT_THAT(path_shorten_exemptions,
+              UnorderedElementsAre(ResourceName({}, ResourceType::kLayout, "keep_path"),
+                                   ResourceName({}, ResourceType::kLayout, "keep_name_and_path")));
   EXPECT_THAT(resource_exclusion,
               UnorderedElementsAre(ResourceName({}, ResourceType::kBool, "remove_me")));
 }
@@ -440,9 +448,10 @@ package:bool/remove_me#remove
   aapt::test::Context context;
   std::unordered_set<ResourceName> resource_exclusion;
   std::set<ResourceName> name_collapse_exemptions;
+  std::set<ResourceName> path_shorten_exemptions;
 
-  EXPECT_FALSE(
-      ParseResourceConfig(content, &context, resource_exclusion, name_collapse_exemptions));
+  EXPECT_FALSE(ParseResourceConfig(content, &context, resource_exclusion, name_collapse_exemptions,
+                                   path_shorten_exemptions));
 }
 
 TEST(UtilTest, ParseConfigInvalidName) {
@@ -452,9 +461,10 @@ package:bool/1231#remove
   aapt::test::Context context;
   std::unordered_set<ResourceName> resource_exclusion;
   std::set<ResourceName> name_collapse_exemptions;
+  std::set<ResourceName> path_shorten_exemptions;
 
-  EXPECT_FALSE(
-      ParseResourceConfig(content, &context, resource_exclusion, name_collapse_exemptions));
+  EXPECT_FALSE(ParseResourceConfig(content, &context, resource_exclusion, name_collapse_exemptions,
+                                   path_shorten_exemptions));
 }
 
 TEST(UtilTest, ParseConfigNoHash) {
@@ -464,9 +474,10 @@ package:bool/my_bool
   aapt::test::Context context;
   std::unordered_set<ResourceName> resource_exclusion;
   std::set<ResourceName> name_collapse_exemptions;
+  std::set<ResourceName> path_shorten_exemptions;
 
-  EXPECT_FALSE(
-      ParseResourceConfig(content, &context, resource_exclusion, name_collapse_exemptions));
+  EXPECT_FALSE(ParseResourceConfig(content, &context, resource_exclusion, name_collapse_exemptions,
+                                   path_shorten_exemptions));
 }
 
 }  // namespace aapt
diff --git a/tools/aapt2/format/binary/TableFlattener.h b/tools/aapt2/format/binary/TableFlattener.h
index 60605d2fb7fd..0633bc81cb25 100644
--- a/tools/aapt2/format/binary/TableFlattener.h
+++ b/tools/aapt2/format/binary/TableFlattener.h
@@ -60,6 +60,9 @@ struct TableFlattenerOptions {
   // Set of resources to avoid collapsing to a single entry in key stringpool.
   std::set<ResourceName> name_collapse_exemptions;
 
+  // Set of resources to avoid path shortening.
+  std::set<ResourceName> path_shorten_exemptions;
+
   // Map from original resource paths to shortened resource paths.
   std::map<std::string, std::string> shortened_path_map;
 
diff --git a/tools/aapt2/optimize/Obfuscator.cpp b/tools/aapt2/optimize/Obfuscator.cpp
index cc21093084b5..8f12f735736e 100644
--- a/tools/aapt2/optimize/Obfuscator.cpp
+++ b/tools/aapt2/optimize/Obfuscator.cpp
@@ -83,13 +83,18 @@ struct PathComparator {
 };
 
 static bool HandleShortenFilePaths(ResourceTable* table,
-                                   std::map<std::string, std::string>& shortened_path_map) {
+                                   std::map<std::string, std::string>& shortened_path_map,
+                                   const std::set<ResourceName>& path_shorten_exemptions) {
   // used to detect collisions
   std::unordered_set<std::string> shortened_paths;
   std::set<FileReference*, PathComparator> file_refs;
   for (auto& package : table->packages) {
     for (auto& type : package->types) {
       for (auto& entry : type->entries) {
+        ResourceName resource_name({}, type->named_type, entry->name);
+        if (path_shorten_exemptions.find(resource_name) != path_shorten_exemptions.end()) {
+          continue;
+        }
         for (auto& config_value : entry->values) {
           FileReference* file_ref = ValueCast<FileReference>(config_value->value.get());
           if (file_ref) {
@@ -188,7 +193,8 @@ bool Obfuscator::Consume(IAaptContext* context, ResourceTable* table) {
   HandleCollapseKeyStringPool(table, options_.collapse_key_stringpool,
                               options_.name_collapse_exemptions, options_.id_resource_map);
   if (shorten_resource_paths_) {
-    return HandleShortenFilePaths(table, options_.shortened_path_map);
+    return HandleShortenFilePaths(table, options_.shortened_path_map,
+                                  options_.path_shorten_exemptions);
   }
   return true;
 }
diff --git a/tools/aapt2/optimize/Obfuscator_test.cpp b/tools/aapt2/optimize/Obfuscator_test.cpp
index 7f57b71ddf76..940cf1096f92 100644
--- a/tools/aapt2/optimize/Obfuscator_test.cpp
+++ b/tools/aapt2/optimize/Obfuscator_test.cpp
@@ -28,6 +28,7 @@ using ::aapt::test::GetValue;
 using ::testing::AnyOf;
 using ::testing::Eq;
 using ::testing::HasSubstr;
+using ::testing::IsFalse;
 using ::testing::IsTrue;
 using ::testing::Not;
 using ::testing::NotNull;
@@ -102,6 +103,44 @@ TEST(ObfuscatorTest, SkipColorFileRefPaths) {
   ASSERT_THAT(path_map.find("res/color-mdp-v21/colorlist.xml"), Eq(path_map.end()));
 }
 
+TEST(ObfuscatorTest, SkipPathShortenExemptions) {
+  std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
+
+  std::unique_ptr<ResourceTable> table =
+      test::ResourceTableBuilder()
+          .AddFileReference("android:drawable/xmlfile", "res/drawables/xmlfile.xml")
+          .AddFileReference("android:drawable/xmlfile2", "res/drawables/xmlfile2.xml")
+          .AddString("android:string/string", "res/should/still/be/the/same.png")
+          .Build();
+
+  OptimizeOptions options{.shorten_resource_paths = true};
+  TableFlattenerOptions& flattenerOptions = options.table_flattener_options;
+  flattenerOptions.path_shorten_exemptions.insert(
+      ResourceName({}, ResourceType::kDrawable, "xmlfile"));
+  std::map<std::string, std::string>& path_map = options.table_flattener_options.shortened_path_map;
+  ASSERT_TRUE(Obfuscator(options).Consume(context.get(), table.get()));
+
+  // Expect that the path map to not contain the first drawable which is in exemption set
+  EXPECT_THAT(path_map.find("res/drawables/xmlfile.xml"), Eq(path_map.end()));
+
+  // Expect that the path map to contain the second drawable which is not in exemption set
+  EXPECT_THAT(path_map.find("res/drawables/xmlfile2.xml"), Not(Eq(path_map.end())));
+
+  FileReference* ref = GetValue<FileReference>(table.get(), "android:drawable/xmlfile");
+  EXPECT_THAT(ref, NotNull());
+  ASSERT_THAT(HasFailure(), IsFalse());
+  // The path of first drawable in exemption was not changed
+  EXPECT_THAT("res/drawables/xmlfile.xml", Eq(*ref->path));
+
+  // The file path of second drawable not in exemption set was changed
+  EXPECT_THAT(path_map.at("res/drawables/xmlfile2.xml"), Not(Eq("res/drawables/xmlfile2.xml")));
+
+  FileReference* ref2 = GetValue<FileReference>(table.get(), "android:drawable/xmlfile2");
+  ASSERT_THAT(ref, NotNull());
+  // The map of second drawable not in exemption correctly points to the new location of the file
+  EXPECT_THAT(path_map["res/drawables/xmlfile2.xml"], Eq(*ref2->path));
+}
+
 TEST(ObfuscatorTest, KeepExtensions) {
   std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();