Node#html_escape_html and #html_escape_href leak the detached cmark buffer

[alexremn]

Markly::Node#html_escape_html and #html_escape_href leak the buffer they allocate, so every render through Markly::Renderer::HTML leaks native memory in proportion to the escaped text. Every text node, inline code span, code block, link destination and title goes through these two methods (Renderer::Generic#escape_html / #escape_href).

Cause

In ext/markly/markly.c (0.19.0 and current main), both functions detach the cmark_strbuf and copy it into a Ruby string, but never free the detached pointer:

result = (char *)cmark_strbuf_detach(&buf);
return rb_str_new2(result);

cmark_strbuf_detach hands ownership of the allocation to the caller. commonmarker 0.23, which this code descends from, freed it after the copy (commonmarker_cstr_adopt → cmark_get_default_mem_allocator()->free(str)).

Reproduction

Ruby 4.0.6 and 3.3, markly 0.19.0, Debian bookworm:

require 'markly'

def rss = File.read('/proc/self/status')[/VmRSS:\s+(\d+)/, 1].to_i

text = ("R&D <b> \"q\" [link](https://example.com/a b?x=1&y=2) " * 20 + "\n\n") * 5
doc = Markly.parse(text)
3.times { Markly::Renderer::HTML.new.render(doc) }
GC.start; before = rss
20_000.times { Markly::Renderer::HTML.new.render(doc) }
GC.start
puts "RSS delta: #{rss - before} KB"

• markly 0.19.0 as released: +194,152 KB (the rendered document is about 9 KB), and it never comes back.
• Same gem built with the patch below: +352 KB.

Patch

--- a/ext/markly/markly.c
+++ b/ext/markly/markly.c
@@ -1209,7 +1209,9 @@
 	if (houdini_escape_href(&buf, (const uint8_t *)RSTRING_PTR(rb_text),
 													(bufsize_t)RSTRING_LEN(rb_text))) {
 		result = (char *)cmark_strbuf_detach(&buf);
-		return rb_str_new2(result);
+		VALUE rb_result = rb_str_new2(result);
+		mem->free(result);
+		return rb_result;
 	}
 
 	return rb_text;
@@ -1229,7 +1231,9 @@
 	if (houdini_escape_html0(&buf, (const uint8_t *)RSTRING_PTR(rb_text),
 													 (bufsize_t)RSTRING_LEN(rb_text), 0)) {
 		result = (char *)cmark_strbuf_detach(&buf);
-		return rb_str_new2(result);
+		VALUE rb_result = rb_str_new2(result);
+		mem->free(result);
+		return rb_result;
 	}
 
 	return rb_text;

mem is already in scope (cmark_node_mem(node)), and it is the allocator the buffer was created with.

Related, not part of the patch

rb_str_new2 measures the result with strlen and returns an ASCII-8BIT string. So html_escape_html truncates its output at the first NUL byte (html_escape_html("a\0b") returns "a"), and both methods return binary rather than the input's encoding. Parsing replaces NUL with U+FFFD, so this does not show up through Markly.parse, only when the methods are called directly. Taking buf.size before detaching and using rb_enc_str_new(result, len, rb_enc_get(rb_text)) would fix both, but it changes the return encoding, so it may deserve its own change.

In the meantime we work around the leak by overriding escape_html / escape_href in our Markly::Renderer::HTML subclass with Ruby ports of houdini's escapers. They are byte-identical to the native output over the CommonMark spec examples.