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

Commit e1094a2e authored by Adam Lesinski's avatar Adam Lesinski
Browse files

AAPT2: Fix issue with String flattening in XmlFlattener 

Compiled Strings (previously not encountered) in an XML resource
were using a different StringPool than the one being referred to
in the XmlFlattener, and so the indices were all wrong.

Bug: 72700446
Test: make aapt2_tests
Change-Id: I663924f8fad50fd4c69cfa196318dc63fb641a25
parent 3e946e93

tools/aapt2/format/binary/XmlFlattener.cpp

+37 −21
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ 
#include "utils/misc.h"



#include "SdkConstants.h"

#include "ValueVisitor.h"

#include "format/binary/ChunkWriter.h"

#include "format/binary/ResourceTypeExtensions.h"

#include "xml/XmlDom.h"
@@ -153,6 +154,9 @@ class XmlFlattenerVisitor : public xml::ConstVisitor { 
 private:

  DISALLOW_COPY_AND_ASSIGN(XmlFlattenerVisitor);



  // We are adding strings to a StringPool whose strings will be sorted and merged with other

  // string pools. That means we can't encode the ID of a string directly. Instead, we defer the

  // writing of the ID here, until after the StringPool is merged and sorted.

  void AddString(const StringPiece& str, uint32_t priority, android::ResStringPool_ref* dest,

                 bool treat_empty_string_as_null = false) {

    if (str.empty() && treat_empty_string_as_null) {
@@ -164,6 +168,9 @@ class XmlFlattenerVisitor : public xml::ConstVisitor { 
    }

  }



  // We are adding strings to a StringPool whose strings will be sorted and merged with other

  // string pools. That means we can't encode the ID of a string directly. Instead, we defer the

  // writing of the ID here, until after the StringPool is merged and sorted.

  void AddString(const StringPool::Ref& ref, android::ResStringPool_ref* dest) {

    string_refs.push_back(StringFlattenDest{ref, dest});

  }
@@ -248,30 +255,39 @@ class XmlFlattenerVisitor : public xml::ConstVisitor { 
        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(true /*preserve_spaces*/);

      str_builder.Append(xml_attr->value);



      if (!options_.keep_raw_values) {

        raw_value = str_builder.ToString();

      std::string processed_str;

      Maybe<StringPiece> compiled_text;

      if (xml_attr->compiled_value != nullptr) {

        // Make sure we're not flattening a String. A String can be referencing a string from

        // a different StringPool than we're using here to build the binary XML.

        String* string_value = ValueCast<String>(xml_attr->compiled_value.get());

        if (string_value != nullptr) {

          // Mark the String's text as needing to be serialized.

          compiled_text = StringPiece(*string_value->value);

        } else {

          // Serialize this compiled value safely.

          CHECK(xml_attr->compiled_value->Flatten(&flat_attr->typedValue));

        }



      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(raw_value, kLowPriority, &flat_attr->rawValue);

      } else {

        // There is no compiled value, so treat the raw string as compiled, once it is processed to

        // make sure escape sequences are properly interpreted.

        processed_str =

            util::StringBuilder(true /*preserve_spaces*/).Append(xml_attr->value).ToString();

        compiled_text = StringPiece(processed_str);

      }



      if (xml_attr->compiled_value) {

        CHECK(xml_attr->compiled_value->Flatten(&flat_attr->typedValue));

      } else {

        // Flatten as a regular string type.

      if (compiled_text) {

        // Write out the compiled text and raw_text.

        flat_attr->typedValue.dataType = android::Res_value::TYPE_STRING;



        AddString(str_builder.ToString(), kLowPriority,

                  (ResStringPool_ref*)&flat_attr->typedValue.data);

        AddString(compiled_text.value(), kLowPriority,

                  reinterpret_cast<ResStringPool_ref*>(&flat_attr->typedValue.data));

        if (options_.keep_raw_values) {

          AddString(xml_attr->value, kLowPriority, &flat_attr->rawValue);

        } else {

          AddString(compiled_text.value(), kLowPriority, &flat_attr->rawValue);

        }

      } else if (options_.keep_raw_values && !xml_attr->value.empty()) {

        AddString(xml_attr->value, kLowPriority, &flat_attr->rawValue);

      }



      flat_attr->typedValue.size = util::HostToDevice16(sizeof(flat_attr->typedValue));

tools/aapt2/format/binary/XmlFlattener_test.cpp

+88 −0
Original line number Diff line number Diff line
@@ -286,4 +286,92 @@ TEST_F(XmlFlattenerTest, ProcessEscapedStrings) { 
  EXPECT_THAT(tree.getText(&len), StrEq(u"\\d{5}"));

}



TEST_F(XmlFlattenerTest, FlattenRawValueOnlyMakesCompiledValueToo) {

  std::unique_ptr<xml::XmlResource> doc = test::BuildXmlDom(R"(<element foo="bar" />)");



  // Raw values are kept when encoding an attribute with no compiled value, regardless of option.

  XmlFlattenerOptions options;

  options.keep_raw_values = false;



  android::ResXMLTree tree;

  ASSERT_TRUE(Flatten(doc.get(), &tree, options));



  while (tree.next() != android::ResXMLTree::START_TAG) {

    ASSERT_THAT(tree.getEventType(), Ne(android::ResXMLTree::BAD_DOCUMENT));

    ASSERT_THAT(tree.getEventType(), Ne(android::ResXMLTree::END_DOCUMENT));

  }



  ASSERT_THAT(tree.getAttributeCount(), Eq(1u));

  EXPECT_THAT(tree.getAttributeValueStringID(0), Ge(0));

  EXPECT_THAT(tree.getAttributeDataType(0), Eq(android::Res_value::TYPE_STRING));

  EXPECT_THAT(tree.getAttributeValueStringID(0), Eq(tree.getAttributeData(0)));

}



TEST_F(XmlFlattenerTest, FlattenCompiledStringValuePreservesRawValue) {

  std::unique_ptr<xml::XmlResource> doc = test::BuildXmlDom(R"(<element foo="bar" />)");

  doc->root->attributes[0].compiled_value =

      util::make_unique<String>(doc->string_pool.MakeRef("bar"));



  // Raw values are kept when encoding a string anyways.

  XmlFlattenerOptions options;

  options.keep_raw_values = false;



  android::ResXMLTree tree;

  ASSERT_TRUE(Flatten(doc.get(), &tree, options));



  while (tree.next() != android::ResXMLTree::START_TAG) {

    ASSERT_THAT(tree.getEventType(), Ne(android::ResXMLTree::BAD_DOCUMENT));

    ASSERT_THAT(tree.getEventType(), Ne(android::ResXMLTree::END_DOCUMENT));

  }



  ASSERT_THAT(tree.getAttributeCount(), Eq(1u));

  EXPECT_THAT(tree.getAttributeValueStringID(0), Ge(0));

  EXPECT_THAT(tree.getAttributeDataType(0), Eq(android::Res_value::TYPE_STRING));

  EXPECT_THAT(tree.getAttributeValueStringID(0), Eq(tree.getAttributeData(0)));

}



TEST_F(XmlFlattenerTest, FlattenCompiledValueExcludesRawValueWithKeepRawOptionFalse) {

  std::unique_ptr<xml::XmlResource> doc = test::BuildXmlDom(R"(<element foo="true" />)");

  doc->root->attributes[0].compiled_value = ResourceUtils::MakeBool(true);



  XmlFlattenerOptions options;

  options.keep_raw_values = false;



  android::ResXMLTree tree;

  ASSERT_TRUE(Flatten(doc.get(), &tree, options));



  while (tree.next() != android::ResXMLTree::START_TAG) {

    ASSERT_THAT(tree.getEventType(), Ne(android::ResXMLTree::BAD_DOCUMENT));

    ASSERT_THAT(tree.getEventType(), Ne(android::ResXMLTree::END_DOCUMENT));

  }



  ASSERT_THAT(tree.getAttributeCount(), Eq(1u));

  EXPECT_THAT(tree.getAttributeValueStringID(0), Eq(-1));

  EXPECT_THAT(tree.getAttributeDataType(0), Eq(android::Res_value::TYPE_INT_BOOLEAN));

}



TEST_F(XmlFlattenerTest, FlattenCompiledValueExcludesRawValueWithKeepRawOptionTrue) {

  std::unique_ptr<xml::XmlResource> doc = test::BuildXmlDom(R"(<element foo="true" />)");

  doc->root->attributes[0].compiled_value = ResourceUtils::MakeBool(true);



  XmlFlattenerOptions options;

  options.keep_raw_values = true;



  android::ResXMLTree tree;

  ASSERT_TRUE(Flatten(doc.get(), &tree, options));



  while (tree.next() != android::ResXMLTree::START_TAG) {

    ASSERT_THAT(tree.getEventType(), Ne(android::ResXMLTree::BAD_DOCUMENT));

    ASSERT_THAT(tree.getEventType(), Ne(android::ResXMLTree::END_DOCUMENT));

  }



  ASSERT_THAT(tree.getAttributeCount(), Eq(1u));

  EXPECT_THAT(tree.getAttributeValueStringID(0), Ge(0));



  size_t len;

  EXPECT_THAT(tree.getAttributeStringValue(0, &len), StrEq(u"true"));



  EXPECT_THAT(tree.getAttributeDataType(0), Eq(android::Res_value::TYPE_INT_BOOLEAN));

}



}  // namespace aapt

tools/aapt2/xml/XmlDom.cpp

+13 −7
Original line number Diff line number Diff line
@@ -248,9 +248,15 @@ static void CopyAttributes(Element* el, android::ResXMLParser* parser, StringPoo 


      android::Res_value res_value;

      if (parser->getAttributeValue(i, &res_value) > 0) {

        // Only compile the value if it is not a string, or it is a string that differs from

        // the raw attribute value.

        int32_t raw_value_idx = parser->getAttributeValueStringID(i);

        if (res_value.dataType != android::Res_value::TYPE_STRING || raw_value_idx < 0 ||

            static_cast<uint32_t>(raw_value_idx) != res_value.data) {

          attr.compiled_value = ResourceUtils::ParseBinaryResValue(

              ResourceType::kAnim, {}, parser->getStrings(), res_value, out_pool);

        }

      }



      el->attributes.push_back(std::move(attr));

    }
@@ -262,8 +268,8 @@ std::unique_ptr<XmlResource> Inflate(const void* data, size_t len, std::string* 
  // an enum, which causes errors when qualifying it with android::

  using namespace android;



  StringPool string_pool;

  std::unique_ptr<Element> root;

  std::unique_ptr<XmlResource> xml_resource = util::make_unique<XmlResource>();



  std::stack<Element*> node_stack;

  std::unique_ptr<Element> pending_element;
@@ -322,12 +328,12 @@ std::unique_ptr<XmlResource> Inflate(const void* data, size_t len, std::string* 
        }



        Element* this_el = el.get();

        CopyAttributes(el.get(), &tree, &string_pool);

        CopyAttributes(el.get(), &tree, &xml_resource->string_pool);



        if (!node_stack.empty()) {

          node_stack.top()->AppendChild(std::move(el));

        } else {

          root = std::move(el);

          xml_resource->root = std::move(el);

        }

        node_stack.push(this_el);

        break;
@@ -359,7 +365,7 @@ std::unique_ptr<XmlResource> Inflate(const void* data, size_t len, std::string* 
        break;

    }

  }

  return util::make_unique<XmlResource>(ResourceFile{}, std::move(string_pool), std::move(root));

  return xml_resource;

}



std::unique_ptr<XmlResource> XmlResource::Clone() const {