Merge change I49b05da2

* changes:
  Various XML fixes.

6 files changed, 100 insertions, 41 deletions

diff --git a/luni/src/test/java/javax/xml/parsers/DocumentBuilderTest.java b/luni/src/test/java/javax/xml/parsers/DocumentBuilderTest.java
index 3bbc4e3..6d3c463 100644
--- a/luni/src/test/java/javax/xml/parsers/DocumentBuilderTest.java
+++ b/luni/src/test/java/javax/xml/parsers/DocumentBuilderTest.java
@@ -18,6 +18,7 @@ package javax.xml.parsers;
 
 import java.io.ByteArrayInputStream;
 import org.w3c.dom.Document;
+import org.w3c.dom.Element;
 import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
 
@@ -25,7 +26,7 @@ import junit.framework.Test;
 import junit.framework.TestSuite;
 
 public class DocumentBuilderTest extends junit.framework.TestCase {
-    private static String parse(String xml) throws Exception {
+    private static Document domOf(String xml) throws Exception {
         DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
         dbf.setCoalescing(true);
         dbf.setExpandEntityReferences(true);
@@ -33,26 +34,61 @@ public class DocumentBuilderTest extends junit.framework.TestCase {
         ByteArrayInputStream stream = new ByteArrayInputStream(xml.getBytes());
         DocumentBuilder builder = dbf.newDocumentBuilder();
         
-        Document doc = builder.parse(stream);
-        
-        Node titleNode = doc.getFirstChild();
-        NodeList children = titleNode.getChildNodes();
+        return builder.parse(stream);
+    }
+    
+    private static String firstChildTextOf(Document doc) throws Exception {
+        NodeList children = doc.getFirstChild().getChildNodes();
         assertEquals(1, children.getLength());
         return children.item(0).getNodeValue();
     }
     
     // http://code.google.com/p/android/issues/detail?id=2607
     public void test_characterReferences() throws Exception {
-        assertEquals("aAb", parse("<p>a&#65;b</p>"));
-        assertEquals("aAb", parse("<p>a&#x41;b</p>"));
+        assertEquals("aAb", firstChildTextOf(domOf("<p>a&#65;b</p>")));
+        assertEquals("aAb", firstChildTextOf(domOf("<p>a&#x41;b</p>")));
     }
 
     // http://code.google.com/p/android/issues/detail?id=2607
     public void test_predefinedEntities() throws Exception {
-        assertEquals("a<b", parse("<p>a&lt;b</p>"));
-        assertEquals("a>b", parse("<p>a&gt;b</p>"));
-        assertEquals("a&b", parse("<p>a&amp;b</p>"));
-        assertEquals("a'b", parse("<p>a&apos;b</p>"));
-        assertEquals("a\"b", parse("<p>a&quot;b</p>"));
+        assertEquals("a<b", firstChildTextOf(domOf("<p>a&lt;b</p>")));
+        assertEquals("a>b", firstChildTextOf(domOf("<p>a&gt;b</p>")));
+        assertEquals("a&b", firstChildTextOf(domOf("<p>a&amp;b</p>")));
+        assertEquals("a'b", firstChildTextOf(domOf("<p>a&apos;b</p>")));
+        assertEquals("a\"b", firstChildTextOf(domOf("<p>a&quot;b</p>")));
+    }
+    
+    private static Element firstElementOf(Document doc) throws Exception {
+        return (Element) doc.getFirstChild();
+    }
+    
+    private static String attrOf(Element e) throws Exception {
+        return e.getAttribute("attr");
+    }
+    
+    // http://code.google.com/p/android/issues/detail?id=2487
+    public void test_cdata_attributes() throws Exception {
+        assertEquals("hello & world", attrOf(firstElementOf(domOf("<?xml version=\"1.0\"?><root attr=\"hello &amp; world\" />"))));
+        try {
+            domOf("<?xml version=\"1.0\"?><root attr=\"hello <![CDATA[ some-cdata ]]> world\" />");
+            fail("SAXParseException not thrown");
+        } catch (org.xml.sax.SAXParseException ex) {
+            // Expected.
+        }
+        assertEquals("hello <![CDATA[ some-cdata ]]> world", attrOf(firstElementOf(domOf("<?xml version=\"1.0\"?><root attr=\"hello &lt;![CDATA[ some-cdata ]]&gt; world\" />"))));
+        assertEquals("hello <![CDATA[ some-cdata ]]> world", attrOf(firstElementOf(domOf("<?xml version=\"1.0\"?><root attr=\"hello &lt;![CDATA[ some-cdata ]]> world\" />"))));
+    }
+    
+    // http://code.google.com/p/android/issues/detail?id=2487
+    public void test_cdata_body() throws Exception {
+        assertEquals("hello & world", firstChildTextOf(domOf("<?xml version=\"1.0\"?><root>hello &amp; world</root>")));
+        assertEquals("hello  some-cdata  world", firstChildTextOf(domOf("<?xml version=\"1.0\"?><root>hello <![CDATA[ some-cdata ]]> world</root>")));
+        assertEquals("hello <![CDATA[ some-cdata ]]> world", firstChildTextOf(domOf("<?xml version=\"1.0\"?><root>hello &lt;![CDATA[ some-cdata ]]&gt; world</root>")));
+        try {
+            domOf("<?xml version=\"1.0\"?><root>hello &lt;![CDATA[ some-cdata ]]> world</root>");
+            fail("SAXParseException not thrown");
+        } catch (org.xml.sax.SAXParseException ex) {
+            // Expected.
+        }
     }
 }
diff --git a/xml/src/main/java/javax/xml/parsers/DocumentBuilderFactory.java b/xml/src/main/java/javax/xml/parsers/DocumentBuilderFactory.java
index 4e186c1..885f1ae 100644
--- a/xml/src/main/java/javax/xml/parsers/DocumentBuilderFactory.java
+++ b/xml/src/main/java/javax/xml/parsers/DocumentBuilderFactory.java
@@ -29,7 +29,7 @@ import org.apache.harmony.xml.parsers.DocumentBuilderFactoryImpl;
  */
 public abstract class DocumentBuilderFactory extends Object {
 
-    private boolean coalesce;
+    private boolean coalesce = true;
 
     private boolean expandEntityReferences;
 
diff --git a/xml/src/main/java/org/apache/harmony/xml/parsers/DocumentBuilderFactoryImpl.java b/xml/src/main/java/org/apache/harmony/xml/parsers/DocumentBuilderFactoryImpl.java
index 4b8eef3..5cdba97 100644
--- a/xml/src/main/java/org/apache/harmony/xml/parsers/DocumentBuilderFactoryImpl.java
+++ b/xml/src/main/java/org/apache/harmony/xml/parsers/DocumentBuilderFactoryImpl.java
@@ -70,10 +70,9 @@ public class DocumentBuilderFactoryImpl extends DocumentBuilderFactory {
          * or by throwing the full SPI monty at it.  
          */
         DocumentBuilderImpl builder = new DocumentBuilderImpl();
-        
+        builder.setCoalescing(isCoalescing());
         builder.setIgnoreComments(isIgnoringComments());
-        builder.setIgnoreElementContentWhitespace(
-                isIgnoringElementContentWhitespace());
+        builder.setIgnoreElementContentWhitespace(isIgnoringElementContentWhitespace());
         builder.setNamespaceAware(isNamespaceAware());
 
         // TODO What about expandEntityReferences?
diff --git a/xml/src/main/java/org/apache/harmony/xml/parsers/DocumentBuilderImpl.java b/xml/src/main/java/org/apache/harmony/xml/parsers/DocumentBuilderImpl.java
index 17438b7..1a8122c 100644
--- a/xml/src/main/java/org/apache/harmony/xml/parsers/DocumentBuilderImpl.java
+++ b/xml/src/main/java/org/apache/harmony/xml/parsers/DocumentBuilderImpl.java
@@ -49,6 +49,8 @@ class DocumentBuilderImpl extends DocumentBuilder {
 
     private static DOMImplementation dom = DOMImplementationImpl.getInstance();
 
+    private boolean coalescing;
+
     private EntityResolver entityResolver;
 
     private ErrorHandler errorHandler;
@@ -261,20 +263,15 @@ class DocumentBuilderImpl extends DocumentBuilder {
                  * whitespace at all.
                  */
                 if (!ignoreElementContentWhitespace) {
-                    appendText(document, node, parser.getText());
+                    appendText(document, node, true, parser.getText());
                 }
-            } else if (token == XmlPullParser.TEXT) {
-                /*
-                 * Found a piece of text. That's the easiest case. We simply
-                 * take it and create a corresponding node.
-                 */
-                appendText(document, node, parser.getText());
-            } else if (token == XmlPullParser.CDSECT) {
+            } else if (token == XmlPullParser.TEXT || token == XmlPullParser.CDSECT) {
                 /*
-                 * Found a CDATA section. That's also trivial. We simply
-                 * take it and create a corresponding node.
+                 * Found a piece of text (possibly encoded as a CDATA section).
+                 * That's the easiest case. We simply take it and create a new text node,
+                 * or merge with an adjacent text node.
                  */
-                node.appendChild(document.createCDATASection(parser.getText()));
+                appendText(document, node, token == XmlPullParser.TEXT, parser.getText());
             } else if (token == XmlPullParser.ENTITY_REF) {
                 /*
                  * Found an entity reference. If an entity resolver is
@@ -289,7 +286,7 @@ class DocumentBuilderImpl extends DocumentBuilder {
 
                 String replacement = resolveStandardEntity(entity);
                 if (replacement != null) {
-                    appendText(document, node, replacement);
+                    appendText(document, node, true, replacement);
                 } else {
                     node.appendChild(document.createEntityReference(entity));
                 }
@@ -374,20 +371,30 @@ class DocumentBuilderImpl extends DocumentBuilder {
         }
     }
 
-    private void appendText(Document document, Node node, String text) {
+    /**
+     * @param isText true for a normal TextNode, false for a CDATA section.
+     * (If we're not coalescing, it matters which kind of node we put into the DOM.)
+     */ 
+    private void appendText(Document document, Node node, boolean isText, String text) {
         // Ignore empty runs.
         if (text.length() == 0) {
             return;
         }
         // Merge with any previous text node if possible.
-        Node lastChild = node.getLastChild();
-        if (lastChild != null && lastChild.getNodeType() == Node.TEXT_NODE) {
-            Text textNode = (Text) lastChild;
-            textNode.setData(textNode.getNodeValue() + text);
-            return;
+        if (coalescing) {
+            Node lastChild = node.getLastChild();
+            if (lastChild != null && lastChild.getNodeType() == Node.TEXT_NODE) {
+                Text textNode = (Text) lastChild;
+                textNode.setData(textNode.getNodeValue() + text);
+                return;
+            }
         }
         // Okay, we really do need a new text node
-        node.appendChild(document.createTextNode(text));
+        if (isText) {
+            node.appendChild(document.createTextNode(text));
+        } else {
+            node.appendChild(document.createCDATASection(text));
+        }
     }
 
     @Override
@@ -409,6 +416,10 @@ class DocumentBuilderImpl extends DocumentBuilder {
         ignoreComments = value;
     }
 
+    public void setCoalescing(boolean value) {
+        coalescing = value;
+    }
+
     /**
      * Controls whether this DocumentBuilder ignores element content whitespace.
      * 
diff --git a/xml/src/main/java/org/kxml2/io/KXmlParser.java b/xml/src/main/java/org/kxml2/io/KXmlParser.java
index 98aae04..c4d8f3d 100644
--- a/xml/src/main/java/org/kxml2/io/KXmlParser.java
+++ b/xml/src/main/java/org/kxml2/io/KXmlParser.java
@@ -319,7 +319,9 @@ public class KXmlParser implements XmlPullParser {
                     return;
 
                 case TEXT :
-                    pushText('<', !token);
+                    // BEGIN android-changed: distinguish attribute values from normal text.
+                    pushText('<', !token, false);
+                    // END android-changed
                     if (depth == 0) {
                         if (isWhitespace)
                             type = IGNORABLE_WHITESPACE;
@@ -680,7 +682,9 @@ public class KXmlParser implements XmlPullParser {
 					read();
 				
                 int p = txtPos;
-                pushText(delimiter, true);
+                // BEGIN android-changed: distinguish attribute values from normal text.
+                pushText(delimiter, true, true);
+                // END android-changed
 
                 attributes[i] = get(p);
                 txtPos = p;
@@ -800,7 +804,7 @@ public class KXmlParser implements XmlPullParser {
     ' ': parse to whitespace or '>'
     */
 
-    private final void pushText(int delimiter, boolean resolveEntities)
+    private final void pushText(int delimiter, boolean resolveEntities, boolean inAttributeValue)
         throws IOException, XmlPullParserException {
 
         int next = peek(0);
@@ -812,21 +816,31 @@ public class KXmlParser implements XmlPullParser {
                 if (next <= ' ' || next == '>')
                     break;
 
+            // BEGIN android-changed: "<" is not allowed in attribute values.
             if (next == '&') {
                 if (!resolveEntities)
                     break;
 
                 pushEntity();
             }
+            else if (next == '<' && inAttributeValue) {
+                error("Illegal: \"<\" inside attribute value");
+            }
             else if (next == '\n' && type == START_TAG) {
                 read();
                 push(' ');
             }
             else
                 push(read());
+            // END android-changed
 
-            if (next == '>' && cbrCount >= 2 && delimiter != ']')
-                error("Illegal: ]]>");
+            // BEGIN android-changed: "]]>" *is* allowed in attribute values, but
+            // is not allowed in regular text between markup.
+            final boolean allowCloseCdata = inAttributeValue;
+            if (!allowCloseCdata && (next == '>' && cbrCount >= 2 && delimiter != ']')) {
+                error("Illegal: \"]]>\" outside CDATA section");
+            }
+            // END android-changed
 
             if (next == ']')
                 cbrCount++;
diff --git a/xml/src/test/java/tests/api/javax/xml/parsers/DocumentBuilderFactoryTest.java b/xml/src/test/java/tests/api/javax/xml/parsers/DocumentBuilderFactoryTest.java
index b6c400f..80c6e3c 100644
--- a/xml/src/test/java/tests/api/javax/xml/parsers/DocumentBuilderFactoryTest.java
+++ b/xml/src/test/java/tests/api/javax/xml/parsers/DocumentBuilderFactoryTest.java
@@ -462,7 +462,6 @@ public class DocumentBuilderFactoryTest extends TestCase {
         method = "setCoalescing",
         args = {boolean.class}
     )
-    @KnownFailure("Should support coalescing")
     public void test_setCoalescingZ() {
         dbf.setCoalescing(true);
         assertTrue(dbf.isCoalescing());