Skip to content

Commit

Permalink
test: add coverage for the memsize_of bug
Browse files Browse the repository at this point in the history
which turns out to be because xmlDTD has "attributes" where xmlNode
has "properties"
  • Loading branch information
flavorjones committed Jul 8, 2023
1 parent 81762fa commit 19a64ba
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 2 deletions.
5 changes: 3 additions & 2 deletions ext/nokogiri/xml_document.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,14 @@ memsize_node(const xmlNodePtr node)
{
/* note we don't count namespace definitions, just going for a good-enough number here */
xmlNodePtr child;
xmlAttrPtr property;
size_t memsize = 0;

memsize += xmlStrlen(node->name);

if (node->type == XML_ELEMENT_NODE) {
for (child = (xmlNodePtr)node->properties; child; child = child->next) {
memsize += sizeof(xmlAttr) + memsize_node(child);
for (property = node->properties; property; property = property->next) {
memsize += sizeof(xmlAttr) + memsize_node((xmlNodePtr)property);
}
}
if (node->type == XML_TEXT_NODE) {
Expand Down
15 changes: 15 additions & 0 deletions test/test_memory_leak.rb
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,21 @@ def test_object_space_memsize_of
assert(bigger_name_size > base_size, "longer tags should increase memsize")
end

def test_object_space_memsize_with_dtd
# https://github.com/sparklemotion/nokogiri/issues/2923
require "objspace"
skip("memsize_of not defined") unless ObjectSpace.respond_to?(:memsize_of)

doc = Nokogiri::XML(<<~XML)
<?xml version="1.0"?>
<!DOCTYPE staff PUBLIC "staff.dtd" [
<!ATTLIST payment type CDATA "check">
]>
<staff></staff>
XML
ObjectSpace.memsize_of(doc) # assert_does_not_crash
end

module MemInfo
# from https://stackoverflow.com/questions/7220896/get-current-ruby-process-memory-usage
# this is only going to work on linux
Expand Down

0 comments on commit 19a64ba

Please sign in to comment.