feat(xmlgen): XML generation with automatic escaping and validation (1/4)

[sthanikan2000]

Problem

Core has no way to generate an XML document. encoding/xml is imported nowhere, and the only templating in the repo (uiprojector/projector.go) performs no escaping — acceptable for a markdown blurb, not for a document that leaves the system.

Changes

New standalone module xmlgen (stdlib + testify; root module untouched). A template is the target XML with text/template actions in it.

func Generate(ctx context.Context, tmpl []byte, data any, opts ...Option) ([]byte, error)
func GenerateTo(ctx context.Context, w io.Writer, tmpl []byte, data any, opts ...Option) error

func WithStrictKeys() Option        // a referenced-but-absent key becomes an error
func WithMaxOutputBytes(n int64) Option
func SkipNamespaceCheck() Option

• Escaping is automatic. After parsing, the template is rewritten so every printing action is piped through an escape function — {{ .name }} becomes {{ .name | xml }}, the technique html/template uses. Rewriting the template rather than the data also covers range variables, map keys and pipeline tails. The rewrite walks every tree a parse produced, not just the main one: {{ define }} and {{ block }} install their bodies as associated templates, and Templates() is provably the complete set (execution reaches a tree only via Lookup, which reads the same map). Opt out per action with raw or cdata; text outside an action is never touched.
• Numbers keep their exact text. JSON is decoded with UseNumber, so 10000000 stays 10000000 rather than becoming 1e+07.
• Output is validated and never rewritten, so the bytes stay signable: exactly one root element, no DOCTYPE, no text outside the root, no undeclared namespace prefix — the last because encoding/xml documents that it does not reject one. The check is not a backstop for raw, which is the deliberate opt-out: a raw value in an attribute can close the quote and forge further attributes, and that output is well-formed. The README says so explicitly.
• Following a pointer or interface is depth-bounded. A caller-supplied cycle would otherwise overflow the stack, which Go treats as fatal and recover cannot catch.
• Wires the module into ci.yml, dependabot.yml and the root README table.

Worth a reviewer's eye: rewrite.go builds a parse.CommandNode literal, so its unexported tr field is nil. Only CommandNode.Copy() dereferences it, reached solely via html/template, which xmlgen never uses. TestInjectEscaping_SurvivesClone pins that.

Testing

cd xmlgen
go test -race ./...
golangci-lint run -c ../.golangci.yml --timeout=5m ./...

115 subtests, race-clean, 0 lint issues. Covers escaping (metacharacters, attribute contexts, a value that is itself valid XML, already-escaped input, invalid UTF-8, control characters, map keys, range variables), every structural rejection above, a UTF-8 BOM, a non-UTF-8 encoding declaration, the size cap, context cancellation, and GenerateTo writing zero bytes when validation fails.

Related

Implements #189. Stacked chain — 1 of 4, review in order:

#199 (this) → #201 helpers and resolvers → #202 golden fixtures → #200 failure paths

🤖 Generated with Claude Code

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: d44e92ec-fff3-48d1-a30f-bacb47e42279

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[ginaxu1]

Please add tests for both define and block with a markup-carrying value, next to TestEscaping_Metacharacters.

Also check this: the rewriter only walks the main parse tree. {{ define }} and {{ block }} install their bodies as associated templates that text/template removes from that tree, and walkNode treats TemplateNode as a no-op. I rendered this template with v set to <Consignee><Name>Evil</Name></Consignee>:

<R>{{block "item" .}}<Item>{{ .v }}</Item>{{end}}</R>

Generate returned:

<R><Item><Consignee><Name>Evil</Name></Consignee></Item></R>

The same injection happens with {{ define }} plus {{ template }}. That contradicts the claim that every printing action is piped through xml and that a value carrying markup cannot forge elements. checkDocument cannot catch this: the output is well-formed. PR 201 does not close the hole. It still rewrites only t.Tree, and a line-item fragment is the natural place someone would reach for block.

[sthanikan2000]

Good catch, and you're right — this was a real hole, not a documentation gap. Fixed in c74da2b.

Confirmed and reproduced, including a case worse than the one you found: {{ block }} nested inside {{ define }} creates a third tree, so an incomplete fix that only followed one level of association would still have leaked.

<R>{{block "item" .}}<Item>{{ .v }}</Item>{{end}}</R>
→ <R><Item><Consignee><Name>Evil</Name></Consignee></Item></R>     (before)
→ <R><Item>&lt;Consignee&gt;&lt;Name&gt;Evil&lt;/Name&gt;…</Item></R>  (after)

Root cause was a false premise I wrote into rewrite.go: "xmlgen parses a single template with no associates." That is wrong — one Parse call returns a map of trees (parse.Parse → map[string]*parse.Tree), and parse.(*Tree).blockControl does block := New(name), leaving only a TemplateNode in the main tree. injectEscaping(t.Tree) never saw the bodies, and as you say checkDocument can't help because the output is well-formed.

Fix: injectEscaping now takes the *template.Template and walks every non-nil tree from t.Templates().

This is complete rather than merely broader, and it's worth stating why: execution reaches a tree only through walkTemplate → s.tmpl.Lookup(name) → walk(dot, tmpl.Root). Lookup reads t.tmpl; Templates() returns that map's values. So the set of exec-reachable trees is exactly what Templates() returns — there is no tree reachable at execution but absent from it. A nested {{define}} inside a list is a parse error, so nothing can hide; {{block}} inside {{define}} shares the parent's tree set and lands in the same map.

Tests added — {{ block }}, {{ define }} + {{ template }}, {{ block }} inside {{ define }}, and a block body reached through range, all with your <Consignee> value, asserting both the escaped bytes and that xml.Unmarshal finds zero child elements. Plus internal tests that the associated trees are rewritten, mutual {{ template }} recursion between two defines, and that compile is idempotent — the root is already a member of Templates(), so that property was previously true only by accident.

---

Two further defects turned up while I was verifying the fix, both now addressed:

1. text() recursed through pointers and interfaces with no depth bound. A caller-supplied cycle overflows the stack, which Go treats as fatal and recover cannot catch — so it would have killed the process rather than failing the render, and limitWriter couldn't catch it because nothing is ever written. Now depth-bounded, returning ErrUnsupportedValue.

2. The README oversold the validator with respect to raw. It said the document check still applies "so a stray & is caught", which implies a backstop. In attribute position that's false: <E ref="{{ raw .ref }}"/> with ref set to " classified="SECRET forges an attribute and the result is perfectly well-formed. The docs now say plainly that the check is not a backstop for raw in attribute or element-name position.

Also reserved break and continue as resolver names in #201 — parse.(*Tree).startParse sets breakOK from !hasFunction("break"), so a resolver by that name would silently demote the keyword and change what every existing {{ break }} does. Not an escaping bypass, but silent wrong output.

The chain has been rebased and force-pushed; #201, #202 and #200 are unchanged apart from that. Subtest counts across the chain: 115 / 153 / 170 / 195.