Read manifest values using resource id in idmap2

Idmap2 currently reads android:targetPackage and android:targetName
from overlay manifests by looking for an attribute with the name of
the attribute resource.

This fixes the divergence from package parsing by finding the
attributes using their resource ids.

Bug: 175060836
Test: libidmap2_tests
Change-Id: I09965e5880a1e6c48c3f8077db0c595484804ce7

cmds/idmap2/include/idmap2/XmlParser.h

@@ -22,6 +22,7 @@
 #include <memory>
 #include <string>
 
+#include "ResourceUtils.h"
 #include "Result.h"
 #include "android-base/macros.h"
 #include "androidfw/ResourceTypes.h"
@@ -39,8 +40,11 @@ class XmlParser {
     Event event() const;
     std::string name() const;
 
-    Result<std::string> GetAttributeStringValue(const std::string& name) const;
     Result<Res_value> GetAttributeValue(const std::string& name) const;
+    Result<Res_value> GetAttributeValue(ResourceId attr, const std::string& label) const;
+
+    Result<std::string> GetAttributeStringValue(const std::string& name) const;
+    Result<std::string> GetAttributeStringValue(ResourceId attr, const std::string& label) const;
 
     bool operator==(const Node& rhs) const;
     bool operator!=(const Node& rhs) const;

cmds/idmap2/libidmap2/ResourceUtils.cpp

@@ -32,6 +32,12 @@ using android::idmap2::ZipFile;
 using android::util::Utf16ToUtf8;
 
 namespace android::idmap2::utils {
+namespace {
+constexpr ResourceId kAttrName = 0x01010003;
+constexpr ResourceId kAttrResourcesMap = 0x01010609;
+constexpr ResourceId kAttrTargetName = 0x0101044d;
+constexpr ResourceId kAttrTargetPackage = 0x01010021;
+}  // namespace
 
 bool IsReference(uint8_t data_type) {
   return data_type == Res_value::TYPE_REFERENCE || data_type == Res_value::TYPE_DYNAMIC_REFERENCE;
@@ -119,7 +125,7 @@ Result<OverlayManifestInfo> ExtractOverlayManifestInfo(const std::string& path,
     }
 
     OverlayManifestInfo info{};
-    if (auto result_str = it.GetAttributeStringValue("name")) {
+    if (auto result_str = it.GetAttributeStringValue(kAttrName, "android:name")) {
       if (*result_str != name) {
         // A value for android:name was found, but either a the name does not match the requested
         // name, or an <overlay> tag with no name was requested.
@@ -132,18 +138,18 @@ Result<OverlayManifestInfo> ExtractOverlayManifestInfo(const std::string& path,
       continue;
     }
 
-    if (auto result_str = it.GetAttributeStringValue("targetPackage")) {
+    if (auto result_str = it.GetAttributeStringValue(kAttrTargetPackage, "android:targetPackage")) {
       info.target_package = *result_str;
     } else {
       return Error("android:targetPackage missing from <overlay> of %s: %s", path.c_str(),
                    result_str.GetErrorMessage().c_str());
     }
 
-    if (auto result_str = it.GetAttributeStringValue("targetName")) {
+    if (auto result_str = it.GetAttributeStringValue(kAttrTargetName, "android:targetName")) {
       info.target_name = *result_str;
     }
 
-    if (auto result_value = it.GetAttributeValue("resourcesMap")) {
+    if (auto result_value = it.GetAttributeValue(kAttrResourcesMap, "android:resourcesMap")) {
       if (IsReference((*result_value).dataType)) {
         info.resource_mapping = (*result_value).data;
       } else {

cmds/idmap2/libidmap2/XmlParser.cpp

@@ -90,15 +90,27 @@ std::string XmlParser::Node::name() const {
   return String8(key16).c_str();
 }
 
-Result<std::string> XmlParser::Node::GetAttributeStringValue(const std::string& name) const {
-  auto value = GetAttributeValue(name);
-  if (!value) {
-    return value.GetError();
+template <typename Func>
+Result<Res_value> FindAttribute(const ResXMLParser& parser, const std::string& label,
+                                Func&& predicate) {
+  for (size_t i = 0; i < parser.getAttributeCount(); i++) {
+    if (!predicate(i)) {
+      continue;
+    }
+    Res_value res_value{};
+    if (parser.getAttributeValue(i, &res_value) == BAD_TYPE) {
+      return Error(R"(Bad value for attribute "%s")", label.c_str());
+    }
+    return res_value;
   }
+  return Error(R"(Failed to find attribute "%s")", label.c_str());
+}
 
-  switch ((*value).dataType) {
+Result<std::string> GetStringValue(const ResXMLParser& parser, const Res_value& value,
+                                   const std::string& label) {
+  switch (value.dataType) {
     case Res_value::TYPE_STRING: {
-      if (auto str = parser_.getStrings().string8ObjectAt((*value).data); str.ok()) {
+      if (auto str = parser.getStrings().string8ObjectAt(value.data); str.ok()) {
         return std::string(str->string());
       }
       break;
@@ -106,31 +118,37 @@ Result<std::string> XmlParser::Node::GetAttributeStringValue(const std::string&
     case Res_value::TYPE_INT_DEC:
     case Res_value::TYPE_INT_HEX:
     case Res_value::TYPE_INT_BOOLEAN: {
-      return std::to_string((*value).data);
+      return std::to_string(value.data);
     }
   }
+  return Error(R"(Failed to convert attribute "%s" value to a string)", label.c_str());
+}
 
-  return Error(R"(Failed to convert attribute "%s" value to a string)", name.c_str());
+Result<Res_value> XmlParser::Node::GetAttributeValue(ResourceId attr,
+                                                     const std::string& label) const {
+  return FindAttribute(parser_, label, [&](size_t index) -> bool {
+    return parser_.getAttributeNameResID(index) == attr;
+  });
 }
 
 Result<Res_value> XmlParser::Node::GetAttributeValue(const std::string& name) const {
-  size_t len;
-  for (size_t i = 0; i < parser_.getAttributeCount(); i++) {
-    const String16 key16(parser_.getAttributeName(i, &len));
+  return FindAttribute(parser_, name, [&](size_t index) -> bool {
+    size_t len;
+    const String16 key16(parser_.getAttributeName(index, &len));
     std::string key = String8(key16).c_str();
-    if (key != name) {
-      continue;
-    }
+    return key == name;
+  });
+}
 
-    Res_value res_value{};
-    if (parser_.getAttributeValue(i, &res_value) == BAD_TYPE) {
-      return Error(R"(Bad value for attribute "%s")", name.c_str());
-    }
+Result<std::string> XmlParser::Node::GetAttributeStringValue(ResourceId attr,
+                                                             const std::string& label) const {
+  auto value = GetAttributeValue(attr, label);
+  return value ? GetStringValue(parser_, *value, label) : value.GetError();
+}
 
-    return res_value;
-  }
-
-  return Error(R"(Failed to find attribute "%s")", name.c_str());
+Result<std::string> XmlParser::Node::GetAttributeStringValue(const std::string& name) const {
+  auto value = GetAttributeValue(name);
+  return value ? GetStringValue(parser_, *value, name) : value.GetError();
 }
 
 Result<std::unique_ptr<const XmlParser>> XmlParser::Create(const void* data, size_t size,

cmds/idmap2/tests/ResourceUtilsTests.cpp

@@ -59,4 +59,26 @@ TEST_F(ResourceUtilsTests, ResToTypeEntryNameNoSuchResourceId) {
   ASSERT_FALSE(name);
 }
 
-}  // namespace android::idmap2
+TEST_F(ResourceUtilsTests, InvalidValidOverlayNameInvalidAttributes) {
+  auto info = utils::ExtractOverlayManifestInfo(GetTestDataPath() + "/overlay/overlay-invalid.apk",
+                                                "InvalidName");
+  ASSERT_FALSE(info);
+}
+
+TEST_F(ResourceUtilsTests, ValidOverlayNameInvalidAttributes) {
+  auto info = utils::ExtractOverlayManifestInfo(GetTestDataPath() + "/overlay/overlay-invalid.apk",
+                                                "ValidName");
+  ASSERT_FALSE(info);
+}
+
+TEST_F(ResourceUtilsTests, ValidOverlayNameAndTargetPackageInvalidAttributes) {
+  auto info = utils::ExtractOverlayManifestInfo(GetTestDataPath() + "/overlay/overlay-invalid.apk",
+                                                "ValidNameAndTargetPackage");
+  ASSERT_TRUE(info);
+  ASSERT_EQ("ValidNameAndTargetPackage", info->name);
+  ASSERT_EQ("Valid", info->target_package);
+  ASSERT_EQ("", info->target_name); // Attribute resource id could not be found
+  ASSERT_EQ(0, info->resource_mapping); // Attribute resource id could not be found
+}
+
+}// namespace android::idmap2

cmds/idmap2/tests/data/overlay/AndroidManifestInvalid.xml

@@ -0,0 +1,34 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright (C) 2018 The Android Open Source Project
+
+     Licensed under the Apache License, Version 2.0 (the "License");
+     you may not use this file except in compliance with the License.
+     You may obtain a copy of the License at
+
+          http://www.apache.org/licenses/LICENSE-2.0
+
+     Unless required by applicable law or agreed to in writing, software
+     distributed under the License is distributed on an "AS IS" BASIS,
+     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+     See the License for the specific language governing permissions and
+     limitations under the License.
+-->
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+          package="test.overlay">
+    <application android:hasCode="false"/>
+
+    <overlay name="InvalidName"
+         android:targetName="Invalid"
+         android:targetPackage="Invalid"
+         android:resourcesMap="@xml/overlays_swap"/>
+
+    <overlay android:name="ValidName"
+         targetName="Invalid"
+         targetPackage="Invalid"
+         resourcesMap="@xml/overlays_swap"/>
+
+    <overlay android:name="ValidNameAndTargetPackage"
+         targetName="Invalid"
+         android:targetPackage="Valid"
+         resourcesMap="@xml/overlays_swap"/>
+</manifest>

cmds/idmap2/tests/data/overlay/build

@@ -38,4 +38,11 @@ aapt2 link \
     -o overlay-legacy.apk \
     compiled.flata
 
+aapt2 link \
+    --no-resource-removal \
+    -I "$FRAMEWORK_RES_APK" \
+    --manifest AndroidManifestInvalid.xml \
+    -o overlay-invalid.apk \
+    compiled.flata
+
 rm compiled.flata