AAPT2: Workaround for findViewById with package ID > 0x7f
The entire View code base checks IDs against View.NO_ID except
findViewById(), which checks to see if the ID is negative.
Any package ID > 0x7f is interpreted as a negative number in Java
(no unsigned ints), so this check prevents the use of IDs > 0x7f.
findViewById is final, so support library workarounds are not possible.
Instead, IDs (@id/foo) are just sentinels, their values don't matter.
If building for pre-O devices, rewrite any references to these IDs of
the for 0xPPTTEEEE, where PP > 7f, to 0x7fPPEEEE.
The symbol table will check for potential collisions against the base
APK, so this should be safe.
Bug: 37498913
Test: manual
Change-Id: Ife3bbd29db287757ef8a2ffd83053d97f1db2613
diff --git a/tools/aapt2/Main.cpp b/tools/aapt2/Main.cpp
index 87fda16..9d00c52 100644
--- a/tools/aapt2/Main.cpp
+++ b/tools/aapt2/Main.cpp
@@ -25,7 +25,7 @@
static const char* sMajorVersion = "2";
// Update minor version whenever a feature or flag is added.
-static const char* sMinorVersion = "13";
+static const char* sMinorVersion = "14";
int PrintVersion() {
std::cerr << "Android Asset Packaging Tool (aapt) " << sMajorVersion << "."
diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index 258516d..8ca0b1f 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -190,6 +190,62 @@
int min_sdk_version_ = 0;
};
+// A custom delegate that generates compatible pre-O IDs for use with feature splits.
+// Feature splits use package IDs > 7f, which in Java (since Java doesn't have unsigned ints)
+// is interpreted as a negative number. Some verification was wrongly assuming negative values
+// were invalid.
+//
+// This delegate will attempt to masquerade any '@id/' references with ID 0xPPTTEEEE,
+// where PP > 7f, as 0x7fPPEEEE. Any potential overlapping is verified and an error occurs if such
+// an overlap exists.
+class FeatureSplitSymbolTableDelegate : public DefaultSymbolTableDelegate {
+ public:
+ FeatureSplitSymbolTableDelegate(IAaptContext* context) : context_(context) {
+ }
+
+ virtual ~FeatureSplitSymbolTableDelegate() = default;
+
+ virtual std::unique_ptr<SymbolTable::Symbol> FindByName(
+ const ResourceName& name,
+ const std::vector<std::unique_ptr<ISymbolSource>>& sources) override {
+ std::unique_ptr<SymbolTable::Symbol> symbol =
+ DefaultSymbolTableDelegate::FindByName(name, sources);
+ if (symbol == nullptr) {
+ return {};
+ }
+
+ // Check to see if this is an 'id' with the target package.
+ if (name.type == ResourceType::kId && symbol->id) {
+ ResourceId* id = &symbol->id.value();
+ if (id->package_id() > kAppPackageId) {
+ // Rewrite the resource ID to be compatible pre-O.
+ ResourceId rewritten_id(kAppPackageId, id->package_id(), id->entry_id());
+
+ // Check that this doesn't overlap another resource.
+ if (DefaultSymbolTableDelegate::FindById(rewritten_id, sources) != nullptr) {
+ // The ID overlaps, so log a message (since this is a weird failure) and fail.
+ context_->GetDiagnostics()->Error(DiagMessage() << "Failed to rewrite " << name
+ << " for pre-O feature split support");
+ return {};
+ }
+
+ if (context_->IsVerbose()) {
+ context_->GetDiagnostics()->Note(DiagMessage() << "rewriting " << name << " (" << *id
+ << ") -> (" << rewritten_id << ")");
+ }
+
+ *id = rewritten_id;
+ }
+ }
+ return symbol;
+ }
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(FeatureSplitSymbolTableDelegate);
+
+ IAaptContext* context_;
+};
+
static bool FlattenXml(xml::XmlResource* xml_res, const StringPiece& path,
Maybe<size_t> max_sdk_level, bool keep_raw_values, IArchiveWriter* writer,
IAaptContext* context) {
@@ -1463,6 +1519,19 @@
context_->GetExternalSymbols()->PrependSource(
util::make_unique<ResourceTableSymbolSource>(&final_table_));
+ // Workaround for pre-O runtime that would treat negative resource IDs
+ // (any ID with a package ID > 7f) as invalid. Intercept any ID (PPTTEEEE) with PP > 0x7f
+ // and type == 'id', and return the ID 0x7fPPEEEE. IDs don't need to be real resources, they
+ // are just identifiers.
+ if (context_->GetMinSdkVersion() < SDK_O && context_->GetPackageType() == PackageType::kApp) {
+ if (context_->IsVerbose()) {
+ context_->GetDiagnostics()->Note(DiagMessage()
+ << "enabling pre-O feature split ID rewriting");
+ }
+ context_->GetExternalSymbols()->SetDelegate(
+ util::make_unique<FeatureSplitSymbolTableDelegate>(context_));
+ }
+
ReferenceLinker linker;
if (!linker.Consume(context_, &final_table_)) {
context_->GetDiagnostics()->Error(DiagMessage() << "failed linking references");
diff --git a/tools/aapt2/java/JavaClassGenerator.cpp b/tools/aapt2/java/JavaClassGenerator.cpp
index a8226c0..2a23aa9 100644
--- a/tools/aapt2/java/JavaClassGenerator.cpp
+++ b/tools/aapt2/java/JavaClassGenerator.cpp
@@ -31,6 +31,7 @@
#include "Resource.h"
#include "ResourceTable.h"
#include "ResourceValues.h"
+#include "SdkConstants.h"
#include "ValueVisitor.h"
#include "java/AnnotationProcessor.h"
#include "java/ClassDefinition.h"
@@ -430,9 +431,15 @@
const ResourceEntry& entry, ClassDefinition* out_class_def,
MethodDefinition* out_rewrite_method,
std::ostream* out_r_txt) {
+ ResourceId real_id = id;
+ if (context_->GetMinSdkVersion() < SDK_O && name.type == ResourceType::kId &&
+ id.package_id() > kAppPackageId) {
+ real_id = ResourceId(kAppPackageId, id.package_id(), id.entry_id());
+ }
+
const std::string field_name = TransformToFieldName(name.entry);
std::unique_ptr<ResourceMember> resource_member =
- util::make_unique<ResourceMember>(field_name, id);
+ util::make_unique<ResourceMember>(field_name, real_id);
// Build the comments and annotations for this entry.
AnnotationProcessor* processor = resource_member->GetCommentBuilder();
@@ -458,7 +465,7 @@
out_class_def->AddMember(std::move(resource_member));
if (out_r_txt != nullptr) {
- *out_r_txt << "int " << name.type << " " << field_name << " " << id << "\n";
+ *out_r_txt << "int " << name.type << " " << field_name << " " << real_id << "\n";
}
if (out_rewrite_method != nullptr) {
diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp
index bcafbca..1a648bf 100644
--- a/tools/aapt2/process/SymbolTable.cpp
+++ b/tools/aapt2/process/SymbolTable.cpp
@@ -34,6 +34,21 @@
namespace aapt {
+SymbolTable::SymbolTable(NameMangler* mangler)
+ : mangler_(mangler),
+ delegate_(util::make_unique<DefaultSymbolTableDelegate>()),
+ cache_(200),
+ id_cache_(200) {
+}
+
+void SymbolTable::SetDelegate(std::unique_ptr<ISymbolTableDelegate> delegate) {
+ CHECK(delegate != nullptr) << "can't set a nullptr delegate";
+ delegate_ = std::move(delegate);
+
+ // Clear the cache in case this delegate changes the order of lookup.
+ cache_.clear();
+}
+
void SymbolTable::AppendSource(std::unique_ptr<ISymbolSource> source) {
sources_.push_back(std::move(source));
@@ -75,28 +90,27 @@
mangled_name = &mangled_name_impl.value();
}
- for (auto& symbolSource : sources_) {
- std::unique_ptr<Symbol> symbol = symbolSource->FindByName(*mangled_name);
- if (symbol) {
- // Take ownership of the symbol into a shared_ptr. We do this because
- // LruCache doesn't support unique_ptr.
- std::shared_ptr<Symbol> shared_symbol(std::move(symbol));
-
- // Since we look in the cache with the unmangled, but package prefixed
- // name, we must put the same name into the cache.
- cache_.put(*name_with_package, shared_symbol);
-
- if (shared_symbol->id) {
- // The symbol has an ID, so we can also cache this!
- id_cache_.put(shared_symbol->id.value(), shared_symbol);
- }
-
- // Returns the raw pointer. Callers are not expected to hold on to this
- // between calls to Find*.
- return shared_symbol.get();
- }
+ std::unique_ptr<Symbol> symbol = delegate_->FindByName(*mangled_name, sources_);
+ if (symbol == nullptr) {
+ return nullptr;
}
- return nullptr;
+
+ // Take ownership of the symbol into a shared_ptr. We do this because
+ // LruCache doesn't support unique_ptr.
+ std::shared_ptr<Symbol> shared_symbol(std::move(symbol));
+
+ // Since we look in the cache with the unmangled, but package prefixed
+ // name, we must put the same name into the cache.
+ cache_.put(*name_with_package, shared_symbol);
+
+ if (shared_symbol->id) {
+ // The symbol has an ID, so we can also cache this!
+ id_cache_.put(shared_symbol->id.value(), shared_symbol);
+ }
+
+ // Returns the raw pointer. Callers are not expected to hold on to this
+ // between calls to Find*.
+ return shared_symbol.get();
}
const SymbolTable::Symbol* SymbolTable::FindById(const ResourceId& id) {
@@ -105,20 +119,19 @@
}
// We did not find it in the cache, so look through the sources.
- for (auto& symbolSource : sources_) {
- std::unique_ptr<Symbol> symbol = symbolSource->FindById(id);
- if (symbol) {
- // Take ownership of the symbol into a shared_ptr. We do this because LruCache
- // doesn't support unique_ptr.
- std::shared_ptr<Symbol> shared_symbol(std::move(symbol));
- id_cache_.put(id, shared_symbol);
-
- // Returns the raw pointer. Callers are not expected to hold on to this
- // between calls to Find*.
- return shared_symbol.get();
- }
+ std::unique_ptr<Symbol> symbol = delegate_->FindById(id, sources_);
+ if (symbol == nullptr) {
+ return nullptr;
}
- return nullptr;
+
+ // Take ownership of the symbol into a shared_ptr. We do this because LruCache
+ // doesn't support unique_ptr.
+ std::shared_ptr<Symbol> shared_symbol(std::move(symbol));
+ id_cache_.put(id, shared_symbol);
+
+ // Returns the raw pointer. Callers are not expected to hold on to this
+ // between calls to Find*.
+ return shared_symbol.get();
}
const SymbolTable::Symbol* SymbolTable::FindByReference(const Reference& ref) {
@@ -140,6 +153,28 @@
return symbol;
}
+std::unique_ptr<SymbolTable::Symbol> DefaultSymbolTableDelegate::FindByName(
+ const ResourceName& name, const std::vector<std::unique_ptr<ISymbolSource>>& sources) {
+ for (auto& source : sources) {
+ std::unique_ptr<SymbolTable::Symbol> symbol = source->FindByName(name);
+ if (symbol) {
+ return symbol;
+ }
+ }
+ return {};
+}
+
+std::unique_ptr<SymbolTable::Symbol> DefaultSymbolTableDelegate::FindById(
+ ResourceId id, const std::vector<std::unique_ptr<ISymbolSource>>& sources) {
+ for (auto& source : sources) {
+ std::unique_ptr<SymbolTable::Symbol> symbol = source->FindById(id);
+ if (symbol) {
+ return symbol;
+ }
+ }
+ return {};
+}
+
std::unique_ptr<SymbolTable::Symbol> ResourceTableSymbolSource::FindByName(
const ResourceName& name) {
Maybe<ResourceTable::SearchResult> result = table_->FindResource(name);
diff --git a/tools/aapt2/process/SymbolTable.h b/tools/aapt2/process/SymbolTable.h
index 298da4d..bd252d2 100644
--- a/tools/aapt2/process/SymbolTable.h
+++ b/tools/aapt2/process/SymbolTable.h
@@ -47,6 +47,7 @@
}
class ISymbolSource;
+class ISymbolTableDelegate;
class NameMangler;
class SymbolTable {
@@ -73,7 +74,11 @@
bool is_public = false;
};
- SymbolTable(NameMangler* mangler) : mangler_(mangler), cache_(200), id_cache_(200) {}
+ SymbolTable(NameMangler* mangler);
+
+ // Overrides the default ISymbolTableDelegate, which allows a custom defined strategy for
+ // looking up resources from a set of sources.
+ void SetDelegate(std::unique_ptr<ISymbolTableDelegate> delegate);
// Appends a symbol source. The cache is not cleared since entries that
// have already been found would take precedence due to ordering.
@@ -99,6 +104,7 @@
private:
NameMangler* mangler_;
+ std::unique_ptr<ISymbolTableDelegate> delegate_;
std::vector<std::unique_ptr<ISymbolSource>> sources_;
// We use shared_ptr because unique_ptr is not supported and
@@ -109,11 +115,41 @@
DISALLOW_COPY_AND_ASSIGN(SymbolTable);
};
-/**
- * An interface that a symbol source implements in order to surface symbol
- * information
- * to the symbol table.
- */
+// Allows the customization of the lookup strategy/order of a symbol from a set of
+// symbol sources.
+class ISymbolTableDelegate {
+ public:
+ ISymbolTableDelegate() = default;
+ virtual ~ISymbolTableDelegate() = default;
+
+ // The name is already mangled and does not need further processing.
+ virtual std::unique_ptr<SymbolTable::Symbol> FindByName(
+ const ResourceName& name, const std::vector<std::unique_ptr<ISymbolSource>>& sources) = 0;
+
+ virtual std::unique_ptr<SymbolTable::Symbol> FindById(
+ ResourceId id, const std::vector<std::unique_ptr<ISymbolSource>>& sources) = 0;
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(ISymbolTableDelegate);
+};
+
+class DefaultSymbolTableDelegate : public ISymbolTableDelegate {
+ public:
+ DefaultSymbolTableDelegate() = default;
+ virtual ~DefaultSymbolTableDelegate() = default;
+
+ virtual std::unique_ptr<SymbolTable::Symbol> FindByName(
+ const ResourceName& name,
+ const std::vector<std::unique_ptr<ISymbolSource>>& sources) override;
+ virtual std::unique_ptr<SymbolTable::Symbol> FindById(
+ ResourceId id, const std::vector<std::unique_ptr<ISymbolSource>>& sources) override;
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(DefaultSymbolTableDelegate);
+};
+
+// An interface that a symbol source implements in order to surface symbol information
+// to the symbol table.
class ISymbolSource {
public:
virtual ~ISymbolSource() = default;
@@ -122,9 +158,7 @@
const ResourceName& name) = 0;
virtual std::unique_ptr<SymbolTable::Symbol> FindById(ResourceId id) = 0;
- /**
- * Default implementation tries the name if it exists, else the ID.
- */
+ // Default implementation tries the name if it exists, else the ID.
virtual std::unique_ptr<SymbolTable::Symbol> FindByReference(
const Reference& ref) {
if (ref.name) {
@@ -136,11 +170,9 @@
}
};
-/**
- * Exposes the resources in a ResourceTable as symbols for SymbolTable.
- * Instances of this class must outlive the encompassed ResourceTable.
- * Lookups by ID are ignored.
- */
+// Exposes the resources in a ResourceTable as symbols for SymbolTable.
+// Instances of this class must outlive the encompassed ResourceTable.
+// Lookups by ID are ignored.
class ResourceTableSymbolSource : public ISymbolSource {
public:
explicit ResourceTableSymbolSource(ResourceTable* table) : table_(table) {}
diff --git a/tools/aapt2/readme.md b/tools/aapt2/readme.md
index daf1ebc..0291720 100644
--- a/tools/aapt2/readme.md
+++ b/tools/aapt2/readme.md
@@ -1,5 +1,19 @@
# Android Asset Packaging Tool 2.0 (AAPT2) release notes
+## Version 2.14
+### `aapt2 link ...`
+- If an app is building with a minSdkVersion < 26 and a --package-id XX where XX > 7F, aapt2
+ will automatically convert any 'id' resource references from the resource ID 0xPPTTEEEE to
+ 0x7FPPEEEE.
+- This is done to workaround a bug in previous versions of the platform that would validate
+ a resource ID by assuming it is larger than 0. In Java, a resource ID with package ID greater
+ than 0x7F is interpreted as a negative number, causing valid feature split IDs like 0x80010000
+ to fail the check.
+- '@id/foo' resources are just sentinel values and do not actually need to resolve to anything.
+ Rewriting these resource IDs to use the package ID 7F while maintaining their definitions under
+ the original package ID is safe. Collisions against the base APK are checked to ensure these
+ rewritten IDs to not overlap with the base.
+
## Version 2.13
### `aapt2 optimize ...`
- aapt2 optimize can now split a binary APK with the same --split parameters as the link
