Skip to content

fix: mark tags fmt cannot format safely instead of crashing (#197)#209

Open
linkdata wants to merge 1 commit into
mainfrom
fix/tagstring-cyclic-stack-overflow-197
Open

fix: mark tags fmt cannot format safely instead of crashing (#197)#209
linkdata wants to merge 1 commit into
mainfrom
fix/tagstring-cyclic-stack-overflow-197

Conversation

@linkdata

@linkdata linkdata commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Fixes #197.

Problem

tag.TagString formatted tags with fmt's %#v and wire.Message.String formatted Dest with %v. Neither verb detects reference cycles, so a self-referential tag list recurses until the runtime aborts the process with fatal error: stack overflow — unrecoverable, from valid input. Confirmed empirically for a cyclic []any, a value-struct-through-slice cycle, and a cyclic map (pointer cycles are safe — fmt prints an address). A compact value that expands exponentially makes fmt buffer 1.3 GB.

Approach

Detect, then delegate. A small pre-check (formatSafe) decides before calling fmt whether a value is safe to hand over — necessary because fmt builds its whole output in an internal buffer before writing, so neither a bounded io.Writer nor recover can contain the overflow or the memory blow-up (both verified). The check walks the value the way fmt does:

  • fmt.Formatter / fmt.GoStringer / fmt.Stringer values are leaves (fmt calls the method, doesn't recurse structure);
  • a pointer is descended only at the top level (fmt prints deeper pointers as addresses), so ordinary pointer-linked tags aren't misflagged;
  • recursion depth is bounded — a reference cycle is simply an unbounded-depth walk, so a depth cap catches every cycle with no active-node bookkeeping (this also means acyclic slice aliases like s[:1] are never misread as cycles);
  • estimated output size is bounded, including the %#v type string (which reflect.StructOf can make enormous), catching exponential/huge blow-ups;
  • a top-level reflect.Value is unwrapped first, matching fmt.

A value that passes is handed to fmt unchanged — every ordinary acyclic tag formats exactly as before, custom formatters (including redaction) included, and Message.String keeps its precise %v output. A value that fails renders as the marker cyclic-tag instead of crashing.

TagString and Message.String share the logic through the new tag.FormatValue(v, goSyntax) — the fmt-path analogue of the cycle detection TagExpand/expand already use for tag expansion. As with TagExpand, formatting methods reachable from the value must terminate and not mutate the graph (documented precondition).

Tests & benchmarks

Covers the issue reproduction and the cyclic struct/map/array/unexported and reflect.Value cases (all → cyclic-tag, no crash), the exponential-graph / huge-shared-string / huge-reflect.StructOf-type-name blow-ups, the depth cap, pointer cycles and large/aliased acyclic slices (not flagged — still byte-identical to fmt), and custom Formatter/GoStringer/Stringer/error handling (including a >1 MiB string-backed redactor). A benchmark guards the deep-chain path that a naive active-node scan made quadratic (~2.9 s → 1.1 µs / 0 allocs). 100% statement coverage of the new code; go vet, gofumpt, staticcheck, golangci-lint (0 issues) and go test -race ./... all pass.

@linkdata
linkdata force-pushed the fix/tagstring-cyclic-stack-overflow-197 branch from c7df1ba to 63c13e5 Compare July 21, 2026 22:49
@linkdata linkdata changed the title fix: bound cyclic tag lists in TagString and Message.String fix: mark tags fmt cannot format safely instead of crashing (#197) Jul 21, 2026
TagString formatted tags with fmt's %#v and Message.String formatted Dest
with %v. Neither verb detects reference cycles, so a self-referential tag
list — a []any that contains itself, or the same cycle reached through a
value struct or a map — recurses until the runtime aborts the process with
"fatal error: stack overflow", an unrecoverable crash from valid, supported
input. A compact value that expands into an enormous string (an
exponentially shared graph, or many references to one large value) makes
fmt buffer gigabytes for the same reason.

Add a small pre-check (formatSafe) that walks a value the way fmt does and
decides whether it is safe to hand over: fmt.Formatter/GoStringer/Stringer
values are leaves, a pointer is descended only at the top level, and the
recursion depth and approximate output size are bounded. A reference cycle
is just an unbounded-depth walk, so the depth bound catches it with no
active-node bookkeeping; the size bound accounts for %#v type strings (which
reflect.StructOf can make huge) and stops exponential or oversized output. A
value that passes is handed to fmt unchanged, so ordinary acyclic tags
format exactly as before, custom formatters included; a value that fails
renders as the marker "cyclic-tag". A top-level reflect.Value is unwrapped
first, as fmt does.

TagString and Message.String share this via the new tag.FormatValue.

Fixes #197
@linkdata
linkdata force-pushed the fix/tagstring-cyclic-stack-overflow-197 branch from 63c13e5 to 601f3e9 Compare July 21, 2026 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TagString and Message.String stack-overflow on cyclic tag lists

1 participant