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
diff --git a/tools/aapt2/StringPool_test.cpp b/tools/aapt2/StringPool_test.cpp
index 2b2d348..562d80e 100644
--- a/tools/aapt2/StringPool_test.cpp
+++ b/tools/aapt2/StringPool_test.cpp
@@ -208,7 +208,8 @@
     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 @@
     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 @@
         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 cc45789..2cf7dda 100644
--- a/tools/aapt2/flatten/XmlFlattener.cpp
+++ b/tools/aapt2/flatten/XmlFlattener.cpp
@@ -57,14 +57,15 @@
             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 @@
             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 @@
             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 @@
             }
             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 4e6eb81..ef06528 100644
--- a/tools/aapt2/flatten/XmlFlattener_test.cpp
+++ b/tools/aapt2/flatten/XmlFlattener_test.cpp
@@ -207,4 +207,23 @@
     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