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
diff --git a/tools/aapt2/optimize/Obfuscator.cpp b/tools/aapt2/optimize/Obfuscator.cpp
index 8f12f735..903cdf8 100644
--- a/tools/aapt2/optimize/Obfuscator.cpp
+++ b/tools/aapt2/optimize/Obfuscator.cpp
@@ -40,9 +40,9 @@
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 @@
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 @@
}
}
-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 @@
}
};
-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 @@
// 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 5ccf5438..79d7e08 100644
--- a/tools/aapt2/optimize/Obfuscator.h
+++ b/tools/aapt2/optimize/Obfuscator.h
@@ -53,7 +53,14 @@
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 b3a915c..c3429e0 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::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 @@
}
}
-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 @@
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 @@
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 @@
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 @@
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 @@
ASSERT_THAT(pbOut, Eq(""));
}
+} // namespace
+
} // namespace aapt