pidgin: d9b2d73b: xmlnode: Fix some brokeness in xmlnode s...
darkrain42 at pidgin.im
darkrain42 at pidgin.im
Sun Sep 4 14:56:09 EDT 2011
----------------------------------------------------------------------
Revision: d9b2d73b4d6e64c7768adff67303cfa8816d6d2e
Parent: 3f9eb1322506cf55ee52bab2f8ebe27c3edd28c6
Author: darkrain42 at pidgin.im
Date: 09/04/11 14:52:18
Branch: im.pidgin.pidgin
URL: http://d.pidgin.im/viewmtn/revision/info/d9b2d73b4d6e64c7768adff67303cfa8816d6d2e
Changelog:
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)
Changes against parent 3f9eb1322506cf55ee52bab2f8ebe27c3edd28c6
patched libpurple/tests/test_xmlnode.c
patched libpurple/xmlnode.c
patched libpurple/xmlnode.h
-------------- next part --------------
============================================================
--- libpurple/xmlnode.c bbd9ea0b506dedb3aa734d0fb4ea162fa6de7d08
+++ libpurple/xmlnode.c aee6abd361df1eb41d15d5ce1333f73a26723fa6
@@ -78,6 +78,19 @@ xmlnode_new_child(xmlnode *parent, const
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 @@ void xmlnode_set_namespace(xmlnode *node
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 @@ xmlnode_to_str_helper(const xmlnode *nod
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)
============================================================
--- libpurple/xmlnode.h c9149d3cf014760cca6561de58ac2834c5e833b0
+++ libpurple/xmlnode.h c2dc393a2b8e3667e6b762a942f25f6c1db453dc
@@ -221,9 +221,28 @@ void xmlnode_set_namespace(xmlnode *node
* @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
*
* @param node The node to qualify
============================================================
--- libpurple/tests/test_xmlnode.c ef8840551902714078a88e375c19a8caf02be7d8
+++ libpurple/tests/test_xmlnode.c a64c414376163c6a17fb2bb21b31fd782e6061d8
@@ -21,6 +21,66 @@ END_TEST
}
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 @@ xmlnode_suite(void)
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;
More information about the Commits
mailing list