commit b435101ec8aa827375e837c4843f6444145e1f4c
author Ryan Mitchell <rtmitchell@google.com> Tue Mar 26 16:07:23 2019 -0700
committer Ryan Mitchell <rtmitchell@google.com> Wed Apr 24 09:59:55 2019 +0000
tree a677f4bbe82cf0e10585dba481941e837dc7b881
parent c37d832194c9a93a720bf14d0a991377e4d4db53

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

tools/aapt2/ResourceTable.cpp

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));