Set resource id correctly when resolve fails

If for some reason the resource id cannot be resolved to a value
(there is no configuration that matches the AssetManager configuration
or some error occurs), set the resource id of the SelectedValue to the
resource id that could not be resolved. This was the behavior before
the AssetManager IncFs hardening refactor.

Bug: 173203252
Test: atest com.google.android.config.pts.PreinstalledAppsTestCase
Test: Chrome icon appears on launcher
Change-Id: Iad1760c0e246da1a4bf64d1c2ec60bb08da32d06
Merged-In: Iad1760c0e246da1a4bf64d1c2ec60bb08da32d06

3 files changed, 44 insertions, 8 deletions

diff --git a/libs/androidfw/AssetManager2.cpp b/libs/androidfw/AssetManager2.cpp
index 3e54dc67db76..8bab73cd15f0 100644
--- a/libs/androidfw/AssetManager2.cpp
+++ b/libs/androidfw/AssetManager2.cpp
@@ -975,19 +975,23 @@ base::expected<std::monostate, NullOrIOError> AssetManager2::ResolveReference(
   for (uint32_t i = 0U;; i++) {
     auto result = GetResource(resolve_resid, true /*may_be_bag*/);
     if (!result.has_value()) {
+      value.resid = resolve_resid;
       return base::unexpected(result.error());
     }
 
+    // If resource resolution fails, the value should be set to the last reference that was able to
+    // be resolved successfully.
+    value = *result;
+    value.flags |= combined_flags;
+
     if (result->type != Res_value::TYPE_REFERENCE ||
         result->data == Res_value::DATA_NULL_UNDEFINED ||
         result->data == resolve_resid || i == kMaxIterations) {
       // This reference can't be resolved, so exit now and let the caller deal with it.
-      value = *result;
-      value.flags |= combined_flags;
       return {};
     }
 
-    combined_flags |= result->flags;
+    combined_flags = result->flags;
     resolve_resid = result->data;
   }
 }
diff --git a/libs/androidfw/AttributeResolution.cpp b/libs/androidfw/AttributeResolution.cpp
index 71919fdbb736..d07b452ad40e 100644
--- a/libs/androidfw/AttributeResolution.cpp
+++ b/libs/androidfw/AttributeResolution.cpp
@@ -265,7 +265,7 @@ base::expected<std::monostate, IOError> ApplyStyle(Theme* theme, ResXMLParser* x
     const size_t xml_attr_idx = xml_attr_finder.Find(cur_ident);
     if (xml_attr_idx != xml_attr_finder.end()) {
       // We found the attribute we were looking for.
-      Res_value attribute_value;
+      Res_value attribute_value{};
       xml_parser->getAttributeValue(xml_attr_idx, &attribute_value);
       value.type = attribute_value.dataType;
       value.data = attribute_value.data;
@@ -377,7 +377,7 @@ base::expected<std::monostate, IOError> RetrieveAttributes(AssetManager2* assetm
 
     // Retrieve the current XML attribute if it matches, and step to next.
     if (ix < xml_attr_count && cur_ident == cur_xml_attr) {
-      Res_value attribute_value;
+      Res_value attribute_value{};
       xml_parser->getAttributeValue(ix, &attribute_value);
       value.type = attribute_value.dataType;
       value.data = attribute_value.data;
diff --git a/libs/androidfw/tests/AssetManager2_test.cpp b/libs/androidfw/tests/AssetManager2_test.cpp
index 3638ce1f92d7..0f5afd45546c 100644
--- a/libs/androidfw/tests/AssetManager2_test.cpp
+++ b/libs/androidfw/tests/AssetManager2_test.cpp
@@ -446,9 +446,6 @@ TEST_F(AssetManager2Test, KeepLastReferenceIdUnmodifiedIfNoReferenceIsResolved)
   AssetManager2 assetmanager;
   assetmanager.SetApkAssets({basic_assets_.get()});
 
-  ResTable_config selected_config;
-  memset(&selected_config, 0, sizeof(selected_config));
-
   // Create some kind of value that is NOT a reference.
   AssetManager2::SelectedValue value{};
   value.cookie = 1;
@@ -461,6 +458,41 @@ TEST_F(AssetManager2Test, KeepLastReferenceIdUnmodifiedIfNoReferenceIsResolved)
   EXPECT_EQ(basic::R::string::test1, value.resid);
 }
 
+TEST_F(AssetManager2Test, ResolveReferenceMissingResource) {
+  AssetManager2 assetmanager;
+  assetmanager.SetApkAssets({basic_assets_.get()});
+
+  const uint32_t kMissingResId = 0x8001ffff;
+  AssetManager2::SelectedValue value{};
+  value.type = Res_value::TYPE_REFERENCE;
+  value.data = kMissingResId;
+
+  auto result = assetmanager.ResolveReference(value);
+  ASSERT_FALSE(result.has_value());
+  EXPECT_EQ(Res_value::TYPE_REFERENCE, value.type);
+  EXPECT_EQ(kMissingResId, value.data);
+  EXPECT_EQ(kMissingResId, value.resid);
+  EXPECT_EQ(-1, value.cookie);
+  EXPECT_EQ(0, value.flags);
+}
+
+TEST_F(AssetManager2Test, ResolveReferenceMissingResourceLib) {
+  AssetManager2 assetmanager;
+  assetmanager.SetApkAssets({libclient_assets_.get()});
+
+  AssetManager2::SelectedValue value{};
+  value.type = Res_value::TYPE_REFERENCE;
+  value.data = libclient::R::string::foo_one;
+
+  auto result = assetmanager.ResolveReference(value);
+  ASSERT_TRUE(result.has_value());
+  EXPECT_EQ(Res_value::TYPE_DYNAMIC_REFERENCE, value.type);
+  EXPECT_EQ(lib_one::R::string::foo, value.data);
+  EXPECT_EQ(libclient::R::string::foo_one, value.resid);
+  EXPECT_EQ(0, value.cookie);
+  EXPECT_EQ(static_cast<uint32_t>(ResTable_typeSpec::SPEC_PUBLIC), value.flags);
+}
+
 static bool IsConfigurationPresent(const std::set<ResTable_config>& configurations,
                                    const ResTable_config& configuration) {
   return configurations.count(configuration) > 0;