From 7b6f79ba94625744e9311b28d33e1e1d92c88cdb Mon Sep 17 00:00:00 2001
From: Mike Dalessio <mike.dalessio@gmail.com>
Date: Mon, 10 May 2021 17:38:28 -0400
Subject: [PATCH] fix: ensure namespaced attributes are reparented properly

Previously, the namespace was dropped from the attribute.

Fixes #2228

Co-authored-by: Nikolaj Potashnikoff <34554890+fiddlededee@users.noreply.github.com>
---
 CHANGELOG.md                      |  5 +++++
 ext/nokogiri/xml_node.c           |  6 ++++--
 test/xml/test_node_reparenting.rb | 24 ++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7f1a6c0e91..b44c37420b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,6 +23,11 @@ Many thanks to Sam Ruby, Steve Checkoway, and Craig Barnes for creating and main
 * `Nokogiri.XSLT` parses stylesheets using `ParseOptions::DEFAULT_XSLT`, which should make some edge-case XSL transformations match libxslt's default behavior. [[#1940](https://github.com/sparklemotion/nokogiri/issues/1940)]
 
 
+### Fixed
+
+* Namespaced attributes are handled properly when their parent node is reparented into another document. Previously, the namespace may have gotten dropped. [[#2228](https://github.com/sparklemotion/nokogiri/issues/2228)]
+
+
 ### Improved
 
 * [CRuby] Speed up (slightly) the compile time of packaged libraries `libiconv`, `libxml2`, and `libxslt` by using autoconf's `--disable-dependency-tracking` option. ("ruby" platform gem only.)
diff --git a/ext/nokogiri/xml_node.c b/ext/nokogiri/xml_node.c
index 849a1402f4..723ff72164 100644
--- a/ext/nokogiri/xml_node.c
+++ b/ext/nokogiri/xml_node.c
@@ -166,7 +166,7 @@ static VALUE
 reparent_node_with(VALUE pivot_obj, VALUE reparentee_obj, pivot_reparentee_func prf)
 {
   VALUE reparented_obj ;
-  xmlNodePtr reparentee, pivot, reparented, next_text, new_next_text, parent ;
+  xmlNodePtr reparentee, original_reparentee, pivot, reparented, next_text, new_next_text, parent ;
   int original_ns_prefix_is_default = 0 ;
 
   if (!rb_obj_is_kind_of(reparentee_obj, cNokogiriXmlNode)) {
@@ -252,7 +252,7 @@ reparent_node_with(VALUE pivot_obj, VALUE reparentee_obj, pivot_reparentee_func
   }
 
 ok:
-  xmlUnlinkNode(reparentee);
+  original_reparentee = reparentee;
 
   if (reparentee->doc != pivot->doc || reparentee->type == XML_TEXT_NODE) {
     /*
@@ -309,6 +309,8 @@ reparent_node_with(VALUE pivot_obj, VALUE reparentee_obj, pivot_reparentee_func
     }
   }
 
+  xmlUnlinkNode(original_reparentee);
+
   if (prf != xmlAddPrevSibling && prf != xmlAddNextSibling
       && reparentee->type == XML_TEXT_NODE && pivot->next && pivot->next->type == XML_TEXT_NODE) {
     /*
diff --git a/test/xml/test_node_reparenting.rb b/test/xml/test_node_reparenting.rb
index 2bd80cf9f0..77fc1ebe42 100644
--- a/test/xml/test_node_reparenting.rb
+++ b/test/xml/test_node_reparenting.rb
@@ -360,6 +360,30 @@ def coerce(data)
                 assert_match(%r{<first xmlns=\"http://tenderlovemaking.com/\">}, @doc.to_xml)
               end
             end
+
+            describe "and a child node has a namespaced attribute" do
+              # https://github.com/sparklemotion/nokogiri/issues/2228
+              it "should not lose attribute namespace" do
+                source_doc = Nokogiri::XML::Document.parse(<<~EOXML)
+                  <pre1:root xmlns:pre1="ns1" xmlns:pre2="ns2">
+                    <pre1:child pre2:attr="attrval">
+                  </pre1:root>
+                EOXML
+                assert(source_node = source_doc.at_xpath("//pre1:child", {"pre1" => "ns1"}))
+                assert_equal("attrval", source_node.attribute_with_ns("attr", "ns2")&.value)
+
+                dest_doc = Nokogiri::XML::Document.parse(<<~EOXML)
+                  <pre1:root xmlns:pre1="ns1" xmlns:pre2="ns2">
+                  </pre1:root>
+                EOXML
+                assert(dest_node = dest_doc.at_xpath("//pre1:root", {"pre1" => "ns1"}))
+
+                inserted = dest_node.add_child(source_node)
+
+                assert_equal("attrval", inserted.attribute_with_ns("attr", "ns2")&.value,
+                             "inserted node attribute should be namespaced")
+              end
+            end
           end
 
           describe "given a parent node with a default and non-default namespace" do