aapt2: Ensure shortened paths are not reserved in Windows

It is possible that the shortened resource path is a reserved name in
Windows (see:
https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file).
We rename the path to something that is guaranteed to be NOT reserved.

Bug: 276340505
Test: atest aapt2_tests
Change-Id: Ibc20f10e2e98755a18190a1ba51e0019dd5c1ba0

3 files changed, 137 insertions, 16 deletions

diff --git a/tools/aapt2/optimize/Obfuscator.cpp b/tools/aapt2/optimize/Obfuscator.cpp
index 8f12f735736e..903cdf852566 100644
--- a/tools/aapt2/optimize/Obfuscator.cpp
+++ b/tools/aapt2/optimize/Obfuscator.cpp
@@ -40,9 +40,9 @@ Obfuscator::Obfuscator(OptimizeOptions& optimizeOptions)
       collapse_key_stringpool_(optimizeOptions.table_flattener_options.collapse_key_stringpool) {
 }
 
-std::string ShortenFileName(android::StringPiece file_path, int output_length) {
+std::string Obfuscator::ShortenFileName(android::StringPiece file_path, int output_length) {
   std::size_t hash_num = std::hash<android::StringPiece>{}(file_path);
-  std::string result = "";
+  std::string result;
   // Convert to (modified) base64 so that it is a proper file path.
   for (int i = 0; i < output_length; i++) {
     uint8_t sextet = hash_num & 0x3f;
@@ -52,10 +52,33 @@ std::string ShortenFileName(android::StringPiece file_path, int output_length) {
   return result;
 }
 
+static std::string RenameDisallowedFileNames(const std::string& file_name) {
+  // We are renaming shortened file names to make sure they not a reserved file name in Windows.
+  // See: https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file. We are renaming
+  // "COM" and "LPT" too because we are appending a number in case of hash collisions; "COM1",
+  // "COM2", etc. are reserved names.
+  static const char* const reserved_windows_names[] = {"CON", "PRN", "AUX", "NUL", "COM", "LPT"};
+  if (file_name.length() == 3) {
+    // Need to convert the file name to uppercase as Windows is case insensitive. E.g., "NuL",
+    // "nul", and "NUl" are also reserved.
+    std::string result_upper_cased(3, 0);
+    std::transform(file_name.begin(), file_name.end(), result_upper_cased.begin(),
+                   [](unsigned char c) { return std::toupper(c); });
+    for (auto reserved_windows_name : reserved_windows_names) {
+      if (result_upper_cased == reserved_windows_name) {
+        // Simple solution to make it a non-reserved name is to add an underscore
+        return "_" + file_name;
+      }
+    }
+  }
+
+  return file_name;
+}
+
 // Return the optimal hash length such that at most 10% of resources collide in
 // their shortened path.
 // Reference: http://matt.might.net/articles/counting-hash-collisions/
-int OptimalShortenedLength(int num_resources) {
+static int OptimalShortenedLength(int num_resources) {
   if (num_resources > 4000) {
     return 3;
   } else {
@@ -63,8 +86,8 @@ int OptimalShortenedLength(int num_resources) {
   }
 }
 
-std::string GetShortenedPath(android::StringPiece shortened_filename,
-                             android::StringPiece extension, int collision_count) {
+static std::string GetShortenedPath(android::StringPiece shortened_filename,
+                                    android::StringPiece extension, int collision_count) {
   std::string shortened_path = std::string("res/") += shortened_filename;
   if (collision_count > 0) {
     shortened_path += std::to_string(collision_count);
@@ -82,9 +105,9 @@ struct PathComparator {
   }
 };
 
-static bool HandleShortenFilePaths(ResourceTable* table,
-                                   std::map<std::string, std::string>& shortened_path_map,
-                                   const std::set<ResourceName>& path_shorten_exemptions) {
+bool Obfuscator::HandleShortenFilePaths(ResourceTable* table,
+                                        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;
@@ -112,7 +135,8 @@ static bool HandleShortenFilePaths(ResourceTable* table,
     // Android detects ColorStateLists via pathname, skip res/color*
     if (util::StartsWith(res_subdir, "res/color")) continue;
 
-    std::string shortened_filename = ShortenFileName(*file_ref->path, num_chars);
+    std::string shortened_filename =
+        RenameDisallowedFileNames(ShortenFileName(*file_ref->path, num_chars));
     int collision_count = 0;
     std::string shortened_path = GetShortenedPath(shortened_filename, extension, collision_count);
     while (shortened_paths.find(shortened_path) != shortened_paths.end()) {
diff --git a/tools/aapt2/optimize/Obfuscator.h b/tools/aapt2/optimize/Obfuscator.h
index 5ccf54383aae..79d7e088d1cc 100644
--- a/tools/aapt2/optimize/Obfuscator.h
+++ b/tools/aapt2/optimize/Obfuscator.h
@@ -53,7 +53,14 @@ class Obfuscator : public IResourceTableConsumer {
       const ResourceNamedType& type_name, const ResourceTableEntryView& entry,
       const android::base::function_ref<void(Result, const ResourceName&)> onObfuscate);
 
+ protected:
+  virtual std::string ShortenFileName(android::StringPiece file_path, int output_length);
+
  private:
+  bool HandleShortenFilePaths(ResourceTable* table,
+                              std::map<std::string, std::string>& shortened_path_map,
+                              const std::set<ResourceName>& path_shorten_exemptions);
+
   TableFlattenerOptions& options_;
   const bool shorten_resource_paths_;
   const bool collapse_key_stringpool_;
diff --git a/tools/aapt2/optimize/Obfuscator_test.cpp b/tools/aapt2/optimize/Obfuscator_test.cpp
index b3a915c9b604..c3429e0fc1d6 100644
--- a/tools/aapt2/optimize/Obfuscator_test.cpp
+++ b/tools/aapt2/optimize/Obfuscator_test.cpp
@@ -19,6 +19,7 @@
 #include <map>
 #include <memory>
 #include <string>
+#include <utility>
 
 #include "ResourceTable.h"
 #include "android-base/file.h"
@@ -26,6 +27,7 @@
 
 using ::aapt::test::GetValue;
 using ::testing::AnyOf;
+using ::testing::Contains;
 using ::testing::Eq;
 using ::testing::HasSubstr;
 using ::testing::IsFalse;
@@ -33,6 +35,10 @@ using ::testing::IsTrue;
 using ::testing::Not;
 using ::testing::NotNull;
 
+namespace aapt {
+
+namespace {
+
 android::StringPiece GetExtension(android::StringPiece path) {
   auto iter = std::find(path.begin(), path.end(), '.');
   return android::StringPiece(iter, path.end() - iter);
@@ -45,7 +51,22 @@ void FillTable(aapt::test::ResourceTableBuilder& builder, int start, int end) {
   }
 }
 
-namespace aapt {
+class FakeObfuscator : public Obfuscator {
+ public:
+  explicit FakeObfuscator(OptimizeOptions& optimize_options,
+                          const std::unordered_map<std::string, std::string>& shortened_name_map)
+      : Obfuscator(optimize_options), shortened_name_map_(shortened_name_map) {
+  }
+
+ protected:
+  std::string ShortenFileName(android::StringPiece file_path, int output_length) override {
+    return shortened_name_map_[std::string(file_path)];
+  }
+
+ private:
+  std::unordered_map<std::string, std::string> shortened_name_map_;
+  DISALLOW_COPY_AND_ASSIGN(FakeObfuscator);
+};
 
 TEST(ObfuscatorTest, FileRefPathsChangedInResourceTable) {
   std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
@@ -127,7 +148,7 @@ TEST(ObfuscatorTest, SkipPathShortenExemptions) {
   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(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));
@@ -161,13 +182,78 @@ TEST(ObfuscatorTest, KeepExtensions) {
   ASSERT_THAT(path_map.find("res/drawable/xmlfile.xml"), Not(Eq(path_map.end())));
   ASSERT_THAT(path_map.find("res/drawable/pngfile.png"), Not(Eq(path_map.end())));
 
-  auto shortend_xml_path = path_map[original_xml_path];
-  auto shortend_png_path = path_map[original_png_path];
-
   EXPECT_THAT(GetExtension(path_map[original_xml_path]), Eq(android::StringPiece(".xml")));
   EXPECT_THAT(GetExtension(path_map[original_png_path]), Eq(android::StringPiece(".png")));
 }
 
+TEST(ObfuscatorTest, ShortenedToReservedWindowsNames) {
+  std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
+
+  std::string original_path_1 = "res/drawable/pngfile_1.png";
+  std::string original_path_2 = "res/drawable/pngfile_2.png";
+  std::string original_path_3 = "res/drawable/pngfile_3.png";
+  std::string original_path_4 = "res/drawable/pngfile_4.png";
+  std::string original_path_5 = "res/drawable/pngfile_5.png";
+  std::string original_path_6 = "res/drawable/pngfile_6.png";
+  std::string original_path_7 = "res/drawable/pngfile_7.png";
+  std::string original_path_8 = "res/drawable/pngfile_8.png";
+  std::string original_path_9 = "res/drawable/pngfile_9.png";
+
+  std::unique_ptr<ResourceTable> table =
+      test::ResourceTableBuilder()
+          .AddFileReference("android:drawable/pngfile_1", original_path_1)
+          .AddFileReference("android:drawable/pngfile_2", original_path_2)
+          .AddFileReference("android:drawable/pngfile_3", original_path_3)
+          .AddFileReference("android:drawable/pngfile_4", original_path_4)
+          .AddFileReference("android:drawable/pngfile_5", original_path_5)
+          .AddFileReference("android:drawable/pngfile_6", original_path_6)
+          .AddFileReference("android:drawable/pngfile_7", original_path_7)
+          .AddFileReference("android:drawable/pngfile_8", original_path_8)
+          .AddFileReference("android:drawable/pngfile_9", original_path_9)
+          .Build();
+
+  OptimizeOptions options{.shorten_resource_paths = true};
+  std::map<std::string, std::string>& path_map = options.table_flattener_options.shortened_path_map;
+  auto obfuscator = FakeObfuscator(
+      options,
+      {
+          {original_path_1, "CON"},
+          {original_path_2, "Prn"},
+          {original_path_3, "AuX"},
+          {original_path_4, "nul"},
+          {original_path_5, "cOM"},
+          {original_path_6, "lPt"},
+          {original_path_7, "lPt"},
+          {original_path_8, "lPt"},  // 6, 7, and 8 will be appended with a number to disambiguate
+          {original_path_9, "F0o"},  // This one is not reserved
+      });
+  ASSERT_TRUE(obfuscator.Consume(context.get(), table.get()));
+
+  // Expect that the path map is populated
+  ASSERT_THAT(path_map.find(original_path_1), Not(Eq(path_map.end())));
+  ASSERT_THAT(path_map.find(original_path_2), Not(Eq(path_map.end())));
+  ASSERT_THAT(path_map.find(original_path_3), Not(Eq(path_map.end())));
+  ASSERT_THAT(path_map.find(original_path_4), Not(Eq(path_map.end())));
+  ASSERT_THAT(path_map.find(original_path_5), Not(Eq(path_map.end())));
+  ASSERT_THAT(path_map.find(original_path_6), Not(Eq(path_map.end())));
+  ASSERT_THAT(path_map.find(original_path_7), Not(Eq(path_map.end())));
+  ASSERT_THAT(path_map.find(original_path_8), Not(Eq(path_map.end())));
+  ASSERT_THAT(path_map.find(original_path_9), Not(Eq(path_map.end())));
+
+  EXPECT_THAT(path_map[original_path_1], Eq("res/_CON.png"));
+  EXPECT_THAT(path_map[original_path_2], Eq("res/_Prn.png"));
+  EXPECT_THAT(path_map[original_path_3], Eq("res/_AuX.png"));
+  EXPECT_THAT(path_map[original_path_4], Eq("res/_nul.png"));
+  EXPECT_THAT(path_map[original_path_5], Eq("res/_cOM.png"));
+  EXPECT_THAT(path_map[original_path_9], Eq("res/F0o.png"));
+
+  std::set<std::string> lpt_shortened_names{path_map[original_path_6], path_map[original_path_7],
+                                            path_map[original_path_8]};
+  EXPECT_THAT(lpt_shortened_names, Contains("res/_lPt.png"));
+  EXPECT_THAT(lpt_shortened_names, Contains("res/_lPt1.png"));
+  EXPECT_THAT(lpt_shortened_names, Contains("res/_lPt2.png"));
+}
+
 TEST(ObfuscatorTest, DeterministicallyHandleCollisions) {
   std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
 
@@ -247,7 +333,9 @@ TEST(ObfuscatorTest, DumpIdResourceMap) {
   ASSERT_TRUE(Obfuscator(options).Consume(context.get(), table.get()));
 
   // Expect that the id resource name map is populated
+  ASSERT_THAT(id_resource_map.find(0x7f020000), Not(Eq(id_resource_map.end())));
   EXPECT_THAT(id_resource_map.at(0x7f020000), Eq("mycolor"));
+  ASSERT_THAT(id_resource_map.find(0x7f030000), Not(Eq(id_resource_map.end())));
   EXPECT_THAT(id_resource_map.at(0x7f030000), Eq("mystring"));
   EXPECT_THAT(id_resource_map.find(0x7f030001), Eq(id_resource_map.end()));
   EXPECT_THAT(id_resource_map.find(0x7f030002), Eq(id_resource_map.end()));
@@ -310,8 +398,8 @@ TEST(ObfuscatorTest, WriteObfuscationMapInProtocolBufferFormat) {
   EXPECT_THAT(pbOut, HasSubstr("mycolor"));
   EXPECT_THAT(pbOut, HasSubstr("mystring"));
   pb::ResourceMappings resourceMappings;
-  EXPECT_THAT(resourceMappings.ParseFromString(pbOut), IsTrue());
-  EXPECT_THAT(resourceMappings.collapsed_names().resource_names_size(), Eq(2));
+  ASSERT_THAT(resourceMappings.ParseFromString(pbOut), IsTrue());
+  ASSERT_THAT(resourceMappings.collapsed_names().resource_names_size(), Eq(2));
   auto& resource_names = resourceMappings.collapsed_names().resource_names();
   EXPECT_THAT(resource_names.at(0).name(), AnyOf(Eq("mycolor"), Eq("mystring")));
   EXPECT_THAT(resource_names.at(1).name(), AnyOf(Eq("mycolor"), Eq("mystring")));
@@ -337,4 +425,6 @@ TEST(ObfuscatorTest, WriteObfuscatingMapWithNonEnabledOption) {
   ASSERT_THAT(pbOut, Eq(""));
 }
 
+}  // namespace
+
 }  // namespace aapt