Merge changes I7d2b7e50,I784406a6 into oc-dev
* changes:
AAPT2: Allow truncating of package names
AAPT2: Make BinaryResourceParser more lenient
diff --git a/tools/aapt2/cmd/Compile.cpp b/tools/aapt2/cmd/Compile.cpp
index 578a8fb..423e790 100644
--- a/tools/aapt2/cmd/Compile.cpp
+++ b/tools/aapt2/cmd/Compile.cpp
@@ -582,6 +582,11 @@
class CompileContext : public IAaptContext {
public:
+ PackageType GetPackageType() override {
+ // Every compilation unit starts as an app and then gets linked as potentially something else.
+ return PackageType::kApp;
+ }
+
void SetVerbose(bool val) {
verbose_ = val;
}
diff --git a/tools/aapt2/cmd/Diff.cpp b/tools/aapt2/cmd/Diff.cpp
index fdc89b2..1a6f348 100644
--- a/tools/aapt2/cmd/Diff.cpp
+++ b/tools/aapt2/cmd/Diff.cpp
@@ -31,6 +31,11 @@
DiffContext() : name_mangler_({}), symbol_table_(&name_mangler_) {
}
+ PackageType GetPackageType() override {
+ // Doesn't matter.
+ return PackageType::kApp;
+ }
+
const std::string& GetCompilationPackage() override {
return empty_;
}
diff --git a/tools/aapt2/cmd/Dump.cpp b/tools/aapt2/cmd/Dump.cpp
index 1bbfb28..57c4574 100644
--- a/tools/aapt2/cmd/Dump.cpp
+++ b/tools/aapt2/cmd/Dump.cpp
@@ -144,6 +144,11 @@
class DumpContext : public IAaptContext {
public:
+ PackageType GetPackageType() override {
+ // Doesn't matter.
+ return PackageType::kApp;
+ }
+
IDiagnostics* GetDiagnostics() override {
return &diagnostics_;
}
diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index b86188f..258516d 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -65,16 +65,7 @@
namespace aapt {
-// The type of package to build.
-enum class PackageType {
- kApp,
- kSharedLib,
- kStaticLib,
-};
-
struct LinkOptions {
- PackageType package_type = PackageType::kApp;
-
std::string output_path;
std::string manifest_path;
std::vector<std::string> include_paths;
@@ -130,6 +121,14 @@
LinkContext() : name_mangler_({}), symbols_(&name_mangler_) {
}
+ PackageType GetPackageType() override {
+ return package_type_;
+ }
+
+ void SetPackageType(PackageType type) {
+ package_type_ = type;
+ }
+
IDiagnostics* GetDiagnostics() override {
return &diagnostics_;
}
@@ -181,6 +180,7 @@
private:
DISALLOW_COPY_AND_ASSIGN(LinkContext);
+ PackageType package_type_ = PackageType::kApp;
StdErrDiagnostics diagnostics_;
NameMangler name_mangler_;
std::string compilation_package_;
@@ -627,7 +627,7 @@
std::string error_str;
std::unique_ptr<ResourceTable> include_static = LoadStaticLibrary(path, &error_str);
if (include_static) {
- if (options_.package_type != PackageType::kStaticLib) {
+ if (context_->GetPackageType() != PackageType::kStaticLib) {
// Can't include static libraries when not building a static library (they have no IDs
// assigned).
context_->GetDiagnostics()->Error(
@@ -1300,7 +1300,7 @@
*/
bool WriteApk(IArchiveWriter* writer, proguard::KeepSet* keep_set, xml::XmlResource* manifest,
ResourceTable* table) {
- const bool keep_raw_values = options_.package_type == PackageType::kStaticLib;
+ const bool keep_raw_values = context_->GetPackageType() == PackageType::kStaticLib;
bool result =
FlattenXml(manifest, "AndroidManifest.xml", {}, keep_raw_values, writer, context_);
if (!result) {
@@ -1325,7 +1325,7 @@
return false;
}
- if (options_.package_type == PackageType::kStaticLib) {
+ if (context_->GetPackageType() == PackageType::kStaticLib) {
if (!FlattenTableToPb(table, writer)) {
return false;
}
@@ -1374,7 +1374,7 @@
context_->SetPackageId(0x01);
// Verify we're building a regular app.
- if (options_.package_type != PackageType::kApp) {
+ if (context_->GetPackageType() != PackageType::kApp) {
context_->GetDiagnostics()->Error(
DiagMessage() << "package 'android' can only be built as a regular app");
return 1;
@@ -1414,7 +1414,7 @@
return 1;
}
- if (options_.package_type != PackageType::kStaticLib) {
+ if (context_->GetPackageType() != PackageType::kStaticLib) {
PrivateAttributeMover mover;
if (!mover.Consume(context_, &final_table_)) {
context_->GetDiagnostics()->Error(DiagMessage() << "failed moving private attributes");
@@ -1469,7 +1469,7 @@
return 1;
}
- if (options_.package_type == PackageType::kStaticLib) {
+ if (context_->GetPackageType() == PackageType::kStaticLib) {
if (!options_.products.empty()) {
context_->GetDiagnostics()->Warn(DiagMessage()
<< "can't select products when building static library");
@@ -1490,7 +1490,7 @@
}
}
- if (options_.package_type != PackageType::kStaticLib && context_->GetMinSdkVersion() > 0) {
+ if (context_->GetPackageType() != PackageType::kStaticLib && context_->GetMinSdkVersion() > 0) {
if (context_->IsVerbose()) {
context_->GetDiagnostics()->Note(DiagMessage()
<< "collapsing resource versions for minimum SDK "
@@ -1514,7 +1514,7 @@
proguard::KeepSet proguard_keep_set;
proguard::KeepSet proguard_main_dex_keep_set;
- if (options_.package_type == PackageType::kStaticLib) {
+ if (context_->GetPackageType() == PackageType::kStaticLib) {
if (options_.table_splitter_options.config_filter != nullptr ||
!options_.table_splitter_options.preferred_densities.empty()) {
context_->GetDiagnostics()->Warn(DiagMessage()
@@ -1641,11 +1641,12 @@
template_options.types = JavaClassGeneratorOptions::SymbolTypes::kAll;
template_options.javadoc_annotations = options_.javadoc_annotations;
- if (options_.package_type == PackageType::kStaticLib || options_.generate_non_final_ids) {
+ if (context_->GetPackageType() == PackageType::kStaticLib ||
+ options_.generate_non_final_ids) {
template_options.use_final = false;
}
- if (options_.package_type == PackageType::kSharedLib) {
+ if (context_->GetPackageType() == PackageType::kSharedLib) {
template_options.use_final = false;
template_options.rewrite_callback_options = OnResourcesLoadedCallbackOptions{};
}
@@ -1922,18 +1923,18 @@
}
if (shared_lib) {
- options.package_type = PackageType::kSharedLib;
+ context.SetPackageType(PackageType::kSharedLib);
context.SetPackageId(0x00);
} else if (static_lib) {
- options.package_type = PackageType::kStaticLib;
+ context.SetPackageType(PackageType::kStaticLib);
context.SetPackageId(kAppPackageId);
} else {
- options.package_type = PackageType::kApp;
+ context.SetPackageType(PackageType::kApp);
context.SetPackageId(kAppPackageId);
}
if (package_id) {
- if (options.package_type != PackageType::kApp) {
+ if (context.GetPackageType() != PackageType::kApp) {
context.GetDiagnostics()->Error(
DiagMessage() << "can't specify --package-id when not building a regular app");
return 1;
@@ -2000,7 +2001,7 @@
}
}
- if (options.package_type != PackageType::kStaticLib && stable_id_file_path) {
+ if (context.GetPackageType() != PackageType::kStaticLib && stable_id_file_path) {
if (!LoadStableIdMap(context.GetDiagnostics(), stable_id_file_path.value(),
&options.stable_id_map)) {
return 1;
@@ -2015,7 +2016,7 @@
".3gpp2", ".amr", ".awb", ".wma", ".wmv", ".webm", ".mkv"});
// Turn off auto versioning for static-libs.
- if (options.package_type == PackageType::kStaticLib) {
+ if (context.GetPackageType() == PackageType::kStaticLib) {
options.no_auto_version = true;
options.no_version_vectors = true;
options.no_version_transitions = true;
diff --git a/tools/aapt2/cmd/Optimize.cpp b/tools/aapt2/cmd/Optimize.cpp
index e99ee8a..78ed49b 100644
--- a/tools/aapt2/cmd/Optimize.cpp
+++ b/tools/aapt2/cmd/Optimize.cpp
@@ -59,6 +59,14 @@
class OptimizeContext : public IAaptContext {
public:
+ OptimizeContext() = default;
+
+ PackageType GetPackageType() override {
+ // Not important here. Using anything other than kApp adds EXTRA validation, which we want to
+ // avoid.
+ return PackageType::kApp;
+ }
+
IDiagnostics* GetDiagnostics() override {
return &diagnostics_;
}
@@ -99,6 +107,8 @@
}
private:
+ DISALLOW_COPY_AND_ASSIGN(OptimizeContext);
+
StdErrDiagnostics diagnostics_;
bool verbose_ = false;
int sdk_version_ = 0;
diff --git a/tools/aapt2/flatten/TableFlattener.cpp b/tools/aapt2/flatten/TableFlattener.cpp
index 3098458..d44b3e0 100644
--- a/tools/aapt2/flatten/TableFlattener.cpp
+++ b/tools/aapt2/flatten/TableFlattener.cpp
@@ -230,15 +230,18 @@
ResTable_package* pkg_header = pkg_writer.StartChunk<ResTable_package>(RES_TABLE_PACKAGE_TYPE);
pkg_header->id = util::HostToDevice32(package_->id.value());
- if (package_->name.size() >= arraysize(pkg_header->name)) {
+ // AAPT truncated the package name, so do the same.
+ // Shared libraries require full package names, so don't truncate theirs.
+ if (context_->GetPackageType() != PackageType::kApp &&
+ package_->name.size() >= arraysize(pkg_header->name)) {
diag_->Error(DiagMessage() << "package name '" << package_->name
- << "' is too long");
+ << "' is too long. "
+ "Shared libraries cannot have truncated package names");
return false;
}
// Copy the package name in device endianness.
- strcpy16_htod(pkg_header->name, arraysize(pkg_header->name),
- util::Utf8ToUtf16(package_->name));
+ strcpy16_htod(pkg_header->name, arraysize(pkg_header->name), util::Utf8ToUtf16(package_->name));
// Serialize the types. We do this now so that our type and key strings
// are populated. We write those first.
diff --git a/tools/aapt2/flatten/TableFlattener_test.cpp b/tools/aapt2/flatten/TableFlattener_test.cpp
index 4196187..8dff3a2 100644
--- a/tools/aapt2/flatten/TableFlattener_test.cpp
+++ b/tools/aapt2/flatten/TableFlattener_test.cpp
@@ -411,4 +411,40 @@
EXPECT_EQ(0x03u, entries.valueAt(idx));
}
+TEST_F(TableFlattenerTest, LongPackageNameIsTruncated) {
+ std::string kPackageName(256, 'F');
+
+ std::unique_ptr<IAaptContext> context =
+ test::ContextBuilder().SetCompilationPackage(kPackageName).SetPackageId(0x7f).Build();
+ std::unique_ptr<ResourceTable> table =
+ test::ResourceTableBuilder()
+ .SetPackageId(kPackageName, 0x7f)
+ .AddSimple(kPackageName + ":id/foo", ResourceId(0x7f010000))
+ .Build();
+
+ ResTable result;
+ ASSERT_TRUE(Flatten(context.get(), {}, table.get(), &result));
+
+ ASSERT_EQ(1u, result.getBasePackageCount());
+ EXPECT_EQ(127u, result.getBasePackageName(0).size());
+}
+
+TEST_F(TableFlattenerTest, LongSharedLibraryPackageNameIsIllegal) {
+ std::string kPackageName(256, 'F');
+
+ std::unique_ptr<IAaptContext> context = test::ContextBuilder()
+ .SetCompilationPackage(kPackageName)
+ .SetPackageId(0x7f)
+ .SetPackageType(PackageType::kSharedLib)
+ .Build();
+ std::unique_ptr<ResourceTable> table =
+ test::ResourceTableBuilder()
+ .SetPackageId(kPackageName, 0x7f)
+ .AddSimple(kPackageName + ":id/foo", ResourceId(0x7f010000))
+ .Build();
+
+ ResTable result;
+ ASSERT_FALSE(Flatten(context.get(), {}, table.get(), &result));
+}
+
} // namespace aapt
diff --git a/tools/aapt2/process/IResourceTableConsumer.h b/tools/aapt2/process/IResourceTableConsumer.h
index 4526a79..30dad802 100644
--- a/tools/aapt2/process/IResourceTableConsumer.h
+++ b/tools/aapt2/process/IResourceTableConsumer.h
@@ -32,9 +32,17 @@
class ResourceTable;
class SymbolTable;
+// The type of package to build.
+enum class PackageType {
+ kApp,
+ kSharedLib,
+ kStaticLib,
+};
+
struct IAaptContext {
virtual ~IAaptContext() = default;
+ virtual PackageType GetPackageType() = 0;
virtual SymbolTable* GetExternalSymbols() = 0;
virtual IDiagnostics* GetDiagnostics() = 0;
virtual const std::string& GetCompilationPackage() = 0;
diff --git a/tools/aapt2/test/Context.h b/tools/aapt2/test/Context.h
index 557cd1b..29d1838 100644
--- a/tools/aapt2/test/Context.h
+++ b/tools/aapt2/test/Context.h
@@ -35,9 +35,17 @@
public:
Context() : name_mangler_({}), symbols_(&name_mangler_), min_sdk_version_(0) {}
- SymbolTable* GetExternalSymbols() override { return &symbols_; }
+ PackageType GetPackageType() override {
+ return package_type_;
+ }
- IDiagnostics* GetDiagnostics() override { return &diagnostics_; }
+ SymbolTable* GetExternalSymbols() override {
+ return &symbols_;
+ }
+
+ IDiagnostics* GetDiagnostics() override {
+ return &diagnostics_;
+ }
const std::string& GetCompilationPackage() override {
CHECK(bool(compilation_package_)) << "package name not set";
@@ -49,17 +57,24 @@
return package_id_.value();
}
- NameMangler* GetNameMangler() override { return &name_mangler_; }
+ NameMangler* GetNameMangler() override {
+ return &name_mangler_;
+ }
- bool IsVerbose() override { return false; }
+ bool IsVerbose() override {
+ return false;
+ }
- int GetMinSdkVersion() override { return min_sdk_version_; }
+ int GetMinSdkVersion() override {
+ return min_sdk_version_;
+ }
private:
DISALLOW_COPY_AND_ASSIGN(Context);
friend class ContextBuilder;
+ PackageType package_type_ = PackageType::kApp;
Maybe<std::string> compilation_package_;
Maybe<uint8_t> package_id_;
StdErrDiagnostics diagnostics_;
@@ -70,6 +85,11 @@
class ContextBuilder {
public:
+ ContextBuilder& SetPackageType(PackageType type) {
+ context_->package_type_ = type;
+ return *this;
+ }
+
ContextBuilder& SetCompilationPackage(const android::StringPiece& package) {
context_->compilation_package_ = package.to_string();
return *this;
@@ -123,15 +143,16 @@
return *this;
}
- std::unique_ptr<ISymbolSource> Build() { return std::move(symbol_source_); }
+ std::unique_ptr<ISymbolSource> Build() {
+ return std::move(symbol_source_);
+ }
private:
class StaticSymbolSource : public ISymbolSource {
public:
StaticSymbolSource() = default;
- std::unique_ptr<SymbolTable::Symbol> FindByName(
- const ResourceName& name) override {
+ std::unique_ptr<SymbolTable::Symbol> FindByName(const ResourceName& name) override {
auto iter = name_map_.find(name);
if (iter != name_map_.end()) {
return CloneSymbol(iter->second);
@@ -153,12 +174,10 @@
private:
std::unique_ptr<SymbolTable::Symbol> CloneSymbol(SymbolTable::Symbol* sym) {
- std::unique_ptr<SymbolTable::Symbol> clone =
- util::make_unique<SymbolTable::Symbol>();
+ std::unique_ptr<SymbolTable::Symbol> clone = util::make_unique<SymbolTable::Symbol>();
clone->id = sym->id;
if (sym->attribute) {
- clone->attribute =
- std::unique_ptr<Attribute>(sym->attribute->Clone(nullptr));
+ clone->attribute = std::unique_ptr<Attribute>(sym->attribute->Clone(nullptr));
}
clone->is_public = sym->is_public;
return clone;
@@ -167,8 +186,7 @@
DISALLOW_COPY_AND_ASSIGN(StaticSymbolSource);
};
- std::unique_ptr<StaticSymbolSource> symbol_source_ =
- util::make_unique<StaticSymbolSource>();
+ std::unique_ptr<StaticSymbolSource> symbol_source_ = util::make_unique<StaticSymbolSource>();
};
} // namespace test
diff --git a/tools/aapt2/unflatten/BinaryResourceParser.cpp b/tools/aapt2/unflatten/BinaryResourceParser.cpp
index 20a4531..42786b5 100644
--- a/tools/aapt2/unflatten/BinaryResourceParser.cpp
+++ b/tools/aapt2/unflatten/BinaryResourceParser.cpp
@@ -22,6 +22,7 @@
#include "android-base/logging.h"
#include "android-base/macros.h"
+#include "android-base/stringprintf.h"
#include "androidfw/ResourceTypes.h"
#include "androidfw/TypeWrappers.h"
@@ -37,6 +38,8 @@
using namespace android;
+using android::base::StringPrintf;
+
namespace {
/*
@@ -87,26 +90,35 @@
bool BinaryResourceParser::Parse() {
ResChunkPullParser parser(data_, data_len_);
- bool error = false;
- while (ResChunkPullParser::IsGoodEvent(parser.Next())) {
- if (parser.chunk()->type != android::RES_TABLE_TYPE) {
- context_->GetDiagnostics()->Warn(DiagMessage(source_)
- << "unknown chunk of type '"
- << (int)parser.chunk()->type << "'");
- continue;
- }
-
- if (!ParseTable(parser.chunk())) {
- error = true;
- }
- }
-
- if (parser.event() == ResChunkPullParser::Event::kBadDocument) {
- context_->GetDiagnostics()->Error(
- DiagMessage(source_) << "corrupt resource table: " << parser.error());
+ if (!ResChunkPullParser::IsGoodEvent(parser.Next())) {
+ context_->GetDiagnostics()->Error(DiagMessage(source_)
+ << "corrupt resources.arsc: " << parser.error());
return false;
}
- return !error;
+
+ if (parser.chunk()->type != android::RES_TABLE_TYPE) {
+ context_->GetDiagnostics()->Error(DiagMessage(source_)
+ << StringPrintf("unknown chunk of type 0x%02x",
+ (int)parser.chunk()->type));
+ return false;
+ }
+
+ if (!ParseTable(parser.chunk())) {
+ return false;
+ }
+
+ if (parser.Next() != ResChunkPullParser::Event::kEndDocument) {
+ if (parser.event() == ResChunkPullParser::Event::kBadDocument) {
+ context_->GetDiagnostics()->Warn(
+ DiagMessage(source_) << "invalid chunk trailing RES_TABLE_TYPE: " << parser.error());
+ } else {
+ context_->GetDiagnostics()->Warn(
+ DiagMessage(source_) << StringPrintf(
+ "unexpected chunk of type 0x%02x trailing RES_TABLE_TYPE",
+ (int)parser.chunk()->type));
+ }
+ }
+ return true;
}
/**
diff --git a/tools/aapt2/unflatten/ResChunkPullParser.cpp b/tools/aapt2/unflatten/ResChunkPullParser.cpp
index 5d71ff3..8d92bd9 100644
--- a/tools/aapt2/unflatten/ResChunkPullParser.cpp
+++ b/tools/aapt2/unflatten/ResChunkPullParser.cpp
@@ -16,9 +16,11 @@
#include "unflatten/ResChunkPullParser.h"
+#include <inttypes.h>
#include <cstddef>
#include "android-base/logging.h"
+#include "android-base/stringprintf.h"
#include "androidfw/ResourceTypes.h"
#include "util/Util.h"
@@ -26,6 +28,13 @@
namespace aapt {
using android::ResChunk_header;
+using android::base::StringPrintf;
+
+static std::string ChunkHeaderDump(const ResChunk_header* header) {
+ return StringPrintf("(type=%02" PRIx16 " header_size=%" PRIu16 " size=%" PRIu32 ")",
+ util::DeviceToHost16(header->type), util::DeviceToHost16(header->headerSize),
+ util::DeviceToHost32(header->size));
+}
ResChunkPullParser::Event ResChunkPullParser::Next() {
if (!IsGoodEvent(event_)) {
@@ -53,18 +62,17 @@
return (event_ = Event::kBadDocument);
}
- if (util::DeviceToHost16(current_chunk_->headerSize) <
- sizeof(ResChunk_header)) {
+ if (util::DeviceToHost16(current_chunk_->headerSize) < sizeof(ResChunk_header)) {
error_ = "chunk has too small header";
current_chunk_ = nullptr;
return (event_ = Event::kBadDocument);
} else if (util::DeviceToHost32(current_chunk_->size) <
util::DeviceToHost16(current_chunk_->headerSize)) {
- error_ = "chunk's total size is smaller than header";
+ error_ = "chunk's total size is smaller than header " + ChunkHeaderDump(current_chunk_);
current_chunk_ = nullptr;
return (event_ = Event::kBadDocument);
} else if (offset + util::DeviceToHost32(current_chunk_->size) > len_) {
- error_ = "chunk's data extends past the end of the document";
+ error_ = "chunk's data extends past the end of the document " + ChunkHeaderDump(current_chunk_);
current_chunk_ = nullptr;
return (event_ = Event::kBadDocument);
}