diff options
-rw-r--r-- | tools/aapt2/cmd/Compile.cpp | 5 | ||||
-rw-r--r-- | tools/aapt2/cmd/Diff.cpp | 5 | ||||
-rw-r--r-- | tools/aapt2/cmd/Dump.cpp | 5 | ||||
-rw-r--r-- | tools/aapt2/cmd/Link.cpp | 51 | ||||
-rw-r--r-- | tools/aapt2/cmd/Optimize.cpp | 10 | ||||
-rw-r--r-- | tools/aapt2/flatten/TableFlattener.cpp | 11 | ||||
-rw-r--r-- | tools/aapt2/flatten/TableFlattener_test.cpp | 36 | ||||
-rw-r--r-- | tools/aapt2/process/IResourceTableConsumer.h | 8 | ||||
-rw-r--r-- | tools/aapt2/test/Context.h | 46 |
9 files changed, 134 insertions, 43 deletions
diff --git a/tools/aapt2/cmd/Compile.cpp b/tools/aapt2/cmd/Compile.cpp index 578a8fb423dc..423e79069eac 100644 --- a/tools/aapt2/cmd/Compile.cpp +++ b/tools/aapt2/cmd/Compile.cpp @@ -582,6 +582,11 @@ static bool CompileFile(IAaptContext* context, const CompileOptions& options, 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 fdc89b2b24bd..1a6f348a3edc 100644 --- a/tools/aapt2/cmd/Diff.cpp +++ b/tools/aapt2/cmd/Diff.cpp @@ -31,6 +31,11 @@ class DiffContext : public IAaptContext { 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 1bbfb28a7870..57c457425e51 100644 --- a/tools/aapt2/cmd/Dump.cpp +++ b/tools/aapt2/cmd/Dump.cpp @@ -144,6 +144,11 @@ void TryDumpFile(IAaptContext* context, const std::string& file_path) { 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 b86188fa8503..258516db2ac9 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -65,16 +65,7 @@ using android::base::StringPrintf; 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 @@ class LinkContext : public IAaptContext { 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 @@ class LinkContext : public IAaptContext { private: DISALLOW_COPY_AND_ASSIGN(LinkContext); + PackageType package_type_ = PackageType::kApp; StdErrDiagnostics diagnostics_; NameMangler name_mangler_; std::string compilation_package_; @@ -627,7 +627,7 @@ class LinkCommand { 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 @@ class LinkCommand { */ 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 @@ class LinkCommand { return false; } - if (options_.package_type == PackageType::kStaticLib) { + if (context_->GetPackageType() == PackageType::kStaticLib) { if (!FlattenTableToPb(table, writer)) { return false; } @@ -1374,7 +1374,7 @@ class LinkCommand { 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 @@ class LinkCommand { 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 @@ class LinkCommand { 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 @@ class LinkCommand { } } - 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 @@ class LinkCommand { 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 @@ class LinkCommand { 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 @@ int Link(const std::vector<StringPiece>& args) { } 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 @@ int Link(const std::vector<StringPiece>& args) { } } - 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 @@ int Link(const std::vector<StringPiece>& args) { ".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 e99ee8aa93c2..78ed49b64dd2 100644 --- a/tools/aapt2/cmd/Optimize.cpp +++ b/tools/aapt2/cmd/Optimize.cpp @@ -59,6 +59,14 @@ struct OptimizeOptions { 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 @@ class OptimizeContext : public IAaptContext { } 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 3098458e4e4f..d44b3e095c0f 100644 --- a/tools/aapt2/flatten/TableFlattener.cpp +++ b/tools/aapt2/flatten/TableFlattener.cpp @@ -230,15 +230,18 @@ class PackageFlattener { 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 419618749a95..8dff3a27b79f 100644 --- a/tools/aapt2/flatten/TableFlattener_test.cpp +++ b/tools/aapt2/flatten/TableFlattener_test.cpp @@ -411,4 +411,40 @@ TEST_F(TableFlattenerTest, FlattenTableReferencingSharedLibraries) { 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 4526a79577d9..30dad8025900 100644 --- a/tools/aapt2/process/IResourceTableConsumer.h +++ b/tools/aapt2/process/IResourceTableConsumer.h @@ -32,9 +32,17 @@ namespace aapt { 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 557cd1b646e3..29d183876c4c 100644 --- a/tools/aapt2/test/Context.h +++ b/tools/aapt2/test/Context.h @@ -35,9 +35,17 @@ class Context : public IAaptContext { public: Context() : name_mangler_({}), symbols_(&name_mangler_), min_sdk_version_(0) {} - SymbolTable* GetExternalSymbols() override { return &symbols_; } + PackageType GetPackageType() override { + return package_type_; + } + + SymbolTable* GetExternalSymbols() override { + return &symbols_; + } - IDiagnostics* GetDiagnostics() override { return &diagnostics_; } + IDiagnostics* GetDiagnostics() override { + return &diagnostics_; + } const std::string& GetCompilationPackage() override { CHECK(bool(compilation_package_)) << "package name not set"; @@ -49,17 +57,24 @@ class Context : public IAaptContext { 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 Context : public IAaptContext { 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 @@ class StaticSymbolSourceBuilder { 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 @@ class StaticSymbolSourceBuilder { 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 @@ class StaticSymbolSourceBuilder { 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 |