AAPT2: Fix issue with deserializing binary XML
We assumed that a raw text value set for an attribute meant there
were no compiled values set either.
This would only really happen for attributes that did not belong to any
namespace (no prefix:), since we always kept their raw string values
in case some code relied on it.
Bug: 72700446
Test: make aapt2_tests
Change-Id: Icba40a1d4b181bfe7cad73131c4dbe5ba7f8b085
diff --git a/tools/aapt2/Debug.cpp b/tools/aapt2/Debug.cpp
index 249557a..f064cb1 100644
--- a/tools/aapt2/Debug.cpp
+++ b/tools/aapt2/Debug.cpp
@@ -450,7 +450,15 @@
if (attr.compiled_value != nullptr) {
attr.compiled_value->PrettyPrint(printer_);
} else {
+ printer_->Print("\"");
printer_->Print(attr.value);
+ printer_->Print("\"");
+ }
+
+ if (!attr.value.empty()) {
+ printer_->Print(" (Raw: \"");
+ printer_->Print(attr.value);
+ printer_->Print("\")");
}
printer_->Println();
}
diff --git a/tools/aapt2/cmd/Convert.cpp b/tools/aapt2/cmd/Convert.cpp
index d80307c..7f956c5 100644
--- a/tools/aapt2/cmd/Convert.cpp
+++ b/tools/aapt2/cmd/Convert.cpp
@@ -139,6 +139,7 @@
BigBuffer buffer(4096);
XmlFlattenerOptions options = {};
options.use_utf16 = utf16;
+ options.keep_raw_values = true;
XmlFlattener flattener(&buffer, options);
if (!flattener.Consume(context_, xml)) {
return false;
diff --git a/tools/aapt2/link/XmlCompatVersioner_test.cpp b/tools/aapt2/link/XmlCompatVersioner_test.cpp
index 1ed4536..a98ab0f 100644
--- a/tools/aapt2/link/XmlCompatVersioner_test.cpp
+++ b/tools/aapt2/link/XmlCompatVersioner_test.cpp
@@ -23,6 +23,7 @@
using ::testing::Eq;
using ::testing::IsNull;
using ::testing::NotNull;
+using ::testing::Pointee;
using ::testing::SizeIs;
namespace aapt {
@@ -287,13 +288,13 @@
ASSERT_THAT(attr, NotNull());
ASSERT_THAT(attr->compiled_value, NotNull());
ASSERT_TRUE(attr->compiled_attribute);
- ASSERT_THAT(*attr->compiled_value, ValueEq(padding_horizontal_value));
+ ASSERT_THAT(attr->compiled_value, Pointee(ValueEq(padding_horizontal_value)));
attr = el->FindAttribute(xml::kSchemaAndroid, "paddingRight");
ASSERT_THAT(attr, NotNull());
ASSERT_THAT(attr->compiled_value, NotNull());
ASSERT_TRUE(attr->compiled_attribute);
- ASSERT_THAT(*attr->compiled_value, ValueEq(padding_horizontal_value));
+ ASSERT_THAT(attr->compiled_value, Pointee(ValueEq(padding_horizontal_value)));
EXPECT_THAT(versioned_docs[1]->file.config.sdkVersion, Eq(SDK_LOLLIPOP_MR1));
el = versioned_docs[1]->root.get();
@@ -302,21 +303,20 @@
attr = el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal");
ASSERT_THAT(attr, NotNull());
- ASSERT_THAT(attr->compiled_value, NotNull());
ASSERT_TRUE(attr->compiled_attribute);
- ASSERT_THAT(*attr->compiled_value, ValueEq(padding_horizontal_value));
+ ASSERT_THAT(attr->compiled_value, Pointee(ValueEq(padding_horizontal_value)));
attr = el->FindAttribute(xml::kSchemaAndroid, "paddingLeft");
ASSERT_THAT(attr, NotNull());
ASSERT_THAT(attr->compiled_value, NotNull());
ASSERT_TRUE(attr->compiled_attribute);
- ASSERT_THAT(*attr->compiled_value, ValueEq(padding_horizontal_value));
+ ASSERT_THAT(attr->compiled_value, Pointee(ValueEq(padding_horizontal_value)));
attr = el->FindAttribute(xml::kSchemaAndroid, "paddingRight");
ASSERT_THAT(attr, NotNull());
ASSERT_THAT(attr->compiled_value, NotNull());
ASSERT_TRUE(attr->compiled_attribute);
- ASSERT_THAT(*attr->compiled_value, ValueEq(padding_horizontal_value));
+ ASSERT_THAT(attr->compiled_value, Pointee(ValueEq(padding_horizontal_value)));
}
} // namespace aapt
diff --git a/tools/aapt2/test/Common.h b/tools/aapt2/test/Common.h
index 4e318a9..aca161a 100644
--- a/tools/aapt2/test/Common.h
+++ b/tools/aapt2/test/Common.h
@@ -146,97 +146,70 @@
return android::StringPiece16(arg) == a;
}
-class ValueEq {
+template <typename T>
+class ValueEqImpl : public ::testing::MatcherInterface<T> {
public:
- template <typename arg_type>
- class BaseImpl : public ::testing::MatcherInterface<arg_type> {
- BaseImpl(const BaseImpl&) = default;
-
- void DescribeTo(::std::ostream* os) const override {
- *os << "is equal to " << *expected_;
- }
-
- void DescribeNegationTo(::std::ostream* os) const override {
- *os << "is not equal to " << *expected_;
- }
-
- protected:
- BaseImpl(const Value* expected) : expected_(expected) {
- }
-
- const Value* expected_;
- };
-
- template <typename T, bool>
- class Impl {};
-
- template <typename T>
- class Impl<T, false> : public ::testing::MatcherInterface<T> {
- public:
- explicit Impl(const Value* expected) : expected_(expected) {
- }
-
- bool MatchAndExplain(T x, ::testing::MatchResultListener* listener) const override {
- return expected_->Equals(&x);
- }
-
- void DescribeTo(::std::ostream* os) const override {
- *os << "is equal to " << *expected_;
- }
-
- void DescribeNegationTo(::std::ostream* os) const override {
- *os << "is not equal to " << *expected_;
- }
-
- private:
- DISALLOW_COPY_AND_ASSIGN(Impl);
-
- const Value* expected_;
- };
-
- template <typename T>
- class Impl<T, true> : public ::testing::MatcherInterface<T> {
- public:
- explicit Impl(const Value* expected) : expected_(expected) {
- }
-
- bool MatchAndExplain(T x, ::testing::MatchResultListener* listener) const override {
- return expected_->Equals(x);
- }
-
- void DescribeTo(::std::ostream* os) const override {
- *os << "is equal to " << *expected_;
- }
-
- void DescribeNegationTo(::std::ostream* os) const override {
- *os << "is not equal to " << *expected_;
- }
-
- private:
- DISALLOW_COPY_AND_ASSIGN(Impl);
-
- const Value* expected_;
- };
-
- ValueEq(const Value& expected) : expected_(&expected) {
+ explicit ValueEqImpl(const Value* expected) : expected_(expected) {
}
- ValueEq(const Value* expected) : expected_(expected) {
- }
- ValueEq(const ValueEq&) = default;
- template <typename T>
- operator ::testing::Matcher<T>() const {
- return ::testing::Matcher<T>(new Impl<T, std::is_pointer<T>::value>(expected_));
+ bool MatchAndExplain(T x, ::testing::MatchResultListener* listener) const override {
+ return expected_->Equals(&x);
+ }
+
+ void DescribeTo(::std::ostream* os) const override {
+ *os << "is equal to " << *expected_;
+ }
+
+ void DescribeNegationTo(::std::ostream* os) const override {
+ *os << "is not equal to " << *expected_;
}
private:
+ DISALLOW_COPY_AND_ASSIGN(ValueEqImpl);
+
const Value* expected_;
};
-// MATCHER_P(ValueEq, a,
-// std::string(negation ? "isn't" : "is") + " equal to " + ::testing::PrintToString(a)) {
-// return arg.Equals(&a);
-//}
+template <typename TValue>
+class ValueEqMatcher {
+ public:
+ ValueEqMatcher(TValue expected) : expected_(std::move(expected)) {
+ }
+
+ template <typename T>
+ operator ::testing::Matcher<T>() const {
+ return ::testing::Matcher<T>(new ValueEqImpl<T>(&expected_));
+ }
+
+ private:
+ TValue expected_;
+};
+
+template <typename TValue>
+class ValueEqPointerMatcher {
+ public:
+ ValueEqPointerMatcher(const TValue* expected) : expected_(expected) {
+ }
+
+ template <typename T>
+ operator ::testing::Matcher<T>() const {
+ return ::testing::Matcher<T>(new ValueEqImpl<T>(expected_));
+ }
+
+ private:
+ const TValue* expected_;
+};
+
+template <typename TValue,
+ typename = typename std::enable_if<!std::is_pointer<TValue>::value, void>::type>
+inline ValueEqMatcher<TValue> ValueEq(TValue value) {
+ return ValueEqMatcher<TValue>(std::move(value));
+}
+
+template <typename TValue>
+inline ValueEqPointerMatcher<TValue> ValueEq(const TValue* value) {
+ return ValueEqPointerMatcher<TValue>(value);
+}
MATCHER_P(StrValueEq, a,
std::string(negation ? "isn't" : "is") + " equal to " + ::testing::PrintToString(a)) {
diff --git a/tools/aapt2/xml/XmlDom.cpp b/tools/aapt2/xml/XmlDom.cpp
index fddb6b8..7b748ce 100644
--- a/tools/aapt2/xml/XmlDom.cpp
+++ b/tools/aapt2/xml/XmlDom.cpp
@@ -244,14 +244,13 @@
str16 = parser->getAttributeStringValue(i, &len);
if (str16) {
attr.value = util::Utf16ToUtf8(StringPiece16(str16, len));
- } else {
- android::Res_value res_value;
- if (parser->getAttributeValue(i, &res_value) > 0) {
- attr.compiled_value = ResourceUtils::ParseBinaryResValue(
- ResourceType::kAnim, {}, parser->getStrings(), res_value, out_pool);
- }
}
+ android::Res_value res_value;
+ if (parser->getAttributeValue(i, &res_value) > 0) {
+ attr.compiled_value = ResourceUtils::ParseBinaryResValue(
+ ResourceType::kAnim, {}, parser->getStrings(), res_value, out_pool);
+ }
el->attributes.push_back(std::move(attr));
}
diff --git a/tools/aapt2/xml/XmlDom_test.cpp b/tools/aapt2/xml/XmlDom_test.cpp
index e5012d6..486b53a 100644
--- a/tools/aapt2/xml/XmlDom_test.cpp
+++ b/tools/aapt2/xml/XmlDom_test.cpp
@@ -23,8 +23,10 @@
#include "test/Test.h"
using ::aapt::io::StringInputStream;
+using ::aapt::test::ValueEq;
using ::testing::Eq;
using ::testing::NotNull;
+using ::testing::Pointee;
using ::testing::SizeIs;
using ::testing::StrEq;
@@ -59,6 +61,16 @@
doc->root->name = "Layout";
doc->root->line_number = 2u;
+ xml::Attribute attr;
+ attr.name = "text";
+ attr.namespace_uri = kSchemaAndroid;
+ attr.compiled_attribute = AaptAttribute(
+ aapt::Attribute(android::ResTable_map::TYPE_REFERENCE | android::ResTable_map::TYPE_STRING),
+ ResourceId(0x01010001u));
+ attr.value = "@string/foo";
+ attr.compiled_value = test::BuildReference("string/foo", ResourceId(0x7f010000u));
+ doc->root->attributes.push_back(std::move(attr));
+
NamespaceDecl decl;
decl.uri = kSchemaAndroid;
decl.prefix = "android";
@@ -66,7 +78,9 @@
doc->root->namespace_decls.push_back(decl);
BigBuffer buffer(4096);
- XmlFlattener flattener(&buffer, {});
+ XmlFlattenerOptions options;
+ options.keep_raw_values = true;
+ XmlFlattener flattener(&buffer, options);
ASSERT_TRUE(flattener.Consume(context.get(), doc.get()));
auto block = util::Copy(buffer);
@@ -75,6 +89,21 @@
EXPECT_THAT(new_doc->root->name, StrEq("Layout"));
EXPECT_THAT(new_doc->root->line_number, Eq(2u));
+
+ ASSERT_THAT(new_doc->root->attributes, SizeIs(1u));
+ EXPECT_THAT(new_doc->root->attributes[0].name, StrEq("text"));
+ EXPECT_THAT(new_doc->root->attributes[0].namespace_uri, StrEq(kSchemaAndroid));
+
+ // We only check that the resource ID was preserved. There is no where to encode the types that
+ // the Attribute accepts (eg: string|reference).
+ ASSERT_TRUE(new_doc->root->attributes[0].compiled_attribute);
+ EXPECT_THAT(new_doc->root->attributes[0].compiled_attribute.value().id,
+ Eq(make_value(ResourceId(0x01010001u))));
+
+ EXPECT_THAT(new_doc->root->attributes[0].value, StrEq("@string/foo"));
+ EXPECT_THAT(new_doc->root->attributes[0].compiled_value,
+ Pointee(ValueEq(Reference(ResourceId(0x7f010000u)))));
+
ASSERT_THAT(new_doc->root->namespace_decls, SizeIs(1u));
EXPECT_THAT(new_doc->root->namespace_decls[0].uri, StrEq(kSchemaAndroid));
EXPECT_THAT(new_doc->root->namespace_decls[0].prefix, StrEq("android"));