DO NOT MERGE: Do not allow overlaying of attributes with conflicting formats
aapt(1) does not allow for attributes to be redefined with a different
format. Also, attributes declared with enums or flags are never allowed to be
redefined. This change will not allow attributes to be redefined with a
conflicting format in aapt2.
Bug: 129146717
Test: aapt2_tests
Change-Id: Idc43d6d689199ba2cdc672d009ede22eaa75a10c
diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp
index 1773b5a..836e199 100644
--- a/tools/aapt2/ResourceTable.cpp
+++ b/tools/aapt2/ResourceTable.cpp
@@ -267,7 +267,8 @@
// A DECL will override a USE without error. Two DECLs must match in their format for there to be
// no error.
ResourceTable::CollisionResult ResourceTable::ResolveValueCollision(Value* existing,
- Value* incoming) {
+ Value* incoming,
+ bool overlay) {
Attribute* existing_attr = ValueCast<Attribute>(existing);
Attribute* incoming_attr = ValueCast<Attribute>(incoming);
if (!incoming_attr) {
@@ -281,7 +282,7 @@
}
// The existing and incoming values are strong, this is an error
// if the values are not both attributes.
- return CollisionResult::kConflict;
+ return overlay ? CollisionResult::kTakeNew : CollisionResult::kConflict;
}
if (!existing_attr) {
@@ -292,7 +293,7 @@
}
// The existing value is not an attribute and it is strong,
// so the incoming attribute value is an error.
- return CollisionResult::kConflict;
+ return overlay ? CollisionResult::kTakeNew : CollisionResult::kConflict;
}
CHECK(incoming_attr != nullptr && existing_attr != nullptr);
@@ -323,8 +324,9 @@
return CollisionResult::kConflict;
}
-ResourceTable::CollisionResult ResourceTable::IgnoreCollision(Value* /** existing **/,
- Value* /** incoming **/) {
+ResourceTable::CollisionResult ResourceTable::IgnoreCollision(Value* /* existing */,
+ Value* /* incoming */,
+ bool /* overlay */) {
return CollisionResult::kKeepBoth;
}
@@ -440,7 +442,7 @@
// Resource does not exist, add it now.
config_value->value = std::move(value);
} else {
- switch (conflict_resolver(config_value->value.get(), value.get())) {
+ switch (conflict_resolver(config_value->value.get(), value.get(), false /* overlay */)) {
case CollisionResult::kKeepBoth:
// Insert the value ignoring for duplicate configurations
entry->values.push_back(util::make_unique<ResourceConfigValue>(config, product));
diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h
index 30ba1ae..e879380 100644
--- a/tools/aapt2/ResourceTable.h
+++ b/tools/aapt2/ResourceTable.h
@@ -228,13 +228,13 @@
enum class CollisionResult { kKeepBoth, kKeepOriginal, kConflict, kTakeNew };
- using CollisionResolverFunc = std::function<CollisionResult(Value*, Value*)>;
+ using CollisionResolverFunc = std::function<CollisionResult(Value*, Value*, bool)>;
// When a collision of resources occurs, this method decides which value to keep.
- static CollisionResult ResolveValueCollision(Value* existing, Value* incoming);
+ static CollisionResult ResolveValueCollision(Value* existing, Value* incoming, bool overlay);
// When a collision of resources occurs, this method keeps both values
- static CollisionResult IgnoreCollision(Value* existing, Value* incoming);
+ static CollisionResult IgnoreCollision(Value* existing, Value* incoming, bool overlay);
bool AddResource(const ResourceNameRef& name, const android::ConfigDescription& config,
const android::StringPiece& product, std::unique_ptr<Value> value,
diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp
index 34b46c5..6960127 100644
--- a/tools/aapt2/ResourceValues.cpp
+++ b/tools/aapt2/ResourceValues.cpp
@@ -574,6 +574,10 @@
}
bool Attribute::IsCompatibleWith(const Attribute& attr) const {
+ if (Equals(&attr)) {
+ return true;
+ }
+
// If the high bits are set on any of these attribute type masks, then they are incompatible.
// We don't check that flags and enums are identical.
if ((type_mask & ~android::ResTable_map::TYPE_ANY) != 0 ||
diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp
index c0802e6..3f65e86 100644
--- a/tools/aapt2/link/TableMerger.cpp
+++ b/tools/aapt2/link/TableMerger.cpp
@@ -188,7 +188,7 @@
}
}
// Delegate to the default handler.
- return ResourceTable::ResolveValueCollision(existing, incoming);
+ return ResourceTable::ResolveValueCollision(existing, incoming, true /* overlay */);
}
static ResourceTable::CollisionResult MergeConfigValue(IAaptContext* context,
@@ -206,15 +206,11 @@
if (overlay) {
collision_result = ResolveMergeCollision(dst_value, src_value, pool);
} else {
- collision_result = ResourceTable::ResolveValueCollision(dst_value, src_value);
+ collision_result = ResourceTable::ResolveValueCollision(dst_value, src_value,
+ false /* overlay */);
}
if (collision_result == CollisionResult::kConflict) {
- if (overlay) {
- return CollisionResult::kTakeNew;
- }
-
- // Error!
context->GetDiagnostics()->Error(DiagMessage(src_value->GetSource())
<< "resource '" << res_name << "' has a conflicting value for "
<< "configuration (" << src_config_value->config << ")");
diff --git a/tools/aapt2/link/TableMerger_test.cpp b/tools/aapt2/link/TableMerger_test.cpp
index 9dd31e6..be9c84b 100644
--- a/tools/aapt2/link/TableMerger_test.cpp
+++ b/tools/aapt2/link/TableMerger_test.cpp
@@ -352,6 +352,110 @@
ASSERT_TRUE(merger.Merge({}, table_b.get(), false /*overlay*/));
}
+TEST_F(TableMergerTest, OverrideAttributeSameFormatsWithOverlay) {
+ std::unique_ptr<ResourceTable> base =
+ test::ResourceTableBuilder()
+ .SetPackageId("", 0x7f)
+ .AddValue("attr/foo", test::AttributeBuilder()
+ .SetTypeMask(android::ResTable_map::TYPE_STRING)
+ .SetWeak(false)
+ .Build())
+ .Build();
+
+ std::unique_ptr<ResourceTable> overlay =
+ test::ResourceTableBuilder()
+ .SetPackageId("", 0x7f)
+ .AddValue("attr/foo", test::AttributeBuilder()
+ .SetTypeMask(android::ResTable_map::TYPE_STRING)
+ .SetWeak(false)
+ .Build())
+ .Build();
+
+ ResourceTable final_table;
+ TableMergerOptions options;
+ options.auto_add_overlay = false;
+ TableMerger merger(context_.get(), &final_table, options);
+
+ ASSERT_TRUE(merger.Merge({}, base.get(), false /*overlay*/));
+ ASSERT_TRUE(merger.Merge({}, overlay.get(), true /*overlay*/));
+}
+
+TEST_F(TableMergerTest, FailToOverrideConflictingAttributeFormatsWithOverlay) {
+ std::unique_ptr<ResourceTable> base =
+ test::ResourceTableBuilder()
+ .SetPackageId("", 0x7f)
+ .AddValue("attr/foo", test::AttributeBuilder()
+ .SetTypeMask(android::ResTable_map::TYPE_ANY)
+ .SetWeak(false)
+ .Build())
+ .Build();
+
+ std::unique_ptr<ResourceTable> overlay =
+ test::ResourceTableBuilder()
+ .SetPackageId("", 0x7f)
+ .AddValue("attr/foo", test::AttributeBuilder()
+ .SetTypeMask(android::ResTable_map::TYPE_STRING)
+ .SetWeak(false)
+ .Build())
+ .Build();
+
+ ResourceTable final_table;
+ TableMergerOptions options;
+ options.auto_add_overlay = false;
+ TableMerger merger(context_.get(), &final_table, options);
+
+ ASSERT_TRUE(merger.Merge({}, base.get(), false /*overlay*/));
+ ASSERT_FALSE(merger.Merge({}, overlay.get(), true /*overlay*/));
+}
+
+TEST_F(TableMergerTest, FailToOverrideConflictingFlagsAndEnumsWithOverlay) {
+ std::unique_ptr<ResourceTable> base =
+ test::ResourceTableBuilder()
+ .SetPackageId("", 0x7f)
+ .AddValue("attr/foo", test::AttributeBuilder()
+ .SetTypeMask(android::ResTable_map::TYPE_FLAGS)
+ .Build())
+ .Build();
+
+ std::unique_ptr<ResourceTable> overlay =
+ test::ResourceTableBuilder()
+ .SetPackageId("", 0x7f)
+ .AddValue("attr/foo", test::AttributeBuilder()
+ .SetTypeMask(android::ResTable_map::TYPE_FLAGS)
+ .SetWeak(false)
+ .Build())
+ .Build();
+
+ ResourceTable final_table;
+ TableMergerOptions options;
+ options.auto_add_overlay = false;
+ TableMerger merger(context_.get(), &final_table, options);
+
+ ASSERT_TRUE(merger.Merge({}, base.get(), false /*overlay*/));
+ ASSERT_FALSE(merger.Merge({}, overlay.get(), true /*overlay*/));
+
+ base = test::ResourceTableBuilder()
+ .SetPackageId("", 0x7f)
+ .AddValue("attr/foo", test::AttributeBuilder()
+ .SetTypeMask(android::ResTable_map::TYPE_ENUM)
+ .Build())
+ .Build();
+
+ overlay = test::ResourceTableBuilder()
+ .SetPackageId("", 0x7f)
+ .AddValue("attr/foo", test::AttributeBuilder()
+ .SetTypeMask(android::ResTable_map::TYPE_ENUM)
+ .SetWeak(false)
+ .Build())
+ .Build();
+
+ ResourceTable final_table2;
+ TableMerger merger2(context_.get(), &final_table2, options);
+
+ ASSERT_TRUE(merger2.Merge({}, base.get(), false /*overlay*/));
+ ASSERT_FALSE(merger2.Merge({}, overlay.get(), true /*overlay*/));
+}
+
TEST_F(TableMergerTest, FailToMergeNewResourceWithoutAutoAddOverlay) {
std::unique_ptr<ResourceTable> table_a =
test::ResourceTableBuilder().SetPackageId("", 0x7f).Build();