Correct C implementation issues for sparklemotion#2494.

TreyE · Apr 3, 2022 · 8b2c3ea · 8b2c3ea
1 parent fb83666
commit 8b2c3ea
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 5 deletions.
diff --git a/ext/nokogiri/xml_node.c b/ext/nokogiri/xml_node.c
@@ -35,10 +35,16 @@ _xml_node_mark(xmlNodePtr node)
 
 
 static void
-relink_namespace(xmlNodePtr reparented)
+relink_namespace(xmlNodePtr reparented, int reconcile_namespaces)
 {
   xmlNodePtr child;
   xmlAttrPtr attr;
+  xmlNsPtr possible_collision_ns;
+  int ns_collision;
+
+  if (reconcile_namespaces && reparented->doc) {
+    xmlReconciliateNs(reparented->doc, reparented);
+  }
 
   if (reparented->type != XML_ATTRIBUTE_NODE &&
       reparented->type != XML_ELEMENT_NODE) { return; }
@@ -89,9 +95,16 @@ relink_namespace(xmlNodePtr reparented)
                       reparented->parent,
                       curr->href
                     );
+      /* Track and check for a namespace which might be 'squatting' on a
+       * the same prefix but a different href. */
+      ns_collision = 0;
+      possible_collision_ns = xmlSearchNs(reparented->doc, reparented->parent, curr->prefix);
+      if (possible_collision_ns && !xmlStrEqual(curr->href, possible_collision_ns->href)) {
+        ns_collision = 1;
+      }
       /* If we find the namespace is already declared, remove it from this
        * definition list. */
-      if (ns && ns != curr && xmlStrEqual(ns->prefix, curr->prefix)) {
+      if (ns && ns != curr && !ns_collision && xmlStrEqual(ns->prefix, curr->prefix)) {
         if (prev) {
           prev->next = curr->next;
         } else {
@@ -132,14 +145,14 @@ relink_namespace(xmlNodePtr reparented)
   /* their namespaces are reparented as well. */
   child = reparented->children;
   while (NULL != child) {
-    relink_namespace(child);
+    relink_namespace(child, 0);
     child = child->next;
   }
 
   if (reparented->type == XML_ELEMENT_NODE) {
     attr = reparented->properties;
     while (NULL != attr) {
-      relink_namespace((xmlNodePtr)attr);
+      relink_namespace((xmlNodePtr)attr, 0);
       attr = attr->next;
     }
   }
@@ -189,6 +202,7 @@ reparent_node_with(VALUE pivot_obj, VALUE reparentee_obj, pivot_reparentee_func
 {
   VALUE reparented_obj ;
   xmlNodePtr reparentee, original_reparentee, pivot, reparented, next_text, new_next_text, parent ;
+  int reconcile_ns = 1;
   int original_ns_prefix_is_default = 0 ;
 
   if (!rb_obj_is_kind_of(reparentee_obj, cNokogiriXmlNode)) {
@@ -198,6 +212,11 @@ reparent_node_with(VALUE pivot_obj, VALUE reparentee_obj, pivot_reparentee_func
     rb_raise(rb_eArgError, "node must be a Nokogiri::XML::Node");
   }
 
+  // Don't reconcile children of fragments.
+  if (rb_obj_is_kind_of(pivot_obj, cNokogiriXmlDocumentFragment)) {
+    reconcile_ns = 0;
+  }
+
   Data_Get_Struct(reparentee_obj, xmlNode, reparentee);
   Data_Get_Struct(pivot_obj, xmlNode, pivot);
 
@@ -379,7 +398,7 @@ reparent_node_with(VALUE pivot_obj, VALUE reparentee_obj, pivot_reparentee_func
   /* if we've created a cycle, raise an exception */
   raise_if_ancestor_of_self(reparented);
 
-  relink_namespace(reparented);
+  relink_namespace(reparented, reconcile_ns);
 
   return reparented_obj ;
 }

diff --git a/test/xml/test_namespace.rb b/test/xml/test_namespace.rb
@@ -93,6 +93,28 @@ def test_maintain_element_namespaces_in_urn
         assert_equal("urn:xmpp:foospec:barfoo", child.namespace.href)
         assert_empty(child.attributes)
       end
+
+      def test_maintain_element_namespaces_with_abbreviation_squating
+        root_namespace_href = "urn:root_namespace"
+        child1_namespace_href = "urn:child1_namespace"
+        doc = Nokogiri::XML(<<-eoxml)
+          <root xmlns="#{root_namespace_href}">
+            <child1 xmlns="#{child1_namespace_href}"/>
+            <child2/>
+          </root>
+        eoxml
+
+        child2 = doc.at_xpath("//ns1:child2", { "ns1" => root_namespace_href })
+        child1 = doc.at_xpath("//ns2:child1", { "ns2" => child1_namespace_href })
+        child1.add_child(child2)
+        new_xml = doc.to_xml
+
+        new_doc = Nokogiri::XML(new_xml)
+        new_child1 = new_doc.at_xpath("//ns2:child1", { "ns2" => child1_namespace_href })
+        new_child2 = new_child1.first_element_child
+        new_child2_ns_href = new_child2.namespace.href
+        assert_equal(root_namespace_href, new_child2_ns_href)
+      end
     end
   end
 end