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

Commit 57cd195c authored by y's avatar y
Browse files

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
parent 65317bac

libs/androidfw/AssetManager2.cpp

+15 −4
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ 


#include "androidfw/AssetManager2.h"



#include <algorithm>

#include <iterator>

#include <set>
@@ -567,6 +568,11 @@ ApkAssetsCookie AssetManager2::ResolveReference(ApkAssetsCookie cookie, Res_valu 
}



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 @@ const ResolvedBag* AssetManager2::GetBag(uint32_t resid) { 
      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 @@ const ResolvedBag* AssetManager2::GetBag(uint32_t resid) { 
  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,

libs/androidfw/include/androidfw/AssetManager2.h

+4 −0
Original line number Diff line number Diff line
@@ -276,6 +276,10 @@ class AssetManager2 { 
  // 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_;

libs/androidfw/tests/AssetManager2_test.cpp

+11 −0
Original line number Diff line number Diff line
@@ -329,6 +329,17 @@ TEST_F(AssetManager2Test, MergesStylesWithParentFromSingleApkAssets) { 
  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()});

libs/androidfw/tests/data/styles/R.h

+3 −0
Original line number Diff line number Diff line
@@ -48,6 +48,9 @@ struct R { 
      StyleOne = 0x7f020000u,

      StyleTwo = 0x7f020001u,

      StyleThree = 0x7f020002u,

      StyleFour = 0x7f020003u,

      StyleFive = 0x7f020004u,

      StyleSix = 0x7f020005u,

    };

  };

};

libs/androidfw/tests/data/styles/res/values/styles.xml

+16 −0
Original line number Diff line number Diff line
@@ -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>