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

Commit 607c608c authored by Mark Punzalan's avatar Mark Punzalan Committed by Android (Google) Code Review
Browse files

Merge "aapt2: Ensure shortened paths are not reserved in Windows" into main

parents fc2112a6 19e1d205
Loading
Loading
Loading
Loading
+33 −9
Original line number Diff line number Diff line
@@ -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,7 +86,7 @@ int OptimalShortenedLength(int num_resources) {
  }
}

std::string GetShortenedPath(android::StringPiece shortened_filename,
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) {
@@ -82,7 +105,7 @@ struct PathComparator {
  }
};

static bool HandleShortenFilePaths(ResourceTable* table,
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
@@ -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()) {
+7 −0
Original line number Diff line number Diff line
@@ -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_;
+97 −7
Original line number Diff line number Diff line
@@ -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