Revert "AssetManager2: Allow out of order type/type spec"
This reverts commit 78695c354342bd95ba5f63937b4e789139b50072.
Bug: 73134570
Change-Id: I6acc35372d9071d067d2fb7caa775ee9ba689811
diff --git a/libs/androidfw/LoadedArsc.cpp b/libs/androidfw/LoadedArsc.cpp
index a65d49b..1d2c597 100644
--- a/libs/androidfw/LoadedArsc.cpp
+++ b/libs/androidfw/LoadedArsc.cpp
@@ -409,10 +409,14 @@
util::ReadUtf16StringFromDevice(header->name, arraysize(header->name),
&loaded_package->package_name_);
- // A map of TypeSpec builders, each associated with an type index.
- // We use these to accumulate the set of Types available for a TypeSpec, and later build a single,
- // contiguous block of memory that holds all the Types together with the TypeSpec.
- std::unordered_map<int, std::unique_ptr<TypeSpecPtrBuilder>> type_builder_map;
+ // A TypeSpec builder. We use this to accumulate the set of Types
+ // available for a TypeSpec, and later build a single, contiguous block
+ // of memory that holds all the Types together with the TypeSpec.
+ std::unique_ptr<TypeSpecPtrBuilder> types_builder;
+
+ // Keep track of the last seen type index. Since type IDs are 1-based,
+ // this records their index, which is 0-based (type ID - 1).
+ uint8_t last_type_idx = 0;
ChunkIterator iter(chunk.data_ptr(), chunk.data_size());
while (iter.HasNext()) {
@@ -446,6 +450,28 @@
case RES_TABLE_TYPE_SPEC_TYPE: {
ATRACE_NAME("LoadTableTypeSpec");
+ // Starting a new TypeSpec, so finish the old one if there was one.
+ if (types_builder) {
+ TypeSpecPtr type_spec_ptr = types_builder->Build();
+ if (type_spec_ptr == nullptr) {
+ LOG(ERROR) << "Too many type configurations, overflow detected.";
+ return {};
+ }
+
+ // We only add the type to the package if there is no IDMAP, or if the type is
+ // overlaying something.
+ if (loaded_idmap == nullptr || type_spec_ptr->idmap_entries != nullptr) {
+ // If this is an overlay, insert it at the target type ID.
+ if (type_spec_ptr->idmap_entries != nullptr) {
+ last_type_idx = dtohs(type_spec_ptr->idmap_entries->target_type_id) - 1;
+ }
+ loaded_package->type_specs_.editItemAt(last_type_idx) = std::move(type_spec_ptr);
+ }
+
+ types_builder = {};
+ last_type_idx = 0;
+ }
+
const ResTable_typeSpec* type_spec = child_chunk.header<ResTable_typeSpec>();
if (type_spec == nullptr) {
LOG(ERROR) << "RES_TABLE_TYPE_SPEC_TYPE too small.";
@@ -480,6 +506,8 @@
return {};
}
+ last_type_idx = type_spec->id - 1;
+
// If this is an overlay, associate the mapping of this type to the target type
// from the IDMAP.
const IdmapEntry_header* idmap_entry_header = nullptr;
@@ -487,13 +515,7 @@
idmap_entry_header = loaded_idmap->GetEntryMapForType(type_spec->id);
}
- std::unique_ptr<TypeSpecPtrBuilder>& builder_ptr = type_builder_map[type_spec->id - 1];
- if (builder_ptr == nullptr) {
- builder_ptr = util::make_unique<TypeSpecPtrBuilder>(type_spec, idmap_entry_header);
- } else {
- LOG(WARNING) << StringPrintf("RES_TABLE_TYPE_SPEC_TYPE already defined for ID %02x",
- type_spec->id);
- }
+ types_builder = util::make_unique<TypeSpecPtrBuilder>(type_spec, idmap_entry_header);
} break;
case RES_TABLE_TYPE_TYPE: {
@@ -508,15 +530,12 @@
}
// Type chunks must be preceded by their TypeSpec chunks.
- std::unique_ptr<TypeSpecPtrBuilder>& builder_ptr = type_builder_map[type->id - 1];
- if (builder_ptr != nullptr) {
- builder_ptr->AddType(type);
- } else {
- LOG(ERROR) << StringPrintf(
- "RES_TABLE_TYPE_TYPE with ID %02x found without preceding RES_TABLE_TYPE_SPEC_TYPE.",
- type->id);
+ if (!types_builder || type->id - 1 != last_type_idx) {
+ LOG(ERROR) << "RES_TABLE_TYPE_TYPE found without preceding RES_TABLE_TYPE_SPEC_TYPE.";
return {};
}
+
+ types_builder->AddType(type);
} break;
case RES_TABLE_LIBRARY_TYPE: {
@@ -542,7 +561,7 @@
arraysize(entry_iter->packageName), &package_name);
if (dtohl(entry_iter->packageId) >= std::numeric_limits<uint8_t>::max()) {
- LOG(ERROR) << StringPrintf(
+ LOG(ERROR) << base::StringPrintf(
"Package ID %02x in RES_TABLE_LIBRARY_TYPE too large for package '%s'.",
dtohl(entry_iter->packageId), package_name.c_str());
return {};
@@ -555,20 +574,14 @@
} break;
default:
- LOG(WARNING) << StringPrintf("Unknown chunk type '%02x'.", chunk.type());
+ LOG(WARNING) << base::StringPrintf("Unknown chunk type '%02x'.", chunk.type());
break;
}
}
- if (iter.HadError()) {
- LOG(ERROR) << iter.GetLastError();
- return {};
- }
-
- // Flatten and construct the TypeSpecs.
- for (auto& entry : type_builder_map) {
- uint8_t type_idx = static_cast<uint8_t>(entry.first);
- TypeSpecPtr type_spec_ptr = entry.second->Build();
+ // Finish the last TypeSpec.
+ if (types_builder) {
+ TypeSpecPtr type_spec_ptr = types_builder->Build();
if (type_spec_ptr == nullptr) {
LOG(ERROR) << "Too many type configurations, overflow detected.";
return {};
@@ -579,15 +592,20 @@
if (loaded_idmap == nullptr || type_spec_ptr->idmap_entries != nullptr) {
// If this is an overlay, insert it at the target type ID.
if (type_spec_ptr->idmap_entries != nullptr) {
- type_idx = dtohs(type_spec_ptr->idmap_entries->target_type_id) - 1;
+ last_type_idx = dtohs(type_spec_ptr->idmap_entries->target_type_id) - 1;
}
- loaded_package->type_specs_.editItemAt(type_idx) = std::move(type_spec_ptr);
+ loaded_package->type_specs_.editItemAt(last_type_idx) = std::move(type_spec_ptr);
}
}
+ if (iter.HadError()) {
+ LOG(ERROR) << iter.GetLastError();
+ return {};
+ }
return std::move(loaded_package);
}
+
bool LoadedArsc::LoadTable(const Chunk& chunk, const LoadedIdmap* loaded_idmap,
bool load_as_shared_library) {
ATRACE_CALL();
@@ -637,7 +655,7 @@
} break;
default:
- LOG(WARNING) << StringPrintf("Unknown chunk type '%02x'.", chunk.type());
+ LOG(WARNING) << base::StringPrintf("Unknown chunk type '%02x'.", chunk.type());
break;
}
}
@@ -669,7 +687,7 @@
break;
default:
- LOG(WARNING) << StringPrintf("Unknown chunk type '%02x'.", chunk.type());
+ LOG(WARNING) << base::StringPrintf("Unknown chunk type '%02x'.", chunk.type());
break;
}
}
diff --git a/libs/androidfw/tests/LoadedArsc_test.cpp b/libs/androidfw/tests/LoadedArsc_test.cpp
index cae632d..bedebd6 100644
--- a/libs/androidfw/tests/LoadedArsc_test.cpp
+++ b/libs/androidfw/tests/LoadedArsc_test.cpp
@@ -16,7 +16,6 @@
#include "androidfw/LoadedArsc.h"
-#include "android-base/file.h"
#include "androidfw/ResourceUtils.h"
#include "TestHelpers.h"
@@ -30,7 +29,6 @@
namespace libclient = com::android::libclient;
namespace sparse = com::android::sparse;
-using ::android::base::ReadFileToString;
using ::testing::Eq;
using ::testing::Ge;
using ::testing::IsNull;
@@ -179,46 +177,6 @@
ASSERT_THAT(LoadedPackage::GetEntry(type_spec->types[0], entry_index), NotNull());
}
-// AAPT(2) generates resource tables with chunks in a certain order. The rule is that
-// a RES_TABLE_TYPE_TYPE with id `i` must always be preceded by a RES_TABLE_TYPE_SPEC_TYPE with
-// id `i`. The RES_TABLE_TYPE_SPEC_TYPE does not need to be directly preceding, however.
-//
-// AAPT(2) generates something like:
-// RES_TABLE_TYPE_SPEC_TYPE id=1
-// RES_TABLE_TYPE_TYPE id=1
-// RES_TABLE_TYPE_SPEC_TYPE id=2
-// RES_TABLE_TYPE_TYPE id=2
-//
-// But the following is valid too:
-// RES_TABLE_TYPE_SPEC_TYPE id=1
-// RES_TABLE_TYPE_SPEC_TYPE id=2
-// RES_TABLE_TYPE_TYPE id=1
-// RES_TABLE_TYPE_TYPE id=2
-//
-TEST(LoadedArscTest, LoadOutOfOrderTypeSpecs) {
- std::string contents;
- ASSERT_TRUE(
- ReadFileFromZipToString(GetTestDataPath() + "/out_of_order_types/out_of_order_types.apk",
- "resources.arsc", &contents));
-
- std::unique_ptr<const LoadedArsc> loaded_arsc = LoadedArsc::Load(StringPiece(contents));
- ASSERT_THAT(loaded_arsc, NotNull());
-
- ASSERT_THAT(loaded_arsc->GetPackages(), SizeIs(1u));
- const auto& package = loaded_arsc->GetPackages()[0];
- ASSERT_THAT(package, NotNull());
-
- const TypeSpec* type_spec = package->GetTypeSpecByTypeIndex(0);
- ASSERT_THAT(type_spec, NotNull());
- ASSERT_THAT(type_spec->type_count, Ge(1u));
- ASSERT_THAT(type_spec->types[0], NotNull());
-
- type_spec = package->GetTypeSpecByTypeIndex(1);
- ASSERT_THAT(type_spec, NotNull());
- ASSERT_THAT(type_spec->type_count, Ge(1u));
- ASSERT_THAT(type_spec->types[0], NotNull());
-}
-
class MockLoadedIdmap : public LoadedIdmap {
public:
MockLoadedIdmap() : LoadedIdmap() {
diff --git a/libs/androidfw/tests/data/out_of_order_types/AndroidManifest.xml b/libs/androidfw/tests/data/out_of_order_types/AndroidManifest.xml
deleted file mode 100644
index 34016db..0000000
--- a/libs/androidfw/tests/data/out_of_order_types/AndroidManifest.xml
+++ /dev/null
@@ -1,18 +0,0 @@
-<?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="com.android.app" />
diff --git a/libs/androidfw/tests/data/out_of_order_types/build b/libs/androidfw/tests/data/out_of_order_types/build
deleted file mode 100755
index 8496f81..0000000
--- a/libs/androidfw/tests/data/out_of_order_types/build
+++ /dev/null
@@ -1,22 +0,0 @@
-#!/bin/bash
-#
-# 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.
-#
-
-set -e
-
-aapt2 compile --dir res -o compiled.flata
-aapt2 link --manifest AndroidManifest.xml -o out_of_order_types.apk compiled.flata
-rm compiled.flata
diff --git a/libs/androidfw/tests/data/out_of_order_types/edited_resources.arsc.txt b/libs/androidfw/tests/data/out_of_order_types/edited_resources.arsc.txt
deleted file mode 100644
index eca8f47..0000000
--- a/libs/androidfw/tests/data/out_of_order_types/edited_resources.arsc.txt
+++ /dev/null
@@ -1,43 +0,0 @@
-00000000: 0200 0c00 ac02 0000 0100 0000 0100 1c00 ................
-00000010: 1c00 0000 0000 0000 0000 0000 0001 0000 ................
-00000020: 1c00 0000 0000 0000 0002 2001 8402 0000 .......... .....
-00000030: 7f00 0000 6300 6f00 6d00 2e00 6100 6e00 ....c.o.m...a.n.
-00000040: 6400 7200 6f00 6900 6400 2e00 6100 7000 d.r.o.i.d...a.p.
-00000050: 7000 0000 0000 0000 0000 0000 0000 0000 p...............
-00000060: 0000 0000 0000 0000 0000 0000 0000 0000 ................
-00000070: 0000 0000 0000 0000 0000 0000 0000 0000 ................
-00000080: 0000 0000 0000 0000 0000 0000 0000 0000 ................
-00000090: 0000 0000 0000 0000 0000 0000 0000 0000 ................
-000000a0: 0000 0000 0000 0000 0000 0000 0000 0000 ................
-000000b0: 0000 0000 0000 0000 0000 0000 0000 0000 ................
-000000c0: 0000 0000 0000 0000 0000 0000 0000 0000 ................
-000000d0: 0000 0000 0000 0000 0000 0000 0000 0000 ................
-000000e0: 0000 0000 0000 0000 0000 0000 0000 0000 ................
-000000f0: 0000 0000 0000 0000 0000 0000 0000 0000 ................
-00000100: 0000 0000 0000 0000 0000 0000 0000 0000 ................
-00000110: 0000 0000 0000 0000 0000 0000 0000 0000 ................
-00000120: 0000 0000 0000 0000 0000 0000 0000 0000 ................
-00000130: 0000 0000 2001 0000 0000 0000 6401 0000 .... .......d...
-00000140: 0000 0000 0000 0000 0100 1c00 4400 0000 ............D...
-00000150: 0200 0000 0000 0000 0000 0000 2400 0000 ............$...
-00000160: 0000 0000 0000 0000 0c00 0000 0400 6200 ..............b.
-00000170: 6f00 6f00 6c00 0000 0700 6900 6e00 7400 o.o.l.....i.n.t.
-00000180: 6500 6700 6500 7200 0000 0000 0100 1c00 e.g.e.r.........
-00000190: 2800 0000 0100 0000 0000 0000 0001 0000 (...............
-000001a0: 2000 0000 0000 0000 0000 0000 0404 7465 .............te
-000001b0: 7374 0000 0202 1000 1400 0000 0100 0000 st..............
-000001c0: 0100 0000 0000 0000 0202 1000 1400 0000
-000001d0: 0200 0000 0100 0000 0000 0000 0102 5400
-000001e0: 6800 0000 0100 0000 0100 0000 5800 0000
-000001f0: 4000 0000 0000 0000 0000 0000 0000 0000
-00000200: 0000 0000 0000 0000 0000 0000 0000 0000
-00000210: 0000 0000 0000 0000 0000 0000 0000 0000
-00000220: 0000 0000 0000 0000 0000 0000 0000 0000
-00000230: 0000 0000 0800 0000 0000 0000 0800 0012
-00000240: ffff ffff 0102 5400 6800 0000 0200 0000 ......T.h.......
-00000250: 0100 0000 5800 0000 4000 0000 0000 0000 ....X...@.......
-00000260: 0000 0000 0000 0000 0000 0000 0000 0000 ................
-00000270: 0000 0000 0000 0000 0000 0000 0000 0000 ................
-00000280: 0000 0000 0000 0000 0000 0000 0000 0000 ................
-00000290: 0000 0000 0000 0000 0000 0000 0800 0000 ................
-000002a0: 0000 0000 0800 0010 0100 0000 ............
diff --git a/libs/androidfw/tests/data/out_of_order_types/out_of_order_types.apk b/libs/androidfw/tests/data/out_of_order_types/out_of_order_types.apk
deleted file mode 100644
index 75146e0..0000000
--- a/libs/androidfw/tests/data/out_of_order_types/out_of_order_types.apk
+++ /dev/null
Binary files differ
diff --git a/libs/androidfw/tests/data/out_of_order_types/res/values/values.xml b/libs/androidfw/tests/data/out_of_order_types/res/values/values.xml
deleted file mode 100644
index 7c54fba..0000000
--- a/libs/androidfw/tests/data/out_of_order_types/res/values/values.xml
+++ /dev/null
@@ -1,20 +0,0 @@
-<?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.
--->
-
-<resources>
- <bool name="test">true</bool>
- <integer name="test">1</integer>
-</resources>
