Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 48448e8a authored by Adam Lesinski's avatar Adam Lesinski
Browse files

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
parent 0ddca920

tools/aapt2/flatten/XmlFlattener.cpp

+26 −22
Original line number Diff line number Diff line
@@ -99,7 +99,11 @@ class XmlFlattenerVisitor : public xml::Visitor { 
    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 @@ 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,



        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++;

    }

  }

tools/aapt2/flatten/XmlFlattener_test.cpp

+67 −31
Original line number Diff line number Diff line
@@ -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<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

tools/aapt2/link/XmlReferenceLinker_test.cpp

+20 −14
Original line number Diff line number Diff line
@@ -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<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 @@ TEST_F(XmlReferenceLinkerTest, LinkBasicAttributes) { 
  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) {

tools/aapt2/xml/XmlDom.cpp

+7 −12
Original line number Diff line number Diff line
@@ -42,7 +42,6 @@ struct Stack { 
  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 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<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) {

tools/aapt2/xml/XmlDom_test.cpp

+9 −6
Original line number Diff line number Diff line
@@ -49,23 +49,26 @@ TEST(XmlDomTest, Inflate) { 
  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