aapt2: Fix issue with Manifest duplicate handling
Fixed a memory-corruption issue that led to multiple duplicate
permission entries being generated for Manifest.java.
Bug: 71641288
Test: make aapt2_tests
Change-Id: I8cd37929c4883aaba2beebbf874c7ee3234d51d8
diff --git a/tools/aapt2/java/ClassDefinition.cpp b/tools/aapt2/java/ClassDefinition.cpp
index b692ccf..f5f5b05 100644
--- a/tools/aapt2/java/ClassDefinition.cpp
+++ b/tools/aapt2/java/ClassDefinition.cpp
@@ -45,8 +45,18 @@
Result result = Result::kAdded;
auto iter = indexed_members_.find(member->GetName());
if (iter != indexed_members_.end()) {
- // Overwrite the entry.
- ordered_members_[iter->second].reset();
+ // Overwrite the entry. Be careful, as the key in indexed_members_ is actually memory owned
+ // by the value at ordered_members_[index]. Since overwriting a value for a key doesn't replace
+ // the key (the initial key inserted into the unordered_map is kept), we must erase and then
+ // insert a new key, whose memory is being kept around. We do all this to avoid using more
+ // memory for each key.
+ size_t index = iter->second;
+
+ // Erase the key + value from the map.
+ indexed_members_.erase(iter);
+
+ // Now clear the memory that was backing the key (now erased).
+ ordered_members_[index].reset();
result = Result::kOverridden;
}
diff --git a/tools/aapt2/java/ManifestClassGenerator_test.cpp b/tools/aapt2/java/ManifestClassGenerator_test.cpp
index c324238..f4e10ab 100644
--- a/tools/aapt2/java/ManifestClassGenerator_test.cpp
+++ b/tools/aapt2/java/ManifestClassGenerator_test.cpp
@@ -129,13 +129,16 @@
std::unique_ptr<xml::XmlResource> manifest = test::BuildXmlDom(R"(
<manifest xmlns:android="http://schemas.android.com/apk/res/android">
<permission android:name="android.permission.ACCESS_INTERNET" />
- <permission android:name="com.android.aapt.test.ACCESS_INTERNET" />
+ <permission android:name="com.android.sample.ACCESS_INTERNET" />
+ <permission android:name="com.android.permission.UNRELATED_PERMISSION" />
+ <permission android:name="com.android.aapt.test.ACCESS_INTERNET" /> -->
</manifest>)");
std::string actual;
ASSERT_TRUE(GetManifestClassText(context.get(), manifest.get(), &actual));
EXPECT_THAT(actual, HasSubstr("ACCESS_INTERNET=\"com.android.aapt.test.ACCESS_INTERNET\";"));
EXPECT_THAT(actual, Not(HasSubstr("ACCESS_INTERNET=\"android.permission.ACCESS_INTERNET\";")));
+ EXPECT_THAT(actual, Not(HasSubstr("ACCESS_INTERNET=\"com.android.sample.ACCESS_INTERNET\";")));
}
static ::testing::AssertionResult GetManifestClassText(IAaptContext* context, xml::XmlResource* res,