The last of the Kxml correctness fixes for Expat compatibility.

With this change we should be able to drop the Expat pull parser
and use Kxml exclusively. I'm deferring that change until after
the current release.

Change-Id: I7c6d6dfe6c1e9ae9417c48603068ddd4ade78b76
http://b/3090550

4 files changed, 115 insertions, 34 deletions

diff --git a/expectations/knownfailures.txt b/expectations/knownfailures.txt
index 5b806ab..54cf6ad 100644
--- a/expectations/knownfailures.txt
+++ b/expectations/knownfailures.txt
@@ -3,6 +3,54 @@
  */
 [
 {
+  description: "KxmlParser doesn't expose DTD text",
+  name: "libcore.xml.KxmlPullParserDtdTest#testDoctypeWithNextToken",
+  bug: 3241492
+},
+{
+  description: "Expat relaxed is different from Kxml relaxed",
+  names: [
+    "libcore.xml.ExpatPullParserTest#testAttributeNoValueWithRelaxed",
+    "libcore.xml.ExpatPullParserTest#testAttributeUnquotedValueWithRelaxed",
+    "libcore.xml.ExpatPullParserTest#testMissingEntitiesWithRelaxed",
+    "libcore.xml.ExpatPullParserTest#testUnterminatedEntityWithRelaxed"
+  ],
+  bug: 3090550
+},
+{
+  description: "ExpatPullParser doesn't support nextToken() or line numbers",
+  names: [
+    "libcore.xml.ExpatPullParserDtdTest#testDoctypeWithNextToken",
+    "libcore.xml.ExpatPullParserTest#testCdataUsingNextToken",
+    "libcore.xml.ExpatPullParserTest#testCommentUsingNextToken",
+    "libcore.xml.ExpatPullParserTest#testCustomEntitiesAreNotEvaluated",
+    "libcore.xml.ExpatPullParserTest#testCustomEntitiesUsingNext",
+    "libcore.xml.ExpatPullParserTest#testCustomEntitiesUsingNextToken",
+    "libcore.xml.ExpatPullParserTest#testEmptyCdataUsingNextToken",
+    "libcore.xml.ExpatPullParserTest#testEmptyComment",
+    "libcore.xml.ExpatPullParserTest#testEmptyEntityReferenceUsingNext",
+    "libcore.xml.ExpatPullParserTest#testEmptyEntityReferenceUsingNextToken",
+    "libcore.xml.ExpatPullParserTest#testEntityInAttributeUsingNextToken",
+    "libcore.xml.ExpatPullParserTest#testLinesAndColumns",
+    "libcore.xml.ExpatPullParserTest#testMissingEntitiesInAttributesUsingNextToken",
+    "libcore.xml.ExpatPullParserTest#testMissingEntitiesInAttributesUsingNextTokenWithRelaxed",
+    "libcore.xml.ExpatPullParserTest#testMissingEntitiesInAttributesUsingNextWithRelaxed",
+    "libcore.xml.ExpatPullParserTest#testMissingEntitiesUsingNextToken",
+    "libcore.xml.ExpatPullParserTest#testMissingEntitiesUsingNextTokenWithRelaxed",
+    "libcore.xml.ExpatPullParserTest#testProcessingInstructionUsingNextToken",
+    "libcore.xml.ExpatPullParserTest#testRegularNumericEntities",
+    "libcore.xml.ExpatPullParserTest#testWhitespaceUsingNextToken",
+    "libcore.xml.ExpatPullParserTest#testXmlDeclaration",
+    "libcore.xml.ExpatPullParserTest#testXmlDeclarationExtraAttributes"
+  ],
+  bug: 3090550
+},
+{
+  description: "ExpatPullParser doesn't handle surrogates properly",
+  name: "libcore.xml.ExpatPullParserTest#testCharacterReferenceOfLastUtf16Surrogate",
+  bug: 3090550
+},
+{
   description: "RandomAccessFile missing finalizer",
   name: "libcore.java.io.RandomAccessFileTest#testRandomAccessFileHasCleanupFinalizer",
   bug: 3015023
diff --git a/luni/src/test/java/libcore/xml/PullParserDtdTest.java b/luni/src/test/java/libcore/xml/PullParserDtdTest.java
index 38b7d8a..0101d7b 100644
--- a/luni/src/test/java/libcore/xml/PullParserDtdTest.java
+++ b/luni/src/test/java/libcore/xml/PullParserDtdTest.java
@@ -182,6 +182,20 @@ public abstract class PullParserDtdTest extends TestCase {
         assertEquals(XmlPullParser.END_DOCUMENT, parser.next());
     }
 
+    /**
+     * Android's ExpatPullParser replaces missing entities with the empty string
+     * when an external DTD is declared.
+     */
+    public void testExternalDtdAndMissingEntity() throws Exception {
+        String xml = "<!DOCTYPE foo SYSTEM \"http://127.0.0.1:1/no-such-file.dtd\">"
+                + "<foo>&a;</foo>";
+        XmlPullParser parser = newPullParser(xml);
+        assertEquals(XmlPullParser.START_TAG, parser.next());
+        assertEquals(XmlPullParser.END_TAG, parser.next());
+        assertEquals(XmlPullParser.END_DOCUMENT, parser.next());
+    }
+
+
     public void testExternalIdIsCaseSensitive() throws Exception {
         // The spec requires 'SYSTEM' in upper case
         String xml = "<!DOCTYPE foo ["
@@ -448,6 +462,10 @@ public abstract class PullParserDtdTest extends TestCase {
         }
     }
 
+    /**
+     * In honeycomb, KxmlParser's DTD handling was improved but no longer
+     * supports returning the full DTD text. http://b/3241492
+     */
     public void testDoctypeWithNextToken() throws Exception {
         String xml = "<!DOCTYPE foo [<!ENTITY bb \"bar baz\">]><foo>a&bb;c</foo>";
         XmlPullParser parser = newPullParser(xml);
@@ -459,7 +477,7 @@ public abstract class PullParserDtdTest extends TestCase {
         assertEquals("a", parser.getText());
         assertEquals(XmlPullParser.ENTITY_REF, parser.nextToken());
         assertEquals("bb", parser.getName());
-        assertEquals("bar baz", parser.getText()); // TODO: this fails on gingerbread
+        assertEquals("bar baz", parser.getText());
         assertEquals(XmlPullParser.TEXT, parser.nextToken());
         assertEquals("c", parser.getText());
         assertEquals(XmlPullParser.END_TAG, parser.next());
diff --git a/luni/src/test/java/libcore/xml/PullParserTest.java b/luni/src/test/java/libcore/xml/PullParserTest.java
index 21342b2..2862de0 100644
--- a/luni/src/test/java/libcore/xml/PullParserTest.java
+++ b/luni/src/test/java/libcore/xml/PullParserTest.java
@@ -102,16 +102,14 @@ public abstract class PullParserTest extends TestCase {
         XmlPullParser parser = newPullParser();
         parser.setInput(new StringReader(
                 "<foo>&#2147483647; &#-2147483648;</foo>"));
-        assertEquals(XmlPullParser.START_TAG, parser.next());
-        assertNextFails(parser);
+        assertParseFailure(parser);
     }
 
     public void testNumericEntitiesLargerThanInt() throws Exception {
         XmlPullParser parser = newPullParser();
         parser.setInput(new StringReader(
                 "<foo>&#2147483648;</foo>"));
-        assertEquals(XmlPullParser.START_TAG, parser.next());
-        assertNextFails(parser);
+        assertParseFailure(parser);
     }
 
     public void testCharacterReferenceOfHexUtf16Surrogates() throws Exception {
@@ -144,8 +142,7 @@ public abstract class PullParserTest extends TestCase {
     public void testOmittedNumericEntities() throws Exception {
         XmlPullParser parser = newPullParser();
         parser.setInput(new StringReader("<foo>&#;</foo>"));
-        assertEquals(XmlPullParser.START_TAG, parser.next());
-        assertNextFails(parser);
+        assertParseFailure(parser);
     }
 
     /**
@@ -243,8 +240,7 @@ public abstract class PullParserTest extends TestCase {
     public void testMissingEntities() throws Exception {
         XmlPullParser parser = newPullParser();
         parser.setInput(new StringReader("<foo>&aaa;</foo>"));
-        assertEquals(XmlPullParser.START_TAG, parser.next());
-        assertNextFails(parser);
+        assertParseFailure(parser);
     }
 
     public void testMissingEntitiesWithRelaxed() throws Exception {
@@ -291,7 +287,7 @@ public abstract class PullParserTest extends TestCase {
     public void testMissingEntitiesInAttributesUsingNext() throws Exception {
         XmlPullParser parser = newPullParser();
         parser.setInput(new StringReader("<foo b='&aaa;'></foo>"));
-        assertNextFails(parser);
+        assertParseFailure(parser);
     }
 
     public void testMissingEntitiesInAttributesUsingNextWithRelaxed() throws Exception {
@@ -345,14 +341,13 @@ public abstract class PullParserTest extends TestCase {
     public void testLessThanInText() throws Exception{
         XmlPullParser parser = newPullParser();
         parser.setInput(new StringReader("<foo><</foo>"));
-        assertEquals(XmlPullParser.START_TAG, parser.next());
-        assertNextFails(parser);
+        assertParseFailure(parser);
     }
 
     public void testLessThanInAttribute() throws Exception{
         XmlPullParser parser = newPullParser();
         parser.setInput(new StringReader("<foo a='<'></foo>"));
-        assertNextFails(parser);
+        assertParseFailure(parser);
     }
 
     public void testQuotesInAttribute() throws Exception{
@@ -381,36 +376,31 @@ public abstract class PullParserTest extends TestCase {
     public void testCdataDelimiterInText() throws Exception{
         XmlPullParser parser = newPullParser();
         parser.setInput(new StringReader("<foo>]]></foo>"));
-        assertEquals(XmlPullParser.START_TAG, parser.next());
-        assertNextFails(parser);
+        assertParseFailure(parser);
     }
 
     public void testUnexpectedEof() throws Exception {
         XmlPullParser parser = newPullParser();
         parser.setInput(new StringReader("<foo><![C"));
-        assertEquals(XmlPullParser.START_TAG, parser.next());
-        assertNextFails(parser);
+        assertParseFailure(parser);
     }
 
     public void testUnexpectedSequence() throws Exception {
         XmlPullParser parser = newPullParser();
         parser.setInput(new StringReader("<foo><![Cdata[bar]]></foo>"));
-        assertEquals(XmlPullParser.START_TAG, parser.next());
-        assertNextFails(parser);
+        assertParseFailure(parser);
     }
 
     public void testThreeDashCommentDelimiter() throws Exception {
         XmlPullParser parser = newPullParser();
         parser.setInput(new StringReader("<foo><!--a---></foo>"));
-        assertEquals(XmlPullParser.START_TAG, parser.next());
-        assertNextFails(parser);
+        assertParseFailure(parser);
     }
 
     public void testTwoDashesInComment() throws Exception {
         XmlPullParser parser = newPullParser();
         parser.setInput(new StringReader("<foo><!-- -- --></foo>"));
-        assertEquals(XmlPullParser.START_TAG, parser.next());
-        assertNextFails(parser);
+        assertParseFailure(parser);
     }
 
     public void testEmptyComment() throws Exception {
@@ -719,9 +709,10 @@ public abstract class PullParserTest extends TestCase {
         assertEquals("ns:default", parser.getNamespaceUri(0));
     }
 
-    private void assertNextFails(XmlPullParser parser) throws IOException {
+    private void assertParseFailure(XmlPullParser parser) throws IOException {
         try {
-            parser.next();
+            while (parser.next() != XmlPullParser.END_DOCUMENT) {
+            }
             fail();
         } catch (XmlPullParserException expected) {
         }
diff --git a/xml/src/main/java/org/kxml2/io/KXmlParser.java b/xml/src/main/java/org/kxml2/io/KXmlParser.java
index ba905a9..5cd2810 100644
--- a/xml/src/main/java/org/kxml2/io/KXmlParser.java
+++ b/xml/src/main/java/org/kxml2/io/KXmlParser.java
@@ -879,17 +879,17 @@ public class KXmlParser implements XmlPullParser, Closeable {
 
         skip();
         int quote = peekCharacter();
+        String entityValue;
         if (quote == '"' || quote == '\'') {
             position++;
-            String value = readValue((char) quote, true, false, ValueContext.ENTITY_DECLARATION);
+            entityValue = readValue((char) quote, true, false, ValueContext.ENTITY_DECLARATION);
             position++;
-            if (generalEntity && processDocDecl) {
-                if (documentEntities == null) {
-                    documentEntities = new HashMap<String, char[]>();
-                }
-                documentEntities.put(name, value.toCharArray());
-            }
         } else if (readExternalId(true, false)) {
+            /*
+             * Map external entities to the empty string. This is dishonest,
+             * but it's consistent with Android's Expat pull parser.
+             */
+            entityValue = "";
             skip();
             if (peekCharacter() == NDATA[0]) {
                 read(NDATA);
@@ -900,6 +900,13 @@ public class KXmlParser implements XmlPullParser, Closeable {
             throw new XmlPullParserException("Expected entity value or external ID", this, null);
         }
 
+        if (generalEntity && processDocDecl) {
+            if (documentEntities == null) {
+                documentEntities = new HashMap<String, char[]>();
+            }
+            documentEntities.put(name, entityValue.toCharArray());
+        }
+
         skip();
         read('>');
     }
@@ -1220,6 +1227,17 @@ public class KXmlParser implements XmlPullParser, Closeable {
             return;
         }
 
+        /*
+         * The parser skipped an external DTD, and now we've encountered an
+         * unknown entity that could have been declared there. Map it to the
+         * empty string. This is dishonest, but it's consistent with Android's
+         * old ExpatPullParser.
+         */
+        if (systemId != null) {
+            out.delete(start, out.length());
+            return;
+        }
+
         // keep the unresolved entity "&code;" in the text for relaxed clients
         unresolved = true;
         if (throwOnResolveFailure) {
@@ -1257,8 +1275,9 @@ public class KXmlParser implements XmlPullParser, Closeable {
          * If we're lucky (which we usually are), we'll return a single slice of
          * the buffer. This fast path avoids allocating a string builder.
          *
-         * There are 5 unlucky characters we could encounter:
+         * There are 6 unlucky characters we could encounter:
          *  - "&":  entities must be resolved.
+         *  - "%":  parameter entities are unsupported in entity values.
          *  - "<":  this isn't permitted in attributes unless relaxed.
          *  - "]":  this requires a lookahead to defend against the forbidden
          *          CDATA section delimiter "]]>".
@@ -1312,7 +1331,8 @@ public class KXmlParser implements XmlPullParser, Closeable {
                     && (c != '\n' || valueContext != ValueContext.ATTRIBUTE)
                     && c != '&'
                     && c != '<'
-                    && (c != ']' || valueContext != ValueContext.TEXT)) {
+                    && (c != ']' || valueContext != ValueContext.TEXT)
+                    && (c != '%' || valueContext != ValueContext.ENTITY_DECLARATION)) {
                 isWhitespace &= (c <= ' ');
                 position++;
                 continue;
@@ -1355,6 +1375,10 @@ public class KXmlParser implements XmlPullParser, Closeable {
                 }
                 isWhitespace = false;
 
+            } else if (c == '%') {
+                throw new XmlPullParserException("This parser doesn't support parameter entities",
+                        this, null);
+
             } else {
                 throw new AssertionError();
             }