AAPT2: Encode empty attribute string values

Due to another bug, empty strings in XML files
were encoded as NULLs. This was only needed for
encoding missing namespace URIs. Attribute
values should remain empty strings.

Bug:29939875
Bug:29462255
Change-Id: I3897661d85865c88bb2b7cf1495da16c30f7272e

3 files changed, 46 insertions, 13 deletions

diff --git a/tools/aapt2/StringPool_test.cpp b/tools/aapt2/StringPool_test.cpp
index 2b2d348fd17c..562d80ed8cdb 100644
--- a/tools/aapt2/StringPool_test.cpp
+++ b/tools/aapt2/StringPool_test.cpp
@@ -208,7 +208,8 @@ TEST(StringPoolTest, FlattenUtf8) {
     StringPool::Ref ref1 = pool.makeRef(u"hello");
     StringPool::Ref ref2 = pool.makeRef(u"goodbye");
     StringPool::Ref ref3 = pool.makeRef(sLongString);
-    StringPool::StyleRef ref4 = pool.makeRef(StyleString{
+    StringPool::Ref ref4 = pool.makeRef(u"");
+    StringPool::StyleRef ref5 = pool.makeRef(StyleString{
             { u"style" },
             { Span{ { u"b" }, 0, 1 }, Span{ { u"i" }, 2, 3 } }
     });
@@ -217,6 +218,7 @@ TEST(StringPoolTest, FlattenUtf8) {
     EXPECT_EQ(1u, ref2.getIndex());
     EXPECT_EQ(2u, ref3.getIndex());
     EXPECT_EQ(3u, ref4.getIndex());
+    EXPECT_EQ(4u, ref5.getIndex());
 
     BigBuffer buffer(1024);
     StringPool::flattenUtf8(&buffer, pool);
@@ -229,9 +231,11 @@ TEST(StringPoolTest, FlattenUtf8) {
         EXPECT_EQ(util::getString(test, 0), u"hello");
         EXPECT_EQ(util::getString(test, 1), u"goodbye");
         EXPECT_EQ(util::getString(test, 2), sLongString);
-        EXPECT_EQ(util::getString(test, 3), u"style");
+        size_t len;
+        EXPECT_NE(nullptr, test.stringAt(3, &len));
+        EXPECT_EQ(util::getString(test, 4), u"style");
 
-        const ResStringPool_span* span = test.styleAt(3);
+        const ResStringPool_span* span = test.styleAt(4);
         ASSERT_NE(nullptr, span);
         EXPECT_EQ(util::getString(test, span->name.index), u"b");
         EXPECT_EQ(0u, span->firstChar);
diff --git a/tools/aapt2/flatten/XmlFlattener.cpp b/tools/aapt2/flatten/XmlFlattener.cpp
index cc457890ff6f..2cf7ddaa2cc6 100644
--- a/tools/aapt2/flatten/XmlFlattener.cpp
+++ b/tools/aapt2/flatten/XmlFlattener.cpp
@@ -57,14 +57,15 @@ struct XmlFlattenerVisitor : public xml::Visitor {
             mBuffer(buffer), mOptions(options) {
     }
 
-    void addString(const StringPiece16& str, uint32_t priority, android::ResStringPool_ref* dest) {
-        if (!str.empty()) {
+    void addString(const StringPiece16& str, uint32_t priority, android::ResStringPool_ref* dest,
+                   bool treatEmptyStringAsNull = false) {
+        if (str.empty() && treatEmptyStringAsNull) {
+            // Some parts of the runtime treat null differently than empty string.
+            dest->index = util::deviceToHost32(-1);
+        } else {
             mStringRefs.push_back(StringFlattenDest{
                     mPool.makeRef(str, StringPool::Context{ priority }),
                     dest });
-        } else {
-            // The device doesn't think a string of size 0 is the same as null.
-            dest->index = util::deviceToHost32(-1);
         }
     }
 
@@ -118,8 +119,14 @@ struct XmlFlattenerVisitor : public xml::Visitor {
             flatNode->comment.index = util::hostToDevice32(-1);
 
             ResXMLTree_attrExt* flatElem = startWriter.nextBlock<ResXMLTree_attrExt>();
-            addString(node->namespaceUri, kLowPriority, &flatElem->ns);
-            addString(node->name, kLowPriority, &flatElem->name);
+
+            // A missing namespace must be null, not an empty string. Otherwise the runtime
+            // complains.
+            addString(node->namespaceUri, kLowPriority, &flatElem->ns,
+                      true /* treatEmptyStringAsNull */);
+            addString(node->name, kLowPriority, &flatElem->name,
+                      true /* treatEmptyStringAsNull */);
+
             flatElem->attributeStart = util::hostToDevice16(sizeof(*flatElem));
             flatElem->attributeSize = util::hostToDevice16(sizeof(ResXMLTree_attribute));
 
@@ -138,7 +145,8 @@ struct XmlFlattenerVisitor : public xml::Visitor {
             flatEndNode->comment.index = util::hostToDevice32(-1);
 
             ResXMLTree_endElementExt* flatEndElem = endWriter.nextBlock<ResXMLTree_endElementExt>();
-            addString(node->namespaceUri, kLowPriority, &flatEndElem->ns);
+            addString(node->namespaceUri, kLowPriority, &flatEndElem->ns,
+                      true /* treatEmptyStringAsNull */);
             addString(node->name, kLowPriority, &flatEndElem->name);
 
             endWriter.finish();
@@ -205,8 +213,10 @@ struct XmlFlattenerVisitor : public xml::Visitor {
             }
             attributeIndex++;
 
-            // Add the namespaceUri to the list of StringRefs to encode.
-            addString(xmlAttr->namespaceUri, kLowPriority, &flatAttr->ns);
+            // Add the namespaceUri to the list of StringRefs to encode. Use null if the namespace
+            // is empty (doesn't exist).
+            addString(xmlAttr->namespaceUri, kLowPriority, &flatAttr->ns,
+                      true /* treatEmptyStringAsNull */);
 
             flatAttr->rawValue.index = util::hostToDevice32(-1);
 
diff --git a/tools/aapt2/flatten/XmlFlattener_test.cpp b/tools/aapt2/flatten/XmlFlattener_test.cpp
index 4e6eb811e572..ef06528e236e 100644
--- a/tools/aapt2/flatten/XmlFlattener_test.cpp
+++ b/tools/aapt2/flatten/XmlFlattener_test.cpp
@@ -207,4 +207,23 @@ TEST_F(XmlFlattenerTest, NoNamespaceIsNotTheSameAsEmptyNamespace) {
     EXPECT_GE(tree.indexOfAttribute(nullptr, 0, kPackage.data(), kPackage.size()), 0);
 }
 
+TEST_F(XmlFlattenerTest, EmptyStringValueInAttributeIsNotNull) {
+    std::unique_ptr<xml::XmlResource> doc = test::buildXmlDom("<View package=\"\"/>");
+
+    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 kPackage = u"package";
+    ssize_t idx = tree.indexOfAttribute(nullptr, 0, kPackage.data(), kPackage.size());
+    ASSERT_GE(idx, 0);
+
+    size_t len;
+    EXPECT_NE(nullptr, tree.getAttributeStringValue(idx, &len));
+}
+
 } // namespace aapt