changeset 32037:114a98da1a5f

xmlnode: Fix some brokeness in xmlnode serialization with prefixed elements. Basically we were treating node->xmlns as the default namespace, but that isn't the case with prefexed elements. In our serialization, I believe we were adding an extraneous xmlns='' to a prefixed element, which changes the (default) namespace for its children. (It's been a bit too long with this in my tree, so I've forgotten the exact details)

--- a/libpurple/tests/test_xmlnode.c	Sun Sep 04 18:45:51 2011 +0000
+++ b/libpurple/tests/test_xmlnode.c	Sun Sep 04 18:52:18 2011 +0000
@@ -21,6 +21,66 @@
 }
 END_TEST
 
+#define check_doc_structure(x) { \
+	xmlnode *ping, *child1, *child2; \
+	fail_if(x == NULL, "Failed to parse document"); \
+	ping = xmlnode_get_child(x, "ping"); \
+	fail_if(ping == NULL, "Failed to find 'ping' child"); \
+	child1 = xmlnode_get_child(ping, "child1"); \
+	fail_if(child1 == NULL, "Failed to find 'child1'"); \
+	child2 = xmlnode_get_child(child1, "child2"); \
+	fail_if(child2 == NULL, "Failed to find 'child2'"); \
+	xmlnode_new_child(child2, "a"); \
+	\
+	assert_string_equal("jabber:client", xmlnode_get_namespace(x)); \
+	/* NOTE: xmlnode_get_namespace() returns the namespace of the element, not the
+	 * current default namespace.  See http://www.w3.org/TR/xml-names/#defaulting and
+	 * http://www.w3.org/TR/xml-names/#dt-defaultNS.
+	 */ \
+	assert_string_equal("urn:xmpp:ping", xmlnode_get_namespace(ping)); \
+	assert_string_equal("jabber:client", xmlnode_get_namespace(child1)); \
+	assert_string_equal("urn:xmpp:ping", xmlnode_get_namespace(child2)); \
+	/*
+	 * This fails (well, actually crashes [the ns is NULL]) unless
+	 * xmlnode_new_child() actually sets the element namespace.
+	assert_string_equal("jabber:client", xmlnode_get_namespace(xmlnode_get_child(child2, "a")));
+	 */ \
+	\
+	assert_string_equal("jabber:client", xmlnode_get_default_namespace(x)); \
+	assert_string_equal("jabber:client", xmlnode_get_default_namespace(ping)); \
+	assert_string_equal("jabber:client", xmlnode_get_default_namespace(child1)); \
+	assert_string_equal("jabber:client", xmlnode_get_default_namespace(child2)); \
+}
+
+START_TEST(test_xmlnode_prefixes)
+{
+	const char *xml_doc =
+		"<iq type='get' xmlns='jabber:client' xmlns:ping='urn:xmpp:ping'>"
+			"<ping:ping>"
+				"<child1>"
+					"<ping:child2></ping:child2>" /* xmlns='jabber:child' */
+				"</child1>"
+			"</ping:ping>"
+		"</iq>";
+	char *str;
+	xmlnode *xml, *reparsed;
+
+	xml = xmlnode_from_str(xml_doc, -1);
+	check_doc_structure(xml);
+
+	/* Check that xmlnode_from_str(xmlnode_to_str(xml, NULL), -1) is idempotent. */
+	str = xmlnode_to_str(xml, NULL);
+	fail_if(str == NULL, "Failed to serialize XMLnode");
+	reparsed = xmlnode_from_str(str, -1);
+	fail_if(reparsed == NULL, "Failed to reparse xml document");
+	check_doc_structure(reparsed);
+
+	g_free(str);
+	xmlnode_free(xml);
+	xmlnode_free(reparsed);
+}
+END_TEST
+
 Suite *
 xmlnode_suite(void)
 {
@@ -28,6 +88,8 @@
 
 	TCase *tc = tcase_create("xmlnode");
 	tcase_add_test(tc, test_xmlnode_billion_laughs_attack);
+	tcase_add_test(tc, test_xmlnode_prefixes);
+
 	suite_add_tcase(s, tc);
 
 	return s;

--- a/libpurple/xmlnode.c	Sun Sep 04 18:45:51 2011 +0000
+++ b/libpurple/xmlnode.c	Sun Sep 04 18:52:18 2011 +0000
@@ -78,6 +78,19 @@
 	node = new_node(name, XMLNODE_TYPE_TAG);
 
 	xmlnode_insert_child(parent, node);
+#if 0
+	/* This would give xmlnodes more appropriate namespacing
+	 * when creating them.  Otherwise, unless an explicit namespace
+	 * is set, xmlnode_get_namespace() will return NULL, when
+	 * there may be a default namespace.
+	 *
+	 * I'm unconvinced that it's useful, and concerned it may break things.
+	 *
+	 * _insert_child would need the same thing, probably (assuming
+	 * xmlns->node == NULL)
+	 */
+	xmlnode_set_namespace(node, xmlnode_get_default_namespace(node))
+#endif
 
 	return node;
 }
@@ -255,13 +268,34 @@
 	node->xmlns = g_strdup(xmlns);
 }
 
-const char *xmlnode_get_namespace(xmlnode *node)
+const char *xmlnode_get_namespace(const xmlnode *node)
 {
 	g_return_val_if_fail(node != NULL, NULL);
 
 	return node->xmlns;
 }
 
+const char *xmlnode_get_default_namespace(const xmlnode *node)
+{
+	const char *ns = NULL;
+	g_return_val_if_fail(node != NULL, NULL);
+
+	/* If this node does *not* have a prefix, node->xmlns is the default
+	 * namespace.  Otherwise, it's the prefix namespace.
+	 */
+	if (!node->prefix && node->xmlns) {
+		return node->xmlns;
+	} else if (node->namespace_map) {
+		ns = g_hash_table_lookup(node->namespace_map, "");
+	}
+
+	/* No default ns found?  Walk up the tree looking for one */
+	if (!(ns && *ns) && node->parent)
+		ns = xmlnode_get_default_namespace(node->parent);
+
+	return ns;
+}
+
 void xmlnode_set_prefix(xmlnode *node, const char *prefix)
 {
 	g_return_if_fail(node != NULL);
@@ -443,12 +477,22 @@
 	if (node->namespace_map) {
 		g_hash_table_foreach(node->namespace_map,
 			(GHFunc)xmlnode_to_str_foreach_append_ns, text);
-	} else if (node->xmlns) {
-		if(!node->parent || !purple_strequal(node->xmlns, node->parent->xmlns))
+	} else {
+		/* Figure out if this node has a different default namespace from parent */
+		const char *xmlns = NULL;
+		const char *parent_xmlns = NULL;
+		if (!prefix)
+			xmlns = node->xmlns;
+
+		if (!xmlns)
+			xmlns = xmlnode_get_default_namespace(node);
+		if (node->parent)
+			parent_xmlns = xmlnode_get_default_namespace(node->parent);
+		if (!purple_strequal(xmlns, parent_xmlns))
 		{
-			char *xmlns = g_markup_escape_text(node->xmlns, -1);
-			g_string_append_printf(text, " xmlns='%s'", xmlns);
-			g_free(xmlns);
+			char *escaped_xmlns = g_markup_escape_text(xmlns, -1);
+			g_string_append_printf(text, " xmlns='%s'", escaped_xmlns);
+			g_free(escaped_xmlns);
 		}
 	}
 	for(c = node->child; c; c = c->next)

--- a/libpurple/xmlnode.h	Sun Sep 04 18:45:51 2011 +0000
+++ b/libpurple/xmlnode.h	Sun Sep 04 18:52:18 2011 +0000
@@ -221,7 +221,26 @@
  * @param node The node to get the namepsace from
  * @return The namespace of this node
  */
-const char *xmlnode_get_namespace(xmlnode *node);
+const char *xmlnode_get_namespace(const xmlnode *node);
+
+/**
+ * Returns the current default namespace.  The default
+ * namespace is the current namespace which applies to child
+ * elements which are unprefixed and which do not contain their
+ * own namespace.
+ *
+ * For example, given:
+ * <iq type='get' xmlns='jabber:client' xmlns:ns1='http://example.org/ns1'>
+ *     <ns1:element><child1/></ns1:element>
+ * </iq>
+ *
+ * The default namespace of all nodes (including 'child1') is "jabber:client",
+ * though the namespace for 'element' is "http://example.org/ns1".
+ *
+ * @param node The node for which to return the default namespace
+ * @return The default namespace of this node
+ */
+const char *xmlnode_get_default_namespace(const xmlnode *node);
 
 /**
  * Sets the prefix of a node