AAPT2: Make BinaryResourceParser more lenient

Trailing data after the main RES_TABLE_TYPE should be logged but not
cause an error.

Bug: 36945869
Test: (aapt2 dump apk/attached/in/bug.apk)
Change-Id: I784406a680b79630798fdb4b7ca81f9d1f2b96d1

2 files changed, 39 insertions, 19 deletions

diff --git a/tools/aapt2/unflatten/BinaryResourceParser.cpp b/tools/aapt2/unflatten/BinaryResourceParser.cpp
index 20a45312993d..42786b5387cf 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 @@ 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;
-  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 (!ResChunkPullParser::IsGoodEvent(parser.Next())) {
+    context_->GetDiagnostics()->Error(DiagMessage(source_)
+                                      << "corrupt resources.arsc: " << parser.error());
+    return false;
+  }
 
-    if (!ParseTable(parser.chunk())) {
-      error = true;
-    }
+  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 (parser.event() == ResChunkPullParser::Event::kBadDocument) {
-    context_->GetDiagnostics()->Error(
-        DiagMessage(source_) << "corrupt resource table: " << parser.error());
+  if (!ParseTable(parser.chunk())) {
     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;
 }
 
 /**
diff --git a/tools/aapt2/unflatten/ResChunkPullParser.cpp b/tools/aapt2/unflatten/ResChunkPullParser.cpp
index 5d71ff315874..8d92bd94b0ae 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 @@ ResChunkPullParser::Event ResChunkPullParser::Next() {
     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);
   }