Merge changes I7d2b7e50,I784406a6 into oc-dev

* changes:
  AAPT2: Allow truncating of package names
  AAPT2: Make BinaryResourceParser more lenient

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;
   }

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_;
   }

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_;
   }

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;

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;

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),
+    strcpy16_htod(pkg_header->name, arraysize(pkg_header->name), util::Utf8ToUtf16(package_->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.

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

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;

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_;
+  }
 
-  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 @@ 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(
+    std::unique_ptr<SymbolTable::Symbol> FindByName(const ResourceName& name) override {
-        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 =
+      std::unique_ptr<SymbolTable::Symbol> clone = util::make_unique<SymbolTable::Symbol>();
-          util::make_unique<SymbolTable::Symbol>();
       clone->id = sym->id;
       if (sym->attribute) {
-        clone->attribute =
+        clone->attribute = std::unique_ptr<Attribute>(sym->attribute->Clone(nullptr));
-            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_ =
+  std::unique_ptr<StaticSymbolSource> symbol_source_ = util::make_unique<StaticSymbolSource>();
-      util::make_unique<StaticSymbolSource>();
 };
 
 }  // namespace test

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 @@ namespace aapt {
 
 using namespace android;
 
+using android::base::StringPrintf;
+
 namespace {
 
 /*
@@ -87,26 +90,35 @@ BinaryResourceParser::BinaryResourceParser(IAaptContext* context, ResourceTable*
 bool BinaryResourceParser::Parse() {
   ResChunkPullParser parser(data_, data_len_);
 
-  bool error = false;
+  if (!ResChunkPullParser::IsGoodEvent(parser.Next())) {
-  while (ResChunkPullParser::IsGoodEvent(parser.Next())) {
+    context_->GetDiagnostics()->Error(DiagMessage(source_)
+                                      << "corrupt resources.arsc: " << parser.error());
+    return false;
+  }
+
   if (parser.chunk()->type != android::RES_TABLE_TYPE) {
-      context_->GetDiagnostics()->Warn(DiagMessage(source_)
+    context_->GetDiagnostics()->Error(DiagMessage(source_)
-                                       << "unknown chunk of type '"
+                                      << StringPrintf("unknown chunk of type 0x%02x",
-                                       << (int)parser.chunk()->type << "'");
+                                                      (int)parser.chunk()->type));
-      continue;
+    return false;
   }
 
   if (!ParseTable(parser.chunk())) {
-      error = true;
-    }
-  }
-
-  if (parser.event() == ResChunkPullParser::Event::kBadDocument) {
-    context_->GetDiagnostics()->Error(
-        DiagMessage(source_) << "corrupt resource table: " << parser.error());
     return false;
   }
-  return !error;
+
+  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;
 }
 
 /**

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 @@ ResChunkPullParser::Event ResChunkPullParser::Next() {
     return (event_ = Event::kBadDocument);
   }
 
-  if (util::DeviceToHost16(current_chunk_->headerSize) <
+  if (util::DeviceToHost16(current_chunk_->headerSize) < sizeof(ResChunk_header)) {
-      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);
   }