AAPT2: GetBag infinite recursion fix
Style resources with circular parental dependencies caused infinite
recursion when calling AssetManager2::GetBag. This fix allows recursion
to cease when a circular dependency is found.
Bug: 77928512
Change-Id: Ib900c36ab1aef5da5b03234a9484c4dad3b63c02
Test: Manual test of b/77928512 and duplicates of 74493983
diff --git a/libs/androidfw/AssetManager2.cpp b/libs/androidfw/AssetManager2.cpp
index 5460b3b..9c1629b 100644
--- a/libs/androidfw/AssetManager2.cpp
+++ b/libs/androidfw/AssetManager2.cpp
@@ -18,6 +18,7 @@
#include "androidfw/AssetManager2.h"
+#include <algorithm>
#include <iterator>
#include <set>
@@ -567,6 +568,11 @@
}
const ResolvedBag* AssetManager2::GetBag(uint32_t resid) {
+ auto found_resids = std::vector<uint32_t>();
+ return GetBag(resid, found_resids);
+}
+
+const ResolvedBag* AssetManager2::GetBag(uint32_t resid, std::vector<uint32_t>& child_resids) {
ATRACE_NAME("AssetManager::GetBag");
auto cached_iter = cached_bags_.find(resid);
@@ -595,10 +601,15 @@
reinterpret_cast<const ResTable_map*>(reinterpret_cast<const uint8_t*>(map) + map->size);
const ResTable_map* const map_entry_end = map_entry + dtohl(map->count);
+ // Keep track of ids that have already been seen to prevent infinite loops caused by circular
+ // dependencies between bags
+ child_resids.push_back(resid);
+
uint32_t parent_resid = dtohl(map->parent.ident);
- if (parent_resid == 0 || parent_resid == resid) {
- // There is no parent, meaning there is nothing to inherit and we can do a simple
- // copy of the entries in the map.
+ if (parent_resid == 0 || std::find(child_resids.begin(), child_resids.end(), parent_resid)
+ != child_resids.end()) {
+ // There is no parent or that a circular dependency exist, meaning there is nothing to
+ // inherit and we can do a simple copy of the entries in the map.
const size_t entry_count = map_entry_end - map_entry;
util::unique_cptr<ResolvedBag> new_bag{reinterpret_cast<ResolvedBag*>(
malloc(sizeof(ResolvedBag) + (entry_count * sizeof(ResolvedBag::Entry))))};
@@ -639,7 +650,7 @@
entry.dynamic_ref_table->lookupResourceId(&parent_resid);
// Get the parent and do a merge of the keys.
- const ResolvedBag* parent_bag = GetBag(parent_resid);
+ const ResolvedBag* parent_bag = GetBag(parent_resid, child_resids);
if (parent_bag == nullptr) {
// Failed to get the parent that should exist.
LOG(ERROR) << base::StringPrintf("Failed to find parent 0x%08x of bag 0x%08x.", parent_resid,
diff --git a/libs/androidfw/include/androidfw/AssetManager2.h b/libs/androidfw/include/androidfw/AssetManager2.h
index ef08897..ad31f69 100644
--- a/libs/androidfw/include/androidfw/AssetManager2.h
+++ b/libs/androidfw/include/androidfw/AssetManager2.h
@@ -276,6 +276,10 @@
// This should always be called when mutating the AssetManager's configuration or ApkAssets set.
void RebuildFilterList();
+ // AssetManager2::GetBag(resid) wraps this function to track which resource ids have already
+ // been seen while traversing bag parents.
+ const ResolvedBag* GetBag(uint32_t resid, std::vector<uint32_t>& child_resids);
+
// The ordered list of ApkAssets to search. These are not owned by the AssetManager, and must
// have a longer lifetime.
std::vector<const ApkAssets*> apk_assets_;
diff --git a/libs/androidfw/tests/AssetManager2_test.cpp b/libs/androidfw/tests/AssetManager2_test.cpp
index 7cac2b3..3118009 100644
--- a/libs/androidfw/tests/AssetManager2_test.cpp
+++ b/libs/androidfw/tests/AssetManager2_test.cpp
@@ -329,6 +329,17 @@
EXPECT_EQ(0, bag_two->entries[5].cookie);
}
+TEST_F(AssetManager2Test, MergeStylesCircularDependency) {
+ AssetManager2 assetmanager;
+ assetmanager.SetApkAssets({style_assets_.get()});
+
+ // GetBag should stop traversing the parents of styles when a circular
+ // dependency is detected
+ const ResolvedBag* bag_one = assetmanager.GetBag(app::R::style::StyleFour);
+ ASSERT_NE(nullptr, bag_one);
+ ASSERT_EQ(3u, bag_one->entry_count);
+}
+
TEST_F(AssetManager2Test, ResolveReferenceToResource) {
AssetManager2 assetmanager;
assetmanager.SetApkAssets({basic_assets_.get()});
diff --git a/libs/androidfw/tests/data/styles/R.h b/libs/androidfw/tests/data/styles/R.h
index 05073a8..538a847 100644
--- a/libs/androidfw/tests/data/styles/R.h
+++ b/libs/androidfw/tests/data/styles/R.h
@@ -48,6 +48,9 @@
StyleOne = 0x7f020000u,
StyleTwo = 0x7f020001u,
StyleThree = 0x7f020002u,
+ StyleFour = 0x7f020003u,
+ StyleFive = 0x7f020004u,
+ StyleSix = 0x7f020005u,
};
};
};
diff --git a/libs/androidfw/tests/data/styles/res/values/styles.xml b/libs/androidfw/tests/data/styles/res/values/styles.xml
index 3c90317..1a23176 100644
--- a/libs/androidfw/tests/data/styles/res/values/styles.xml
+++ b/libs/androidfw/tests/data/styles/res/values/styles.xml
@@ -63,4 +63,20 @@
<item name="attr_five">5</item>
</style>
+ <!-- Circular parental dependency -->
+ <public type="style" name="StyleFour" id="0x7f020003" />
+ <style name="StyleFour" parent="StyleFive">
+ <item name="attr_one">1</item>
+ </style>
+
+ <public type="style" name="StyleFive" id="0x7f020004" />
+ <style name="StyleFive" parent="StyleSix">
+ <item name="attr_two">2</item>
+ </style>
+
+ <public type="style" name="StyleSix" id="0x7f020005" />
+ <style name="StyleSix" parent="StyleFour">
+ <item name="attr_three">3</item>
+ </style>
+
</resources>
diff --git a/libs/androidfw/tests/data/styles/styles.apk b/libs/androidfw/tests/data/styles/styles.apk
index 72abf8f..cd5c7a1 100644
--- a/libs/androidfw/tests/data/styles/styles.apk
+++ b/libs/androidfw/tests/data/styles/styles.apk
Binary files differ