Skip to content

openapi: raw YAML tree walks each carry their own stack, depth cap and alias rule #788

Closed

Description

@OmarAlJarrah

Problem

The compiler walks raw yaml.Node trees in many places, and each walk carries its own stack or recursion, depth cap and decision about aliases. At 210283f, among others:

  • sourceindex/sourceindex.go walk: iterative, explicit frame stack, ancestor set, node bound; does not follow aliases.
  • load/external.go holdSpellings: iterative, explicit stack, follows an alias only to read a $ref's key and value.
  • overlay/overlay.go substituteGrafts and expandAlias: recursive, counting down a budget passed by pointer.
  • load/load.go walkNumericScalars: recursive with a depth parameter.
  • internal/harness/order.go reverseNode: recursive with maxReverseDepth.

Each had to decide the same four things: how deep, how many nodes, whether an alias is an edge, and what to do when a bound is hit. Where the answer is "iterative with an explicit stack" it is re-derived each time, and the bounded-recursion rule is easiest to get wrong exactly here: untrusted input chooses the depth.

The converters value.fromNodeAt and rawConv.node also recurse with depth caps, but they build a result bottom-up and are not walks; they stay as they are.

Proposed direction

A small iterative walker in compilers/openapi/internal taking a root, a depth bound, a node bound (the counter from #785) and an alias policy (follow or skip), calling a visitor with the node and its depth and letting it prune. Migrate the walks whose semantics match; leave those that are really searches over a different graph (reach, scan) alone.

Keeps to compilers/openapi/internal; the framework-promotion rule (docs/micro-compiler-design.md §3) applies if another compiler shows the same need.

Acceptance

  • Each migrated walk loses its private stack and bound; its existing bound tests pass.
  • The walker reports truncation to the caller rather than stopping silently, as sourceindex.Index.Truncated does.
  • Deep (thousands of levels) and wide inputs are covered by a test at the walker, not once per caller.
  • Goldens and the harness sweep unchanged.

Activity

  1. added
    priority:P4Refactors, tests, CI, hostile-input hardening, cosmetic diagnostics, other formats
    on Oct 19, 2026
  2. OmarAlJarrah commented on Oct 20, 2026

    @OmarAlJarrah
    MemberAuthor

    #1016 adds one walk to this list: bind/walk.go finder.walk.

    • It is iterative, with an explicit frame stack.
    • Each node costs one step of the binding's work budget, which is reported as openapi/budget-exceeded when exhausted.
    • An alias is read at every position it is written, as the document's JSON places it.
    • Merge keys are read through nodeview up to MergeDepthLimit. A mapping that merges deeper is reported as an openapi/degraded-construct warning.

    It moves through positions by the OpenAPI grammar rather than visiting every node, so it sits closer to the searches this issue leaves alone than to a plain walk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority:P4Refactors, tests, CI, hostile-input hardening, cosmetic diagnostics, other formatstype:choreRefactor or cleanup, no behavior change

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions