Fixing a critical regression in our SAX parsing.
We weren't correctly covering the case where namespaces were off, but
elements contained namespaces. See bug 2400596.
diff --git a/libcore/xml/src/main/native/org_apache_harmony_xml_ExpatParser.cpp b/libcore/xml/src/main/native/org_apache_harmony_xml_ExpatParser.cpp
index 1944bdc..4721800 100644
--- a/libcore/xml/src/main/native/org_apache_harmony_xml_ExpatParser.cpp
+++ b/libcore/xml/src/main/native/org_apache_harmony_xml_ExpatParser.cpp
@@ -471,7 +471,9 @@
}
/**
- * Returns the element or attribute local name, like "h1". Never empty.
+ * Returns the element or attribute local name, like "h1". Never empty. When
+ * namespace processing is disabled, this may contain a prefix, yielding a
+ * local name like "html:h1". In such cases, the qName will always be empty.
*/
jstring localName() {
return internString(mEnv, mParsingContext, mLocalName);
@@ -504,13 +506,17 @@
bool matchesQName(const char* qName) {
char* lastColon = strrchr(qName, ':');
- // if the input doesn't have a colon, there's no namespace prefix. Our
- // prefix must be empty and the qName must equal our localName
- if (lastColon == NULL) {
- return strlen(mPrefix) == 0 && strcmp(qName, mLocalName) == 0;
+ // Compare local names only if either:
+ // - the input qualified name doesn't have a colon (like "h1")
+ // - this element doesn't have a prefix. Such is the case when it
+ // doesn't belong to a namespace, or when this parser's namespace
+ // processing is disabled. In the latter case, this element's local
+ // name may still contain a colon (like "html:h1").
+ if (lastColon == NULL || *mPrefix == 0) {
+ return strcmp(qName, mLocalName) == 0;
}
- // otherwise the prefixes must be equal and our localName must equal qName
+ // Otherwise compare both prefix and local name
size_t prefixLength = lastColon - qName;
return strlen(mPrefix) == prefixLength
&& strncmp(qName, mPrefix, prefixLength) == 0
@@ -1004,7 +1010,7 @@
}
/**
- * Creates a new Expat parser. Called from the Java ExpatParser contructor.
+ * Creates a new Expat parser. Called from the Java ExpatParser constructor.
*
* @param object the Java ExpatParser instance
* @param javaEncoding the character encoding name
@@ -1208,11 +1214,6 @@
jint attributePointer, jint index) {
XML_Parser parser = (XML_Parser) pointer;
ParsingContext* context = (ParsingContext*) XML_GetUserData(parser);
-
- if (!context->processNamespaces) {
- return emptyString;
- }
-
return ExpatElementName(env, context, attributePointer, index).uri();
}
@@ -1229,11 +1230,6 @@
jint attributePointer, jint index) {
XML_Parser parser = (XML_Parser) pointer;
ParsingContext* context = (ParsingContext*) XML_GetUserData(parser);
-
- if (!context->processNamespaces) {
- return emptyString;
- }
-
return ExpatElementName(env, context, attributePointer, index).localName();
}
@@ -1250,11 +1246,6 @@
jint attributePointer, jint index) {
XML_Parser parser = (XML_Parser) pointer;
ParsingContext* context = (ParsingContext*) XML_GetUserData(parser);
-
- if (context->processNamespaces) {
- return emptyString;
- }
-
return ExpatElementName(env, context, attributePointer, index).qName();
}
diff --git a/libcore/xml/src/test/java/org/apache/harmony/xml/NamespacedAttributesLookupTest.java b/libcore/xml/src/test/java/org/apache/harmony/xml/NamespacedAttributesLookupTest.java
new file mode 100644
index 0000000..4f58262
--- /dev/null
+++ b/libcore/xml/src/test/java/org/apache/harmony/xml/NamespacedAttributesLookupTest.java
@@ -0,0 +1,139 @@
+/*
+ * Copyright (C) 2010 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.harmony.xml;
+
+import junit.framework.TestCase;
+import org.xml.sax.Attributes;
+import org.xml.sax.InputSource;
+import org.xml.sax.XMLReader;
+import org.xml.sax.helpers.DefaultHandler;
+
+import javax.xml.parsers.SAXParserFactory;
+import java.io.StringReader;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+/**
+ * Tests that we both report and retrieve attributes using the appropriate
+ * names for different combinations of namespaces and namespace prefixes.
+ */
+public class NamespacedAttributesLookupTest extends TestCase {
+
+ private static final String SAX_PROPERTY_NS =
+ "http://xml.org/sax/features/namespaces";
+ private static final String SAX_PROPERTY_NS_PREFIXES =
+ "http://xml.org/sax/features/namespace-prefixes";
+
+ private static String xml = "<?xml version='1.0' encoding='UTF-8'?>" +
+ "<test xmlns='http://foo' xmlns:bar='http://bar' xmlns:baz='http://baz' baz:c='a'>" +
+ "<b c='w' bar:c='x'/>" +
+ "<bar:e baz:c='y' bar:c='z'/>" +
+ "</test>";
+
+ public void testNamespace() throws Exception {
+ List<String> expected = Arrays.asList(
+ "http://foo,test\n" +
+ " http://baz,c\n" +
+ " http://bar+c=null,\n" +
+ " bar:c=null\n",
+
+ "http://foo,b\n" +
+ " ,c\n" +
+ " http://bar,c\n" +
+ " http://bar+c=x,\n" +
+ " bar:c=x\n",
+
+ "http://bar,e\n" +
+ " http://baz,c\n" +
+ " http://bar,c\n" +
+ " http://bar+c=z,\n" +
+ " bar:c=z\n");
+
+ boolean namespace = true;
+ boolean namespacePrefixes = false;
+ assertEquals(expected, getStartElements(xml, namespace, namespacePrefixes));
+ }
+
+ public void testNamespacePrefixes() throws Exception {
+ List<String> expected = Arrays.asList(
+ "test\n" +
+ " xmlns\n" +
+ " xmlns:bar\n" +
+ " xmlns:baz\n" +
+ " baz:c\n" +
+ " http://bar+c=null,\n" +
+ " bar:c=null\n",
+
+ "b\n" +
+ " c\n" +
+ " bar:c\n" +
+ " http://bar+c=null,\n" +
+ " bar:c=x\n",
+
+ "bar:e\n" +
+ " baz:c\n" +
+ " bar:c\n" +
+ " http://bar+c=null,\n" +
+ " bar:c=z\n");
+
+ boolean namespace = false;
+ boolean namespacePrefixes = true;
+ assertEquals(expected, getStartElements(xml, namespace, namespacePrefixes));
+ }
+
+ public List<String> getStartElements(String xml, final boolean namespace, boolean namespacePrefixes)
+ throws Exception {
+ final List<String> result = new ArrayList<String>();
+ XMLReader reader = SAXParserFactory.newInstance().newSAXParser().getXMLReader();
+ reader.setFeature(SAX_PROPERTY_NS, namespace);
+ reader.setFeature(SAX_PROPERTY_NS_PREFIXES, namespacePrefixes);
+ reader.setContentHandler(new DefaultHandler() {
+ @Override public final void startElement(
+ String uri, String localName, String qName, Attributes attributes) {
+ StringBuilder serialized = new StringBuilder();
+ /*
+ * Only supply the uri+localName or qname depending on whether namespaces are
+ * enabled. It's an optional parameter and the RI only supplies one or the other.
+ */
+ if (namespace) {
+ serialized.append(uri).append(",");
+ serialized.append(localName);
+ } else {
+ serialized.append(qName);
+ }
+ for (int i = 0; i < attributes.getLength(); i++) {
+ serialized.append("\n ");
+ if (namespace) {
+ serialized.append(attributes.getURI(i)).append(",");
+ serialized.append(attributes.getLocalName(i));
+ } else {
+ serialized.append(attributes.getQName(i));
+ }
+ }
+ serialized.append("\n http://bar+c=")
+ .append(attributes.getValue("http://bar", "c")).append(",")
+ .append("\n bar:c=")
+ .append(attributes.getValue("bar:c"))
+ .append("\n");
+ result.add(serialized.toString());
+ }
+ });
+ reader.parse(new InputSource(new StringReader(xml)));
+ return result;
+ }
+}
