diff --git a/uiterm/tree.go b/uiterm/tree.go index 0990c72..2ec28c0 100644 --- a/uiterm/tree.go +++ b/uiterm/tree.go @@ -89,10 +89,10 @@ func (t *Tree) rebuild(preserveActive bool, sameItem func(previous, current Tree previousLine := t.activeLine lines := []renderedTreeItem{} for _, item := range t.Generator(nil) { - children := t.rebuild_rec(item, 0) - if children != nil { - lines = append(lines, children...) + if len(lines) >= maxTreeLines { + break } + lines = t.rebuild_rec(lines, item, 0) } t.lines = lines if preserveActive { @@ -113,21 +113,31 @@ func (t *Tree) rebuild(preserveActive bool, sameItem func(previous, current Tree } } -func (t *Tree) rebuild_rec(parent TreeItem, level int) []renderedTreeItem { - if parent == nil { - return nil - } - lines := []renderedTreeItem{ - renderedTreeItem{ - Level: level, - Item: parent, - }, +// A server is free to describe a channel graph in which a channel is its own +// ancestor. The generator follows parent/child links literally, so without +// these limits such a graph recurses until the process is out of memory. +// Real trees are orders of magnitude smaller than either bound. +const ( + maxTreeDepth = 64 + maxTreeLines = 100000 +) + +// rebuild_rec appends parent and its descendants to lines. Accumulating into +// one slice keeps maxTreeLines a budget for the whole tree rather than for +// each level, and avoids building a slice per node. +func (t *Tree) rebuild_rec(lines []renderedTreeItem, parent TreeItem, level int) []renderedTreeItem { + if parent == nil || level >= maxTreeDepth || len(lines) >= maxTreeLines { + return lines } + lines = append(lines, renderedTreeItem{ + Level: level, + Item: parent, + }) for _, item := range t.Generator(parent) { - children := t.rebuild_rec(item, level+1) - if children != nil { - lines = append(lines, children...) + if len(lines) >= maxTreeLines { + break } + lines = t.rebuild_rec(lines, item, level+1) } return lines } diff --git a/uiterm/tree_regression_test.go b/uiterm/tree_regression_test.go new file mode 100644 index 0000000..15bacea --- /dev/null +++ b/uiterm/tree_regression_test.go @@ -0,0 +1,65 @@ +package uiterm + +import ( + "testing" + "time" +) + +// cyclicItem reports itself as its own child, standing in for a channel graph +// in which a channel is its own ancestor. +type cyclicItem struct{ name string } + +func (i *cyclicItem) String() string { return i.name } + +func (i *cyclicItem) TreeItemStyle(fg, bg Attribute, active bool) (Attribute, Attribute) { + return fg, bg +} + +// Regression: rebuild_rec followed parent/child links with no depth limit, so +// a cyclic channel graph recursed until the process ran out of memory. A +// rebuild must now terminate and stay bounded. +func TestTreeRebuildTerminatesOnCyclicGraph(t *testing.T) { + t.Parallel() + + self := &cyclicItem{name: "loop"} + tree := Tree{ + Generator: func(item TreeItem) []TreeItem { + return []TreeItem{self} + }, + } + + done := make(chan struct{}) + go func() { + tree.rebuild(false, nil) + close(done) + }() + + select { + case <-done: + case <-timeoutAfterSeconds(10): + t.Fatal("rebuild did not terminate on a cyclic tree") + } + + if len(tree.lines) == 0 { + t.Fatal("expected the bounded rebuild to still produce lines") + } + if len(tree.lines) > maxTreeLines { + t.Fatalf("rebuild produced %d lines, above the %d cap", + len(tree.lines), maxTreeLines) + } + for _, line := range tree.lines { + if line.Level >= maxTreeDepth { + t.Fatalf("rebuild recursed to level %d, at or past the %d cap", + line.Level, maxTreeDepth) + } + } +} + +func timeoutAfterSeconds(n int) <-chan struct{} { + ch := make(chan struct{}) + go func() { + time.Sleep(time.Duration(n) * time.Second) + close(ch) + }() + return ch +}