From 93ff40f24d4054bbbaa5375ff2eae5f38e284d73 Mon Sep 17 00:00:00 2001 From: Yurii Zubrytskyi Date: Wed, 27 Nov 2024 14:13:35 -0800 Subject: [res] Make TargetResourcesContainer valid after an error ApkResourcesContainer class has a variant that is either a zip file object, or a full blown AssetManager with ApkAssets with the same zip file inside. But if transition from one to the other fails, it could end up in a state with neither, causing a crash if the users try accessing it. This change ensures that: 1. The zip file object is only moved into ApkAssets if the loading succeeds 2. If any part of the further initialization of ResState fails, we take the zip file out and put it back into the variant + Add unit tests for both ApkAssets and libidmap2 to ensure they don't crash in any of those scenarios (they used to) + Enable root tests in libidmap2 as they were never running Flag: EXEMPT bugfix Bug: 381108280 Test: atest libandroidfw_tests idmap2_tests + boot Change-Id: I8f4803fdf03a41ba7a6892e6aed07f04b77788ce --- libs/androidfw/ApkAssets.cpp | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) (limited to 'libs/androidfw/ApkAssets.cpp') diff --git a/libs/androidfw/ApkAssets.cpp b/libs/androidfw/ApkAssets.cpp index 49254d1c6f6e..dbb891455ddd 100644 --- a/libs/androidfw/ApkAssets.cpp +++ b/libs/androidfw/ApkAssets.cpp @@ -40,20 +40,21 @@ ApkAssets::ApkAssets(PrivateConstructorUtil, std::unique_ptr resources_as } ApkAssetsPtr ApkAssets::Load(const std::string& path, package_property_t flags) { - return Load(ZipAssetsProvider::Create(path, flags), flags); + return LoadImpl(ZipAssetsProvider::Create(path, flags), flags); } ApkAssetsPtr ApkAssets::LoadFromFd(base::unique_fd fd, const std::string& debug_name, package_property_t flags, off64_t offset, off64_t len) { - return Load(ZipAssetsProvider::Create(std::move(fd), debug_name, offset, len), flags); + return LoadImpl(ZipAssetsProvider::Create(std::move(fd), debug_name, offset, len), flags); } -ApkAssetsPtr ApkAssets::Load(std::unique_ptr assets, package_property_t flags) { +ApkAssetsPtr ApkAssets::LoadImpl(std::unique_ptr&& assets, + package_property_t flags) { return LoadImpl(std::move(assets), flags, nullptr /* idmap_asset */, nullptr /* loaded_idmap */); } -ApkAssetsPtr ApkAssets::LoadTable(std::unique_ptr resources_asset, - std::unique_ptr assets, +ApkAssetsPtr ApkAssets::LoadTable(std::unique_ptr&& resources_asset, + std::unique_ptr&& assets, package_property_t flags) { if (resources_asset == nullptr) { return {}; @@ -97,10 +98,10 @@ ApkAssetsPtr ApkAssets::LoadOverlay(const std::string& idmap_path, package_prope std::move(loaded_idmap)); } -ApkAssetsPtr ApkAssets::LoadImpl(std::unique_ptr assets, +ApkAssetsPtr ApkAssets::LoadImpl(std::unique_ptr&& assets, package_property_t property_flags, - std::unique_ptr idmap_asset, - std::unique_ptr loaded_idmap) { + std::unique_ptr&& idmap_asset, + std::unique_ptr&& loaded_idmap) { if (assets == nullptr) { return {}; } @@ -119,11 +120,11 @@ ApkAssetsPtr ApkAssets::LoadImpl(std::unique_ptr assets, std::move(idmap_asset), std::move(loaded_idmap)); } -ApkAssetsPtr ApkAssets::LoadImpl(std::unique_ptr resources_asset, - std::unique_ptr assets, +ApkAssetsPtr ApkAssets::LoadImpl(std::unique_ptr&& resources_asset, + std::unique_ptr&& assets, package_property_t property_flags, - std::unique_ptr idmap_asset, - std::unique_ptr loaded_idmap) { + std::unique_ptr&& idmap_asset, + std::unique_ptr&& loaded_idmap) { if (assets == nullptr ) { return {}; } -- cgit v1.2.3-59-g8ed1b