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
diff --git a/libs/androidfw/.clang-format b/libs/androidfw/.clang-format
index ee1bee2..c91502a 100644
--- a/libs/androidfw/.clang-format
+++ b/libs/androidfw/.clang-format
@@ -1,2 +1,7 @@
BasedOnStyle: Google
ColumnLimit: 100
+AllowShortBlocksOnASingleLine: false
+AllowShortFunctionsOnASingleLine: false
+CommentPragmas: NOLINT:.*
+DerivePointerAlignment: false
+PointerAlignment: Left
diff --git a/libs/androidfw/ResourceTypes.cpp b/libs/androidfw/ResourceTypes.cpp
index 359cfac..244c525 100644
--- a/libs/androidfw/ResourceTypes.cpp
+++ b/libs/androidfw/ResourceTypes.cpp
@@ -6431,32 +6431,42 @@
}
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;
}
- 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",
- (int) newEntryCount, (int) existingType->entryCount);
- // We should normally abort here, but some legacy apps declare
- // resources in the 'android' package (old bug in AAPT).
+ 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",
+ (int) newEntryCount, (int) existingType->entryCount);
+ // We should normally abort here, but some legacy apps declare
+ // resources in the 'android' package (old bug in AAPT).
+ }
}
- }
- Type* t = new Type(header, package, newEntryCount);
- t->typeSpec = typeSpec;
- t->typeSpecFlags = (const uint32_t*)(
- ((const uint8_t*)typeSpec) + dtohs(typeSpec->header.headerSize));
- if (idmapIndex >= 0) {
- t->idmapEntries = idmapEntries[idmapIndex];
+ Type* t = new Type(header, package, newEntryCount);
+ t->typeSpec = typeSpec;
+ t->typeSpecFlags = (const uint32_t*)(
+ ((const uint8_t*)typeSpec) + dtohs(typeSpec->header.headerSize));
+ if (idmapIndex >= 0) {
+ t->idmapEntries = idmapEntries[idmapIndex];
+ }
+ typeList.add(t);
+ group->largestTypeId = max(group->largestTypeId, typeSpec->id);
}
- typeList.add(t);
- group->largestTypeId = max(group->largestTypeId, typeSpec->id);
} else {
ALOGV("Skipping empty ResTable_typeSpec for type %d", typeSpec->id);
}
@@ -6499,31 +6509,40 @@
}
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;
}
- TypeList& typeList = group->types.editItemAt(typeIndex);
- if (typeList.isEmpty()) {
- ALOGE("No TypeSpec for type %d", type->id);
- return (mError=BAD_TYPE);
- }
+ if (addToType) {
+ TypeList& typeList = group->types.editItemAt(typeIndex);
+ if (typeList.isEmpty()) {
+ ALOGE("No TypeSpec for type %d", type->id);
+ return (mError=BAD_TYPE);
+ }
- Type* t = typeList.editItemAt(typeList.size() - 1);
- if (t->package != package) {
- ALOGE("No TypeSpec for type %d", type->id);
- return (mError=BAD_TYPE);
- }
+ Type* t = typeList.editItemAt(typeList.size() - 1);
+ if (t->package != package) {
+ ALOGE("No TypeSpec for type %d", type->id);
+ return (mError=BAD_TYPE);
+ }
- t->configs.add(type);
+ t->configs.add(type);
- if (kDebugTableGetEntry) {
- ResTable_config thisConfig;
- thisConfig.copyFromDtoH(type->config);
- ALOGI("Adding config to type %d: %s\n", type->id,
- thisConfig.toString().string());
+ if (kDebugTableGetEntry) {
+ ResTable_config thisConfig;
+ thisConfig.copyFromDtoH(type->config);
+ ALOGI("Adding config to type %d: %s\n", type->id,
+ thisConfig.toString().string());
+ }
}
} else {
ALOGV("Skipping empty ResTable_type for type %d", type->id);
diff --git a/libs/androidfw/tests/Idmap_test.cpp b/libs/androidfw/tests/Idmap_test.cpp
index 0928b1b..d12be18 100644
--- a/libs/androidfw/tests/Idmap_test.cpp
+++ b/libs/androidfw/tests/Idmap_test.cpp
@@ -30,25 +30,23 @@
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",
- "resources.arsc", &overlay_data_));
+ 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 @@
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 @@
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 resName;
- ASSERT_TRUE(
- target_table_.getResourceName(R::array::integerArray1, false, &resName));
+ ResTable::resource_name res_name;
+ ASSERT_TRUE(target_table_.getResourceName(R::array::integerArray1, false, &res_name));
- ASSERT_TRUE(resName.package != NULL);
- ASSERT_TRUE(resName.type != NULL);
- ASSERT_TRUE(resName.name != NULL);
+ ASSERT_TRUE(res_name.package != NULL);
+ ASSERT_TRUE(res_name.type != NULL);
+ ASSERT_TRUE(res_name.name != NULL);
- 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));
+ 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));
+
+ Res_value val;
+ ssize_t block = table.getResource(kNonOverlaidResourceId, &val, false /*mayBeBag*/);
+ ASSERT_GE(block, 0);
+
+ // 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
diff --git a/libs/androidfw/tests/data/overlay/overlay.apk b/libs/androidfw/tests/data/overlay/overlay.apk
index e0e0543..40bf17c 100644
--- a/libs/androidfw/tests/data/overlay/overlay.apk
+++ b/libs/androidfw/tests/data/overlay/overlay.apk
Binary files differ
diff --git a/libs/androidfw/tests/data/overlay/res/values/values.xml b/libs/androidfw/tests/data/overlay/res/values/values.xml
index 3e1af98..8e4417e 100644
--- a/libs/androidfw/tests/data/overlay/res/values/values.xml
+++ b/libs/androidfw/tests/data/overlay/res/values/values.xml
@@ -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>
diff --git a/tools/aapt2/.clang-format b/tools/aapt2/.clang-format
index 71c5ef2..c91502a 100644
--- a/tools/aapt2/.clang-format
+++ b/tools/aapt2/.clang-format
@@ -1,3 +1,7 @@
BasedOnStyle: Google
ColumnLimit: 100
-
+AllowShortBlocksOnASingleLine: false
+AllowShortFunctionsOnASingleLine: false
+CommentPragmas: NOLINT:.*
+DerivePointerAlignment: false
+PointerAlignment: Left