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

Commit ed69ce84 authored by Adam Lesinski's avatar Adam Lesinski
Browse files

libandroidfw: Fix mass logspam of ResourceTypes warnings 

An overlay was incorrectly leaking its own resources into the
framework resource package, which caused warnings for every app
that tried to access framework resources (all of them).

This change skips including any resources that are not overlaying
anything (not present in IDMAP).

Bug: 36256974
Test: make libandroidfw_tests
Change-Id: I8c710af6849bb848938825aacca02799ee96c003
parent 5c690d55

libs/androidfw/.clang-format

+5 −0
Original line number Diff line number Diff line 
BasedOnStyle: Google

ColumnLimit: 100

AllowShortBlocksOnASingleLine: false

AllowShortFunctionsOnASingleLine: false

CommentPragmas: NOLINT:.*

DerivePointerAlignment: false

PointerAlignment: Left

libs/androidfw/ResourceTypes.cpp

+52 −33
Original line number Diff line number Diff line
@@ -6431,17 +6431,26 @@ status_t ResTable::parsePackage(const ResTable_package* const pkg, 
            }



            if (newEntryCount > 0) {

                bool addToType = true;

                uint8_t typeIndex = typeSpec->id - 1;

                ssize_t idmapIndex = idmapEntries.indexOfKey(typeSpec->id);

                if (idmapIndex >= 0) {

                    typeIndex = idmapEntries[idmapIndex].targetTypeId() - 1;

                } else if (header->resourceIDMap != NULL) {

                    // This is an overlay, but the types in this overlay are not

                    // overlaying anything according to the idmap. We can skip these

                    // as they will otherwise conflict with the other resources in the package

                    // without a mapping.

                    addToType = false;

                }



                if (addToType) {

                    TypeList& typeList = group->types.editItemAt(typeIndex);

                    if (!typeList.isEmpty()) {

                        const Type* existingType = typeList[0];

                        if (existingType->entryCount != newEntryCount && idmapIndex < 0) {

                        ALOGW("ResTable_typeSpec entry count inconsistent: given %d, previously %d",

                            ALOGW("ResTable_typeSpec entry count inconsistent: "

                                  "given %d, previously %d",

                                  (int) newEntryCount, (int) existingType->entryCount);

                            // We should normally abort here, but some legacy apps declare

                            // resources in the 'android' package (old bug in AAPT).
@@ -6457,6 +6466,7 @@ status_t ResTable::parsePackage(const ResTable_package* const pkg, 
                    }

                    typeList.add(t);

                    group->largestTypeId = max(group->largestTypeId, typeSpec->id);

                }

            } else {

                ALOGV("Skipping empty ResTable_typeSpec for type %d", typeSpec->id);

            }
@@ -6499,12 +6509,20 @@ status_t ResTable::parsePackage(const ResTable_package* const pkg, 
            }



            if (newEntryCount > 0) {

                bool addToType = true;

                uint8_t typeIndex = type->id - 1;

                ssize_t idmapIndex = idmapEntries.indexOfKey(type->id);

                if (idmapIndex >= 0) {

                    typeIndex = idmapEntries[idmapIndex].targetTypeId() - 1;

                } else if (header->resourceIDMap != NULL) {

                    // This is an overlay, but the types in this overlay are not

                    // overlaying anything according to the idmap. We can skip these

                    // as they will otherwise conflict with the other resources in the package

                    // without a mapping.

                    addToType = false;

                }



                if (addToType) {

                    TypeList& typeList = group->types.editItemAt(typeIndex);

                    if (typeList.isEmpty()) {

                        ALOGE("No TypeSpec for type %d", type->id);
@@ -6525,6 +6543,7 @@ status_t ResTable::parsePackage(const ResTable_package* const pkg, 
                        ALOGI("Adding config to type %d: %s\n", type->id,

                                thisConfig.toString().string());

                    }

                }

            } else {

                ALOGV("Skipping empty ResTable_type for type %d", type->id);

            }

libs/androidfw/tests/Idmap_test.cpp

+56 −44
Original line number Diff line number Diff line
@@ -30,25 +30,23 @@ class IdmapTest : public ::testing::Test { 
 protected:

  void SetUp() override {

    std::string contents;

    ASSERT_TRUE(ReadFileFromZipToString(GetTestDataPath() + "/basic/basic.apk",

                                        "resources.arsc", &contents));

    ASSERT_EQ(NO_ERROR,

              target_table_.add(contents.data(), contents.size(), 0, true));

    ASSERT_TRUE(ReadFileFromZipToString(GetTestDataPath() + "/basic/basic.apk", "resources.arsc",

                                        &contents));

    ASSERT_EQ(NO_ERROR, target_table_.add(contents.data(), contents.size(), 0, true));



    ASSERT_TRUE(

        ReadFileFromZipToString(GetTestDataPath() + "/overlay/overlay.apk",

    ASSERT_TRUE(ReadFileFromZipToString(GetTestDataPath() + "/overlay/overlay.apk",

                                        "resources.arsc", &overlay_data_));

    ResTable overlay_table;

    ASSERT_EQ(NO_ERROR,

              overlay_table.add(overlay_data_.data(), overlay_data_.size()));

    ASSERT_EQ(NO_ERROR, overlay_table.add(overlay_data_.data(), overlay_data_.size()));



    char target_name[256] = "com.android.basic";

    ASSERT_EQ(NO_ERROR,

              target_table_.createIdmap(overlay_table, 0, 0, target_name,

                                        target_name, &data_, &data_size_));

    ASSERT_EQ(NO_ERROR, target_table_.createIdmap(overlay_table, 0, 0, target_name, target_name,

                                                  &data_, &data_size_));

  }



  void TearDown() override { ::free(data_); }

  void TearDown() override {

    ::free(data_);

  }



  ResTable target_table_;

  std::string overlay_data_;
@@ -56,13 +54,12 @@ class IdmapTest : public ::testing::Test { 
  size_t data_size_ = 0;

};



TEST_F(IdmapTest, canLoadIdmap) {

TEST_F(IdmapTest, CanLoadIdmap) {

  ASSERT_EQ(NO_ERROR,

            target_table_.add(overlay_data_.data(), overlay_data_.size(), data_,

                              data_size_));

            target_table_.add(overlay_data_.data(), overlay_data_.size(), data_, data_size_));

}



TEST_F(IdmapTest, overlayOverridesResourceValue) {

TEST_F(IdmapTest, OverlayOverridesResourceValue) {

  Res_value val;

  ssize_t block = target_table_.getResource(R::string::test2, &val, false);

  ASSERT_GE(block, 0);
@@ -71,45 +68,60 @@ TEST_F(IdmapTest, overlayOverridesResourceValue) { 
  ASSERT_TRUE(pool != NULL);

  ASSERT_LT(val.data, pool->size());



  size_t strLen;

  const char16_t* targetStr16 = pool->stringAt(val.data, &strLen);

  ASSERT_TRUE(targetStr16 != NULL);

  ASSERT_EQ(String16("test2"), String16(targetStr16, strLen));

  size_t str_len;

  const char16_t* target_str16 = pool->stringAt(val.data, &str_len);

  ASSERT_TRUE(target_str16 != NULL);

  ASSERT_EQ(String16("test2"), String16(target_str16, str_len));



  ASSERT_EQ(NO_ERROR,

            target_table_.add(overlay_data_.data(), overlay_data_.size(), data_,

                              data_size_));

            target_table_.add(overlay_data_.data(), overlay_data_.size(), data_, data_size_));



  ssize_t newBlock = target_table_.getResource(R::string::test2, &val, false);

  ASSERT_GE(newBlock, 0);

  ASSERT_NE(block, newBlock);

  ssize_t new_block = target_table_.getResource(R::string::test2, &val, false);

  ASSERT_GE(new_block, 0);

  ASSERT_NE(block, new_block);

  ASSERT_EQ(Res_value::TYPE_STRING, val.dataType);

  pool = target_table_.getTableStringBlock(newBlock);

  pool = target_table_.getTableStringBlock(new_block);

  ASSERT_TRUE(pool != NULL);

  ASSERT_LT(val.data, pool->size());



  targetStr16 = pool->stringAt(val.data, &strLen);

  ASSERT_TRUE(targetStr16 != NULL);

  ASSERT_EQ(String16("test2-overlay"), String16(targetStr16, strLen));

  target_str16 = pool->stringAt(val.data, &str_len);

  ASSERT_TRUE(target_str16 != NULL);

  ASSERT_EQ(String16("test2-overlay"), String16(target_str16, str_len));

}



TEST_F(IdmapTest, overlaidResourceHasSameName) {

TEST_F(IdmapTest, OverlaidResourceHasSameName) {

  ASSERT_EQ(NO_ERROR,

            target_table_.add(overlay_data_.data(), overlay_data_.size(), data_,

                              data_size_));

            target_table_.add(overlay_data_.data(), overlay_data_.size(), data_, data_size_));



  ResTable::resource_name res_name;

  ASSERT_TRUE(target_table_.getResourceName(R::array::integerArray1, false, &res_name));



  ASSERT_TRUE(res_name.package != NULL);

  ASSERT_TRUE(res_name.type != NULL);

  ASSERT_TRUE(res_name.name != NULL);



  ResTable::resource_name resName;

  ASSERT_TRUE(

      target_table_.getResourceName(R::array::integerArray1, false, &resName));

  EXPECT_EQ(String16("com.android.basic"), String16(res_name.package, res_name.packageLen));

  EXPECT_EQ(String16("array"), String16(res_name.type, res_name.typeLen));

  EXPECT_EQ(String16("integerArray1"), String16(res_name.name, res_name.nameLen));

}



constexpr const uint32_t kNonOverlaidResourceId = 0x7fff0000u;



TEST_F(IdmapTest, OverlayDoesNotIncludeNonOverlaidResources) {

  // First check that the resource we're trying to not include when overlaid is present when

  // the overlay is loaded as a standalone APK.

  ResTable table;

  ASSERT_EQ(NO_ERROR, table.add(overlay_data_.data(), overlay_data_.size(), 0, true));



  ASSERT_TRUE(resName.package != NULL);

  ASSERT_TRUE(resName.type != NULL);

  ASSERT_TRUE(resName.name != NULL);

  Res_value val;

  ssize_t block = table.getResource(kNonOverlaidResourceId, &val, false /*mayBeBag*/);

  ASSERT_GE(block, 0);



  EXPECT_EQ(String16("com.android.basic"),

            String16(resName.package, resName.packageLen));

  EXPECT_EQ(String16("array"), String16(resName.type, resName.typeLen));

  EXPECT_EQ(String16("integerArray1"), String16(resName.name, resName.nameLen));

  // Now add the overlay and verify that the unoverlaid resource is gone.

  ASSERT_EQ(NO_ERROR,

            target_table_.add(overlay_data_.data(), overlay_data_.size(), data_, data_size_));

  block = target_table_.getResource(kNonOverlaidResourceId, &val, false /*mayBeBag*/);

  ASSERT_LT(block, 0);

}



}  // namespace

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

+2 −0
Original line number Diff line number Diff line
@@ -20,4 +20,6 @@ 
        <item>10</item>

        <item>11</item>

    </integer-array>

    <public type="animator" name="unoverlaid" id="0x7fff0000" />

    <item type="animator" name="unoverlaid">@null</item>

</resources>