AAPT2: Loosen loading apk format requirements

The Android runtime and AAPT are more lenient of apk format, allowing
for duplicate enty, types, and configs. This change loosens the
ResourceTable's checks on resource uniqueness when apks are loaded; not
when ResourceTables are being created by aapt2.

Bug: 36051266
Test: Tested using apks in bug with allow_duplicates on and off
Change-Id: I9296417bf2dc53e1e891479a53679a0388210d50

tools/aapt2/LoadedApk.cpp

@@ -76,7 +76,7 @@ std::unique_ptr<LoadedApk> LoadedApk::LoadProtoApkFromFileCollection(
     }
 
     std::string error;
-    table = util::make_unique<ResourceTable>();
+    table = util::make_unique<ResourceTable>(/** validate_resources **/ false);
     if (!DeserializeTableFromPb(pb_table, collection.get(), table.get(), &error)) {
       diag->Error(DiagMessage(source)
                   << "failed to deserialize " << kProtoResourceTablePath << ": " << error);
@@ -120,7 +120,7 @@ std::unique_ptr<LoadedApk> LoadedApk::LoadBinaryApkFromFileCollection(
 
   io::IFile* table_file = collection->FindFile(kApkResourceTablePath);
   if (table_file != nullptr) {
-    table = util::make_unique<ResourceTable>();
+    table = util::make_unique<ResourceTable>(/** validate_resources **/ false);
     std::unique_ptr<io::IData> data = table_file->OpenAsData();
     if (data == nullptr) {
       diag->Error(DiagMessage(source) << "failed to open " << kApkResourceTablePath);

tools/aapt2/Resource.cpp

@@ -96,6 +96,8 @@ StringPiece to_string(ResourceType type) {
       return "styleable";
     case ResourceType::kTransition:
       return "transition";
+    case ResourceType::kUnknown:
+      return "unknown";
     case ResourceType::kXml:
       return "xml";
   }

tools/aapt2/Resource.h

@@ -66,6 +66,11 @@ enum class ResourceType {
   kStyle,
   kStyleable,
   kTransition,
+
+  // Not a parsed type. It is only used when loading resource tables that may have modified type
+  // names
+  kUnknown,
+
   kXml,
 };
 

tools/aapt2/ResourceTable.cpp

@@ -26,6 +26,7 @@
 #include "androidfw/ResourceTypes.h"
 
 #include "ConfigDescription.h"
+#include "Debug.h"
 #include "NameMangler.h"
 #include "ResourceValues.h"
 #include "ValueVisitor.h"
@@ -38,8 +39,9 @@ using ::android::base::StringPrintf;
 
 namespace aapt {
 
-static bool less_than_type(const std::unique_ptr<ResourceTableType>& lhs, ResourceType rhs) {
-  return lhs->type < rhs;
+static bool less_than_type_and_id(const std::unique_ptr<ResourceTableType>& lhs,
+                                  const std::pair<ResourceType, Maybe<uint8_t>>& rhs) {
+  return lhs->type < rhs.first || (lhs->type == rhs.first && rhs.second && lhs->id < rhs.second);
 }
 
 template <typename T>
@@ -51,7 +53,7 @@ template <typename T>
 static bool less_than_struct_with_name_and_id(const std::unique_ptr<T>& lhs,
                                               const std::pair<StringPiece, Maybe<uint8_t>>& rhs) {
   int name_cmp = lhs->name.compare(0, lhs->name.size(), rhs.first.data(), rhs.first.size());
-  return name_cmp < 0 || (name_cmp == 0 && lhs->id < rhs.second);
+  return name_cmp < 0 || (name_cmp == 0 && rhs.second && lhs->id < rhs.second);
 }
 
 ResourceTablePackage* ResourceTable::FindPackage(const StringPiece& name) const {
@@ -115,42 +117,52 @@ ResourceTablePackage* ResourceTable::FindOrCreatePackage(const StringPiece& name
   return packages.emplace(iter, std::move(new_package))->get();
 }
 
-ResourceTableType* ResourceTablePackage::FindType(ResourceType type) {
+ResourceTableType* ResourceTablePackage::FindType(ResourceType type, const Maybe<uint8_t> id) {
   const auto last = types.end();
-  auto iter = std::lower_bound(types.begin(), last, type, less_than_type);
-  if (iter != last && (*iter)->type == type) {
+  auto iter = std::lower_bound(types.begin(), last, std::make_pair(type, id),
+                               less_than_type_and_id);
+  if (iter != last && (*iter)->type == type && (!id || id == (*iter)->id)) {
     return iter->get();
   }
   return nullptr;
 }
 
-ResourceTableType* ResourceTablePackage::FindOrCreateType(ResourceType type) {
+ResourceTableType* ResourceTablePackage::FindOrCreateType(ResourceType type,
+                                                          const Maybe<uint8_t> id) {
   const auto last = types.end();
-  auto iter = std::lower_bound(types.begin(), last, type, less_than_type);
-  if (iter != last && (*iter)->type == type) {
+  auto iter = std::lower_bound(types.begin(), last, std::make_pair(type, id),
+                               less_than_type_and_id);
+  if (iter != last && (*iter)->type == type && (!id || id == (*iter)->id)) {
     return iter->get();
   }
-  return types.emplace(iter, new ResourceTableType(type))->get();
+
+  auto new_type = new ResourceTableType(type);
+  new_type->id = id;
+  return types.emplace(iter, std::move(new_type))->get();
 }
 
-ResourceEntry* ResourceTableType::FindEntry(const StringPiece& name) {
+ResourceEntry* ResourceTableType::FindEntry(const StringPiece& name, const Maybe<uint8_t> id) {
   const auto last = entries.end();
-  auto iter =
-      std::lower_bound(entries.begin(), last, name, less_than_struct_with_name<ResourceEntry>);
-  if (iter != last && name == (*iter)->name) {
+  auto iter = std::lower_bound(entries.begin(), last, std::make_pair(name, id),
+      less_than_struct_with_name_and_id<ResourceEntry>);
+  if (iter != last && name == (*iter)->name && (!id || id == (*iter)->id)) {
     return iter->get();
   }
   return nullptr;
 }
 
-ResourceEntry* ResourceTableType::FindOrCreateEntry(const StringPiece& name) {
+ResourceEntry* ResourceTableType::FindOrCreateEntry(const StringPiece& name,
+                                                    const Maybe<uint8_t> id) {
   auto last = entries.end();
-  auto iter =
-      std::lower_bound(entries.begin(), last, name, less_than_struct_with_name<ResourceEntry>);
-  if (iter != last && name == (*iter)->name) {
+  auto iter = std::lower_bound(entries.begin(), last, std::make_pair(name, id),
+                               less_than_struct_with_name_and_id<ResourceEntry>);
+  if (iter != last && name == (*iter)->name && (!id || id == (*iter)->id)) {
     return iter->get();
   }
-  return entries.emplace(iter, new ResourceEntry(name))->get();
+
+  auto new_entry = new ResourceEntry(name);
+  new_entry->id = id;
+  return entries.emplace(iter, std::move(new_entry))->get();
 }
 
 ResourceConfigValue* ResourceEntry::FindValue(const ConfigDescription& config) {
@@ -302,9 +314,15 @@ ResourceTable::CollisionResult ResourceTable::ResolveValueCollision(Value* exist
     // Keep the existing attribute.
     return CollisionResult::kKeepOriginal;
   }
+
   return CollisionResult::kConflict;
 }
 
+ResourceTable::CollisionResult ResourceTable::IgnoreCollision(Value* /** existing **/,
+                                                              Value* /** incoming **/) {
+  return CollisionResult::kKeepBoth;
+}
+
 static StringPiece ResourceNameValidator(const StringPiece& name) {
   if (!IsValidResourceEntryName(name)) {
     return name;
@@ -321,15 +339,17 @@ bool ResourceTable::AddResource(const ResourceNameRef& name,
                                 const StringPiece& product,
                                 std::unique_ptr<Value> value,
                                 IDiagnostics* diag) {
-  return AddResourceImpl(name, {}, config, product, std::move(value), ResourceNameValidator,
-                         ResolveValueCollision, diag);
+  return AddResourceImpl(name, ResourceId{}, config, product, std::move(value),
+                         (validate_resources_ ? ResourceNameValidator : SkipNameValidator),
+                         (validate_resources_ ? ResolveValueCollision : IgnoreCollision), diag);
 }
 
 bool ResourceTable::AddResourceWithId(const ResourceNameRef& name, const ResourceId& res_id,
                                       const ConfigDescription& config, const StringPiece& product,
                                       std::unique_ptr<Value> value, IDiagnostics* diag) {
-  return AddResourceImpl(name, res_id, config, product, std::move(value), ResourceNameValidator,
-                         ResolveValueCollision, diag);
+  return AddResourceImpl(name, res_id, config, product, std::move(value),
+                         (validate_resources_ ? ResourceNameValidator : SkipNameValidator),
+                         (validate_resources_ ? ResolveValueCollision : IgnoreCollision), diag);
 }
 
 bool ResourceTable::AddFileReference(const ResourceNameRef& name,
@@ -337,14 +357,18 @@ bool ResourceTable::AddFileReference(const ResourceNameRef& name,
                                      const Source& source,
                                      const StringPiece& path,
                                      IDiagnostics* diag) {
-  return AddFileReferenceImpl(name, config, source, path, nullptr, ResourceNameValidator, diag);
+  return AddFileReferenceImpl(name, config, source, path, nullptr,
+                              (validate_resources_ ? ResourceNameValidator : SkipNameValidator),
+                              diag);
 }
 
 bool ResourceTable::AddFileReferenceMangled(const ResourceNameRef& name,
                                             const ConfigDescription& config, const Source& source,
                                             const StringPiece& path, io::IFile* file,
                                             IDiagnostics* diag) {
-  return AddFileReferenceImpl(name, config, source, path, file, SkipNameValidator, diag);
+  return AddFileReferenceImpl(name, config, source, path, file,
+                              (validate_resources_ ? ResourceNameValidator : SkipNameValidator),
+                              diag);
 }
 
 bool ResourceTable::AddFileReferenceImpl(const ResourceNameRef& name,
@@ -363,7 +387,7 @@ bool ResourceTable::AddResourceMangled(const ResourceNameRef& name, const Config
                                        const StringPiece& product, std::unique_ptr<Value> value,
                                        IDiagnostics* diag) {
   return AddResourceImpl(name, ResourceId{}, config, product, std::move(value), SkipNameValidator,
-                         ResolveValueCollision, diag);
+                         (validate_resources_ ? ResolveValueCollision : IgnoreCollision), diag);
 }
 
 bool ResourceTable::AddResourceWithIdMangled(const ResourceNameRef& name, const ResourceId& id,
@@ -371,7 +395,7 @@ bool ResourceTable::AddResourceWithIdMangled(const ResourceNameRef& name, const
                                              const StringPiece& product,
                                              std::unique_ptr<Value> value, IDiagnostics* diag) {
   return AddResourceImpl(name, id, config, product, std::move(value), SkipNameValidator,
-                         ResolveValueCollision, diag);
+                         (validate_resources_ ? ResolveValueCollision : IgnoreCollision), diag);
 }
 
 bool ResourceTable::ValidateName(NameValidator name_validator, const ResourceNameRef& name,
@@ -398,37 +422,57 @@ bool ResourceTable::AddResourceImpl(const ResourceNameRef& name, const ResourceI
     return false;
   }
 
+  // Check for package names appearing twice with two different package ids
   ResourceTablePackage* package = FindOrCreatePackage(name.package);
   if (res_id.is_valid_dynamic() && package->id && package->id.value() != res_id.package_id()) {
-    diag->Error(DiagMessage(source) << "trying to add resource '" << name << "' with ID " << res_id
-                                    << " but package '" << package->name << "' already has ID "
-                                    << StringPrintf("%02x", package->id.value()));
+    diag->Error(DiagMessage(source)
+                    << "trying to add resource '" << name << "' with ID " << res_id
+                    << " but package '" << package->name << "' already has ID "
+                    << StringPrintf("%02x", package->id.value()));
     return false;
   }
 
-  ResourceTableType* type = package->FindOrCreateType(name.type);
-  if (res_id.is_valid_dynamic() && type->id && type->id.value() != res_id.type_id()) {
+  // Whether or not to error on duplicate resources
+  bool check_id = validate_resources_ && res_id.is_valid_dynamic();
+  // Whether or not to create a duplicate resource if the id does not match
+  bool use_id = !validate_resources_ && res_id.is_valid_dynamic();
+
+  ResourceTableType* type = package->FindOrCreateType(name.type, use_id ? res_id.type_id()
+                                                                        : Maybe<uint8_t>());
+
+  // Check for types appearing twice with two different type ids
+  if (check_id && type->id && type->id.value() != res_id.type_id()) {
     diag->Error(DiagMessage(source)
-                << "trying to add resource '" << name << "' with ID " << res_id << " but type '"
-                << type->type << "' already has ID " << StringPrintf("%02x", type->id.value()));
+                    << "trying to add resource '" << name << "' with ID " << res_id
+                    << " but type '" << type->type << "' already has ID "
+                    << StringPrintf("%02x", type->id.value()));
     return false;
   }
 
-  ResourceEntry* entry = type->FindOrCreateEntry(name.entry);
-  if (res_id.is_valid_dynamic() && entry->id && entry->id.value() != res_id.entry_id()) {
+  ResourceEntry* entry = type->FindOrCreateEntry(name.entry, use_id ? res_id.entry_id()
+                                                                    : Maybe<uint8_t>());
+
+  // Check for entries appearing twice with two different entry ids
+  if (check_id && entry->id && entry->id.value() != res_id.entry_id()) {
     diag->Error(DiagMessage(source)
-                << "trying to add resource '" << name << "' with ID " << res_id
-                << " but resource already has ID "
-                << ResourceId(package->id.value(), type->id.value(), entry->id.value()));
+                    << "trying to add resource '" << name << "' with ID " << res_id
+                    << " but resource already has ID "
+                    << ResourceId(package->id.value(), type->id.value(), entry->id.value()));
     return false;
   }
 
   ResourceConfigValue* config_value = entry->FindOrCreateValue(config, product);
-  if (config_value->value == nullptr) {
+  if (!config_value->value) {
     // Resource does not exist, add it now.
     config_value->value = std::move(value);
   } else {
     switch (conflict_resolver(config_value->value.get(), value.get())) {
+      case CollisionResult::kKeepBoth:
+        // Insert the value ignoring for duplicate configurations
+        entry->values.push_back(util::make_unique<ResourceConfigValue>(config, product));
+        entry->values.back()->value = std::move(value);
+        break;
+
       case CollisionResult::kTakeNew:
         // Take the incoming value.
         config_value->value = std::move(value);
@@ -450,17 +494,22 @@ bool ResourceTable::AddResourceImpl(const ResourceNameRef& name, const ResourceI
     type->id = res_id.type_id();
     entry->id = res_id.entry_id();
   }
+
   return true;
 }
 
+bool ResourceTable::GetValidateResources() {
+  return validate_resources_;
+}
+
 bool ResourceTable::SetVisibility(const ResourceNameRef& name, const Visibility& visibility,
                                   IDiagnostics* diag) {
-  return SetVisibilityImpl(name, visibility, ResourceId{}, ResourceNameValidator, diag);
+  return SetVisibilityImpl(name, visibility, {}, ResourceNameValidator, diag);
 }
 
 bool ResourceTable::SetVisibilityMangled(const ResourceNameRef& name, const Visibility& visibility,
                                          IDiagnostics* diag) {
-  return SetVisibilityImpl(name, visibility, ResourceId{}, SkipNameValidator, diag);
+  return SetVisibilityImpl(name, visibility, {}, SkipNameValidator, diag);
 }
 
 bool ResourceTable::SetVisibilityWithId(const ResourceNameRef& name, const Visibility& visibility,
@@ -484,28 +533,42 @@ bool ResourceTable::SetVisibilityImpl(const ResourceNameRef& name, const Visibil
     return false;
   }
 
+  // Check for package names appearing twice with two different package ids
   ResourceTablePackage* package = FindOrCreatePackage(name.package);
   if (res_id.is_valid_dynamic() && package->id && package->id.value() != res_id.package_id()) {
-    diag->Error(DiagMessage(source) << "trying to add resource '" << name << "' with ID " << res_id
-                                    << " but package '" << package->name << "' already has ID "
-                                    << StringPrintf("%02x", package->id.value()));
+    diag->Error(DiagMessage(source)
+                    << "trying to add resource '" << name << "' with ID " << res_id
+                    << " but package '" << package->name << "' already has ID "
+                    << StringPrintf("%02x", package->id.value()));
     return false;
   }
 
-  ResourceTableType* type = package->FindOrCreateType(name.type);
-  if (res_id.is_valid_dynamic() && type->id && type->id.value() != res_id.type_id()) {
+  // Whether or not to error on duplicate resources
+  bool check_id = validate_resources_ && res_id.is_valid_dynamic();
+  // Whether or not to create a duplicate resource if the id does not match
+  bool use_id = !validate_resources_ && res_id.is_valid_dynamic();
+
+  ResourceTableType* type = package->FindOrCreateType(name.type, use_id ? res_id.type_id()
+                                                                        : Maybe<uint8_t>());
+
+  // Check for types appearing twice with two different type ids
+  if (check_id && type->id && type->id.value() != res_id.type_id()) {
     diag->Error(DiagMessage(source)
-                << "trying to add resource '" << name << "' with ID " << res_id << " but type '"
-                << type->type << "' already has ID " << StringPrintf("%02x", type->id.value()));
+                    << "trying to add resource '" << name << "' with ID " << res_id
+                    << " but type '" << type->type << "' already has ID "
+                    << StringPrintf("%02x", type->id.value()));
     return false;
   }
 
-  ResourceEntry* entry = type->FindOrCreateEntry(name.entry);
-  if (res_id.is_valid_dynamic() && entry->id && entry->id.value() != res_id.entry_id()) {
+  ResourceEntry* entry = type->FindOrCreateEntry(name.entry, use_id ? res_id.entry_id()
+                                                                    : Maybe<uint8_t>());
+
+  // Check for entries appearing twice with two different entry ids
+  if (check_id && entry->id && entry->id.value() != res_id.entry_id()) {
     diag->Error(DiagMessage(source)
-                << "trying to add resource '" << name << "' with ID " << res_id
-                << " but resource already has ID "
-                << ResourceId(package->id.value(), type->id.value(), entry->id.value()));
+                    << "trying to add resource '" << name << "' with ID " << res_id
+                    << " but resource already has ID "
+                    << ResourceId(package->id.value(), type->id.value(), entry->id.value()));
     return false;
   }
 

tools/aapt2/ResourceTable.h

@@ -146,8 +146,9 @@ class ResourceTableType {
 
   explicit ResourceTableType(const ResourceType type) : type(type) {}
 
-  ResourceEntry* FindEntry(const android::StringPiece& name);
-  ResourceEntry* FindOrCreateEntry(const android::StringPiece& name);
+  ResourceEntry* FindEntry(const android::StringPiece& name, Maybe<uint8_t> id = Maybe<uint8_t>());
+  ResourceEntry* FindOrCreateEntry(const android::StringPiece& name,
+                                   Maybe<uint8_t> id = Maybe<uint8_t>());
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ResourceTableType);
@@ -163,8 +164,9 @@ class ResourceTablePackage {
   std::vector<std::unique_ptr<ResourceTableType>> types;
 
   ResourceTablePackage() = default;
-  ResourceTableType* FindType(ResourceType type);
-  ResourceTableType* FindOrCreateType(const ResourceType type);
+  ResourceTableType* FindType(ResourceType type, Maybe<uint8_t> id = Maybe<uint8_t>());
+  ResourceTableType* FindOrCreateType(const ResourceType type,
+                                      Maybe<uint8_t> id = Maybe<uint8_t>());
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ResourceTablePackage);
@@ -174,14 +176,18 @@ class ResourceTablePackage {
 class ResourceTable {
  public:
   ResourceTable() = default;
+  explicit ResourceTable(bool validate_resources) : validate_resources_(validate_resources) {}
 
-  enum class CollisionResult { kKeepOriginal, kConflict, kTakeNew };
+  enum class CollisionResult { kKeepBoth, kKeepOriginal, kConflict, kTakeNew };
 
   using CollisionResolverFunc = std::function<CollisionResult(Value*, Value*)>;
 
   // When a collision of resources occurs, this method decides which value to keep.
   static CollisionResult ResolveValueCollision(Value* existing, Value* incoming);
 
+  // When a collision of resources occurs, this method keeps both values
+  static CollisionResult IgnoreCollision(Value* existing, Value* incoming);
+
   bool AddResource(const ResourceNameRef& name, const ConfigDescription& config,
                    const android::StringPiece& product, std::unique_ptr<Value> value,
                    IDiagnostics* diag);
@@ -208,6 +214,8 @@ class ResourceTable {
                                 const android::StringPiece& product, std::unique_ptr<Value> value,
                                 IDiagnostics* diag);
 
+  bool GetValidateResources();
+
   bool SetVisibility(const ResourceNameRef& name, const Visibility& visibility, IDiagnostics* diag);
   bool SetVisibilityMangled(const ResourceNameRef& name, const Visibility& visibility,
                             IDiagnostics* diag);
@@ -299,6 +307,9 @@ class ResourceTable {
                           const Visibility& symbol, NameValidator name_validator,
                           IDiagnostics* diag);
 
+  // Controls whether the table validates resource names and prevents duplicate resource names
+  bool validate_resources_ = true;
+
   DISALLOW_COPY_AND_ASSIGN(ResourceTable);
 };
 

tools/aapt2/cmd/Dump.cpp

@@ -124,7 +124,7 @@ static bool TryDumpFile(IAaptContext* context, const std::string& file_path,
   std::string err;
   std::unique_ptr<io::ZipFileCollection> zip = io::ZipFileCollection::Create(file_path, &err);
   if (zip) {
-    ResourceTable table;
+    ResourceTable table(/** validate_resources **/ false);
     bool proto = false;
     if (io::IFile* file = zip->FindFile("resources.pb")) {
       proto = true;

tools/aapt2/format/binary/BinaryResourceParser.cpp

@@ -338,10 +338,13 @@ bool BinaryResourceParser::ParseType(const ResourceTablePackage* package,
 
   const std::string type_str = util::GetString(type_pool_, type->id - 1);
 
-  const ResourceType* parsed_type = ParseResourceType(type_str);
-  if (!parsed_type) {
-    diag_->Error(DiagMessage(source_)
-                 << "invalid type name '" << type_str << "' for type with ID " << (int)type->id);
+  // Be lenient on the name of the type if the table is lenient on resource validation.
+  auto parsed_type = ResourceType::kUnknown;
+  if (const ResourceType* parsed = ParseResourceType(type_str)) {
+    parsed_type = *parsed;
+  } else if (table_->GetValidateResources()) {
+    diag_->Error(DiagMessage(source_) << "invalid type name '" << type_str << "' for type with ID "
+                                      << (int) type->id);
     return false;
   }
 
@@ -352,7 +355,7 @@ bool BinaryResourceParser::ParseType(const ResourceTablePackage* package,
       continue;
     }
 
-    const ResourceName name(package->name, *parsed_type,
+    const ResourceName name(package->name, parsed_type,
                             util::GetString(key_pool_, util::DeviceToHost32(entry->key.index)));
 
     const ResourceId res_id(package->id.value(), type->id, static_cast<uint16_t>(it.index()));