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/LoadedApk.cpp b/tools/aapt2/LoadedApk.cpp
index 1dd46ba..b32766b 100644
--- a/tools/aapt2/LoadedApk.cpp
+++ b/tools/aapt2/LoadedApk.cpp
@@ -76,7 +76,7 @@
}
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 @@
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);
diff --git a/tools/aapt2/Resource.cpp b/tools/aapt2/Resource.cpp
index b78f48c..ae01170 100644
--- a/tools/aapt2/Resource.cpp
+++ b/tools/aapt2/Resource.cpp
@@ -96,6 +96,8 @@
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 6fcf0f6..879d0bd 100644
--- a/tools/aapt2/Resource.h
+++ b/tools/aapt2/Resource.h
@@ -66,6 +66,11 @@
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 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;
}
diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h
index 8534eaa..c40323c 100644
--- a/tools/aapt2/ResourceTable.h
+++ b/tools/aapt2/ResourceTable.h
@@ -146,8 +146,9 @@
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 @@
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 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 @@
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 @@
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 8b1f672..6e9c800 100644
--- a/tools/aapt2/cmd/Dump.cpp
+++ b/tools/aapt2/cmd/Dump.cpp
@@ -124,7 +124,7 @@
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;
diff --git a/tools/aapt2/format/binary/BinaryResourceParser.cpp b/tools/aapt2/format/binary/BinaryResourceParser.cpp
index 8215ddf..3a39a6b 100644
--- a/tools/aapt2/format/binary/BinaryResourceParser.cpp
+++ b/tools/aapt2/format/binary/BinaryResourceParser.cpp
@@ -338,10 +338,13 @@
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 @@
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()));