Repository navigation
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
It changes user-facing behavior and exit codes of the core explode path for all non-YAML encoders, so a maintainer should confirm the error-on-cycle semantics and message wording.
No open issues 🎉
What changed in this PR
This PR fixes #2167, where converting a self-referential YAML structure (e.g. &self [*self]) to JSON silently produced incorrect output ([[[]]]) with exit code 0 instead of erroring. The fix adds a cycle check in explodeOperator that runs before explodeNode mutates nodes (which would otherwise hide the cycle), returning cannot explode circular alias reference when a finite expansion is impossible. Because non-YAML encoders explode aliases at print time (printer.go runs explode when CanHandleAliases() is false), this surfaces as an error for JSON/other formats while YAML output (which preserves aliases) is unaffected.
The new checkForAliasCycles is a standard depth-first grey/black cycle detector: visited[node]=false marks a node on the current traversal path and true marks a completed node, so a back-edge to an in-progress node is a true cycle, while repeated references to already-completed acyclic values are allowed. It follows both AliasNode.Alias and Content, covering sequence, mapping, indirect, and merge-anchor cycles. I verified it does not produce false positives for repeated aliases or diamond (shared-subtree) structures, and only inspects the selected nodes so acyclic selections from cyclic documents still succeed.
Changes:
- Add a pre-explode cycle guard (
checkForAliasCycles) inexplodeOperatorthat errors on circular alias references. - Add
expressionScenariotests covering sequence/merge cycles, repeated (non-cyclic) aliases, and YAML passthrough. - Add
TestJSONAliasCyclescovering sequence/mapping/indirect cycles, repeated aliases, and acyclic selection from a cyclic document.
| File | Description |
|---|---|
| pkg/yqlib/operator_anchors_aliases.go | Adds checkForAliasCycles and invokes it in explodeOperator before explodeNode mutates nodes. |
| pkg/yqlib/operator_anchors_aliases_test.go | Adds scenarios for cycle rejection (sequence + merge) and YAML alias preservation. |
| pkg/yqlib/json_test.go | Adds TestJSONAliasCycles validating JSON-output cycle errors and acyclic cases. |
Review effort: Balanced
Fixes #2167.
Converting
&self [*self]to JSON currently succeeds with[[[]]], silently losing the circular reference. Check the selected nodes for alias cycles beforeexplodemutates them, and returncannot explode circular alias referencewhen expansion cannot produce a finite tree.The check distinguishes nodes on the current traversal path from completed nodes, so repeated references to the same acyclic value remain valid. YAML output still preserves circular aliases; selecting an acyclic value from a cyclic document still works. No dependency, workflow, or security-check configuration changes are included.
Validation on Linux with Go 1.26.9, based on
a814ff5166040ec745a96bddd79ee31c3a548e83:[[[]]].go test ./pkg/yqlib -run 'TestJSONAliasCycles|TestAnchorAliasOperator' -count=1: passes, including both merge-anchor modes.TestGoInstallCompatibility.go test ./...has the same single failing test,TestJSONScenarios, on the untouched base and on this branch. Its existing malformed-JSON case expectsjson: string of object unexpected end of JSON input, but the current JSON dependency returnsinvalid character 'b' after object key:value pair. This change leaves that unrelated assertion intact.make local buildreturns success despite that unit-test failure becausescripts/test.shsubsequently runs the install-compatibility test, so the full suite is not being reported as green.The issue timeline had no assignee or work claim when checked, and searches of open and closed PRs for the issue number, circular references and alias expansion found no competing fix.