From 33af6c730f9f7fc51f04516c7a22cac82cb9823e Mon Sep 17 00:00:00 2001
From: Adam Lesinski <adamlesinski@google.com>
Date: Wed, 29 Mar 2017 13:00:35 -0700
Subject: [PATCH] AAPT2: Parse an ID encoded as a map

ID types should not be encoded as a map. AAPT and AAPT2 emit
IDs as boolean types.

Some apps exist that for some reason have their ID types encoded
as empty maps. This is the case only for the auto generated IDs
from enum values in <attr> tags.

Allow IDs as maps and ignore their content when processing an APK
for optimizing.

Also fixes an issue with expected size of the ResTable_package struct.

Bug: 35861796
Test: tested against the APK in b/35861796
Change-Id: I29a19cd9777bb10bed6766cd42e35e50e098797b
---
 libs/androidfw/LoadedArsc.cpp                  |  4 +++-
 tools/aapt2/Main.cpp                           |  2 +-
 tools/aapt2/readme.md                          |  8 ++++++++
 tools/aapt2/unflatten/BinaryResourceParser.cpp | 15 +++++++++++----
 4 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/libs/androidfw/LoadedArsc.cpp b/libs/androidfw/LoadedArsc.cpp
index 621d8c0e96f04..bd7b80469ddc6 100644
--- a/libs/androidfw/LoadedArsc.cpp
+++ b/libs/androidfw/LoadedArsc.cpp
@@ -416,7 +416,9 @@ std::unique_ptr<LoadedPackage> LoadedPackage::Load(const Chunk& chunk) {
   ATRACE_CALL();
   std::unique_ptr<LoadedPackage> loaded_package{new LoadedPackage()};
 
-  const ResTable_package* header = chunk.header<ResTable_package>();
+  constexpr size_t kMinPackageSize =
+      sizeof(ResTable_package) - sizeof(ResTable_package::typeIdOffset);
+  const ResTable_package* header = chunk.header<ResTable_package, kMinPackageSize>();
   if (header == nullptr) {
     LOG(ERROR) << "Chunk RES_TABLE_PACKAGE_TYPE is too small.";
     return {};
diff --git a/tools/aapt2/Main.cpp b/tools/aapt2/Main.cpp
index 5e9b81a29af9b..36c1de6a4f26c 100644
--- a/tools/aapt2/Main.cpp
+++ b/tools/aapt2/Main.cpp
@@ -25,7 +25,7 @@ namespace aapt {
 static const char* sMajorVersion = "2";
 
 // Update minor version whenever a feature or flag is added.
-static const char* sMinorVersion = "11";
+static const char* sMinorVersion = "12";
 
 int PrintVersion() {
   std::cerr << "Android Asset Packaging Tool (aapt) " << sMajorVersion << "."
diff --git a/tools/aapt2/readme.md b/tools/aapt2/readme.md
index 9899f803b339e..2e674bddce7e9 100644
--- a/tools/aapt2/readme.md
+++ b/tools/aapt2/readme.md
@@ -1,5 +1,13 @@
 # Android Asset Packaging Tool 2.0 (AAPT2) release notes
 
+## Version 2.12
+### `aapt2 optimize ...`
+- aapt2 optimize now understands map (complex) values under the type `id`. It ignores their
+  contents and interprets them as a sentinel `id` type. This was added to support existing
+  apps that build with their `id` types as map values.
+  AAPT and AAPT2 always generate a simple value for the type `ID`, so it is unclear how some
+  these apps are encoded.
+
 ## Version 2.11
 ### `aapt2 link ...`
 - Adds the ability to specify assets directories with the -A parameter. Assets work just like
diff --git a/tools/aapt2/unflatten/BinaryResourceParser.cpp b/tools/aapt2/unflatten/BinaryResourceParser.cpp
index 66bcfa034dfb0..35bf618c1635e 100644
--- a/tools/aapt2/unflatten/BinaryResourceParser.cpp
+++ b/tools/aapt2/unflatten/BinaryResourceParser.cpp
@@ -168,10 +168,11 @@ bool BinaryResourceParser::ParseTable(const ResChunk_header* chunk) {
 }
 
 bool BinaryResourceParser::ParsePackage(const ResChunk_header* chunk) {
-  const ResTable_package* package_header = ConvertTo<ResTable_package>(chunk);
+  constexpr size_t kMinPackageSize =
+      sizeof(ResTable_package) - sizeof(ResTable_package::typeIdOffset);
+  const ResTable_package* package_header = ConvertTo<ResTable_package, kMinPackageSize>(chunk);
   if (!package_header) {
-    context_->GetDiagnostics()->Error(DiagMessage(source_)
-                                      << "corrupt ResTable_package chunk");
+    context_->GetDiagnostics()->Error(DiagMessage(source_) << "corrupt ResTable_package chunk");
     return false;
   }
 
@@ -498,8 +499,14 @@ std::unique_ptr<Value> BinaryResourceParser::ParseMapEntry(
       return ParseArray(name, config, map);
     case ResourceType::kPlurals:
       return ParsePlural(name, config, map);
+    case ResourceType::kId:
+      // Special case: An ID is not a bag, but some apps have defined the auto-generated
+      // IDs that come from declaring an enum value in an attribute as an empty map...
+      // We can ignore the value here.
+      return util::make_unique<Id>();
     default:
-      LOG(FATAL) << "unknown map type";
+      context_->GetDiagnostics()->Error(DiagMessage() << "illegal map type '" << ToString(name.type)
+                                                      << "' (" << (int)name.type << ")");
       break;
   }
   return {};