diff options
-rw-r--r-- | libs/androidfw/ResourceTypes.cpp | 94 | ||||
-rw-r--r-- | libs/androidfw/include/androidfw/ResourceTypes.h | 5 | ||||
-rw-r--r-- | tools/aapt2/Resource.h | 2 | ||||
-rw-r--r-- | tools/aapt2/ResourceTable.cpp | 9 | ||||
-rw-r--r-- | tools/aapt2/ResourceTable.h | 5 | ||||
-rw-r--r-- | tools/aapt2/cmd/Link.cpp | 12 | ||||
-rw-r--r-- | tools/aapt2/format/binary/BinaryResourceParser.cpp | 2 | ||||
-rw-r--r-- | tools/aapt2/format/binary/ResEntryWriter.cpp | 4 | ||||
-rw-r--r-- | tools/aapt2/format/binary/ResEntryWriter.h | 2 | ||||
-rw-r--r-- | tools/aapt2/format/binary/TableFlattener.cpp | 3 | ||||
-rw-r--r-- | tools/aapt2/format/binary/TableFlattener_test.cpp | 19 | ||||
-rw-r--r-- | tools/aapt2/link/FlaggedResources_test.cpp | 48 | ||||
-rw-r--r-- | tools/aapt2/link/FlaggedXmlVersioner.cpp | 27 |
13 files changed, 198 insertions, 34 deletions
diff --git a/libs/androidfw/ResourceTypes.cpp b/libs/androidfw/ResourceTypes.cpp index a18c5f5f92f6..8ecd6ba9b253 100644 --- a/libs/androidfw/ResourceTypes.cpp +++ b/libs/androidfw/ResourceTypes.cpp @@ -6520,41 +6520,79 @@ base::expected<StringPiece16, NullOrIOError> StringPoolRef::string16() const { } bool ResTable::getResourceFlags(uint32_t resID, uint32_t* outFlags) const { - if (mError != NO_ERROR) { - return false; - } + if (mError != NO_ERROR) { + return false; + } - const ssize_t p = getResourcePackageIndex(resID); - const int t = Res_GETTYPE(resID); - const int e = Res_GETENTRY(resID); + const ssize_t p = getResourcePackageIndex(resID); + const int t = Res_GETTYPE(resID); + const int e = Res_GETENTRY(resID); - if (p < 0) { - if (Res_GETPACKAGE(resID)+1 == 0) { - ALOGW("No package identifier when getting flags for resource number 0x%08x", resID); - } else { - ALOGW("No known package when getting flags for resource number 0x%08x", resID); - } - return false; - } - if (t < 0) { - ALOGW("No type identifier when getting flags for resource number 0x%08x", resID); - return false; + if (p < 0) { + if (Res_GETPACKAGE(resID)+1 == 0) { + ALOGW("No package identifier when getting flags for resource number 0x%08x", resID); + } else { + ALOGW("No known package when getting flags for resource number 0x%08x", resID); } + return false; + } + if (t < 0) { + ALOGW("No type identifier when getting flags for resource number 0x%08x", resID); + return false; + } - const PackageGroup* const grp = mPackageGroups[p]; - if (grp == NULL) { - ALOGW("Bad identifier when getting flags for resource number 0x%08x", resID); - return false; - } + const PackageGroup* const grp = mPackageGroups[p]; + if (grp == NULL) { + ALOGW("Bad identifier when getting flags for resource number 0x%08x", resID); + return false; + } - Entry entry; - status_t err = getEntry(grp, t, e, NULL, &entry); - if (err != NO_ERROR) { - return false; + Entry entry; + status_t err = getEntry(grp, t, e, NULL, &entry); + if (err != NO_ERROR) { + return false; + } + + *outFlags = entry.specFlags; + return true; +} + +bool ResTable::getResourceEntryFlags(uint32_t resID, uint32_t* outFlags) const { + if (mError != NO_ERROR) { + return false; + } + + const ssize_t p = getResourcePackageIndex(resID); + const int t = Res_GETTYPE(resID); + const int e = Res_GETENTRY(resID); + + if (p < 0) { + if (Res_GETPACKAGE(resID)+1 == 0) { + ALOGW("No package identifier when getting flags for resource number 0x%08x", resID); + } else { + ALOGW("No known package when getting flags for resource number 0x%08x", resID); } + return false; + } + if (t < 0) { + ALOGW("No type identifier when getting flags for resource number 0x%08x", resID); + return false; + } - *outFlags = entry.specFlags; - return true; + const PackageGroup* const grp = mPackageGroups[p]; + if (grp == NULL) { + ALOGW("Bad identifier when getting flags for resource number 0x%08x", resID); + return false; + } + + Entry entry; + status_t err = getEntry(grp, t, e, NULL, &entry); + if (err != NO_ERROR) { + return false; + } + + *outFlags = entry.entry->flags(); + return true; } bool ResTable::isPackageDynamic(uint8_t packageID) const { diff --git a/libs/androidfw/include/androidfw/ResourceTypes.h b/libs/androidfw/include/androidfw/ResourceTypes.h index 0d45149267cf..63b28da075cd 100644 --- a/libs/androidfw/include/androidfw/ResourceTypes.h +++ b/libs/androidfw/include/androidfw/ResourceTypes.h @@ -1593,6 +1593,8 @@ union ResTable_entry // If set, this is a compact entry with data type and value directly // encoded in the this entry, see ResTable_entry::compact FLAG_COMPACT = 0x0008, + // If set, this entry relies on read write android feature flags + FLAG_USES_FEATURE_FLAGS = 0x0010, }; struct Full { @@ -1622,6 +1624,7 @@ union ResTable_entry uint16_t flags() const { return dtohs(full.flags); }; bool is_compact() const { return flags() & FLAG_COMPACT; } bool is_complex() const { return flags() & FLAG_COMPLEX; } + bool uses_feature_flags() const { return flags() & FLAG_USES_FEATURE_FLAGS; } size_t size() const { return is_compact() ? sizeof(ResTable_entry) : dtohs(this->full.size); @@ -2039,6 +2042,8 @@ public: bool getResourceFlags(uint32_t resID, uint32_t* outFlags) const; + bool getResourceEntryFlags(uint32_t resID, uint32_t* outFlags) const; + /** * Returns whether or not the package for the given resource has been dynamically assigned. * If the resource can't be found, returns 'false'. diff --git a/tools/aapt2/Resource.h b/tools/aapt2/Resource.h index 0d261abd728d..e51477c668dd 100644 --- a/tools/aapt2/Resource.h +++ b/tools/aapt2/Resource.h @@ -249,6 +249,8 @@ struct ResourceFile { // Flag std::optional<FeatureFlagAttribute> flag; + + bool uses_readwrite_feature_flags = false; }; /** diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp index 5435cba290fc..db7dddc49a99 100644 --- a/tools/aapt2/ResourceTable.cpp +++ b/tools/aapt2/ResourceTable.cpp @@ -664,6 +664,7 @@ bool ResourceTable::AddResource(NewResource&& res, android::IDiagnostics* diag) if (!config_value->value) { // Resource does not exist, add it now. config_value->value = std::move(res.value); + config_value->uses_readwrite_feature_flags = res.uses_readwrite_feature_flags; } else { // When validation is enabled, ensure that a resource cannot have multiple values defined for // the same configuration unless protected by flags. @@ -681,12 +682,14 @@ bool ResourceTable::AddResource(NewResource&& res, android::IDiagnostics* diag) ConfigKey{&res.config, res.product}, lt_config_key_ref()), util::make_unique<ResourceConfigValue>(res.config, res.product)); (*it)->value = std::move(res.value); + (*it)->uses_readwrite_feature_flags = res.uses_readwrite_feature_flags; break; } case CollisionResult::kTakeNew: // Take the incoming value. config_value->value = std::move(res.value); + config_value->uses_readwrite_feature_flags = res.uses_readwrite_feature_flags; break; case CollisionResult::kConflict: @@ -843,6 +846,12 @@ NewResourceBuilder& NewResourceBuilder::SetAllowMangled(bool allow_mangled) { return *this; } +NewResourceBuilder& NewResourceBuilder::SetUsesReadWriteFeatureFlags( + bool uses_readwrite_feature_flags) { + res_.uses_readwrite_feature_flags = uses_readwrite_feature_flags; + return *this; +} + NewResource NewResourceBuilder::Build() { return std::move(res_); } diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h index b0e185536d16..778b43adb50b 100644 --- a/tools/aapt2/ResourceTable.h +++ b/tools/aapt2/ResourceTable.h @@ -104,6 +104,9 @@ class ResourceConfigValue { // The actual Value. std::unique_ptr<Value> value; + // Whether the value uses read/write feature flags + bool uses_readwrite_feature_flags = false; + ResourceConfigValue(const android::ConfigDescription& config, android::StringPiece product) : config(config), product(product) { } @@ -284,6 +287,7 @@ struct NewResource { std::optional<AllowNew> allow_new; std::optional<StagedId> staged_id; bool allow_mangled = false; + bool uses_readwrite_feature_flags = false; }; struct NewResourceBuilder { @@ -297,6 +301,7 @@ struct NewResourceBuilder { NewResourceBuilder& SetAllowNew(AllowNew allow_new); NewResourceBuilder& SetStagedId(StagedId id); NewResourceBuilder& SetAllowMangled(bool allow_mangled); + NewResourceBuilder& SetUsesReadWriteFeatureFlags(bool uses_feature_flags); NewResource Build(); private: diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp index 2a7921600477..755dbb6f8e42 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -673,11 +673,13 @@ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archiv // Update the output format of this XML file. file_ref->type = XmlFileTypeForOutputFormat(options_.output_format); - bool result = table->AddResource(NewResourceBuilder(file.name) - .SetValue(std::move(file_ref), file.config) - .SetAllowMangled(true) - .Build(), - context_->GetDiagnostics()); + bool result = table->AddResource( + NewResourceBuilder(file.name) + .SetValue(std::move(file_ref), file.config) + .SetAllowMangled(true) + .SetUsesReadWriteFeatureFlags(doc->file.uses_readwrite_feature_flags) + .Build(), + context_->GetDiagnostics()); if (!result) { return false; } diff --git a/tools/aapt2/format/binary/BinaryResourceParser.cpp b/tools/aapt2/format/binary/BinaryResourceParser.cpp index 2e20e8175213..bac871b8bdc3 100644 --- a/tools/aapt2/format/binary/BinaryResourceParser.cpp +++ b/tools/aapt2/format/binary/BinaryResourceParser.cpp @@ -414,6 +414,8 @@ bool BinaryResourceParser::ParseType(const ResourceTablePackage* package, .SetId(res_id, OnIdConflict::CREATE_ENTRY) .SetAllowMangled(true); + res_builder.SetUsesReadWriteFeatureFlags(entry->uses_feature_flags()); + if (entry->flags() & ResTable_entry::FLAG_PUBLIC) { Visibility visibility{Visibility::Level::kPublic}; diff --git a/tools/aapt2/format/binary/ResEntryWriter.cpp b/tools/aapt2/format/binary/ResEntryWriter.cpp index 9dc205f4c1ba..0be392164453 100644 --- a/tools/aapt2/format/binary/ResEntryWriter.cpp +++ b/tools/aapt2/format/binary/ResEntryWriter.cpp @@ -199,6 +199,10 @@ void WriteEntry(const FlatEntry* entry, T* out_result, bool compact = false) { flags |= ResTable_entry::FLAG_WEAK; } + if (entry->uses_readwrite_feature_flags) { + flags |= ResTable_entry::FLAG_USES_FEATURE_FLAGS; + } + if constexpr (std::is_same_v<ResTable_entry_ext, T>) { flags |= ResTable_entry::FLAG_COMPLEX; } diff --git a/tools/aapt2/format/binary/ResEntryWriter.h b/tools/aapt2/format/binary/ResEntryWriter.h index c11598ec12f7..f54b29aa8f2a 100644 --- a/tools/aapt2/format/binary/ResEntryWriter.h +++ b/tools/aapt2/format/binary/ResEntryWriter.h @@ -38,6 +38,8 @@ struct FlatEntry { // The entry string pool index to the entry's name. uint32_t entry_key; + + bool uses_readwrite_feature_flags; }; // Pair of ResTable_entry and Res_value. These pairs are stored sequentially in values buffer. diff --git a/tools/aapt2/format/binary/TableFlattener.cpp b/tools/aapt2/format/binary/TableFlattener.cpp index 1a82021bce71..50144ae816b6 100644 --- a/tools/aapt2/format/binary/TableFlattener.cpp +++ b/tools/aapt2/format/binary/TableFlattener.cpp @@ -502,7 +502,8 @@ class PackageFlattener { // Group values by configuration. for (auto& config_value : entry.values) { config_to_entry_list_map[config_value->config].push_back( - FlatEntry{&entry, config_value->value.get(), local_key_index}); + FlatEntry{&entry, config_value->value.get(), local_key_index, + config_value->uses_readwrite_feature_flags}); } } diff --git a/tools/aapt2/format/binary/TableFlattener_test.cpp b/tools/aapt2/format/binary/TableFlattener_test.cpp index 0f1168514c4a..9156b96b67ec 100644 --- a/tools/aapt2/format/binary/TableFlattener_test.cpp +++ b/tools/aapt2/format/binary/TableFlattener_test.cpp @@ -1069,4 +1069,23 @@ TEST_F(TableFlattenerTest, FlattenTypeEntryWithNameCollapseInExemption) { testing::IsTrue()); } +TEST_F(TableFlattenerTest, UsesReadWriteFeatureFlagSerializesCorrectly) { + std::unique_ptr<ResourceTable> table = + test::ResourceTableBuilder() + .Add(NewResourceBuilder("com.app.a:color/foo") + .SetValue(util::make_unique<BinaryPrimitive>( + uint8_t(Res_value::TYPE_INT_COLOR_ARGB8), 0xffaabbcc)) + .SetUsesReadWriteFeatureFlags(true) + .SetId(0x7f020000) + .Build()) + .Build(); + ResTable res_table; + TableFlattenerOptions options; + ASSERT_TRUE(Flatten(context_.get(), options, table.get(), &res_table)); + + uint32_t flags; + ASSERT_TRUE(res_table.getResourceEntryFlags(0x7f020000, &flags)); + ASSERT_EQ(flags, ResTable_entry::FLAG_USES_FEATURE_FLAGS); +} + } // namespace aapt diff --git a/tools/aapt2/link/FlaggedResources_test.cpp b/tools/aapt2/link/FlaggedResources_test.cpp index dbef77615515..47a71fe36e9f 100644 --- a/tools/aapt2/link/FlaggedResources_test.cpp +++ b/tools/aapt2/link/FlaggedResources_test.cpp @@ -14,6 +14,9 @@ * limitations under the License. */ +#include <regex> +#include <string> + #include "LoadedApk.h" #include "cmd/Dump.h" #include "io/StringStream.h" @@ -183,4 +186,49 @@ TEST_F(FlaggedResourcesTest, ReadWriteFlagInPathFails) { "Only read only flags may be used with resources: test.package.rwFlag")); } +TEST_F(FlaggedResourcesTest, ReadWriteFlagInXmlGetsFlagged) { + auto apk_path = file::BuildPath({android::base::GetExecutableDirectory(), "resapp.apk"}); + auto loaded_apk = LoadedApk::LoadApkFromPath(apk_path, &noop_diag); + + std::string output; + DumpChunksToString(loaded_apk.get(), &output); + + // The actual line looks something like: + // [ResTable_entry] id: 0x0000 name: layout1 keyIndex: 14 size: 8 flags: 0x0010 + // + // This regex matches that line and captures the name and the flag value for checking. + std::regex regex("[0-9a-zA-Z:_\\]\\[ ]+name: ([0-9a-zA-Z]+)[0-9a-zA-Z: ]+flags: (0x\\d{4})"); + std::smatch match; + + std::stringstream ss(output); + std::string line; + bool found = false; + int fields_flagged = 0; + while (std::getline(ss, line)) { + bool first_line = false; + if (line.contains("config: v36")) { + std::getline(ss, line); + first_line = true; + } + if (!line.contains("flags")) { + continue; + } + if (std::regex_search(line, match, regex) && (match.size() == 3)) { + unsigned int hex_value; + std::stringstream hex_ss; + hex_ss << std::hex << match[2]; + hex_ss >> hex_value; + if (hex_value & android::ResTable_entry::FLAG_USES_FEATURE_FLAGS) { + fields_flagged++; + if (first_line && match[1] == "layout1") { + found = true; + } + } + } + } + ASSERT_TRUE(found) << "No entry for layout1 at v36 with FLAG_USES_FEATURE_FLAGS bit set"; + // There should only be 1 entry that has the FLAG_USES_FEATURE_FLAGS bit of flags set to 1 + ASSERT_EQ(fields_flagged, 1); +} + } // namespace aapt diff --git a/tools/aapt2/link/FlaggedXmlVersioner.cpp b/tools/aapt2/link/FlaggedXmlVersioner.cpp index 75c6f17dcb51..8a3337c446cb 100644 --- a/tools/aapt2/link/FlaggedXmlVersioner.cpp +++ b/tools/aapt2/link/FlaggedXmlVersioner.cpp @@ -66,6 +66,28 @@ class AllDisabledFlagsVisitor : public xml::Visitor { bool had_flags_ = false; }; +// An xml visitor that goes through the a doc and determines if any elements are behind a flag. +class FindFlagsVisitor : public xml::Visitor { + public: + void Visit(xml::Element* node) override { + if (had_flags_) { + return; + } + auto* attr = node->FindAttribute(xml::kSchemaAndroid, xml::kAttrFeatureFlag); + if (attr != nullptr) { + had_flags_ = true; + return; + } + VisitChildren(node); + } + + bool HadFlags() const { + return had_flags_; + } + + bool had_flags_ = false; +}; + std::vector<std::unique_ptr<xml::XmlResource>> FlaggedXmlVersioner::Process(IAaptContext* context, xml::XmlResource* doc) { std::vector<std::unique_ptr<xml::XmlResource>> docs; @@ -74,15 +96,20 @@ std::vector<std::unique_ptr<xml::XmlResource>> FlaggedXmlVersioner::Process(IAap // Support for read/write flags was added in baklava so if the doc will only get used on // baklava or later we can just return the original doc. docs.push_back(doc->Clone()); + FindFlagsVisitor visitor; + doc->root->Accept(&visitor); + docs.back()->file.uses_readwrite_feature_flags = visitor.HadFlags(); } else { auto preBaklavaVersion = doc->Clone(); AllDisabledFlagsVisitor visitor; preBaklavaVersion->root->Accept(&visitor); + preBaklavaVersion->file.uses_readwrite_feature_flags = false; docs.push_back(std::move(preBaklavaVersion)); if (visitor.HadFlags()) { auto baklavaVersion = doc->Clone(); baklavaVersion->file.config.sdkVersion = SDK_BAKLAVA; + baklavaVersion->file.uses_readwrite_feature_flags = true; docs.push_back(std::move(baklavaVersion)); } } |