Skip to content

Commit

Permalink
fix: Document#remove_namespaces! use-after-free bug
Browse files Browse the repository at this point in the history
  • Loading branch information
flavorjones committed Oct 16, 2022
1 parent 5f58b34 commit 73d73d6
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 1 deletion.
6 changes: 5 additions & 1 deletion ext/nokogiri/xml_document.c
Expand Up @@ -104,7 +104,11 @@ recursively_remove_namespaces_from_node(xmlNodePtr node)
(node->type == XML_XINCLUDE_START) ||
(node->type == XML_XINCLUDE_END)) &&
node->nsDef) {
xmlFreeNsList(node->nsDef);
xmlNsPtr curr = node->nsDef;
while (curr) {
noko_xml_document_pin_namespace(curr, node->doc);
curr = curr->next;
}
node->nsDef = NULL;
}

Expand Down
22 changes: 22 additions & 0 deletions test/test_compaction.rb
Expand Up @@ -39,5 +39,27 @@

doc.at_xpath("//root:first", "root" => "http://example.com/root").namespace_scopes.inspect
end

it "remove_namespaces!" do
skip unless GC.respond_to?(:verify_compaction_references)

doc = Nokogiri::XML(<<~XML)
<root xmlns:a="http://a.flavorjon.es/" xmlns:b="http://b.flavorjon.es/">
<a:foo>hello from a</a:foo>
<b:foo>hello from b</b:foo>
<container xmlns:c="http://c.flavorjon.es/">
<c:foo c:attr='attr-value'>hello from c</c:foo>
</container>
</root>
XML

namespaces = doc.root.namespaces
namespaces.each(&:inspect)
doc.remove_namespaces!

GC.verify_compaction_references(double_heap: true, toward: :empty)

namespaces.each(&:inspect)
end
end
end
41 changes: 41 additions & 0 deletions test/test_memory_leak.rb
Expand Up @@ -191,6 +191,47 @@ def test_leaking_namespace_node_strings_with_prefix
end
end

def test_document_remove_namespaces_with_ruby_objects
xml = <<~XML
<root xmlns:a="http://a.flavorjon.es/" xmlns:b="http://b.flavorjon.es/">
<a:foo>hello from a</a:foo>
<b:foo>hello from b</b:foo>
<container xmlns:c="http://c.flavorjon.es/">
<c:foo c:attr='attr-value'>hello from c</c:foo>
</container>
</root>
XML

20.times do
10_000.times do
doc = Nokogiri::XML::Document.parse(xml)
doc.namespaces.each(&:inspect)
doc.remove_namespaces!
end
puts MemInfo.rss
end
end

def test_document_remove_namespaces_without_ruby_objects
xml = <<~XML
<root xmlns:a="http://a.flavorjon.es/" xmlns:b="http://b.flavorjon.es/">
<a:foo>hello from a</a:foo>
<b:foo>hello from b</b:foo>
<container xmlns:c="http://c.flavorjon.es/">
<c:foo c:attr='attr-value'>hello from c</c:foo>
</container>
</root>
XML

20.times do
20_000.times do
doc = Nokogiri::XML::Document.parse(xml)
doc.remove_namespaces!
end
puts MemInfo.rss
end
end

def test_leaking_dtd_nodes_after_internal_subset_removal
# see https://github.com/sparklemotion/nokogiri/issues/1784
100_000.times do |i|
Expand Down

0 comments on commit 73d73d6

Please sign in to comment.