From c1676807f45a862256fb5b2804f15dbf4a90a11e Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Mon, 20 May 2019 16:47:08 -0700 Subject: Retain parsed attribute type If the value of an attribute enum is defined as a hexadecimal integer, flatten uses of the attribute as with the android::Res_value::TYPE_INT_HEX type. This change adds a "type" field to pb::Attribute::Symbol, which if left unset, will have a default value of android::Res_value::TYPE_INT_DEC when deserialized by aapt2. Bug: 124474141 Test: aapt2_tests and manual compilation of files and inspection using `aapt2 dump chunks` Change-Id: Ibf12394284fdbe3a8047f7ecf4fe68517dfc3abb --- tools/aapt2/ResourceUtils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/aapt2/ResourceUtils.cpp') diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp index e0040e486a23..bd2ab5377311 100644 --- a/tools/aapt2/ResourceUtils.cpp +++ b/tools/aapt2/ResourceUtils.cpp @@ -378,7 +378,7 @@ std::unique_ptr TryParseEnumSymbol(const Attribute* enum_attr, const ResourceName& enum_symbol_resource_name = symbol.symbol.name.value(); if (trimmed_str == enum_symbol_resource_name.entry) { android::Res_value value = {}; - value.dataType = android::Res_value::TYPE_INT_DEC; + value.dataType = symbol.type; value.data = symbol.value; return util::make_unique(value); } -- cgit v1.2.3-59-g8ed1b From 4abc82859dc7f7ed5f01aaadb360542ae8ca303c Mon Sep 17 00:00:00 2001 From: Donald Chai Date: Fri, 13 Dec 2019 23:23:07 -0800 Subject: Fix deserialization of @id aliases I didn't realize that this was actually a thing, but: @id/id1 leads to a resource table which 'aapt dump resources' will render as: resource 0x7f030000 com.pkg:id/id1: t=0x03 d=0x00000001 (s=0x0008 r=0x00) resource 0x7f030001 com.pkg:id/id2: t=0x01 d=0x7f030000 (s=0x0008 r=0x00) while 'aapt2 dump resources' printed: type id id=03 entryCount=2 resource 0x7f030000 id/id1 () (id) resource 0x7f030001 id/id2 () (id) After this change, it prints: type id id=03 entryCount=2 resource 0x7f030000 id/id1 () (id) resource 0x7f030001 id/id2 () @id/id1 Bug: 69445910 Change-Id: I0001ff09345434577ac4c1d32f96a781f4190d8d Tested: manual, see above --- tools/aapt2/ResourceUtils.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'tools/aapt2/ResourceUtils.cpp') diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp index bd2ab5377311..03009aaeead2 100644 --- a/tools/aapt2/ResourceUtils.cpp +++ b/tools/aapt2/ResourceUtils.cpp @@ -738,7 +738,13 @@ std::unique_ptr ParseBinaryResValue(const ResourceType& type, const Config const android::Res_value& res_value, StringPool* dst_pool) { if (type == ResourceType::kId) { - return util::make_unique(); + if (res_value.dataType != android::Res_value::TYPE_REFERENCE && + res_value.dataType != android::Res_value::TYPE_DYNAMIC_REFERENCE) { + // plain "id" resources are actually encoded as dummy values (aapt1 uses an empty string, + // while aapt2 uses a false boolean). + return util::make_unique(); + } + // fall through to regular reference deserialization logic } const uint32_t data = util::DeviceToHost32(res_value.data); -- cgit v1.2.3-59-g8ed1b From cd78febeac2956525931b1f0f15ad507bec6a24c Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Wed, 18 Dec 2019 15:20:48 -0800 Subject: Recognize dynamic res ids as valid Shared libraries are assigned package id 0. Resource ids that start with 0x00 are not invalid. This change changes is_valid to accept dynamic resource ids and adds an is_valid_static method for when an id must have a non-zero package id. This also fixes an issue that made layouts in shared libraries that use internal attributes exclude compiled resource ids from the binay xml output. Bug: 146491000 Test: Build a shared library with a layout that uses app attributes as well as android attribute and verify that all attributes have assigned resource ids using `aapt2 dump xmltree` Change-Id: Ibc0407c610ffc98d7aaf233c37c065912ab0d516 --- tools/aapt2/Resource.h | 9 +++++---- tools/aapt2/ResourceTable.cpp | 16 ++++++++-------- tools/aapt2/ResourceUtils.cpp | 2 +- tools/aapt2/ResourceValues.cpp | 6 +++--- tools/aapt2/process/SymbolTable.cpp | 4 ++-- 5 files changed, 19 insertions(+), 18 deletions(-) (limited to 'tools/aapt2/ResourceUtils.cpp') diff --git a/tools/aapt2/Resource.h b/tools/aapt2/Resource.h index 67ba895e51d1..c49c370bcc44 100644 --- a/tools/aapt2/Resource.h +++ b/tools/aapt2/Resource.h @@ -147,10 +147,11 @@ struct ResourceId { ResourceId(uint32_t res_id); // NOLINT(google-explicit-constructor) ResourceId(uint8_t p, uint8_t t, uint16_t e); - bool is_valid() const; + // Returns true if the ID is a valid ID that is not dynamic (package ID cannot be 0) + bool is_valid_static() const; // Returns true if the ID is a valid ID or dynamic ID (package ID can be 0). - bool is_valid_dynamic() const; + bool is_valid() const; uint8_t package_id() const; uint8_t type_id() const; @@ -233,11 +234,11 @@ inline ResourceId::ResourceId(uint32_t res_id) : id(res_id) {} inline ResourceId::ResourceId(uint8_t p, uint8_t t, uint16_t e) : id((p << 24) | (t << 16) | e) {} -inline bool ResourceId::is_valid() const { +inline bool ResourceId::is_valid_static() const { return (id & 0xff000000u) != 0 && (id & 0x00ff0000u) != 0; } -inline bool ResourceId::is_valid_dynamic() const { +inline bool ResourceId::is_valid() const { return (id & 0x00ff0000u) != 0; } diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp index 1773b5a8addf..e0a9a31eee8b 100644 --- a/tools/aapt2/ResourceTable.cpp +++ b/tools/aapt2/ResourceTable.cpp @@ -398,7 +398,7 @@ bool ResourceTable::AddResourceImpl(const ResourceNameRef& name, const ResourceI // Check for package names appearing twice with two different package ids ResourceTablePackage* package = FindOrCreatePackage(name.package); - if (res_id.is_valid_dynamic() && package->id && package->id.value() != res_id.package_id()) { + if (res_id.is_valid() && package->id && package->id.value() != res_id.package_id()) { diag->Error(DiagMessage(source) << "trying to add resource '" << name << "' with ID " << res_id << " but package '" << package->name << "' already has ID " @@ -407,9 +407,9 @@ bool ResourceTable::AddResourceImpl(const ResourceNameRef& name, const ResourceI } // Whether or not to error on duplicate resources - bool check_id = validate_resources_ && res_id.is_valid_dynamic(); + bool check_id = validate_resources_ && res_id.is_valid(); // Whether or not to create a duplicate resource if the id does not match - bool use_id = !validate_resources_ && res_id.is_valid_dynamic(); + bool use_id = !validate_resources_ && res_id.is_valid(); ResourceTableType* type = package->FindOrCreateType(name.type, use_id ? res_id.type_id() : Maybe()); @@ -463,7 +463,7 @@ bool ResourceTable::AddResourceImpl(const ResourceNameRef& name, const ResourceI } } - if (res_id.is_valid_dynamic()) { + if (res_id.is_valid()) { package->id = res_id.package_id(); type->id = res_id.type_id(); entry->id = res_id.entry_id(); @@ -504,7 +504,7 @@ bool ResourceTable::SetVisibilityImpl(const ResourceNameRef& name, const Visibil // Check for package names appearing twice with two different package ids ResourceTablePackage* package = FindOrCreatePackage(name.package); - if (res_id.is_valid_dynamic() && package->id && package->id.value() != res_id.package_id()) { + if (res_id.is_valid() && package->id && package->id.value() != res_id.package_id()) { diag->Error(DiagMessage(source) << "trying to add resource '" << name << "' with ID " << res_id << " but package '" << package->name << "' already has ID " @@ -513,9 +513,9 @@ bool ResourceTable::SetVisibilityImpl(const ResourceNameRef& name, const Visibil } // Whether or not to error on duplicate resources - bool check_id = validate_resources_ && res_id.is_valid_dynamic(); + bool check_id = validate_resources_ && res_id.is_valid(); // Whether or not to create a duplicate resource if the id does not match - bool use_id = !validate_resources_ && res_id.is_valid_dynamic(); + bool use_id = !validate_resources_ && res_id.is_valid(); ResourceTableType* type = package->FindOrCreateType(name.type, use_id ? res_id.type_id() : Maybe()); @@ -541,7 +541,7 @@ bool ResourceTable::SetVisibilityImpl(const ResourceNameRef& name, const Visibil return false; } - if (res_id.is_valid_dynamic()) { + if (res_id.is_valid()) { package->id = res_id.package_id(); type->id = res_id.type_id(); entry->id = res_id.entry_id(); diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp index 03009aaeead2..3623b1112bc6 100644 --- a/tools/aapt2/ResourceUtils.cpp +++ b/tools/aapt2/ResourceUtils.cpp @@ -516,7 +516,7 @@ Maybe ParseResourceId(const StringPiece& str) { if (android::ResTable::stringToInt(str16.data(), str16.size(), &value)) { if (value.dataType == android::Res_value::TYPE_INT_HEX) { ResourceId id(value.data); - if (id.is_valid_dynamic()) { + if (id.is_valid()) { return id; } } diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp index 34b46c552e0c..4f0fa8ae29ba 100644 --- a/tools/aapt2/ResourceValues.cpp +++ b/tools/aapt2/ResourceValues.cpp @@ -117,7 +117,7 @@ bool Reference::Equals(const Value* value) const { bool Reference::Flatten(android::Res_value* out_value) const { const ResourceId resid = id.value_or_default(ResourceId(0)); - const bool dynamic = resid.is_valid_dynamic() && is_dynamic; + const bool dynamic = resid.is_valid() && is_dynamic; if (reference_type == Reference::Type::kResource) { if (dynamic) { @@ -159,7 +159,7 @@ void Reference::Print(std::ostream* out) const { *out << name.value(); } - if (id && id.value().is_valid_dynamic()) { + if (id && id.value().is_valid()) { if (name) { *out << " "; } @@ -196,7 +196,7 @@ static void PrettyPrintReferenceImpl(const Reference& ref, bool print_package, P printer->Print("/"); printer->Print(name.entry); } - } else if (ref.id && ref.id.value().is_valid_dynamic()) { + } else if (ref.id && ref.id.value().is_valid()) { printer->Print(ref.id.value().to_string()); } } diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp index bc09f19b94d5..83e20b5833b9 100644 --- a/tools/aapt2/process/SymbolTable.cpp +++ b/tools/aapt2/process/SymbolTable.cpp @@ -340,7 +340,7 @@ std::unique_ptr AssetManagerSymbolSource::FindByName( } res_id = asset_manager_.GetResourceId(real_name.to_string()); - if (res_id.is_valid() && asset_manager_.GetResourceFlags(res_id.id, &type_spec_flags)) { + if (res_id.is_valid_static() && asset_manager_.GetResourceFlags(res_id.id, &type_spec_flags)) { found = true; return false; } @@ -379,7 +379,7 @@ static Maybe GetResourceName(android::AssetManager2& am, std::unique_ptr AssetManagerSymbolSource::FindById( ResourceId id) { - if (!id.is_valid()) { + if (!id.is_valid_static()) { // Exit early and avoid the error logs from AssetManager. return {}; } -- cgit v1.2.3-59-g8ed1b From e9fedbe223bc1eb4498e375f4714ca73444bd1ce Mon Sep 17 00:00:00 2001 From: Clark DuVall Date: Thu, 9 Jan 2020 11:52:52 -0800 Subject: Fix serializing dynamic references to proto The is_dynamic bit on references was getting lost when converting to proto and back. This was preventing bundles from being used as shared libraries, since layout inflation would fail when it hit a dynamic reference. Bug: 146491000 Test: Build a shared library with a layout that has a dynamic reference, and attempt to inflate that layout. Change-Id: Ia0e615670d2ac52f9266e3eec8813af7687d3323 --- tools/aapt2/ResourceUtils.cpp | 7 ++++- tools/aapt2/ResourceUtils_test.cpp | 14 +++++++++ tools/aapt2/Resources.proto | 8 +++++ tools/aapt2/format/proto/ProtoDeserialize.cpp | 1 + tools/aapt2/format/proto/ProtoSerialize.cpp | 3 ++ tools/aapt2/format/proto/ProtoSerialize_test.cpp | 37 ++++++++++++++++++++++++ 6 files changed, 69 insertions(+), 1 deletion(-) (limited to 'tools/aapt2/ResourceUtils.cpp') diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp index 3623b1112bc6..469128b1e50b 100644 --- a/tools/aapt2/ResourceUtils.cpp +++ b/tools/aapt2/ResourceUtils.cpp @@ -800,7 +800,12 @@ std::unique_ptr ParseBinaryResValue(const ResourceType& type, const Config } // This is a normal reference. - return util::make_unique(data, ref_type); + auto reference = util::make_unique(data, ref_type); + if (res_value.dataType == android::Res_value::TYPE_DYNAMIC_REFERENCE || + res_value.dataType == android::Res_value::TYPE_DYNAMIC_ATTRIBUTE) { + reference->is_dynamic = true; + } + return reference; } break; } diff --git a/tools/aapt2/ResourceUtils_test.cpp b/tools/aapt2/ResourceUtils_test.cpp index c016cb44af00..b08bf9a1ff17 100644 --- a/tools/aapt2/ResourceUtils_test.cpp +++ b/tools/aapt2/ResourceUtils_test.cpp @@ -109,6 +109,20 @@ TEST(ResourceUtilsTest, ParsePrivateReference) { EXPECT_TRUE(private_ref); } +TEST(ResourceUtilsTest, ParseBinaryDynamicReference) { + android::Res_value value = {}; + value.data = util::HostToDevice32(0x01); + value.dataType = android::Res_value::TYPE_DYNAMIC_REFERENCE; + std::unique_ptr item = ResourceUtils::ParseBinaryResValue(ResourceType::kId, + android::ConfigDescription(), + android::ResStringPool(), value, + nullptr); + + Reference* ref = ValueCast(item.get()); + EXPECT_TRUE(ref->is_dynamic); + EXPECT_EQ(ref->id.value().id, 0x01); +} + TEST(ResourceUtilsTest, FailToParseAutoCreateNonIdReference) { bool create = false; bool private_ref = false; diff --git a/tools/aapt2/Resources.proto b/tools/aapt2/Resources.proto index 7498e132d943..8a2f5afa7255 100644 --- a/tools/aapt2/Resources.proto +++ b/tools/aapt2/Resources.proto @@ -269,6 +269,11 @@ message CompoundValue { } } +// Message holding a boolean, so it can be optionally encoded. +message Boolean { + bool value = 1; +} + // A value that is a reference to another resource. This reference can be by name or resource ID. message Reference { enum Type { @@ -289,6 +294,9 @@ message Reference { // Whether this reference is referencing a private resource (@*package:type/entry). bool private = 4; + + // Whether this reference is dynamic. + Boolean is_dynamic = 5; } // A value that represents an ID. This is just a placeholder, as ID values are used to occupy a diff --git a/tools/aapt2/format/proto/ProtoDeserialize.cpp b/tools/aapt2/format/proto/ProtoDeserialize.cpp index efbf636878f2..4cd6e930915d 100644 --- a/tools/aapt2/format/proto/ProtoDeserialize.cpp +++ b/tools/aapt2/format/proto/ProtoDeserialize.cpp @@ -634,6 +634,7 @@ static bool DeserializeReferenceFromPb(const pb::Reference& pb_ref, Reference* o std::string* out_error) { out_ref->reference_type = DeserializeReferenceTypeFromPb(pb_ref.type()); out_ref->private_reference = pb_ref.private_(); + out_ref->is_dynamic = pb_ref.is_dynamic().value(); if (pb_ref.id() != 0) { out_ref->id = ResourceId(pb_ref.id()); diff --git a/tools/aapt2/format/proto/ProtoSerialize.cpp b/tools/aapt2/format/proto/ProtoSerialize.cpp index e4b3fce52166..d9f6c193fc2f 100644 --- a/tools/aapt2/format/proto/ProtoSerialize.cpp +++ b/tools/aapt2/format/proto/ProtoSerialize.cpp @@ -418,6 +418,9 @@ static void SerializeReferenceToPb(const Reference& ref, pb::Reference* pb_ref) pb_ref->set_private_(ref.private_reference); pb_ref->set_type(SerializeReferenceTypeToPb(ref.reference_type)); + if (ref.is_dynamic) { + pb_ref->mutable_is_dynamic()->set_value(ref.is_dynamic); + } } template diff --git a/tools/aapt2/format/proto/ProtoSerialize_test.cpp b/tools/aapt2/format/proto/ProtoSerialize_test.cpp index e7f23302652c..61a8335e17a7 100644 --- a/tools/aapt2/format/proto/ProtoSerialize_test.cpp +++ b/tools/aapt2/format/proto/ProtoSerialize_test.cpp @@ -608,4 +608,41 @@ TEST(ProtoSerializeTest, SerializeAndDeserializeOverlayable) { ASSERT_FALSE(search_result.value().entry->overlayable_item); } +TEST(ProtoSerializeTest, SerializeAndDeserializeDynamicReference) { + Reference ref(ResourceId(0x00010001)); + ref.is_dynamic = true; + + pb::Item pb_item; + SerializeItemToPb(ref, &pb_item); + + ASSERT_TRUE(pb_item.has_ref()); + EXPECT_EQ(pb_item.ref().id(), ref.id.value().id); + EXPECT_TRUE(pb_item.ref().is_dynamic().value()); + + std::unique_ptr item = DeserializeItemFromPb(pb_item, android::ResStringPool(), + android::ConfigDescription(), nullptr, + nullptr, nullptr); + Reference* actual_ref = ValueCast(item.get()); + EXPECT_EQ(actual_ref->id.value().id, ref.id.value().id); + EXPECT_TRUE(actual_ref->is_dynamic); +} + +TEST(ProtoSerializeTest, SerializeAndDeserializeNonDynamicReference) { + Reference ref(ResourceId(0x00010001)); + + pb::Item pb_item; + SerializeItemToPb(ref, &pb_item); + + ASSERT_TRUE(pb_item.has_ref()); + EXPECT_EQ(pb_item.ref().id(), ref.id.value().id); + EXPECT_FALSE(pb_item.ref().has_is_dynamic()); + + std::unique_ptr item = DeserializeItemFromPb(pb_item, android::ResStringPool(), + android::ConfigDescription(), nullptr, + nullptr, nullptr); + Reference* actual_ref = ValueCast(item.get()); + EXPECT_EQ(actual_ref->id.value().id, ref.id.value().id); + EXPECT_FALSE(actual_ref->is_dynamic); +} + } // namespace aapt -- cgit v1.2.3-59-g8ed1b