Revert^2 "libandroidfw hardening for IncFs"

55ef6167a2c235bd88c7216238b2001b46795b79

Change-Id: I02d4890d181655dfd0a14c188468db512559d27b
diff --git a/tools/aapt2/Debug.cpp b/tools/aapt2/Debug.cpp
index 439f231..82da249 100644
--- a/tools/aapt2/Debug.cpp
+++ b/tools/aapt2/Debug.cpp
@@ -436,9 +436,9 @@
   for (size_t i=0; i<N; i++) {
     size_t len;
     if (pool->isUTF8()) {
-      uniqueStrings.add(pool->string8At(i, &len));
+      uniqueStrings.add(UnpackOptionalString(pool->string8At(i), &len));
     } else {
-      uniqueStrings.add(pool->stringAt(i, &len));
+      uniqueStrings.add(UnpackOptionalString(pool->stringAt(i), &len));
     }
   }
 
@@ -450,8 +450,8 @@
 
   const size_t NS = pool->size();
   for (size_t s=0; s<NS; s++) {
-    String8 str = pool->string8ObjectAt(s);
-    printer->Print(StringPrintf("String #%zd : %s\n", s, str.string()));
+    auto str = pool->string8ObjectAt(s);
+    printer->Print(StringPrintf("String #%zd : %s\n", s, str.has_value() ? str->string() : ""));
   }
 }
 
diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp
index 7dfc983..5b43df6 100644
--- a/tools/aapt2/ResourceUtils.cpp
+++ b/tools/aapt2/ResourceUtils.cpp
@@ -751,10 +751,12 @@
   switch (res_value.dataType) {
     case android::Res_value::TYPE_STRING: {
       const std::string str = util::GetString(src_pool, data);
-      const android::ResStringPool_span* spans = src_pool.styleAt(data);
+      auto spans_result = src_pool.styleAt(data);
 
       // Check if the string has a valid style associated with it.
-      if (spans != nullptr && spans->name.index != android::ResStringPool_span::END) {
+      if (spans_result.has_value() &&
+          (*spans_result)->name.index != android::ResStringPool_span::END) {
+        const android::ResStringPool_span* spans = spans_result->unsafe_ptr();
         StyleString style_str = {str};
         while (spans->name.index != android::ResStringPool_span::END) {
           style_str.spans.push_back(Span{util::GetString(src_pool, spans->name.index),
diff --git a/tools/aapt2/StringPool_test.cpp b/tools/aapt2/StringPool_test.cpp
index 9a7238b..6e5200b 100644
--- a/tools/aapt2/StringPool_test.cpp
+++ b/tools/aapt2/StringPool_test.cpp
@@ -223,11 +223,11 @@
   std::unique_ptr<uint8_t[]> data = util::Copy(buffer);
   ResStringPool test;
   ASSERT_EQ(test.setTo(data.get(), buffer.size()), NO_ERROR);
-  size_t len = 0;
-  const char16_t* str = test.stringAt(0, &len);
-  EXPECT_THAT(len, Eq(1u));
-  EXPECT_THAT(str, Pointee(Eq(u'\u093f')));
-  EXPECT_THAT(str[1], Eq(0u));
+  auto str = test.stringAt(0);
+  ASSERT_TRUE(str.has_value());
+  EXPECT_THAT(str->size(), Eq(1u));
+  EXPECT_THAT(str->data(), Pointee(Eq(u'\u093f')));
+  EXPECT_THAT(str->data()[1], Eq(0u));
 }
 
 constexpr const char* sLongString =
@@ -278,14 +278,15 @@
     EXPECT_THAT(util::GetString(test, 3), Eq(sLongString));
     EXPECT_THAT(util::GetString16(test, 3), Eq(util::Utf8ToUtf16(sLongString)));
 
-    size_t len;
-    EXPECT_TRUE(test.stringAt(4, &len) != nullptr || test.string8At(4, &len) != nullptr);
+    EXPECT_TRUE(test.stringAt(4).has_value() || test.string8At(4).has_value());
 
     EXPECT_THAT(util::GetString(test, 0), Eq("style"));
     EXPECT_THAT(util::GetString16(test, 0), Eq(u"style"));
 
-    const ResStringPool_span* span = test.styleAt(0);
-    ASSERT_THAT(span, NotNull());
+    auto span_result = test.styleAt(0);
+    ASSERT_TRUE(span_result.has_value());
+
+    const ResStringPool_span* span = span_result->unsafe_ptr();
     EXPECT_THAT(util::GetString(test, span->name.index), Eq("b"));
     EXPECT_THAT(util::GetString16(test, span->name.index), Eq(u"b"));
     EXPECT_THAT(span->firstChar, Eq(0u));
@@ -318,16 +319,17 @@
   // Check that the codepoints are encoded using two three-byte surrogate pairs
   ResStringPool test;
   ASSERT_EQ(test.setTo(data.get(), buffer.size()), NO_ERROR);
-  size_t len;
-  const char* str = test.string8At(0, &len);
-  ASSERT_THAT(str, NotNull());
-  EXPECT_THAT(std::string(str, len), Eq("\xED\xA0\x81\xED\xB0\x80"));
-  str = test.string8At(1, &len);
-  ASSERT_THAT(str, NotNull());
-  EXPECT_THAT(std::string(str, len), Eq("foo \xED\xA0\x81\xED\xB0\xB7 bar"));
-  str = test.string8At(2, &len);
-  ASSERT_THAT(str, NotNull());
-  EXPECT_THAT(std::string(str, len), Eq("\xED\xA0\x81\xED\xB0\x80\xED\xA0\x81\xED\xB0\xB7"));
+  auto str = test.string8At(0);
+  ASSERT_TRUE(str.has_value());
+  EXPECT_THAT(str->to_string(), Eq("\xED\xA0\x81\xED\xB0\x80"));
+
+  str = test.string8At(1);
+  ASSERT_TRUE(str.has_value());
+  EXPECT_THAT(str->to_string(), Eq("foo \xED\xA0\x81\xED\xB0\xB7 bar"));
+
+  str = test.string8At(2);
+  ASSERT_TRUE(str.has_value());
+  EXPECT_THAT(str->to_string(), Eq("\xED\xA0\x81\xED\xB0\x80\xED\xA0\x81\xED\xB0\xB7"));
 
   // Check that retrieving the strings returns the original UTF-8 character bytes
   EXPECT_THAT(util::GetString(test, 0), Eq("\xF0\x90\x90\x80"));
diff --git a/tools/aapt2/cmd/Compile_test.cpp b/tools/aapt2/cmd/Compile_test.cpp
index 0aab94d3..8cbd998 100644
--- a/tools/aapt2/cmd/Compile_test.cpp
+++ b/tools/aapt2/cmd/Compile_test.cpp
@@ -314,8 +314,10 @@
   ASSERT_NE(content_values.find(relative_path_values_colors), -1);
   ASSERT_EQ(content_values.find(path_values_colors), -1);
 
-  Link({"-o", apk_path, "--manifest", GetDefaultManifest(), "--proto-format"},
-      compiled_files_dir,  &diag);
+  ASSERT_TRUE(Link({"-o", apk_path,
+                    "--manifest", GetDefaultManifest(),
+                    "--proto-format"},
+                    compiled_files_dir, &diag));
 
   std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(apk_path, &diag);
   ResourceTable* resource_table = apk.get()->GetResourceTable();
diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index 118a76f..f92c602 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -732,22 +732,6 @@
   return true;
 }
 
-static android::ApkAssetsCookie FindFrameworkAssetManagerCookie(
-    const android::AssetManager2& assets) {
-  using namespace android;
-
-  // Find the system package (0x01). AAPT always generates attributes with the type 0x01, so
-  // we're looking for the first attribute resource in the system package.
-  Res_value val{};
-  ResTable_config config{};
-  uint32_t type_spec_flags;
-  ApkAssetsCookie idx = assets.GetResource(0x01010000, true /** may_be_bag */,
-                                           0 /** density_override */, &val, &config,
-                                           &type_spec_flags);
-
-  return idx;
-}
-
 class Linker {
  public:
   Linker(LinkContext* context, const LinkOptions& options)
@@ -760,8 +744,12 @@
   void ExtractCompileSdkVersions(android::AssetManager2* assets) {
     using namespace android;
 
-    android::ApkAssetsCookie cookie = FindFrameworkAssetManagerCookie(*assets);
-    if (cookie == android::kInvalidCookie) {
+    // Find the system package (0x01). AAPT always generates attributes with the type 0x01, so
+    // we're looking for the first attribute resource in the system package.
+    android::ApkAssetsCookie cookie;
+    if (auto value = assets->GetResource(0x01010000, true /** may_be_bag */); value.has_value()) {
+      cookie = value->cookie;
+    } else {
       // No Framework assets loaded. Not a failure.
       return;
     }
diff --git a/tools/aapt2/format/binary/TableFlattener_test.cpp b/tools/aapt2/format/binary/TableFlattener_test.cpp
index 6932baf..f8b8a1c 100644
--- a/tools/aapt2/format/binary/TableFlattener_test.cpp
+++ b/tools/aapt2/format/binary/TableFlattener_test.cpp
@@ -189,16 +189,16 @@
                      ResTable_config::CONFIG_VERSION));
 
   std::u16string foo_str = u"foo";
-  ssize_t idx = res_table.getTableStringBlock(0)->indexOfString(foo_str.data(), foo_str.size());
-  ASSERT_GE(idx, 0);
+  auto idx = res_table.getTableStringBlock(0)->indexOfString(foo_str.data(), foo_str.size());
+  ASSERT_TRUE(idx.has_value());
   EXPECT_TRUE(Exists(&res_table, "com.app.test:string/test", ResourceId(0x7f040000), {},
-                     Res_value::TYPE_STRING, (uint32_t)idx, 0u));
+                     Res_value::TYPE_STRING, (uint32_t)*idx, 0u));
 
   std::u16string bar_path = u"res/layout/bar.xml";
   idx = res_table.getTableStringBlock(0)->indexOfString(bar_path.data(), bar_path.size());
-  ASSERT_GE(idx, 0);
+  ASSERT_TRUE(idx.has_value());
   EXPECT_TRUE(Exists(&res_table, "com.app.test:layout/bar", ResourceId(0x7f050000), {},
-                     Res_value::TYPE_STRING, (uint32_t)idx, 0u));
+                     Res_value::TYPE_STRING, (uint32_t)*idx, 0u));
 }
 
 TEST_F(TableFlattenerTest, FlattenEntriesWithGapsInIds) {
@@ -603,16 +603,16 @@
                      2u, ResTable_config::CONFIG_VERSION));
 
   std::u16string foo_str = u"foo";
-  ssize_t idx = res_table.getTableStringBlock(0)->indexOfString(foo_str.data(), foo_str.size());
-  ASSERT_GE(idx, 0);
+  auto idx = res_table.getTableStringBlock(0)->indexOfString(foo_str.data(), foo_str.size());
+  ASSERT_TRUE(idx.has_value());
   EXPECT_TRUE(Exists(&res_table, "com.app.test:string/0_resource_name_obfuscated",
-                     ResourceId(0x7f040000), {}, Res_value::TYPE_STRING, (uint32_t)idx, 0u));
+                     ResourceId(0x7f040000), {}, Res_value::TYPE_STRING, (uint32_t)*idx, 0u));
 
   std::u16string bar_path = u"res/layout/bar.xml";
   idx = res_table.getTableStringBlock(0)->indexOfString(bar_path.data(), bar_path.size());
-  ASSERT_GE(idx, 0);
+  ASSERT_TRUE(idx.has_value());
   EXPECT_TRUE(Exists(&res_table, "com.app.test:layout/0_resource_name_obfuscated",
-                     ResourceId(0x7f050000), {}, Res_value::TYPE_STRING, (uint32_t)idx, 0u));
+                     ResourceId(0x7f050000), {}, Res_value::TYPE_STRING, (uint32_t)*idx, 0u));
 }
 
 TEST_F(TableFlattenerTest, ObfuscatingResourceNamesWithNameCollapseExemptionsSucceeds) {
@@ -659,16 +659,16 @@
                      2u, ResTable_config::CONFIG_VERSION));
 
   std::u16string foo_str = u"foo";
-  ssize_t idx = res_table.getTableStringBlock(0)->indexOfString(foo_str.data(), foo_str.size());
-  ASSERT_GE(idx, 0);
+  auto idx = res_table.getTableStringBlock(0)->indexOfString(foo_str.data(), foo_str.size());
+  ASSERT_TRUE(idx.has_value());
   EXPECT_TRUE(Exists(&res_table, "com.app.test:string/test", ResourceId(0x7f040000), {},
-                     Res_value::TYPE_STRING, (uint32_t)idx, 0u));
+                     Res_value::TYPE_STRING, (uint32_t)*idx, 0u));
 
   std::u16string bar_path = u"res/layout/bar.xml";
   idx = res_table.getTableStringBlock(0)->indexOfString(bar_path.data(), bar_path.size());
-  ASSERT_GE(idx, 0);
+  ASSERT_TRUE(idx.has_value());
   EXPECT_TRUE(Exists(&res_table, "com.app.test:layout/0_resource_name_obfuscated",
-                     ResourceId(0x7f050000), {}, Res_value::TYPE_STRING, (uint32_t)idx, 0u));
+                     ResourceId(0x7f050000), {}, Res_value::TYPE_STRING, (uint32_t)*idx, 0u));
 }
 
 TEST_F(TableFlattenerTest, FlattenOverlayable) {
diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp
index 897fa80..ad716c7 100644
--- a/tools/aapt2/process/SymbolTable.cpp
+++ b/tools/aapt2/process/SymbolTable.cpp
@@ -265,21 +265,22 @@
 
 static std::unique_ptr<SymbolTable::Symbol> LookupAttributeInTable(
     android::AssetManager2& am, ResourceId id) {
+  using namespace android;
   if (am.GetApkAssets().empty()) {
     return {};
   }
 
-  const android::ResolvedBag* bag = am.GetBag(id.id);
-  if (bag == nullptr) {
+  auto bag_result = am.GetBag(id.id);
+  if (!bag_result.has_value()) {
     return nullptr;
   }
 
   // We found a resource.
   std::unique_ptr<SymbolTable::Symbol> s = util::make_unique<SymbolTable::Symbol>(id);
-
+  const ResolvedBag* bag = *bag_result;
   const size_t count = bag->entry_count;
   for (uint32_t i = 0; i < count; i++) {
-    if (bag->entries[i].key == android::ResTable_map::ATTR_TYPE) {
+    if (bag->entries[i].key == ResTable_map::ATTR_TYPE) {
       s->attribute = std::make_shared<Attribute>(bag->entries[i].value.data);
       break;
     }
@@ -287,25 +288,25 @@
 
   if (s->attribute) {
     for (size_t i = 0; i < count; i++) {
-      const android::ResolvedBag::Entry& map_entry = bag->entries[i];
+      const ResolvedBag::Entry& map_entry = bag->entries[i];
       if (Res_INTERNALID(map_entry.key)) {
         switch (map_entry.key) {
-          case android::ResTable_map::ATTR_MIN:
+          case ResTable_map::ATTR_MIN:
             s->attribute->min_int = static_cast<int32_t>(map_entry.value.data);
             break;
-          case android::ResTable_map::ATTR_MAX:
+          case ResTable_map::ATTR_MAX:
             s->attribute->max_int = static_cast<int32_t>(map_entry.value.data);
             break;
         }
         continue;
       }
 
-      android::AssetManager2::ResourceName name;
-      if (!am.GetResourceName(map_entry.key, &name)) {
+      auto name = am.GetResourceName(map_entry.key);
+      if (!name.has_value()) {
         return nullptr;
       }
 
-      Maybe<ResourceName> parsed_name = ResourceUtils::ToResourceName(name);
+      Maybe<ResourceName> parsed_name = ResourceUtils::ToResourceName(*name);
       if (!parsed_name) {
         return nullptr;
       }
@@ -328,7 +329,7 @@
 
   bool found = false;
   ResourceId res_id = 0;
-  uint32_t type_spec_flags;
+  uint32_t type_spec_flags = 0;
   ResourceName real_name;
 
   // There can be mangled resources embedded within other packages. Here we will
@@ -340,8 +341,19 @@
       real_name.package = package_name;
     }
 
-    res_id = asset_manager_.GetResourceId(real_name.to_string());
-    if (res_id.is_valid_static() && asset_manager_.GetResourceFlags(res_id.id, &type_spec_flags)) {
+    auto real_res_id = asset_manager_.GetResourceId(real_name.to_string());
+    if (!real_res_id.has_value()) {
+      return true;
+    }
+
+    res_id.id = *real_res_id;
+    if (!res_id.is_valid_static()) {
+      return true;
+    }
+
+    auto value = asset_manager_.GetResource(res_id.id, true /* may_be_bag */);
+    if (value.has_value()) {
+      type_spec_flags = value->flags;
       found = true;
       return false;
     }
@@ -371,11 +383,11 @@
 
 static Maybe<ResourceName> GetResourceName(android::AssetManager2& am,
                                            ResourceId id) {
-  android::AssetManager2::ResourceName name;
-  if (!am.GetResourceName(id.id, &name)) {
+  auto name = am.GetResourceName(id.id);
+  if (!name.has_value()) {
     return {};
   }
-  return ResourceUtils::ToResourceName(name);
+  return ResourceUtils::ToResourceName(*name);
 }
 
 std::unique_ptr<SymbolTable::Symbol> AssetManagerSymbolSource::FindById(
@@ -394,9 +406,8 @@
     return {};
   }
 
-
-  uint32_t type_spec_flags = 0;
-  if (!asset_manager_.GetResourceFlags(id.id, &type_spec_flags)) {
+  auto value = asset_manager_.GetResource(id.id, true /* may_be_bag */);
+  if (!value.has_value()) {
     return {};
   }
 
@@ -411,7 +422,7 @@
   }
 
   if (s) {
-    s->is_public = (type_spec_flags & android::ResTable_typeSpec::SPEC_PUBLIC) != 0;
+    s->is_public = (value->flags & android::ResTable_typeSpec::SPEC_PUBLIC) != 0;
     return s;
   }
   return {};
diff --git a/tools/aapt2/util/Util.cpp b/tools/aapt2/util/Util.cpp
index 37ce65e..ef33c34 100644
--- a/tools/aapt2/util/Util.cpp
+++ b/tools/aapt2/util/Util.cpp
@@ -531,19 +531,15 @@
 }
 
 StringPiece16 GetString16(const android::ResStringPool& pool, size_t idx) {
-  size_t len;
-  const char16_t* str = pool.stringAt(idx, &len);
-  if (str != nullptr) {
-    return StringPiece16(str, len);
+  if (auto str = pool.stringAt(idx)) {
+    return *str;
   }
   return StringPiece16();
 }
 
 std::string GetString(const android::ResStringPool& pool, size_t idx) {
-  size_t len;
-  const char* str = pool.string8At(idx, &len);
-  if (str != nullptr) {
-    return ModifiedUtf8ToUtf8(std::string(str, len));
+  if (auto str = pool.string8At(idx)) {
+    return ModifiedUtf8ToUtf8(str->to_string());
   }
   return Utf16ToUtf8(GetString16(pool, idx));
 }