AAPT2: Fix string escaping
We were processing escaped strings too early, before
parsing of values into types. Now the escaped strings get
processed when they are being flattened.
Bug: 37715376
Test: make aapt2_tests
Change-Id: Ic59aa2e3a20c40756c219752ff74b2a4f8a602ba
diff --git a/tools/aapt2/flatten/XmlFlattener.cpp b/tools/aapt2/flatten/XmlFlattener.cpp
index 366c373..e98d2d7 100644
--- a/tools/aapt2/flatten/XmlFlattener.cpp
+++ b/tools/aapt2/flatten/XmlFlattener.cpp
@@ -99,7 +99,11 @@
flat_node->comment.index = util::HostToDevice32(-1);
ResXMLTree_cdataExt* flat_text = writer.NextBlock<ResXMLTree_cdataExt>();
- AddString(node->text, kLowPriority, &flat_text->data);
+
+ // Process plain strings to make sure they get properly escaped.
+ util::StringBuilder builder;
+ builder.Append(node->text);
+ AddString(builder.ToString(), kLowPriority, &flat_text->data);
writer.Finish();
}
@@ -184,17 +188,14 @@
writer.Finish();
}
- void WriteAttributes(xml::Element* node, ResXMLTree_attrExt* flat_elem,
- ChunkWriter* writer) {
+ void WriteAttributes(xml::Element* node, ResXMLTree_attrExt* flat_elem, ChunkWriter* writer) {
filtered_attrs_.clear();
filtered_attrs_.reserve(node->attributes.size());
// Filter the attributes.
for (xml::Attribute& attr : node->attributes) {
- if (options_.max_sdk_level && attr.compiled_attribute &&
- attr.compiled_attribute.value().id) {
- size_t sdk_level =
- FindAttributeSdkLevel(attr.compiled_attribute.value().id.value());
+ if (options_.max_sdk_level && attr.compiled_attribute && attr.compiled_attribute.value().id) {
+ size_t sdk_level = FindAttributeSdkLevel(attr.compiled_attribute.value().id.value());
if (sdk_level > options_.max_sdk_level.value()) {
continue;
}
@@ -211,8 +212,7 @@
const ResourceId kIdAttr(0x010100d0);
- std::sort(filtered_attrs_.begin(), filtered_attrs_.end(),
- cmp_xml_attribute_by_id);
+ std::sort(filtered_attrs_.begin(), filtered_attrs_.end(), cmp_xml_attribute_by_id);
flat_elem->attributeCount = util::HostToDevice16(filtered_attrs_.size());
@@ -234,18 +234,15 @@
}
attribute_index++;
- // Add the namespaceUri to the list of StringRefs to encode. Use null if
- // the namespace
+ // Add the namespaceUri to the list of StringRefs to encode. Use null if the namespace
// is empty (doesn't exist).
AddString(xml_attr->namespace_uri, kLowPriority, &flat_attr->ns,
true /* treat_empty_string_as_null */);
flat_attr->rawValue.index = util::HostToDevice32(-1);
- if (!xml_attr->compiled_attribute ||
- !xml_attr->compiled_attribute.value().id) {
- // The attribute has no associated ResourceID, so the string order
- // doesn't matter.
+ if (!xml_attr->compiled_attribute || !xml_attr->compiled_attribute.value().id) {
+ // The attribute has no associated ResourceID, so the string order doesn't matter.
AddString(xml_attr->name, kLowPriority, &flat_attr->name);
} else {
// Attribute names are stored without packages, but we use
@@ -255,8 +252,7 @@
// pools that we later combine.
//
// Lookup the StringPool for this package and make the reference there.
- const xml::AaptAttribute& aapt_attr =
- xml_attr->compiled_attribute.value();
+ const xml::AaptAttribute& aapt_attr = xml_attr->compiled_attribute.value();
StringPool::Ref name_ref =
package_pools[aapt_attr.id.value().package_id()].MakeRef(
@@ -266,10 +262,18 @@
AddString(name_ref, &flat_attr->name);
}
+ // Process plain strings to make sure they get properly escaped.
+ StringPiece raw_value = xml_attr->value;
+ util::StringBuilder str_builder;
+ if (!options_.keep_raw_values) {
+ str_builder.Append(xml_attr->value);
+ raw_value = str_builder.ToString();
+ }
+
if (options_.keep_raw_values || !xml_attr->compiled_value) {
// Keep raw values if the value is not compiled or
// if we're building a static library (need symbols).
- AddString(xml_attr->value, kLowPriority, &flat_attr->rawValue);
+ AddString(raw_value, kLowPriority, &flat_attr->rawValue);
}
if (xml_attr->compiled_value) {
@@ -277,12 +281,12 @@
} else {
// Flatten as a regular string type.
flat_attr->typedValue.dataType = android::Res_value::TYPE_STRING;
- AddString(xml_attr->value, kLowPriority,
- (ResStringPool_ref*)&flat_attr->typedValue.data);
+
+ AddString(str_builder.ToString(), kLowPriority,
+ (ResStringPool_ref*) &flat_attr->typedValue.data);
}
- flat_attr->typedValue.size =
- util::HostToDevice16(sizeof(flat_attr->typedValue));
+ flat_attr->typedValue.size = util::HostToDevice16(sizeof(flat_attr->typedValue));
flat_attr++;
}
}
diff --git a/tools/aapt2/flatten/XmlFlattener_test.cpp b/tools/aapt2/flatten/XmlFlattener_test.cpp
index f0613e7..cfa89bb 100644
--- a/tools/aapt2/flatten/XmlFlattener_test.cpp
+++ b/tools/aapt2/flatten/XmlFlattener_test.cpp
@@ -82,72 +82,71 @@
android::ResXMLTree tree;
ASSERT_TRUE(Flatten(doc.get(), &tree));
- ASSERT_EQ(tree.next(), android::ResXMLTree::START_NAMESPACE);
+ ASSERT_EQ(android::ResXMLTree::START_NAMESPACE, tree.next());
size_t len;
const char16_t* namespace_prefix = tree.getNamespacePrefix(&len);
- EXPECT_EQ(StringPiece16(namespace_prefix, len), u"test");
+ EXPECT_EQ(StringPiece16(u"test"), StringPiece16(namespace_prefix, len));
const char16_t* namespace_uri = tree.getNamespaceUri(&len);
- ASSERT_EQ(StringPiece16(namespace_uri, len), u"http://com.test");
+ ASSERT_EQ(StringPiece16(u"http://com.test"), StringPiece16(namespace_uri, len));
- ASSERT_EQ(tree.next(), android::ResXMLTree::START_TAG);
+ ASSERT_EQ(android::ResXMLTree::START_TAG, tree.next());
- ASSERT_EQ(tree.getElementNamespace(&len), nullptr);
+ ASSERT_EQ(nullptr, tree.getElementNamespace(&len));
const char16_t* tag_name = tree.getElementName(&len);
- EXPECT_EQ(StringPiece16(tag_name, len), u"View");
+ EXPECT_EQ(StringPiece16(u"View"), StringPiece16(tag_name, len));
ASSERT_EQ(1u, tree.getAttributeCount());
- ASSERT_EQ(tree.getAttributeNamespace(0, &len), nullptr);
+ ASSERT_EQ(nullptr, tree.getAttributeNamespace(0, &len));
const char16_t* attr_name = tree.getAttributeName(0, &len);
- EXPECT_EQ(StringPiece16(attr_name, len), u"attr");
+ EXPECT_EQ(StringPiece16(u"attr"), StringPiece16(attr_name, len));
- EXPECT_EQ(0, tree.indexOfAttribute(nullptr, 0, u"attr",
- StringPiece16(u"attr").size()));
+ EXPECT_EQ(0, tree.indexOfAttribute(nullptr, 0, u"attr", StringPiece16(u"attr").size()));
- ASSERT_EQ(tree.next(), android::ResXMLTree::START_TAG);
+ ASSERT_EQ(android::ResXMLTree::START_TAG, tree.next());
- ASSERT_EQ(tree.getElementNamespace(&len), nullptr);
+ ASSERT_EQ(nullptr, tree.getElementNamespace(&len));
tag_name = tree.getElementName(&len);
- EXPECT_EQ(StringPiece16(tag_name, len), u"Layout");
+ EXPECT_EQ(StringPiece16(u"Layout"), StringPiece16(tag_name, len));
ASSERT_EQ(1u, tree.getAttributeCount());
const char16_t* attr_namespace = tree.getAttributeNamespace(0, &len);
- EXPECT_EQ(StringPiece16(attr_namespace, len), u"http://com.test");
+ EXPECT_EQ(StringPiece16(u"http://com.test"), StringPiece16(attr_namespace, len));
attr_name = tree.getAttributeName(0, &len);
- EXPECT_EQ(StringPiece16(attr_name, len), u"hello");
+ EXPECT_EQ(StringPiece16(u"hello"), StringPiece16(attr_name, len));
- ASSERT_EQ(tree.next(), android::ResXMLTree::END_TAG);
- ASSERT_EQ(tree.next(), android::ResXMLTree::START_TAG);
+ ASSERT_EQ(android::ResXMLTree::END_TAG, tree.next());
+ ASSERT_EQ(android::ResXMLTree::START_TAG, tree.next());
- ASSERT_EQ(tree.getElementNamespace(&len), nullptr);
+ ASSERT_EQ(nullptr, tree.getElementNamespace(&len));
tag_name = tree.getElementName(&len);
- EXPECT_EQ(StringPiece16(tag_name, len), u"Layout");
+ EXPECT_EQ(StringPiece16(u"Layout"), StringPiece16(tag_name, len));
ASSERT_EQ(0u, tree.getAttributeCount());
- ASSERT_EQ(tree.next(), android::ResXMLTree::TEXT);
+ ASSERT_EQ(android::ResXMLTree::TEXT, tree.next());
const char16_t* text = tree.getText(&len);
- EXPECT_EQ(StringPiece16(text, len), u"Some text\\");
+ EXPECT_EQ(StringPiece16(u"Some text\\"), StringPiece16(text, len));
- ASSERT_EQ(tree.next(), android::ResXMLTree::END_TAG);
- ASSERT_EQ(tree.getElementNamespace(&len), nullptr);
+ ASSERT_EQ(android::ResXMLTree::END_TAG, tree.next());
+ ASSERT_EQ(nullptr, tree.getElementNamespace(&len));
tag_name = tree.getElementName(&len);
- EXPECT_EQ(StringPiece16(tag_name, len), u"Layout");
+ EXPECT_EQ(StringPiece16(u"Layout"), StringPiece16(tag_name, len));
- ASSERT_EQ(tree.next(), android::ResXMLTree::END_TAG);
- ASSERT_EQ(tree.getElementNamespace(&len), nullptr);
+ ASSERT_EQ(android::ResXMLTree::END_TAG, tree.next());
+ ASSERT_EQ(nullptr, tree.getElementNamespace(&len));
tag_name = tree.getElementName(&len);
- EXPECT_EQ(StringPiece16(tag_name, len), u"View");
+ EXPECT_EQ(StringPiece16(u"View"), StringPiece16(tag_name, len));
- ASSERT_EQ(tree.next(), android::ResXMLTree::END_NAMESPACE);
+ ASSERT_EQ(android::ResXMLTree::END_NAMESPACE, tree.next());
namespace_prefix = tree.getNamespacePrefix(&len);
- EXPECT_EQ(StringPiece16(namespace_prefix, len), u"test");
+ EXPECT_EQ(StringPiece16(u"test"), StringPiece16(namespace_prefix, len));
namespace_uri = tree.getNamespaceUri(&len);
- ASSERT_EQ(StringPiece16(namespace_uri, len), u"http://com.test");
+ ASSERT_EQ(StringPiece16(u"http://com.test"), StringPiece16(namespace_uri, len));
- ASSERT_EQ(tree.next(), android::ResXMLTree::END_DOCUMENT);
+ ASSERT_EQ(android::ResXMLTree::END_DOCUMENT, tree.next());
}
TEST_F(XmlFlattenerTest, FlattenCompiledXmlAndStripSdk21) {
@@ -300,4 +299,41 @@
EXPECT_EQ(ResourceId(0x80020000), tree.getAttributeData(idx));
}
+TEST_F(XmlFlattenerTest, ProcessEscapedStrings) {
+ std::unique_ptr<xml::XmlResource> doc = test::BuildXmlDom(
+ R"EOF(<element value="\?hello" pattern="\\d{5}">\\d{5}</element>)EOF");
+
+ android::ResXMLTree tree;
+ ASSERT_TRUE(Flatten(doc.get(), &tree));
+
+ while (tree.next() != android::ResXMLTree::START_TAG) {
+ ASSERT_NE(tree.getEventType(), android::ResXMLTree::BAD_DOCUMENT);
+ ASSERT_NE(tree.getEventType(), android::ResXMLTree::END_DOCUMENT);
+ }
+
+ const StringPiece16 kValue = u"value";
+ const StringPiece16 kPattern = u"pattern";
+
+ size_t len;
+ ssize_t idx;
+ const char16_t* str16;
+
+ idx = tree.indexOfAttribute(nullptr, 0, kValue.data(), kValue.size());
+ ASSERT_GE(idx, 0);
+ str16 = tree.getAttributeStringValue(idx, &len);
+ ASSERT_NE(nullptr, str16);
+ EXPECT_EQ(StringPiece16(u"?hello"), StringPiece16(str16, len));
+
+ idx = tree.indexOfAttribute(nullptr, 0, kPattern.data(), kPattern.size());
+ ASSERT_GE(idx, 0);
+ str16 = tree.getAttributeStringValue(idx, &len);
+ ASSERT_NE(nullptr, str16);
+ EXPECT_EQ(StringPiece16(u"\\d{5}"), StringPiece16(str16, len));
+
+ ASSERT_EQ(android::ResXMLTree::TEXT, tree.next());
+ str16 = tree.getText(&len);
+ ASSERT_NE(nullptr, str16);
+ EXPECT_EQ(StringPiece16(u"\\d{5}"), StringPiece16(str16, len));
+}
+
} // namespace aapt
diff --git a/tools/aapt2/link/XmlReferenceLinker_test.cpp b/tools/aapt2/link/XmlReferenceLinker_test.cpp
index cc59416..66ecc15 100644
--- a/tools/aapt2/link/XmlReferenceLinker_test.cpp
+++ b/tools/aapt2/link/XmlReferenceLinker_test.cpp
@@ -81,6 +81,7 @@
android:layout_width="match_parent"
android:background="@color/green"
android:text="hello"
+ android:attr="\?hello"
nonAaptAttr="1"
nonAaptAttrRef="@id/id"
class="hello" />)EOF");
@@ -89,35 +90,40 @@
ASSERT_TRUE(linker.Consume(context_.get(), doc.get()));
xml::Element* view_el = xml::FindRootElement(doc.get());
- ASSERT_NE(view_el, nullptr);
+ ASSERT_NE(nullptr, view_el);
xml::Attribute* xml_attr = view_el->FindAttribute(xml::kSchemaAndroid, "layout_width");
- ASSERT_NE(xml_attr, nullptr);
+ ASSERT_NE(nullptr, xml_attr);
AAPT_ASSERT_TRUE(xml_attr->compiled_attribute);
AAPT_ASSERT_TRUE(xml_attr->compiled_attribute.value().id);
- EXPECT_EQ(xml_attr->compiled_attribute.value().id.value(), ResourceId(0x01010000));
- ASSERT_NE(xml_attr->compiled_value, nullptr);
- ASSERT_NE(ValueCast<BinaryPrimitive>(xml_attr->compiled_value.get()), nullptr);
+ EXPECT_EQ(ResourceId(0x01010000), xml_attr->compiled_attribute.value().id.value());
+ ASSERT_NE(nullptr, xml_attr->compiled_value);
+ ASSERT_NE(nullptr, ValueCast<BinaryPrimitive>(xml_attr->compiled_value.get()));
xml_attr = view_el->FindAttribute(xml::kSchemaAndroid, "background");
- ASSERT_NE(xml_attr, nullptr);
+ ASSERT_NE(nullptr, xml_attr);
AAPT_ASSERT_TRUE(xml_attr->compiled_attribute);
AAPT_ASSERT_TRUE(xml_attr->compiled_attribute.value().id);
- EXPECT_EQ(xml_attr->compiled_attribute.value().id.value(), ResourceId(0x01010001));
- ASSERT_NE(xml_attr->compiled_value, nullptr);
+ EXPECT_EQ(ResourceId(0x01010001), xml_attr->compiled_attribute.value().id.value());
+ ASSERT_NE(nullptr, xml_attr->compiled_value);
Reference* ref = ValueCast<Reference>(xml_attr->compiled_value.get());
- ASSERT_NE(ref, nullptr);
+ ASSERT_NE(nullptr, ref);
AAPT_ASSERT_TRUE(ref->name);
- EXPECT_EQ(ref->name.value(), test::ParseNameOrDie("color/green")); // Make sure the name
+ EXPECT_EQ(test::ParseNameOrDie("color/green"), ref->name.value()); // Make sure the name
// didn't change.
AAPT_ASSERT_TRUE(ref->id);
- EXPECT_EQ(ref->id.value(), ResourceId(0x7f020000));
+ EXPECT_EQ(ResourceId(0x7f020000), ref->id.value());
xml_attr = view_el->FindAttribute(xml::kSchemaAndroid, "text");
- ASSERT_NE(xml_attr, nullptr);
+ ASSERT_NE(nullptr, xml_attr);
AAPT_ASSERT_TRUE(xml_attr->compiled_attribute);
ASSERT_FALSE(xml_attr->compiled_value); // Strings don't get compiled for memory sake.
+ xml_attr = view_el->FindAttribute(xml::kSchemaAndroid, "attr");
+ ASSERT_NE(nullptr, xml_attr);
+ AAPT_ASSERT_TRUE(xml_attr->compiled_attribute);
+ ASSERT_FALSE(xml_attr->compiled_value); // Should be a plain string.
+
xml_attr = view_el->FindAttribute("", "nonAaptAttr");
ASSERT_NE(nullptr, xml_attr);
AAPT_ASSERT_FALSE(xml_attr->compiled_attribute);
@@ -131,9 +137,9 @@
ASSERT_NE(nullptr, ValueCast<Reference>(xml_attr->compiled_value.get()));
xml_attr = view_el->FindAttribute("", "class");
- ASSERT_NE(xml_attr, nullptr);
+ ASSERT_NE(nullptr, xml_attr);
AAPT_ASSERT_FALSE(xml_attr->compiled_attribute);
- ASSERT_EQ(xml_attr->compiled_value, nullptr);
+ ASSERT_EQ(nullptr, xml_attr->compiled_value);
}
TEST_F(XmlReferenceLinkerTest, PrivateSymbolsAreNotLinked) {
diff --git a/tools/aapt2/xml/XmlDom.cpp b/tools/aapt2/xml/XmlDom.cpp
index 6055190..98f5f1d 100644
--- a/tools/aapt2/xml/XmlDom.cpp
+++ b/tools/aapt2/xml/XmlDom.cpp
@@ -42,7 +42,6 @@
std::stack<xml::Node*> node_stack;
std::string pending_comment;
std::unique_ptr<xml::Text> last_text_node;
- util::StringBuilder pending_text;
};
/**
@@ -66,14 +65,12 @@
static void FinishPendingText(Stack* stack) {
if (stack->last_text_node != nullptr) {
- if (!stack->pending_text.IsEmpty()) {
- stack->last_text_node->text = stack->pending_text.ToString();
- stack->pending_text = {};
+ if (!stack->last_text_node->text.empty()) {
stack->node_stack.top()->AppendChild(std::move(stack->last_text_node));
} else {
// Drop an empty text node.
- stack->last_text_node = nullptr;
}
+ stack->last_text_node = nullptr;
}
}
@@ -138,13 +135,11 @@
while (*attrs) {
Attribute attribute;
SplitName(*attrs++, &attribute.namespace_uri, &attribute.name);
- util::StringBuilder builder;
- builder.Append(*attrs++);
- attribute.value = builder.ToString();
+ attribute.value = *attrs++;
// Insert in sorted order.
- auto iter = std::lower_bound(el->attributes.begin(), el->attributes.end(),
- attribute, less_attribute);
+ auto iter = std::lower_bound(el->attributes.begin(), el->attributes.end(), attribute,
+ less_attribute);
el->attributes.insert(iter, std::move(attribute));
}
@@ -173,14 +168,14 @@
// See if we can just append the text to a previous text node.
if (stack->last_text_node != nullptr) {
- stack->pending_text.Append(str);
+ stack->last_text_node->text.append(str.data(), str.size());
return;
}
stack->last_text_node = util::make_unique<Text>();
stack->last_text_node->line_number = XML_GetCurrentLineNumber(parser);
stack->last_text_node->column_number = XML_GetCurrentColumnNumber(parser);
- stack->pending_text.Append(str);
+ stack->last_text_node->text = str.to_string();
}
static void XMLCALL CommentDataHandler(void* user_data, const char* comment) {
diff --git a/tools/aapt2/xml/XmlDom_test.cpp b/tools/aapt2/xml/XmlDom_test.cpp
index 0fc3cec6..fb18ea3 100644
--- a/tools/aapt2/xml/XmlDom_test.cpp
+++ b/tools/aapt2/xml/XmlDom_test.cpp
@@ -49,23 +49,26 @@
EXPECT_EQ(ns->namespace_prefix, "android");
}
-TEST(XmlDomTest, HandleEscapes) {
- std::unique_ptr<xml::XmlResource> doc = test::BuildXmlDom(
- R"EOF(<shortcode pattern="\\d{5}">\\d{5}</shortcode>)EOF");
+// Escaping is handled after parsing of the values for resource-specific values.
+TEST(XmlDomTest, ForwardEscapes) {
+ std::unique_ptr<xml::XmlResource> doc = test::BuildXmlDom(R"EOF(
+ <element value="\?hello" pattern="\\d{5}">\\d{5}</element>)EOF");
xml::Element* el = xml::FindRootElement(doc->root.get());
ASSERT_NE(nullptr, el);
xml::Attribute* attr = el->FindAttribute({}, "pattern");
ASSERT_NE(nullptr, attr);
+ EXPECT_EQ("\\\\d{5}", attr->value);
- EXPECT_EQ("\\d{5}", attr->value);
+ attr = el->FindAttribute({}, "value");
+ ASSERT_NE(nullptr, attr);
+ EXPECT_EQ("\\?hello", attr->value);
ASSERT_EQ(1u, el->children.size());
-
xml::Text* text = xml::NodeCast<xml::Text>(el->children[0].get());
ASSERT_NE(nullptr, text);
- EXPECT_EQ("\\d{5}", text->text);
+ EXPECT_EQ("\\\\d{5}", text->text);
}
} // namespace aapt