[Tales-of-zuna]

What

Fixes #3937. Form-style binding (form, query, multipart, header, uri) crashes the process with fatal error: stack overflow when the target struct refers to itself through a pointer, e.g.:

type Node struct {
	Name   string `form:"name"`
	Parent *Node
}

It happens on every request, even an empty one, and Recovery() cannot catch a stack overflow, so a single handler takes the whole server down.

Why it happens

mapping allocates every nil pointer to a struct and walks its fields. For a self-referencing type, each new level contains another nil pointer, so the walk never ends.

Fix

Track the struct types whose fields are being mapped on the current path, and don't walk a type that is already on it.

• A recursive field can still be set as a whole value from a matching key (e.g. ?Parent={"Name":"p"} via the existing JSON path); otherwise it stays nil.
• Non-recursive structs bind exactly as before, at any depth, including sibling fields of the same type.
• No public API change: only the unexported mapping gains a parameter.

Compared with #4337

#4337 caps recursion depth at 10, but the cap also counts levels of non-recursive structs, so a legitimate struct nested ~6 levels deep silently loses its deepest fields with no error (reproduction posted on #4337). This PR stops only on actual type cycles. TestMappingDeepNonRecursiveStruct guards that case.

Tests

• Mapping-level: direct and mutual recursion, fields outside the cycle, whole-value set, deep non-recursive chain, same-type siblings.
• One subtest per binder: query, form, multipart, header, uri.
• Every new recursion test crashes on master and passes with this change. go test -race ./... and go vet ./... pass.

Benchmarks

benchstat, n=6, before/after: B/op and allocs/op are unchanged for all binding benchmarks. MapFormFull and SliceValidationError show no significant change. MapFormName (single field) goes 148.9ns → 156.4ns (+5%, about 7ns per bind) for the path check.

Pull Request Checklist

• Open your pull request against the master branch.
• All tests pass in available continuous integration systems (e.g., GitHub Actions).
• Tests are added or modified as needed to cover code changes.
• If the pull request introduces a new feature, the feature is documented in the docs/doc.md. (Bug fix, no docs needed.)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.65%. Comparing base (0f09c3a) to head (3a05f77).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4916   +/-   ##
=======================================
  Coverage   98.65%   98.65%           
=======================================
  Files          48       48           
  Lines        3188     3191    +3     
=======================================
+ Hits         3145     3148    +3     
  Misses         43       43           

Flag	Coverage Δ
--ldflags="-checklinkname=0" -tags sonic	98.64% <100.00%> (+<0.01%)	⬆️
-tags go_json	98.57% <100.00%> (+<0.01%)	⬆️
-tags nomsgpack	98.63% <100.00%> (+<0.01%)	⬆️
go-1.26	98.33% <100.00%> (+<0.01%)	⬆️
go-1.27	98.67% <100.00%> (+<0.01%)	⬆️
macos-latest	98.65% <100.00%> (+<0.01%)	⬆️
ubuntu-latest	98.65% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[rahul05ranjan]

I traced the recursion mechanism on the PR head and the fix is sound:

• The crash premise checks out: a nil *Node field makes the ptr branch reflect.New a node and descend into it, the struct branch walks the fields, hits the same nil ptr again, and the loop never terminates until the goroutine stack is gone — on every bind, since even an empty form walks the whole tree.
• parents is a proper path-stack, not a global visited-set: each struct level appends its own type to the slice it received and passes the longer slice down, while sibling fields still get the shorter one. So a type reused across different subtrees (DAG shapes, your TestMappingDeepNonRecursiveStruct chain) still binds, and only true cycles on one path stop. The append pattern is also safe against the usual slice-aliasing trap, since a callee's writes land beyond the length any sibling can observe.
• The check's position matters and you got it right: it sits in the struct branch, after tryToSetValue, so keyed/whole-value binding still reaches self-referencing fields — that's what makes TestMappingRecursiveStructFieldAsWholeValue pass, via the fallback of an empty tag to the field name (tryToSetValue), the form["Parent"] hit in setByForm, and finally the JSON unmarshal in setWithProperType's reflect.Struct case. Good test to have, because that path is non-obvious.

One behavior note worth a line in the docs eventually: post-fix, a nested form key through a cycle (Parent.name=...) silently produces a nil Parent instead of binding or erroring. Given the pre-fix alternative was a fatal stack overflow, nothing working regresses, but "silently dropped" vs "rejected" is a choice someone may want to find documented.

Mutually-recursive cycleA/cycleB coverage and the per-binding-mode integration tests over all five binders round this out nicely. On CI: the only red check is the Trivy scan, which doesn't look code-related.

Couldn't run the suite here (no Go toolchain on this machine) — the above is a static trace of the PR head.

[Tales-of-zuna]

Thanks for the careful trace, @rahul05ranjan!

On the Parent.name=... note: gin's form binding has no dotted-key syntax for nested structs, and unknown keys are ignored silently. So that key is also dropped for an ordinary non-recursive struct on master (checked: child.name=x leaves a *Child field nil, while name=x fills it). The case is not specific to this change.

What is specific: a self-referencing field cannot be filled from flat keys, because binding stops at the cycle (before this change it crashed). Binding it as a whole value, e.g. Parent={"name":"root"}, still works and is covered by TestMappingRecursiveStructFieldAsWholeValue. Happy to add a short comment in mapping saying this if maintainers want it.

[rahul05ranjan]

Thanks for verifying the dropped-key behavior against master — that settles it. With the Parent.name case confirmed unreachable and the recursion bounded by the depth counter, this looks solid from my side.