Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cruby): reconciliation of ns on unparented nodes #3462

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 19 additions & 11 deletions ext/java/nokogiri/XmlNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,7 @@ public class XmlNode extends RubyObject
}

Element e = (Element) node;
Node parent = e.getParentNode();

// disable error checking to prevent lines like the following
// from throwing a `NAMESPACE_ERR' exception:
Expand All @@ -439,22 +440,27 @@ public class XmlNode extends RubyObject
set_namespace(context, ((XmlNode)parent(context)).namespace(context));
}
} else {
String currentPrefix = e.getParentNode().lookupPrefix(nsURI);
String currentURI = e.getParentNode().lookupNamespaceURI(prefix);
boolean isDefault = e.getParentNode().isDefaultNamespace(nsURI);

// add xmlns attribute if this is a new root node or if the node's
// namespace isn't a default namespace in the new document
if (e.getParentNode().getNodeType() == Node.DOCUMENT_NODE) {
if (parent == null || parent.getNodeType() == Node.DOCUMENT_NODE) {
// this is the root node, so we must set the namespaces attributes anyway
e.setAttribute(prefix == null ? "xmlns" : "xmlns:" + prefix, nsURI);
} else if (prefix == null) {
// this is a default namespace but isn't the default where this node is being added
if (!isDefault) { e.setAttribute("xmlns", nsURI); }
} else if (!prefix.equals(currentPrefix) || !nsURI.equals(currentURI)) {
// this is a prefixed namespace
// but doesn't have the same prefix or the prefix is set to a different URI
e.setAttribute("xmlns:" + prefix, nsURI);
boolean isDefault = parent.isDefaultNamespace(nsURI);

if (!isDefault) {
e.setAttribute("xmlns", nsURI);
}
} else if (parent.getNodeType() == Node.ELEMENT_NODE) {
String currentPrefix = parent.lookupPrefix(nsURI);
String currentURI = parent.lookupNamespaceURI(prefix);

if (!prefix.equals(currentPrefix) || !nsURI.equals(currentURI)) {
// this is a prefixed namespace
// but doesn't have the same prefix or the prefix is set to a different URI
e.setAttribute("xmlns:" + prefix, nsURI);
}
}
}

Expand Down Expand Up @@ -492,7 +498,7 @@ public class XmlNode extends RubyObject

// if this namespace is a duplicate of what's already in the document, remove it.
// a "duplicate" here is if the prefix and the URI both match what resolves in the parent.
if (e.getParentNode().getNodeType() == Node.ELEMENT_NODE) {
if (parent != null && parent.getNodeType() == Node.ELEMENT_NODE) {
RubyArray<?> nsdefs = this.namespace_definitions(context);
for (int j = 0 ; j < nsdefs.getLength() ; j++) {
XmlNamespace ns = (XmlNamespace)nsdefs.get(j);
Expand Down Expand Up @@ -1524,6 +1530,8 @@ public class XmlNode extends RubyObject
// and keep the return value. -mbklein
String new_name = NokogiriHelpers.newQName(ns.getPrefix(), node);
this.node = NokogiriHelpers.renameNode(node, ns.getHref(), new_name);

this.relink_namespace(context);
}

clearXpathContext(getNode());
Expand Down
4 changes: 4 additions & 0 deletions ext/nokogiri/xml_node.c
Original file line number Diff line number Diff line change
Expand Up @@ -1334,6 +1334,10 @@ set_namespace(VALUE self, VALUE namespace)

xmlSetNs(node, ns);

if (ns && ns->prefix) {
xmlReconciliateNs(node->doc, node);
}

return self;
}

Expand Down
13 changes: 13 additions & 0 deletions test/xml/test_node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1336,6 +1336,19 @@ def test_namespace_in_rendered_xml
assert_match(/xmlns="bar"/, subject.to_xml)
end

# https://github.com/sparklemotion/nokogiri/issues/3459
def test_namespace_set_on_unparented_node_also_sets_a_definition
doc = Nokogiri::XML(%{<root xmlns:foo="http://example.com/foo"></root>})
ns = doc.root.namespace_definitions.first

node = doc.create_element("other")
node.namespace = ns

assert_pattern do
node.namespace_definitions => [ { prefix: "foo", href: "http://example.com/foo" } ]
end
end

# issue 771
def test_format_noblank
content = <<~XML
Expand Down
23 changes: 23 additions & 0 deletions test/xml/test_node_reparenting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,29 @@ def coerce(data)
end
end
end

# https://github.com/sparklemotion/nokogiri/issues/3459
describe "reparenting with duplicate namespace prefixes" do
it "stitches together ok" do
doc = Nokogiri::XML(<<~XML)
<dnd:adventure xmlns:dnd="http://www.w3.org/dungeons#">
<dnd:party xmlns:dnd="http://www.w3.org/dragons#">
<dnd:members>
</dnd:members>
</dnd:party>
</dnd:adventure>
XML

dungeons_ns = doc.root.namespace_definitions.find { |ns| ns.prefix == "dnd" }
parent = doc.xpath("//ns:members", ns: "http://www.w3.org/dragons#").first

node = doc.create_element("character")
node.namespace = dungeons_ns
parent.add_child(node)

assert_includes(doc.to_xml, %{<dnd:character xmlns:dnd="http://www.w3.org/dungeons#"/>})
end
end
end
end
end
Expand Down
Loading