Revert^2 "libandroidfw hardening for IncFs"

55ef6167a2c235bd88c7216238b2001b46795b79

Change-Id: I02d4890d181655dfd0a14c188468db512559d27b
Merged-In: I02d4890d181655dfd0a14c188468db512559d27b

6 files changed, 69 insertions, 91 deletions

diff --git a/cmds/idmap2/idmap2/Lookup.cpp b/cmds/idmap2/idmap2/Lookup.cpp
index c44170928992..8323d0ba2415 100644
--- a/cmds/idmap2/idmap2/Lookup.cpp
+++ b/cmds/idmap2/idmap2/Lookup.cpp
@@ -45,11 +45,8 @@ using android::ApkAssets;
 using android::ApkAssetsCookie;
 using android::AssetManager2;
 using android::ConfigDescription;
-using android::is_valid_resid;
-using android::kInvalidCookie;
 using android::Res_value;
 using android::ResStringPool;
-using android::ResTable_config;
 using android::StringPiece16;
 using android::base::StringPrintf;
 using android::idmap2::CommandLineOptions;
@@ -59,7 +56,6 @@ using android::idmap2::ResourceId;
 using android::idmap2::Result;
 using android::idmap2::Unit;
 using android::idmap2::utils::ExtractOverlayManifestInfo;
-using android::util::Utf16ToUtf8;
 
 namespace {
 
@@ -69,25 +65,23 @@ Result<ResourceId> WARN_UNUSED ParseResReference(const AssetManager2& am, const
 
   // first, try to parse as a hex number
   char* endptr = nullptr;
-  ResourceId resid;
-  resid = strtol(res.c_str(), &endptr, kBaseHex);
+  const ResourceId parsed_resid = strtol(res.c_str(), &endptr, kBaseHex);
   if (*endptr == '\0') {
-    return resid;
+    return parsed_resid;
   }
 
   // next, try to parse as a package:type/name string
-  resid = am.GetResourceId(res, "", fallback_package);
-  if (is_valid_resid(resid)) {
-    return resid;
+  if (auto resid = am.GetResourceId(res, "", fallback_package)) {
+    return *resid;
   }
 
   // end of the road: res could not be parsed
   return Error("failed to obtain resource id for %s", res.c_str());
 }
 
-void PrintValue(AssetManager2* const am, const Res_value& value, const ApkAssetsCookie& cookie,
+void PrintValue(AssetManager2* const am, const AssetManager2::SelectedValue& value,
                 std::string* const out) {
-  switch (value.dataType) {
+  switch (value.type) {
     case Res_value::TYPE_INT_DEC:
       out->append(StringPrintf("%d", value.data));
       break;
@@ -98,30 +92,21 @@ void PrintValue(AssetManager2* const am, const Res_value& value, const ApkAssets
       out->append(value.data != 0 ? "true" : "false");
       break;
     case Res_value::TYPE_STRING: {
-      const ResStringPool* pool = am->GetStringPoolForCookie(cookie);
+      const ResStringPool* pool = am->GetStringPoolForCookie(value.cookie);
       out->append("\"");
-      size_t len;
-      if (pool->isUTF8()) {
-        const char* str = pool->string8At(value.data, &len);
-        out->append(str, len);
-      } else {
-        const char16_t* str16 = pool->stringAt(value.data, &len);
-        out->append(Utf16ToUtf8(StringPiece16(str16, len)));
+      if (auto str = pool->string8ObjectAt(value.data)) {
+        out->append(*str);
       }
-      out->append("\"");
     } break;
     default:
-      out->append(StringPrintf("dataType=0x%02x data=0x%08x", value.dataType, value.data));
+      out->append(StringPrintf("dataType=0x%02x data=0x%08x", value.type, value.data));
       break;
   }
 }
 
 Result<std::string> WARN_UNUSED GetValue(AssetManager2* const am, ResourceId resid) {
-  Res_value value;
-  ResTable_config config;
-  uint32_t flags;
-  ApkAssetsCookie cookie = am->GetResource(resid, true, 0, &value, &config, &flags);
-  if (cookie == kInvalidCookie) {
+  auto value = am->GetResource(resid);
+  if (!value.has_value()) {
     return Error("no resource 0x%08x in asset manager", resid);
   }
 
@@ -129,41 +114,35 @@ Result<std::string> WARN_UNUSED GetValue(AssetManager2* const am, ResourceId res
 
   // TODO(martenkongstad): use optional parameter GetResource(..., std::string*
   // stacktrace = NULL) instead
-  out.append(StringPrintf("cookie=%d ", cookie));
+  out.append(StringPrintf("cookie=%d ", value->cookie));
 
   out.append("config='");
-  out.append(config.toString().c_str());
+  out.append(value->config.toString().c_str());
   out.append("' value=");
 
-  if (value.dataType == Res_value::TYPE_REFERENCE) {
-    const android::ResolvedBag* bag = am->GetBag(static_cast<uint32_t>(value.data));
-    if (bag == nullptr) {
-      out.append(StringPrintf("dataType=0x%02x data=0x%08x", value.dataType, value.data));
+  if (value->type == Res_value::TYPE_REFERENCE) {
+    auto bag_result = am->GetBag(static_cast<uint32_t>(value->data));
+    if (!bag_result.has_value()) {
+      out.append(StringPrintf("dataType=0x%02x data=0x%08x", value->type, value->data));
       return out;
     }
+
     out.append("[");
-    Res_value bag_val;
-    ResTable_config selected_config;
-    uint32_t flags;
-    uint32_t ref;
-    ApkAssetsCookie bag_cookie;
+    const android::ResolvedBag* bag = bag_result.value();
     for (size_t i = 0; i < bag->entry_count; ++i) {
-      const android::ResolvedBag::Entry& entry = bag->entries[i];
-      bag_val = entry.value;
-      bag_cookie = am->ResolveReference(entry.cookie, &bag_val, &selected_config, &flags, &ref);
-      if (bag_cookie == kInvalidCookie) {
-        out.append(
-            StringPrintf("Error: dataType=0x%02x data=0x%08x", bag_val.dataType, bag_val.data));
+      AssetManager2::SelectedValue entry(bag, bag->entries[i]);
+      if (am->ResolveReference(entry).has_value()) {
+        out.append(StringPrintf("Error: dataType=0x%02x data=0x%08x", entry.type, entry.data));
         continue;
       }
-      PrintValue(am, bag_val, bag_cookie, &out);
+      PrintValue(am, entry, &out);
       if (i != bag->entry_count - 1) {
         out.append(", ");
       }
     }
     out.append("]");
   } else {
-    PrintValue(am, value, cookie, &out);
+    PrintValue(am, *value, &out);
   }
 
   return out;
diff --git a/cmds/idmap2/include/idmap2/ResourceMapping.h b/cmds/idmap2/include/idmap2/ResourceMapping.h
index 0a58ec43d8ff..f66916ccb78c 100644
--- a/cmds/idmap2/include/idmap2/ResourceMapping.h
+++ b/cmds/idmap2/include/idmap2/ResourceMapping.h
@@ -117,7 +117,8 @@ class ResourceMapping {
   static Result<ResourceMapping> CreateResourceMappingLegacy(const AssetManager2* target_am,
                                                              const AssetManager2* overlay_am,
                                                              const LoadedPackage* target_package,
-                                                             const LoadedPackage* overlay_package);
+                                                             const LoadedPackage* overlay_package,
+                                                             LogInfo& log_info);
 
   // Removes resources that do not pass policy or overlayable checks of the target package.
   void FilterOverlayableResources(const AssetManager2* target_am,
diff --git a/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp b/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
index a93202a64d31..3037a791328e 100644
--- a/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
+++ b/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
@@ -100,10 +100,9 @@ void PrettyPrintVisitor::visit(const IdmapData& data) {
     stream_ << TAB << base::StringPrintf("0x%08x -> ", target_entry.target_id)
             << utils::DataTypeToString(target_entry.value.data_type);
 
-    size_t unused;
     if (target_entry.value.data_type == Res_value::TYPE_STRING) {
-      auto str = string_pool.stringAt(target_entry.value.data_value - string_pool_offset, &unused);
-      stream_ << " \"" << StringPiece16(str) << "\"";
+      auto str = string_pool.stringAt(target_entry.value.data_value - string_pool_offset);
+      stream_ << " \"" << str.value_or(StringPiece16(u"")) << "\"";
     } else {
       stream_ << " " << base::StringPrintf("0x%08x", target_entry.value.data_value);
     }
diff --git a/cmds/idmap2/libidmap2/ResourceMapping.cpp b/cmds/idmap2/libidmap2/ResourceMapping.cpp
index 31f1c16ba5a6..d777cbfa9a14 100644
--- a/cmds/idmap2/libidmap2/ResourceMapping.cpp
+++ b/cmds/idmap2/libidmap2/ResourceMapping.cpp
@@ -71,9 +71,10 @@ Result<Unit> CheckOverlayable(const LoadedPackage& target_package,
   if (!target_package.DefinesOverlayable()) {
     return (sDefaultPolicies & fulfilled_policies) != 0
                ? Result<Unit>({})
-               : Error("overlay must be preinstalled, signed with the same signature as the target,"
-                       " or signed with the same signature as the package referenced through"
-                       " <overlay-config-signature>.");
+               : Error(
+                     "overlay must be preinstalled, signed with the same signature as the target,"
+                     " or signed with the same signature as the package referenced through"
+                     " <overlay-config-signature>.");
   }
 
   const OverlayableInfo* overlayable_info = target_package.GetOverlayableInfo(target_resource);
@@ -119,32 +120,25 @@ const LoadedPackage* GetPackageAtIndex0(const LoadedArsc& loaded_arsc) {
 
 Result<std::unique_ptr<Asset>> OpenNonAssetFromResource(const ResourceId& resource_id,
                                                         const AssetManager2& asset_manager) {
-  Res_value value{};
-  ResTable_config selected_config{};
-  uint32_t flags;
-  auto cookie =
-      asset_manager.GetResource(resource_id, /* may_be_bag */ false,
-                                /* density_override */ 0U, &value, &selected_config, &flags);
-  if (cookie == kInvalidCookie) {
+  auto value = asset_manager.GetResource(resource_id);
+  if (!value.has_value()) {
     return Error("failed to find resource for id 0x%08x", resource_id);
   }
 
-  if (value.dataType != Res_value::TYPE_STRING) {
+  if (value->type != Res_value::TYPE_STRING) {
     return Error("resource for is 0x%08x is not a file", resource_id);
   }
 
-  auto string_pool = asset_manager.GetStringPoolForCookie(cookie);
-  size_t len;
-  auto file_path16 = string_pool->stringAt(value.data, &len);
-  if (file_path16 == nullptr) {
-    return Error("failed to find string for index %d", value.data);
+  auto string_pool = asset_manager.GetStringPoolForCookie(value->cookie);
+  auto file = string_pool->string8ObjectAt(value->data);
+  if (!file.has_value()) {
+    return Error("failed to find string for index %d", value->data);
   }
 
   // Load the overlay resource mappings from the file specified using android:resourcesMap.
-  auto file_path = String8(String16(file_path16));
-  auto asset = asset_manager.OpenNonAsset(file_path.c_str(), Asset::AccessMode::ACCESS_BUFFER);
+  auto asset = asset_manager.OpenNonAsset(file->c_str(), Asset::AccessMode::ACCESS_BUFFER);
   if (asset == nullptr) {
-    return Error("file \"%s\" not found", file_path.c_str());
+    return Error("file \"%s\" not found", file->c_str());
   }
 
   return asset;
@@ -190,16 +184,16 @@ Result<ResourceMapping> ResourceMapping::CreateResourceMapping(const AssetManage
       return Error(R"(<item> tag missing expected attribute "value")");
     }
 
-    ResourceId target_id =
+    auto target_id_result =
         target_am->GetResourceId(*target_resource, "", target_package->GetPackageName());
-    if (target_id == 0U) {
+    if (!target_id_result.has_value()) {
       log_info.Warning(LogMessage() << "failed to find resource \"" << *target_resource
                                     << "\" in target resources");
       continue;
     }
 
     // Retrieve the compile-time resource id of the target resource.
-    target_id = REWRITE_PACKAGE(target_id, target_package_id);
+    uint32_t target_id = REWRITE_PACKAGE(*target_id_result, target_package_id);
 
     if (overlay_resource->dataType == Res_value::TYPE_STRING) {
       overlay_resource->data += string_pool_offset;
@@ -220,7 +214,7 @@ Result<ResourceMapping> ResourceMapping::CreateResourceMapping(const AssetManage
 
 Result<ResourceMapping> ResourceMapping::CreateResourceMappingLegacy(
     const AssetManager2* target_am, const AssetManager2* overlay_am,
-    const LoadedPackage* target_package, const LoadedPackage* overlay_package) {
+    const LoadedPackage* target_package, const LoadedPackage* overlay_package, LogInfo& log_info) {
   ResourceMapping resource_mapping;
   const uint8_t target_package_id = target_package->GetPackageId();
   const auto end = overlay_package->end();
@@ -234,13 +228,15 @@ Result<ResourceMapping> ResourceMapping::CreateResourceMappingLegacy(
     // Find the resource with the same type and entry name within the target package.
     const std::string full_name =
         base::StringPrintf("%s:%s", target_package->GetPackageName().c_str(), name->c_str());
-    ResourceId target_resource = target_am->GetResourceId(full_name);
-    if (target_resource == 0U) {
+    auto target_resource_result = target_am->GetResourceId(full_name);
+    if (!target_resource_result.has_value()) {
+      log_info.Warning(LogMessage() << "failed to find resource \"" << full_name
+                                    << "\" in target resources");
       continue;
     }
 
     // Retrieve the compile-time resource id of the target resource.
-    target_resource = REWRITE_PACKAGE(target_resource, target_package_id);
+    ResourceId target_resource = REWRITE_PACKAGE(*target_resource_result, target_package_id);
     resource_mapping.AddMapping(target_resource, overlay_resid,
                                 false /* rewrite_overlay_reference */);
   }
@@ -347,7 +343,9 @@ Result<ResourceMapping> ResourceMapping::FromApkAssets(const ApkAssets& target_a
     auto& string_pool = (*parser)->get_strings();
     string_pool_data_length = string_pool.bytes();
     string_pool_data.reset(new uint8_t[string_pool_data_length]);
-    memcpy(string_pool_data.get(), string_pool.data(), string_pool_data_length);
+
+    // Overlays should not be incrementally installed, so calling unsafe_ptr is fine here.
+    memcpy(string_pool_data.get(), string_pool.data().unsafe_ptr(), string_pool_data_length);
 
     // Offset string indices by the size of the overlay resource table string pool.
     string_pool_offset = overlay_arsc->GetStringPool()->size();
@@ -358,7 +356,7 @@ Result<ResourceMapping> ResourceMapping::FromApkAssets(const ApkAssets& target_a
     // If no file is specified using android:resourcesMap, it is assumed that the overlay only
     // defines resources intended to override target resources of the same type and name.
     resource_mapping = CreateResourceMappingLegacy(&target_asset_manager, &overlay_asset_manager,
-                                                   target_pkg, overlay_pkg);
+                                                   target_pkg, overlay_pkg, log_info);
   }
 
   if (!resource_mapping) {
diff --git a/cmds/idmap2/libidmap2/ResourceUtils.cpp b/cmds/idmap2/libidmap2/ResourceUtils.cpp
index 98d026bc70dc..e817140238ae 100644
--- a/cmds/idmap2/libidmap2/ResourceUtils.cpp
+++ b/cmds/idmap2/libidmap2/ResourceUtils.cpp
@@ -72,21 +72,21 @@ StringPiece DataTypeToString(uint8_t data_type) {
 }
 
 Result<std::string> ResToTypeEntryName(const AssetManager2& am, uint32_t resid) {
-  AssetManager2::ResourceName name;
-  if (!am.GetResourceName(resid, &name)) {
+  const auto name = am.GetResourceName(resid);
+  if (!name.has_value()) {
     return Error("no resource 0x%08x in asset manager", resid);
   }
   std::string out;
-  if (name.type != nullptr) {
-    out.append(name.type, name.type_len);
+  if (name->type != nullptr) {
+    out.append(name->type, name->type_len);
   } else {
-    out += Utf16ToUtf8(StringPiece16(name.type16, name.type_len));
+    out += Utf16ToUtf8(StringPiece16(name->type16, name->type_len));
   }
   out.append("/");
-  if (name.entry != nullptr) {
-    out.append(name.entry, name.entry_len);
+  if (name->entry != nullptr) {
+    out.append(name->entry, name->entry_len);
   } else {
-    out += Utf16ToUtf8(StringPiece16(name.entry16, name.entry_len));
+    out += Utf16ToUtf8(StringPiece16(name->entry16, name->entry_len));
   }
   return out;
 }
diff --git a/cmds/idmap2/libidmap2/XmlParser.cpp b/cmds/idmap2/libidmap2/XmlParser.cpp
index 526a560907aa..7c55b64566f2 100644
--- a/cmds/idmap2/libidmap2/XmlParser.cpp
+++ b/cmds/idmap2/libidmap2/XmlParser.cpp
@@ -98,18 +98,19 @@ Result<std::string> XmlParser::Node::GetAttributeStringValue(const std::string&
 
   switch ((*value).dataType) {
     case Res_value::TYPE_STRING: {
-      size_t len;
-      const String16 value16(parser_.getStrings().stringAt((*value).data, &len));
-      return std::string(String8(value16).c_str());
+      if (auto str = parser_.getStrings().string8ObjectAt((*value).data)) {
+        return std::string(str->string());
+      }
+      break;
     }
     case Res_value::TYPE_INT_DEC:
     case Res_value::TYPE_INT_HEX:
     case Res_value::TYPE_INT_BOOLEAN: {
       return std::to_string((*value).data);
     }
-    default:
-      return Error(R"(Failed to convert attribute "%s" value to a string)", name.c_str());
   }
+
+  return Error(R"(Failed to convert attribute "%s" value to a string)", name.c_str());
 }
 
 Result<Res_value> XmlParser::Node::GetAttributeValue(const std::string& name) const {