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
diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp
index d0faac3..58b5e8f 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 @@
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 @@
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 @@
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 @@
// 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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;
}