Fix unexpected EOF handling by KXmlParser.

Avoid read position slipping beyond limit on EOF, causing an
ArrayIndexOutOfBoundsException.

Also fix two incorrect checks whether to read more data.

Bug: 9012715
Change-Id: I40f60becd97eb4d9f33842bea2c19e9dc531d3b7

2 files changed, 49 insertions, 5 deletions

diff --git a/luni/src/test/java/libcore/xml/PullParserTest.java b/luni/src/test/java/libcore/xml/PullParserTest.java
index 71125aa..c59b358 100644
--- a/luni/src/test/java/libcore/xml/PullParserTest.java
+++ b/luni/src/test/java/libcore/xml/PullParserTest.java
@@ -742,8 +742,42 @@ public abstract class PullParserTest extends TestCase {
         assertEquals("foo", parser.getName());
     }
 
+    public void testEofInElementSpecRelaxed() throws Exception {
+        assertRelaxedParseFailure("<!DOCTYPE foo [<!ELEMENT foo (unterminated");
+    }
+
+    public void testEofInAttributeValue() throws Exception {
+        assertParseFailure("<!DOCTYPE foo [<!ATTLIST foo x y \"unterminated");
+    }
+
+    public void testEofInEntityValue() throws Exception {
+        assertParseFailure("<!DOCTYPE foo [<!ENTITY aaa \"unterminated");
+    }
+
+    public void testEofInStartTagAttributeValue() throws Exception {
+        assertParseFailure("<long foo=\"unterminated");
+    }
+
+    public void testEofInReadCharRelaxed() throws Exception {
+        assertRelaxedParseFailure("<!DOCTYPE foo [<!ELEMENT foo ()"); // EOF in read('>')
+    }
+
+    public void testEofAfterReadCharArrayRelaxed() throws Exception {
+        assertRelaxedParseFailure("<!DOCTYPE foo [<!ELEMENT foo EMPTY"); // EOF in read('>')
+    }
+
     private void assertParseFailure(String xml) throws Exception {
         XmlPullParser parser = newPullParser();
+        assertParseFailure(xml, parser);
+    }
+
+    private void assertRelaxedParseFailure(String xml) throws Exception {
+        XmlPullParser parser = newPullParser();
+        parser.setFeature("http://xmlpull.org/v1/doc/features.html#relaxed", true);
+        assertParseFailure(xml, parser);
+    }
+
+    private void assertParseFailure(String xml, XmlPullParser parser) throws Exception {
         parser.setInput(new StringReader(xml));
         try {
             while (parser.next() != XmlPullParser.END_DOCUMENT) {
diff --git a/xml/src/main/java/org/kxml2/io/KXmlParser.java b/xml/src/main/java/org/kxml2/io/KXmlParser.java
index 80fd8ca..84376ae 100644
--- a/xml/src/main/java/org/kxml2/io/KXmlParser.java
+++ b/xml/src/main/java/org/kxml2/io/KXmlParser.java
@@ -468,7 +468,7 @@ public class KXmlParser implements XmlPullParser, Closeable {
 
         search:
         while (true) {
-            if (position + delimiter.length >= limit) {
+            if (position + delimiter.length > limit) {
                 if (start < position && returnText) {
                     if (result == null) {
                         result = new StringBuilder();
@@ -756,6 +756,9 @@ public class KXmlParser implements XmlPullParser, Closeable {
                     depth++;
                 } else if (c == ')') {
                     depth--;
+                } else if (c == -1) {
+                    throw new XmlPullParserException(
+                            "Unterminated element content spec", this, null);
                 }
                 position++;
                 c = peekCharacter();
@@ -857,7 +860,9 @@ public class KXmlParser implements XmlPullParser, Closeable {
                 position++;
                 // TODO: does this do escaping correctly?
                 String value = readValue((char) c, true, true, ValueContext.ATTRIBUTE);
-                position++;
+                if (peekCharacter() == c) {
+                    position++;
+                }
                 defineAttributeDefault(elementName, attributeName, value);
             }
         }
@@ -907,7 +912,9 @@ public class KXmlParser implements XmlPullParser, Closeable {
         if (quote == '"' || quote == '\'') {
             position++;
             entityValue = readValue((char) quote, true, false, ValueContext.ENTITY_DECLARATION);
-            position++;
+            if (peekCharacter() == quote) {
+                position++;
+            }
         } else if (readExternalId(true, false)) {
             /*
              * Map external entities to the empty string. This is dishonest,
@@ -1109,7 +1116,7 @@ public class KXmlParser implements XmlPullParser, Closeable {
                 attributes[i + 3] = readValue(delimiter, true, throwOnResolveFailure,
                         ValueContext.ATTRIBUTE);
 
-                if (delimiter != ' ') {
+                if (delimiter != ' ' && peekCharacter() == delimiter) {
                     position++; // end quote
                 }
             } else if (relaxed) {
@@ -1424,12 +1431,15 @@ public class KXmlParser implements XmlPullParser, Closeable {
         int c = peekCharacter();
         if (c != expected) {
             checkRelaxed("expected: '" + expected + "' actual: '" + ((char) c) + "'");
+            if (c == -1) {
+                return; // On EOF, don't move position beyond limit
+            }
         }
         position++;
     }
 
     private void read(char[] chars) throws IOException, XmlPullParserException {
-        if (position + chars.length >= limit && !fillBuffer(chars.length)) {
+        if (position + chars.length > limit && !fillBuffer(chars.length)) {
             checkRelaxed("expected: '" + new String(chars) + "' but was EOF");
             return;
         }