Skip to content
Merged
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
11 changes: 11 additions & 0 deletions FIDELITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,17 @@ The committed PNGs under `testdata/renders/` back every claim here. Reproduce
them with the commands at the bottom. The measured-vs-Chrome numbers live in
[`bench/REPORT.md`](bench/REPORT.md).

## 2026-10-17 (round 173) — the SAME memory-exhaustion shape as round 172, but simpler to trigger: a single `innerHTML` assignment, no loop at all, found by generalizing the lesson rather than re-checking the same code path (security audit, patch)

Continuing round 172's own sweep (JS methods that can allocate unboundedly from a tree it already has) into the one other place a script routinely hands this engine a whole subtree at once: `innerHTML`/`outerHTML`/`insertAdjacentHTML`. All three ultimately call `dom.ParseFragment` (or, for the top-level page, `dom.Parse`), both of which share one core: `convertChildren`.

- **Finding**: `golang.org/x/net/html`'s own parser refuses markup nested past 512 levels, but that guard is about DEPTH — a WIDE document (millions of sibling elements, never nesting past a handful of levels) sails straight through it. `convertChildren` had no count of its own at all. Confirmed live, two ways: `root.innerHTML = "<div>x</div>".repeat(3000000)` — one statement, no loop, the repeated fragment a mere 13 bytes — allocated **~11.5GB of real memory in 15 seconds**; a 25-iteration `root.innerHTML = root.outerHTML + root.outerHTML` loop (the same doubling shape as round 172's `cloneNode` case, but through serialize-then-reparse instead of direct cloning) reached ~6GB.
- **Fixed**: `convertChildren` now carries a shared node-count `budget` (`maxParsedNodes = 200,000`, matching `js.maxCloneNodes`'s own value and reasoning — the same bug, one round earlier, for `cloneNode`'s bushy-growth case) threaded through both its own recursion and the `<template>` content-fragment branch. Past zero, remaining siblings/descendants are simply not converted — the real tree loses nodes near the bound rather than crashing or exhausting memory, the same degradation shape as every other budget cap in this codebase.
- **Measured effect**: the flat `repeat(3000000)` reproduction drops from ~11.5GB/15.4s to ~1.1GB/1.8s; the `outerHTML`-doubling loop drops from ~6GB/8.3s to ~1.4GB/3.5s.
- **Test**: `dom/mutate_test.go`'s `TestParseFragmentCapsTotalNodeCount` and `dom/dom_test.go`'s `TestParseCapsTotalNodeCount` (the shared `countTreeNodes` helper, new in `mutate_test.go`), each building a `maxParsedNodes+1000`-repeat string. `Parse`'s own count needed a `+1` margin over `ParseFragment`'s: `Parse` wraps `convertChildren`'s budgeted output in a synthetic `Document` node created BEFORE the budget starts counting, so the full tree is one node larger than the budget alone would suggest — confirmed precisely (it reported exactly `maxParsedNodes+1`, not a looser overshoot) before adjusting the assertion rather than second-guessing the fix.
- **Coverage**: `convertChildren` at 100%; `dom` package 98.4%→98.5% (confirmed via `git stash`, no regression — and this time stashing tracked and untracked paths SEPARATELY, per round 172's own named lesson, which held up cleanly).
- **Same honest caveat as round 172's own `cloneNode` fix**: this bounds one `Parse`/`ParseFragment` call, not a whole render — a script calling `innerHTML =` many times could still accumulate several×`maxParsedNodes` before the JS script timeout stops it. Bounded and linear now, not a hard per-render ceiling.

## 2026-10-17 (round 172) — closing round 171's own open lead: 25 lines of ordinary-looking script allocated ~21GB of REAL memory, and neither an existing heap watchdog nor this round's own depth cap could stop it (security audit, patch)

Round 171 flagged, but did not characterize, a 25-iteration `root.appendChild(root.cloneNode(true))` loop that rendered a 10.5-million-pixel-tall page without crashing — unclear at the time whether that was genuinely safe (sparse, unpainted memory) or just not yet pushed hard enough. Measured directly with `runtime.MemStats` before/after: **~19.75GB of real HeapAlloc, ~21GB of real Sys**, in under 9 seconds, from 25 ordinary DOM calls on a trivial page. Not sparse at all — `image.NewRGBA` eagerly allocates and zero-fills every byte, and `newCanvas`'s own fill pass then writes to all of it.
Expand Down
35 changes: 31 additions & 4 deletions dom/dom.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ func Parse(htmlSrc string) (*Node, error) {
return nil, err
}
doc := &Node{Type: Document, Quirks: !hasDoctype(root)}
convertChildren(root, doc)
budget := maxParsedNodes
convertChildren(root, doc, &budget)
attachDeclarativeShadowRoots(doc)
return doc, nil
}
Expand All @@ -184,21 +185,45 @@ func hasDoctype(root *html.Node) bool {
return false
}

// maxParsedNodes bounds the total nodes one Parse/ParseFragment call may
// produce. golang.org/x/net/html already refuses markup nested past 512
// levels, but that guard is about DEPTH, not total node count — a WIDE
// document (millions of SIBLING elements, never nesting past a handful of
// levels) sails through it untouched. Confirmed live: a single statement,
// `root.innerHTML = "<div>x</div>".repeat(3000000)`, allocates ~11.5GB of
// real memory in 15 seconds — no loop, no deep nesting, no large string by
// pathological standards (the repeated fragment is 13 bytes). 200,000
// matches js.maxCloneNodes (the same shape of bug, found one round
// earlier, for cloneNode's own bushy-growth case) — generous for any real
// page while keeping one parse's own worst case a bounded amount of memory.
const maxParsedNodes = 200000

// convertChildren walks h's children, converting element, text and comment
// nodes and dropping everything else (doctype, processing instructions — no
// real page's rendering depends on either surviving into the DOM). A
// <template>'s own children are parsed into its Content fragment instead of
// its Children, per spec (see Node.Content's doc comment) — this is the ONE
// place besides NewElement a template can originate from, so both must agree.
func convertChildren(h *html.Node, parent *Node) {
//
// budget bounds the TOTAL nodes this call (and its own recursion) may add —
// see maxParsedNodes. Past zero, remaining siblings/descendants are simply
// not converted (the real tree loses nodes near this bound rather than
// crashing or exhausting memory) — the same "reduce, don't drop entirely"
// shape as the budget caps elsewhere in this codebase (css's
// expandFunctionalPseudosBudget, js's cloneNode).
func convertChildren(h *html.Node, parent *Node, budget *int) {
for c := h.FirstChild; c != nil; c = c.NextSibling {
if *budget <= 0 {
return
}
switch c.Type {
case html.CommentNode:
parent.Children = append(parent.Children, &Node{
Type: Comment,
Text: c.Data,
Parent: parent,
})
*budget--
case html.ElementNode:
tag := strings.ToLower(c.Data)
el := &Node{
Expand All @@ -211,11 +236,12 @@ func convertChildren(h *html.Node, parent *Node) {
el.Attr[strings.ToLower(a.Key)] = a.Val
}
parent.Children = append(parent.Children, el)
*budget--
if tag == "template" {
el.Content = &Node{Type: Element, Tag: "#fragment", Attr: map[string]string{}}
convertChildren(c, el.Content)
convertChildren(c, el.Content, budget)
} else {
convertChildren(c, el)
convertChildren(c, el, budget)
}
case html.TextNode:
// Preserve the raw text; whitespace handling is a layout concern.
Expand All @@ -224,6 +250,7 @@ func convertChildren(h *html.Node, parent *Node) {
Text: c.Data,
Parent: parent,
})
*budget--
}
}
}
Expand Down
24 changes: 23 additions & 1 deletion dom/dom_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@

package dom

import "testing"
import (
"strings"
"testing"
)

func TestParseAndAttributes(t *testing.T) {
root, err := Parse(`<!DOCTYPE html><html><head><title> Hi </title></head>` +
Expand Down Expand Up @@ -151,3 +154,22 @@ func TestParseQuirksMode(t *testing.T) {
t.Error("<!DOCTYPE html> present: want Quirks=false")
}
}

func TestParseCapsTotalNodeCount(t *testing.T) {
// See TestParseFragmentCapsTotalNodeCount (mutate_test.go) for the full
// rationale: golang.org/x/net/html's own depth guard (512 levels) does
// nothing against a WIDE document, and this is the top-level Parse
// (the main document fetch) rather than ParseFragment, sharing the same
// convertChildren budget.
root, err := Parse(`<html><body>` + strings.Repeat("<div>x</div>", maxParsedNodes+1000) + `</body></html>`)
if err != nil {
t.Fatal(err)
}
// +1: root itself is the synthetic Document wrapper Parse creates BEFORE
// calling convertChildren, so it is one node outside the budget
// convertChildren's own recursion tracks (confirmed: without the +1,
// this reported exactly maxParsedNodes+1, not a looser overshoot).
if got := countTreeNodes([]*Node{root}); got > maxParsedNodes+1 {
t.Errorf("Parse produced %d nodes, want <= %d (maxParsedNodes+1)", got, maxParsedNodes+1)
}
}
3 changes: 2 additions & 1 deletion dom/mutate.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,8 @@ func ParseFragment(htmlSrc string) ([]*Node, error) {
container.AppendChild(p)
}
holder := &Node{Type: Element, Tag: "body", Attr: map[string]string{}}
convertChildren(container, holder)
budget := maxParsedNodes
convertChildren(container, holder, &budget)
for _, c := range holder.Children {
c.Parent = nil
}
Expand Down
36 changes: 35 additions & 1 deletion dom/mutate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@

package dom

import "testing"
import (
"strings"
"testing"
)

func TestNewElementAndText(t *testing.T) {
el := NewElement("DIV")
Expand Down Expand Up @@ -248,6 +251,37 @@ func TestParseFragment(t *testing.T) {
}
}

func countTreeNodes(nodes []*Node) int {
c := 0
for _, n := range nodes {
c++
c += countTreeNodes(n.Children)
if n.Content != nil {
c++
c += countTreeNodes(n.Content.Children)
}
}
return c
}

func TestParseFragmentCapsTotalNodeCount(t *testing.T) {
// A WIDE document (many sibling elements, never nesting past a handful
// of levels) sails straight through golang.org/x/net/html's own
// 512-level DEPTH guard untouched — that guard is about depth, not
// total node count. Confirmed live: a single statement,
// `root.innerHTML = "<div>x</div>".repeat(3000000)` (SetInnerHTML calls
// ParseFragment), allocated ~11.5GB of real memory in 15 seconds with
// no loop and no deep nesting at all — the 13-byte repeated fragment is
// nowhere near a pathologically large string by itself.
nodes, err := ParseFragment(strings.Repeat("<div>x</div>", maxParsedNodes+1000))
if err != nil {
t.Fatal(err)
}
if got := countTreeNodes(nodes); got > maxParsedNodes {
t.Errorf("ParseFragment produced %d nodes, want <= %d (maxParsedNodes)", got, maxParsedNodes)
}
}

func TestSerializeTextNode(t *testing.T) {
// A bare text node serializes (and escapes) directly.
if got := OuterHTML(NewText("<b>")); got != "&lt;b&gt;" {
Expand Down
Loading