diff --git a/tools/aapt2/flatten/XmlFlattener.cpp b/tools/aapt2/flatten/XmlFlattener.cpp index 366c373223dc8..e98d2d758df47 100644 --- a/tools/aapt2/flatten/XmlFlattener.cpp +++ b/tools/aapt2/flatten/XmlFlattener.cpp @@ -99,7 +99,11 @@ class XmlFlattenerVisitor : public xml::Visitor { flat_node->comment.index = util::HostToDevice32(-1); ResXMLTree_cdataExt* flat_text = writer.NextBlock(); - 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 @@ class XmlFlattenerVisitor : public xml::Visitor { 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 @@ class XmlFlattenerVisitor : public xml::Visitor { 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 @@ class XmlFlattenerVisitor : public xml::Visitor { } 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 @@ class XmlFlattenerVisitor : public xml::Visitor { // 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 @@ class XmlFlattenerVisitor : public xml::Visitor { 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 @@ class XmlFlattenerVisitor : public xml::Visitor { } 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 f0613e74b5f82..cfa89bb4b09f9 100644 --- a/tools/aapt2/flatten/XmlFlattener_test.cpp +++ b/tools/aapt2/flatten/XmlFlattener_test.cpp @@ -82,72 +82,71 @@ TEST_F(XmlFlattenerTest, FlattenXmlWithNoCompiledAttributes) { 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 @@ TEST_F(XmlFlattenerTest, FlattenNonStandardPackageId) { EXPECT_EQ(ResourceId(0x80020000), tree.getAttributeData(idx)); } +TEST_F(XmlFlattenerTest, ProcessEscapedStrings) { + std::unique_ptr doc = test::BuildXmlDom( + R"EOF(\\d{5})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 cc59416cb2b94..66ecc15730edb 100644 --- a/tools/aapt2/link/XmlReferenceLinker_test.cpp +++ b/tools/aapt2/link/XmlReferenceLinker_test.cpp @@ -81,6 +81,7 @@ TEST_F(XmlReferenceLinkerTest, LinkBasicAttributes) { 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 @@ TEST_F(XmlReferenceLinkerTest, LinkBasicAttributes) { 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(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(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(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 @@ TEST_F(XmlReferenceLinkerTest, LinkBasicAttributes) { ASSERT_NE(nullptr, ValueCast(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 60551901fe8df..98f5f1d06df9e 100644 --- a/tools/aapt2/xml/XmlDom.cpp +++ b/tools/aapt2/xml/XmlDom.cpp @@ -42,7 +42,6 @@ struct Stack { std::stack node_stack; std::string pending_comment; std::unique_ptr last_text_node; - util::StringBuilder pending_text; }; /** @@ -66,14 +65,12 @@ static void SplitName(const char* name, std::string* out_ns, 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 @@ static void XMLCALL StartElementHandler(void* user_data, const char* name, 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 @@ static void XMLCALL CharacterDataHandler(void* user_data, const char* s, int len // 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(); 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 0fc3cec66666d..fb18ea316ca68 100644 --- a/tools/aapt2/xml/XmlDom_test.cpp +++ b/tools/aapt2/xml/XmlDom_test.cpp @@ -49,23 +49,26 @@ TEST(XmlDomTest, Inflate) { EXPECT_EQ(ns->namespace_prefix, "android"); } -TEST(XmlDomTest, HandleEscapes) { - std::unique_ptr doc = test::BuildXmlDom( - R"EOF(\\d{5})EOF"); +// Escaping is handled after parsing of the values for resource-specific values. +TEST(XmlDomTest, ForwardEscapes) { + std::unique_ptr doc = test::BuildXmlDom(R"EOF( + \\d{5})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(el->children[0].get()); ASSERT_NE(nullptr, text); - EXPECT_EQ("\\d{5}", text->text); + EXPECT_EQ("\\\\d{5}", text->text); } } // namespace aapt