From 83a37adfbb3c9fc139cc7881995ddd63db4caff8 Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Mon, 6 Aug 2018 16:32:24 -0700 Subject: [PATCH] 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 | 4 +- tools/aapt2/Resource.cpp | 2 + tools/aapt2/Resource.h | 5 + tools/aapt2/ResourceTable.cpp | 171 ++++++++++++------ tools/aapt2/ResourceTable.h | 21 ++- tools/aapt2/cmd/Dump.cpp | 2 +- .../format/binary/BinaryResourceParser.cpp | 13 +- 7 files changed, 151 insertions(+), 67 deletions(-) diff --git a/tools/aapt2/LoadedApk.cpp b/tools/aapt2/LoadedApk.cpp index 1dd46ba813db5..b32766be8d180 100644 --- a/tools/aapt2/LoadedApk.cpp +++ b/tools/aapt2/LoadedApk.cpp @@ -76,7 +76,7 @@ std::unique_ptr LoadedApk::LoadProtoApkFromFileCollection( } std::string error; - table = util::make_unique(); + table = util::make_unique(/** 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::LoadBinaryApkFromFileCollection( io::IFile* table_file = collection->FindFile(kApkResourceTablePath); if (table_file != nullptr) { - table = util::make_unique(); + table = util::make_unique(/** validate_resources **/ false); std::unique_ptr data = table_file->OpenAsData(); if (data == nullptr) { diag->Error(DiagMessage(source) << "failed to open " << kApkResourceTablePath); diff --git a/tools/aapt2/Resource.cpp b/tools/aapt2/Resource.cpp index b78f48ce7f176..ae01170a68943 100644 --- a/tools/aapt2/Resource.cpp +++ b/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"; } diff --git a/tools/aapt2/Resource.h b/tools/aapt2/Resource.h index 6fcf0f6d19cd0..879d0bd8d94ec 100644 --- a/tools/aapt2/Resource.h +++ b/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, }; diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp index d0faac30425a7..58b5e8f0212c6 100644 --- a/tools/aapt2/ResourceTable.cpp +++ b/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& lhs, ResourceType rhs) { - return lhs->type < rhs; +static bool less_than_type_and_id(const std::unique_ptr& lhs, + const std::pair>& rhs) { + return lhs->type < rhs.first || (lhs->type == rhs.first && rhs.second && lhs->id < rhs.second); } template @@ -51,7 +53,7 @@ template static bool less_than_struct_with_name_and_id(const std::unique_ptr& lhs, const std::pair>& 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 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 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 id) { const auto last = entries.end(); - auto iter = - std::lower_bound(entries.begin(), last, name, less_than_struct_with_name); - 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); + 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 id) { auto last = entries.end(); - auto iter = - std::lower_bound(entries.begin(), last, name, less_than_struct_with_name); - 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); + 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, 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, 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, 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, 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()); + + // 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()); + + // 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(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()); + + // 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()); + + // 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; } diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h index 8534eaaf03664..c40323c34f481 100644 --- a/tools/aapt2/ResourceTable.h +++ b/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 id = Maybe()); + ResourceEntry* FindOrCreateEntry(const android::StringPiece& name, + Maybe id = Maybe()); private: DISALLOW_COPY_AND_ASSIGN(ResourceTableType); @@ -163,8 +164,9 @@ class ResourceTablePackage { std::vector> types; ResourceTablePackage() = default; - ResourceTableType* FindType(ResourceType type); - ResourceTableType* FindOrCreateType(const ResourceType type); + ResourceTableType* FindType(ResourceType type, Maybe id = Maybe()); + ResourceTableType* FindOrCreateType(const ResourceType type, + Maybe id = Maybe()); 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; // 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, IDiagnostics* diag); @@ -208,6 +214,8 @@ class ResourceTable { const android::StringPiece& product, std::unique_ptr 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); }; diff --git a/tools/aapt2/cmd/Dump.cpp b/tools/aapt2/cmd/Dump.cpp index 8b1f67204c226..6e9c80024ddd0 100644 --- a/tools/aapt2/cmd/Dump.cpp +++ b/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 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; diff --git a/tools/aapt2/format/binary/BinaryResourceParser.cpp b/tools/aapt2/format/binary/BinaryResourceParser.cpp index 8215ddf0461cd..3a39a6baeeebb 100644 --- a/tools/aapt2/format/binary/BinaryResourceParser.cpp +++ b/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(it.index()));