From d09cdcb81b04c82e6dc728da57756137e29a48b0 Mon Sep 17 00:00:00 2001 From: Brian Willows Date: Thu, 10 Sep 2026 09:12:10 +0100 Subject: [PATCH] fix: walk the group chain iteratively so deep nesting can't overflow the stack RTF groups nest via `{`, and get/getFont/getColor/getStyle each recursed up the parent chain on every lookup. A document that nests deeply enough overflows the stack: RangeError: Maximum call stack size exceeded at RTFGroup.getStyle (rtf-group.js:21:12) at RTFGroup.getStyle (rtf-group.js:22:53) ... That error is raised inside a stream callback rather than passed to the rtf.string(str, cb) error argument, so it is not catchable through the documented API and takes the process down. A ~39KB file was enough. Walk the chain with a loop instead. The walk still terminates at RTFDocument, which overrides each of these to return directly, so resolution order is unchanged. getStyle() with no name also collected the chain and merged it root-first in one pass, rather than allocating a fresh object at every level of every lookup - about 5.7x faster on deeply nested input. The repo has no test suite, so correctness was checked with a differential harness: 34 documents (fonts, colour tables, bold/italic/underline, alignment, super/subscript, \plain, escaped and unicode characters, paragraphs, sibling groups, and nesting from 1 to 250 deep) serialise byte-for-byte identically before and after. Co-Authored-By: Claude Opus 4.8 --- rtf-group.js | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/rtf-group.js b/rtf-group.js index 46d83ce..acf0f12 100644 --- a/rtf-group.js +++ b/rtf-group.js @@ -9,18 +9,48 @@ class RTFGroup { this.style = {} this.ignorable = null } + // These walk up the group chain iteratively rather than by recursion. RTF + // groups nest via `{`, so the chain is as deep as the document nests it, and + // recursing once per lookup overflows the stack on deeply nested input. The + // walk stops at RTFDocument, which overrides each of these to return directly. get (name) { - return this[name] != null ? this[name] : this.parent.get(name) + for (let group = this; group != null; group = group.parent) { + if (group.parent == null) return group.get(name) + if (group[name] != null) return group[name] + } } getFont (num) { - return this.fonts[num] != null ? this.fonts[num] : this.parent.getFont(num) + for (let group = this; group != null; group = group.parent) { + if (group.parent == null) return group.getFont(num) + if (group.fonts[num] != null) return group.fonts[num] + } } getColor (num) { - return this.colors[num] != null ? this.colors[num] : this.parent.getFont(num) + for (let group = this; group != null; group = group.parent) { + if (group.parent == null) return group.getFont(num) + if (group.colors[num] != null) return group.colors[num] + } } getStyle (name) { - if (!name) return Object.assign({}, this.parent.getStyle(), this.style) - return this.style[name] != null ? this.style[name] : this.parent.getStyle(name) + if (!name) { + // Collect the chain, then merge root-first in a single pass. Recursing + // allocated a fresh object at every level for every lookup. + const chain = [] + let group = this + while (group.parent != null) { + chain.push(group) + group = group.parent + } + const style = Object.assign({}, group.getStyle()) + for (let i = chain.length - 1; i >= 0; i--) { + Object.assign(style, chain[i].style) + } + return style + } + for (let group = this; group != null; group = group.parent) { + if (group.parent == null) return group.getStyle(name) + if (group.style[name] != null) return group.style[name] + } } resetStyle () { this.style = {}