From 0a8aae0b76e1417e665cc821d15f271211cd0bc9 Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Wed, 15 Jul 2026 04:31:17 +0300 Subject: [PATCH 1/7] openapi3: add T.WalkParameters to visit every parameter in a document (#1224) --- .github/docs/openapi3.txt | 26 ++++ openapi3/walk_parameters.go | 128 ++++++++++++++++++ openapi3/walk_parameters_test.go | 108 +++++++++++++++ openapi3/{walk.go => walk_schemas.go} | 0 .../{walk_test.go => walk_schemas_test.go} | 0 5 files changed, 262 insertions(+) create mode 100644 openapi3/walk_parameters.go create mode 100644 openapi3/walk_parameters_test.go rename openapi3/{walk.go => walk_schemas.go} (100%) rename openapi3/{walk_test.go => walk_schemas_test.go} (100%) diff --git a/.github/docs/openapi3.txt b/.github/docs/openapi3.txt index c04d34ec2..725e1a1b8 100644 --- a/.github/docs/openapi3.txt +++ b/.github/docs/openapi3.txt @@ -3343,6 +3343,18 @@ func (doc *T) ValidateSchemaJSON(schema *Schema, value any, opts ...SchemaValida format validators. This is a convenience method that automatically applies the document's format validators. +func (doc *T) WalkParameters(fn WalkParametersFunc) error + WalkParameters visits every parameter reachable from the document exactly + once, invoking fn for each. It follows resolved $ref targets (param.Value), + so each distinct *Parameter is visited a single time regardless of how + many references point at it. Maps are visited in sorted key order, so the + traversal is deterministic. + + It covers components.parameters, path items, operations, callbacks, + and webhooks. It is the parameter counterpart of WalkSchemas: useful for + validation, linting, parameter transformation, and documentation, without + re-deriving the (easy to get wrong) traversal. + func (doc *T) WalkSchemas(fn WalkSchemasFunc) error WalkSchemas visits every schema reachable from the document exactly once, invoking fn for each. It follows resolved $ref targets (schema.Value) and @@ -3691,6 +3703,20 @@ type ValidationOptions struct { } ValidationOptions provides configuration for validating OpenAPI documents. +type WalkParametersFunc func(jsonPointer string, param *ParameterRef) error + WalkParametersFunc is called once for each parameter visited by + WalkParameters. + + jsonPointer is the RFC 6901 JSON Pointer of the parameter + within the document, e.g. "/components/parameters/Limit" or + "/paths/~1pets/get/parameters/0". For a parameter referenced from several + places it is the pointer of the first visit, and components are visited + first, so a shared parameter is reported at its definition. param is non-nil + and param.Value is non-nil; the callback may modify param.Value in place, + so WalkParameters serves transformers and not only read-only inspection. + + Returning a non-nil error aborts the walk and is returned by WalkParameters. + type WalkSchemasFunc func(jsonPointer string, schema *SchemaRef) error WalkSchemasFunc is called once for each schema visited by WalkSchemas. diff --git a/openapi3/walk_parameters.go b/openapi3/walk_parameters.go new file mode 100644 index 000000000..d60edb6c7 --- /dev/null +++ b/openapi3/walk_parameters.go @@ -0,0 +1,128 @@ +package openapi3 + +import ( + "maps" + "slices" + "strconv" + "strings" +) + +// WalkParametersFunc is called once for each parameter visited by +// WalkParameters. +// +// jsonPointer is the RFC 6901 JSON Pointer of the parameter within the +// document, e.g. "/components/parameters/Limit" or +// "/paths/~1pets/get/parameters/0". For a parameter referenced from several +// places it is the pointer of the first visit, and components are visited +// first, so a shared parameter is reported at its definition. param is +// non-nil and param.Value is non-nil; the callback may modify param.Value in +// place, so WalkParameters serves transformers and not only read-only +// inspection. +// +// Returning a non-nil error aborts the walk and is returned by +// WalkParameters. +type WalkParametersFunc func(jsonPointer string, param *ParameterRef) error + +// WalkParameters visits every parameter reachable from the document exactly +// once, invoking fn for each. It follows resolved $ref targets (param.Value), +// so each distinct *Parameter is visited a single time regardless of how many +// references point at it. Maps are visited in sorted key order, so the +// traversal is deterministic. +// +// It covers components.parameters, path items, operations, callbacks, and +// webhooks. It is the parameter counterpart of WalkSchemas: useful for +// validation, linting, parameter transformation, and documentation, without +// re-deriving the (easy to get wrong) traversal. +func (doc *T) WalkParameters(fn WalkParametersFunc) error { + if doc == nil { + return nil + } + w := parameterWalker{fn: fn, seen: make(map[*Parameter]struct{})} + return w.document(doc) +} + +type parameterWalker struct { + fn WalkParametersFunc + seen map[*Parameter]struct{} +} + +func (w *parameterWalker) document(doc *T) error { + if c := doc.Components; c != nil { + for _, name := range slices.Sorted(maps.Keys(c.Parameters)) { + if err := w.parameter("/components/parameters/"+escapeRefString(name), c.Parameters[name]); err != nil { + return err + } + } + for _, name := range slices.Sorted(maps.Keys(c.Callbacks)) { + if cbr := c.Callbacks[name]; cbr != nil && cbr.Value != nil { + if err := w.callback("/components/callbacks/"+escapeRefString(name), cbr.Value); err != nil { + return err + } + } + } + } + if doc.Paths != nil { + items := doc.Paths.Map() + for _, path := range slices.Sorted(maps.Keys(items)) { + if err := w.pathItem("/paths/"+escapeRefString(path), items[path]); err != nil { + return err + } + } + } + for _, name := range slices.Sorted(maps.Keys(doc.Webhooks)) { + if err := w.pathItem("/webhooks/"+escapeRefString(name), doc.Webhooks[name]); err != nil { + return err + } + } + return nil +} + +func (w *parameterWalker) pathItem(ptr string, item *PathItem) error { + if item == nil { + return nil + } + for i, pr := range item.Parameters { + if err := w.parameter(ptr+"/parameters/"+strconv.Itoa(i), pr); err != nil { + return err + } + } + ops := item.Operations() + for _, method := range slices.Sorted(maps.Keys(ops)) { + op := ops[method] + opPtr := ptr + "/" + strings.ToLower(method) + for i, pr := range op.Parameters { + if err := w.parameter(opPtr+"/parameters/"+strconv.Itoa(i), pr); err != nil { + return err + } + } + for _, name := range slices.Sorted(maps.Keys(op.Callbacks)) { + if cbr := op.Callbacks[name]; cbr != nil && cbr.Value != nil { + if err := w.callback(opPtr+"/callbacks/"+escapeRefString(name), cbr.Value); err != nil { + return err + } + } + } + } + return nil +} + +func (w *parameterWalker) callback(ptr string, cb *Callback) error { + items := cb.Map() + for _, expr := range slices.Sorted(maps.Keys(items)) { + if err := w.pathItem(ptr+"/"+escapeRefString(expr), items[expr]); err != nil { + return err + } + } + return nil +} + +func (w *parameterWalker) parameter(ptr string, pr *ParameterRef) error { + if pr == nil || pr.Value == nil { + return nil + } + if _, ok := w.seen[pr.Value]; ok { + return nil + } + w.seen[pr.Value] = struct{}{} + return w.fn(ptr, pr) +} diff --git a/openapi3/walk_parameters_test.go b/openapi3/walk_parameters_test.go new file mode 100644 index 000000000..951509850 --- /dev/null +++ b/openapi3/walk_parameters_test.go @@ -0,0 +1,108 @@ +package openapi3_test + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" +) + +// walkParametersSpec exercises parameters in components (shared via $ref from +// two operations: dedup), path items, operations, a callback, and a webhook. +var walkParametersSpec = []byte(` +openapi: 3.1.0 +info: {title: t, version: "1"} +paths: + /pets: + parameters: + - name: tenant + in: header + schema: {type: string} + get: + parameters: + - $ref: '#/components/parameters/Limit' + - name: filter + in: query + schema: {type: string} + responses: + "200": {description: ok} + post: + parameters: + - $ref: '#/components/parameters/Limit' + callbacks: + onEvent: + '{$request.body#/url}': + post: + parameters: + - name: attempt + in: query + schema: {type: integer} + responses: + "200": {description: ok} + responses: + "201": {description: created} +webhooks: + newPet: + post: + parameters: + - name: signature + in: header + schema: {type: string} + responses: + "200": {description: ok} +components: + parameters: + Limit: + name: limit + in: query + schema: {type: integer} +`) + +func TestWalkParameters(t *testing.T) { + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData(walkParametersSpec) + require.NoError(t, err) + + visited := map[string]string{} // pointer -> parameter name + require.NoError(t, doc.WalkParameters(func(jsonPointer string, param *openapi3.ParameterRef) error { + require.NotNil(t, param) + require.NotNil(t, param.Value) + _, dup := visited[jsonPointer] + require.Falsef(t, dup, "pointer %q visited twice", jsonPointer) + visited[jsonPointer] = param.Value.Name + return nil + })) + + require.Equal(t, map[string]string{ + // the shared Limit parameter is visited once, at its definition + "/components/parameters/Limit": "limit", + "/paths/~1pets/parameters/0": "tenant", + "/paths/~1pets/get/parameters/1": "filter", + "/paths/~1pets/post/callbacks/onEvent/{$request.body#~1url}/post/parameters/0": "attempt", + "/webhooks/newPet/post/parameters/0": "signature", + }, visited) +} + +func TestWalkParameters_ErrorAborts(t *testing.T) { + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData(walkParametersSpec) + require.NoError(t, err) + + boom := errors.New("boom") + calls := 0 + require.ErrorIs(t, doc.WalkParameters(func(string, *openapi3.ParameterRef) error { + calls++ + return boom + }), boom) + require.Equal(t, 1, calls, "the walk stops at the first error") +} + +func TestWalkParameters_NilDoc(t *testing.T) { + var doc *openapi3.T + require.NoError(t, doc.WalkParameters(func(string, *openapi3.ParameterRef) error { + t.Fatal("no visits on a nil document") + return nil + })) +} diff --git a/openapi3/walk.go b/openapi3/walk_schemas.go similarity index 100% rename from openapi3/walk.go rename to openapi3/walk_schemas.go diff --git a/openapi3/walk_test.go b/openapi3/walk_schemas_test.go similarity index 100% rename from openapi3/walk_test.go rename to openapi3/walk_schemas_test.go From a3567a512a9eae71e977f26805aebae09d962400 Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Sat, 11 Jul 2026 12:31:10 +0300 Subject: [PATCH 2/7] openapi3: preserve origin for a $ref to a schema under an arbitrary top-level key A $ref to a schema stored under an arbitrary top-level key in another file (e.g. "./schemas.yaml#/User", the Swagger-2-era definitions bag) resolves through T.Extensions, a generic map that carries no origin, and the json round-trip that decodes it into the typed component drops origin entirely. With IncludeOrigin enabled the resolved schema then has a nil Origin, unlike the /components/schemas path. Re-attach origins on that path: re-derive the component file's origin tree, walk to the ref fragment, and apply the subtree, so the resolved object carries the same origins a typed resolution would, with the original file's line numbers. Best-effort; any failure leaves the object without origins, as before. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01BqJA1X6suZYtR3tRbX8sLj --- openapi3/loader.go | 34 +++++++++++++++++++ openapi3/marsh.go | 21 ++++++++++++ openapi3/origin_external_ref_test.go | 31 +++++++++++++++++ openapi3/testdata/origin/arbitrary_key.yaml | 14 ++++++++ .../origin/arbitrary_key_schemas.yaml | 7 ++++ 5 files changed, 107 insertions(+) create mode 100644 openapi3/origin_external_ref_test.go create mode 100644 openapi3/testdata/origin/arbitrary_key.yaml create mode 100644 openapi3/testdata/origin/arbitrary_key_schemas.yaml diff --git a/openapi3/loader.go b/openapi3/loader.go index 4c2ebe288..a43a813a7 100644 --- a/openapi3/loader.go +++ b/openapi3/loader.go @@ -516,6 +516,10 @@ func (loader *Loader) resolveComponent(doc *T, ref string, path *url.URL, resolv if err := codec(cursor, resolved); err != nil { return nil, nil, fmt.Errorf("bad data in %q (expecting %s)", ref, readableType(resolved)) } + // The value came from a generic map in T.Extensions (a $ref to an + // arbitrary top-level key), so the json round-trip above stripped its + // origins. Re-attach them from the file, best-effort. + loader.attachOriginToResolved(resolved, componentPath, fragment) return componentDoc, componentPath, nil default: @@ -523,6 +527,36 @@ func (loader *Loader) resolveComponent(doc *T, ref string, path *url.URL, resolv } } +// attachOriginToResolved re-attaches source origins to a component resolved +// through the generic-map path: a $ref to a schema under an arbitrary top-level +// key lands in T.Extensions, and the json round-trip in resolveComponent strips +// its origin. It re-derives the component file's origin tree, walks to the ref +// fragment, and applies that subtree, so the object carries the same origins a +// typed resolution would, with the original file's line numbers. Best-effort: +// any failure leaves the object without origins, exactly as before. +func (loader *Loader) attachOriginToResolved(resolved any, componentPath *url.URL, fragment string) { + if !loader.IncludeOrigin || componentPath == nil { + return + } + data, err := loader.readURL(componentPath) + if err != nil { + return + } + tree := originTree(data, componentPath) + if tree == nil { + return + } + for _, part := range strings.Split(strings.Trim(fragment, "/"), "/") { + if part == "" { + continue + } + if tree = tree.Fields[unescapeRefString(part)]; tree == nil { + return + } + } + applyOrigins(resolved, tree) +} + func readableType(x any) string { switch x.(type) { case *Callback: diff --git a/openapi3/marsh.go b/openapi3/marsh.go index 6483fa9a4..45e0c229a 100644 --- a/openapi3/marsh.go +++ b/openapi3/marsh.go @@ -43,3 +43,24 @@ func unmarshal(data []byte, v any, includeOrigin bool, location *url.URL) error // If both unmarshaling attempts fail, return a new error that includes both errors return fmt.Errorf("failed to unmarshal data: json error: %v, yaml error: %v", jsonErr, yamlErr) } + +// originTree parses data solely for its origin tree, used to re-attach source +// origins to a component resolved through the generic-map path: a $ref to a +// schema under an arbitrary top-level key lands in T.Extensions and loses its +// origin in the json round-trip (see resolveComponent). Returns nil when parsing +// fails; callers degrade to no origin. +func originTree(data []byte, location *url.URL) *yaml.OriginTree { + var file string + if location != nil { + file = location.String() + } + var v any + tree, err := yaml.Unmarshal(data, &v, yaml.DecodeOpts{ + Origin: yaml.OriginOpt{Enabled: true, File: file}, + DisableTimestamps: true, + }) + if err != nil { + return nil + } + return tree +} diff --git a/openapi3/origin_external_ref_test.go b/openapi3/origin_external_ref_test.go new file mode 100644 index 000000000..5410310e6 --- /dev/null +++ b/openapi3/origin_external_ref_test.go @@ -0,0 +1,31 @@ +package openapi3 + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +// A $ref to a schema stored under an arbitrary top-level key in another file +// (e.g. "./schemas.yaml#/User", the Swagger-2-era "definitions bag") must carry +// the origin of the file it lives in, like a $ref into /components/schemas does. +// The key is not a typed field of T, so the loader reaches it through T.Extensions +// (a generic map); the origin must survive that path. +func TestOrigin_ExternalRefToArbitraryTopLevelKey(t *testing.T) { + loader := NewLoader() + loader.IncludeOrigin = true + loader.IsExternalRefsAllowed = true + + doc, err := loader.LoadFromFile("testdata/origin/arbitrary_key.yaml") + require.NoError(t, err) + + user := doc.Paths.Value("/users").Get.Responses.Value("200").Value.Content["application/json"].Schema.Value + require.NotNil(t, user, "the User schema resolves") + + require.NotNil(t, user.Origin, "a schema $ref'd under an arbitrary top-level key must carry an origin") + require.NotNil(t, user.Origin.Key, "the origin has a key location") + require.Equal(t, "arbitrary_key_schemas.yaml", filepath.Base(user.Origin.Key.File), + "origin points at the file the schema lives in, not the referencing document") + require.NotZero(t, user.Origin.Key.Line, "the origin has a source line") +} diff --git a/openapi3/testdata/origin/arbitrary_key.yaml b/openapi3/testdata/origin/arbitrary_key.yaml new file mode 100644 index 000000000..11cf195fe --- /dev/null +++ b/openapi3/testdata/origin/arbitrary_key.yaml @@ -0,0 +1,14 @@ +openapi: 3.0.0 +info: + title: Arbitrary top-level key ref + version: 1.0.0 +paths: + /users: + get: + responses: + "200": + description: OK + content: + application/json: + schema: + $ref: "./arbitrary_key_schemas.yaml#/User" diff --git a/openapi3/testdata/origin/arbitrary_key_schemas.yaml b/openapi3/testdata/origin/arbitrary_key_schemas.yaml new file mode 100644 index 000000000..8474e357b --- /dev/null +++ b/openapi3/testdata/origin/arbitrary_key_schemas.yaml @@ -0,0 +1,7 @@ +User: + type: object + properties: + id: + type: string + name: + type: string From fd6dbfcc2b2c93d7028c4932aff3a11a65bda6fa Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Mon, 13 Jul 2026 23:38:39 +0300 Subject: [PATCH 3/7] openapi3: move originTree into origin.go It is origin machinery, and marsh.go is expected to hold the only direct yaml uses outside origin.go (the CI unmarshal gate pins that count). --- openapi3/marsh.go | 21 --------------------- openapi3/origin.go | 22 ++++++++++++++++++++++ 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/openapi3/marsh.go b/openapi3/marsh.go index 45e0c229a..6483fa9a4 100644 --- a/openapi3/marsh.go +++ b/openapi3/marsh.go @@ -43,24 +43,3 @@ func unmarshal(data []byte, v any, includeOrigin bool, location *url.URL) error // If both unmarshaling attempts fail, return a new error that includes both errors return fmt.Errorf("failed to unmarshal data: json error: %v, yaml error: %v", jsonErr, yamlErr) } - -// originTree parses data solely for its origin tree, used to re-attach source -// origins to a component resolved through the generic-map path: a $ref to a -// schema under an arbitrary top-level key lands in T.Extensions and loses its -// origin in the json round-trip (see resolveComponent). Returns nil when parsing -// fails; callers degrade to no origin. -func originTree(data []byte, location *url.URL) *yaml.OriginTree { - var file string - if location != nil { - file = location.String() - } - var v any - tree, err := yaml.Unmarshal(data, &v, yaml.DecodeOpts{ - Origin: yaml.OriginOpt{Enabled: true, File: file}, - DisableTimestamps: true, - }) - if err != nil { - return nil - } - return tree -} diff --git a/openapi3/origin.go b/openapi3/origin.go index e55695a5b..c24655340 100644 --- a/openapi3/origin.go +++ b/openapi3/origin.go @@ -1,6 +1,7 @@ package openapi3 import ( + "net/url" "reflect" "sort" "strings" @@ -324,3 +325,24 @@ func jsonTagName(f reflect.StructField) string { name, _, _ := strings.Cut(tag, ",") return name } + +// originTree parses data solely for its origin tree, used to re-attach source +// origins to a component resolved through the generic-map path: a $ref to a +// schema under an arbitrary top-level key lands in T.Extensions and loses its +// origin in the json round-trip (see resolveComponent). Returns nil when parsing +// fails; callers degrade to no origin. +func originTree(data []byte, location *url.URL) *yaml.OriginTree { + var file string + if location != nil { + file = location.String() + } + var v any + tree, err := yaml.Unmarshal(data, &v, yaml.DecodeOpts{ + Origin: yaml.OriginOpt{Enabled: true, File: file}, + DisableTimestamps: true, + }) + if err != nil { + return nil + } + return tree +} From e9dae0fa88415aac7985a061480b8f1df925c63b Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Tue, 14 Jul 2026 19:46:32 +0300 Subject: [PATCH 4/7] openapi3: iterate fragment parts with strings.SplitSeq --- openapi3/loader.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openapi3/loader.go b/openapi3/loader.go index a43a813a7..a1d9bf9b6 100644 --- a/openapi3/loader.go +++ b/openapi3/loader.go @@ -546,7 +546,7 @@ func (loader *Loader) attachOriginToResolved(resolved any, componentPath *url.UR if tree == nil { return } - for _, part := range strings.Split(strings.Trim(fragment, "/"), "/") { + for part := range strings.SplitSeq(strings.Trim(fragment, "/"), "/") { if part == "" { continue } From cd5ba43c6ea3f9e82461cc4aae2dfbbe64a8ed52 Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Wed, 15 Jul 2026 06:44:37 +0300 Subject: [PATCH 5/7] openapi3: retain origin trees at load; re-attach origins without re-reading attachOriginToResolved re-read and re-parsed the component file even though it had been read once already, and with a custom ReadFromURIFunc the second read could be a remote fetch. Retain each visited document's origin tree at unmarshal time instead (keyed like visitedDocuments, only when IncludeOrigin is set), and make the re-attachment a lookup plus tree walk. A test pins that resolving the arbitrary-key $ref reads each file exactly once. --- openapi3/loader.go | 36 ++++++++++++++++++---------- openapi3/marsh.go | 11 +++++---- openapi3/origin.go | 24 +++---------------- openapi3/origin_external_ref_test.go | 24 +++++++++++++++++++ 4 files changed, 57 insertions(+), 38 deletions(-) diff --git a/openapi3/loader.go b/openapi3/loader.go index a1d9bf9b6..8e78e1567 100644 --- a/openapi3/loader.go +++ b/openapi3/loader.go @@ -64,6 +64,12 @@ type Loader struct { visitedDocuments map[string]*T + // originTrees retains each visited document's origin tree (keyed like + // visitedDocuments) when IncludeOrigin is set, so resolveComponent can + // re-attach origins to components that lose them in the generic-map path + // without re-reading or re-parsing the file. + originTrees map[string]*originTree + visitedRefs map[string]struct{} visitedPath []string backtrack map[string][]func(value any) @@ -133,7 +139,7 @@ func (loader *Loader) loadSingleElementFromURI(ref string, rootPath *url.URL, el if err != nil { return nil, err } - if err := unmarshal(data, element, loader.IncludeOrigin, resolvedPath); err != nil { + if _, err := unmarshal(data, element, loader.IncludeOrigin, resolvedPath); err != nil { return nil, err } @@ -169,7 +175,7 @@ func (loader *Loader) LoadFromIoReader(reader io.Reader) (*T, error) { func (loader *Loader) LoadFromData(data []byte) (*T, error) { loader.resetVisitedPathItemRefs() doc := &T{} - if err := unmarshal(data, doc, loader.IncludeOrigin, nil); err != nil { + if _, err := unmarshal(data, doc, loader.IncludeOrigin, nil); err != nil { return nil, err } if err := loader.ResolveRefsIn(doc, nil); err != nil { @@ -198,9 +204,16 @@ func (loader *Loader) loadFromDataWithPathInternal(data []byte, location *url.UR doc := &T{} loader.visitedDocuments[uri] = doc - if err := unmarshal(data, doc, loader.IncludeOrigin, location); err != nil { + tree, err := unmarshal(data, doc, loader.IncludeOrigin, location) + if err != nil { return nil, err } + if tree != nil { + if loader.originTrees == nil { + loader.originTrees = make(map[string]*originTree) + } + loader.originTrees[uri] = tree + } doc.url = copyURI(location) @@ -468,7 +481,7 @@ func (loader *Loader) resolveComponent(doc *T, ref string, path *url.URL, resolv if err2 != nil { return nil, nil, err } - if err2 = unmarshal(data, &cursor, loader.IncludeOrigin, path); err2 != nil { + if _, err2 = unmarshal(data, &cursor, loader.IncludeOrigin, path); err2 != nil { return nil, nil, err } if cursor, err2 = drill(cursor); err2 != nil || cursor == nil { @@ -530,19 +543,16 @@ func (loader *Loader) resolveComponent(doc *T, ref string, path *url.URL, resolv // attachOriginToResolved re-attaches source origins to a component resolved // through the generic-map path: a $ref to a schema under an arbitrary top-level // key lands in T.Extensions, and the json round-trip in resolveComponent strips -// its origin. It re-derives the component file's origin tree, walks to the ref -// fragment, and applies that subtree, so the object carries the same origins a -// typed resolution would, with the original file's line numbers. Best-effort: -// any failure leaves the object without origins, exactly as before. +// its origin. It walks the document's retained origin tree (see originTrees, +// populated when the file was first unmarshaled: no re-read, no re-parse) down +// to the ref fragment and applies that subtree, so the object carries the same +// origins a typed resolution would, with the original file's line numbers. +// Best-effort: a missing tree or fragment leaves the object without origins. func (loader *Loader) attachOriginToResolved(resolved any, componentPath *url.URL, fragment string) { if !loader.IncludeOrigin || componentPath == nil { return } - data, err := loader.readURL(componentPath) - if err != nil { - return - } - tree := originTree(data, componentPath) + tree := loader.originTrees[componentPath.String()] if tree == nil { return } diff --git a/openapi3/marsh.go b/openapi3/marsh.go index 6483fa9a4..f895e6e40 100644 --- a/openapi3/marsh.go +++ b/openapi3/marsh.go @@ -17,12 +17,15 @@ func unmarshalError(jsonUnmarshalErr error) error { return jsonUnmarshalErr } -func unmarshal(data []byte, v any, includeOrigin bool, location *url.URL) error { +// unmarshal decodes data into v. It returns the document origin tree when +// includeOrigin is set and the data took the yaml path (json input carries no +// origins), so the caller can retain it (see Loader.originTrees). +func unmarshal(data []byte, v any, includeOrigin bool, location *url.URL) (*originTree, error) { var jsonErr, yamlErr error // See https://github.com/getkin/kin-openapi/issues/680 if jsonErr = json.Unmarshal(data, v); jsonErr == nil { - return nil + return nil, nil } // UnmarshalStrict(data, v) TODO: investigate how ymlv3 handles duplicate map keys @@ -35,11 +38,11 @@ func unmarshal(data []byte, v any, includeOrigin bool, location *url.URL) error DisableTimestamps: true, }); err == nil { applyOrigins(v, tree) - return nil + return tree, nil } else { yamlErr = err } // If both unmarshaling attempts fail, return a new error that includes both errors - return fmt.Errorf("failed to unmarshal data: json error: %v, yaml error: %v", jsonErr, yamlErr) + return nil, fmt.Errorf("failed to unmarshal data: json error: %v, yaml error: %v", jsonErr, yamlErr) } diff --git a/openapi3/origin.go b/openapi3/origin.go index c24655340..75e1e70ad 100644 --- a/openapi3/origin.go +++ b/openapi3/origin.go @@ -1,7 +1,6 @@ package openapi3 import ( - "net/url" "reflect" "sort" "strings" @@ -326,23 +325,6 @@ func jsonTagName(f reflect.StructField) string { return name } -// originTree parses data solely for its origin tree, used to re-attach source -// origins to a component resolved through the generic-map path: a $ref to a -// schema under an arbitrary top-level key lands in T.Extensions and loses its -// origin in the json round-trip (see resolveComponent). Returns nil when parsing -// fails; callers degrade to no origin. -func originTree(data []byte, location *url.URL) *yaml.OriginTree { - var file string - if location != nil { - file = location.String() - } - var v any - tree, err := yaml.Unmarshal(data, &v, yaml.DecodeOpts{ - Origin: yaml.OriginOpt{Enabled: true, File: file}, - DisableTimestamps: true, - }) - if err != nil { - return nil - } - return tree -} +// originTree aliases the decoder-side origin tree, so the loader and marsh can +// carry it without referencing the yaml package directly. +type originTree = yaml.OriginTree diff --git a/openapi3/origin_external_ref_test.go b/openapi3/origin_external_ref_test.go index 5410310e6..cdfbeff3f 100644 --- a/openapi3/origin_external_ref_test.go +++ b/openapi3/origin_external_ref_test.go @@ -1,6 +1,7 @@ package openapi3 import ( + "net/url" "path/filepath" "testing" @@ -29,3 +30,26 @@ func TestOrigin_ExternalRefToArbitraryTopLevelKey(t *testing.T) { "origin points at the file the schema lives in, not the referencing document") require.NotZero(t, user.Origin.Key.Line, "the origin has a source line") } + +// Re-attaching origins reuses the origin tree retained at load time: resolving +// the arbitrary-key $ref must not read any file a second time. +func TestOrigin_ExternalRefToArbitraryTopLevelKey_NoRereads(t *testing.T) { + reads := map[string]int{} + loader := NewLoader() + loader.IncludeOrigin = true + loader.ReadFromURIFunc = func(l *Loader, location *url.URL) ([]byte, error) { + reads[location.String()]++ + return DefaultReadFromURI(l, location) + } + + doc, err := loader.LoadFromFile("testdata/origin/arbitrary_key.yaml") + require.NoError(t, err) + + user := doc.Paths.Value("/users").Get.Responses.Value("200").Value.Content["application/json"].Schema.Value + require.NotNil(t, user.Origin, "origins are attached") + + require.Len(t, reads, 2, "the root and the $ref'd file") + for location, n := range reads { + require.Equalf(t, 1, n, "%s must be read exactly once", location) + } +} From 857f2f98ae1511467eeb5418410529ed23a0eb41 Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Wed, 15 Jul 2026 07:03:11 +0300 Subject: [PATCH 6/7] openapi3: key retained origin trees by document, not URL string The URL-string key relied on resolveRefAndDocument returning the same string the document was loaded under; true today, but a silent lookup miss away from breaking. Key by the document pointer instead: visitedDocuments guarantees one *T per document, and resolveComponent already holds it, so insert and lookup cannot disagree. This also serves documents loaded from memory (LoadFromData has no URL), so a same-document $ref to an arbitrary top-level key now gets its origin too, with a test. --- openapi3/loader.go | 48 +++++++++++++++++----------- openapi3/origin_external_ref_test.go | 32 +++++++++++++++++++ 2 files changed, 61 insertions(+), 19 deletions(-) diff --git a/openapi3/loader.go b/openapi3/loader.go index 8e78e1567..2f2480c07 100644 --- a/openapi3/loader.go +++ b/openapi3/loader.go @@ -64,11 +64,12 @@ type Loader struct { visitedDocuments map[string]*T - // originTrees retains each visited document's origin tree (keyed like - // visitedDocuments) when IncludeOrigin is set, so resolveComponent can - // re-attach origins to components that lose them in the generic-map path - // without re-reading or re-parsing the file. - originTrees map[string]*originTree + // originTrees retains each loaded document's origin tree, keyed by the + // document itself so insert and lookup cannot disagree, populated when + // IncludeOrigin is set. resolveComponent uses it to re-attach origins to + // components that lose them in the generic-map path, without re-reading + // or re-parsing the file. + originTrees map[*T]*originTree visitedRefs map[string]struct{} visitedPath []string @@ -146,6 +147,18 @@ func (loader *Loader) loadSingleElementFromURI(ref string, rootPath *url.URL, el return resolvedPath, nil } +// rememberOriginTree retains doc's origin tree for attachOriginToResolved. +// tree is nil when IncludeOrigin is off or the data took the json path. +func (loader *Loader) rememberOriginTree(doc *T, tree *originTree) { + if tree == nil { + return + } + if loader.originTrees == nil { + loader.originTrees = make(map[*T]*originTree) + } + loader.originTrees[doc] = tree +} + func (loader *Loader) readURL(location *url.URL) ([]byte, error) { if f := loader.ReadFromURIFunc; f != nil { return f(loader, location) @@ -175,9 +188,11 @@ func (loader *Loader) LoadFromIoReader(reader io.Reader) (*T, error) { func (loader *Loader) LoadFromData(data []byte) (*T, error) { loader.resetVisitedPathItemRefs() doc := &T{} - if _, err := unmarshal(data, doc, loader.IncludeOrigin, nil); err != nil { + tree, err := unmarshal(data, doc, loader.IncludeOrigin, nil) + if err != nil { return nil, err } + loader.rememberOriginTree(doc, tree) if err := loader.ResolveRefsIn(doc, nil); err != nil { return nil, err } @@ -208,12 +223,7 @@ func (loader *Loader) loadFromDataWithPathInternal(data []byte, location *url.UR if err != nil { return nil, err } - if tree != nil { - if loader.originTrees == nil { - loader.originTrees = make(map[string]*originTree) - } - loader.originTrees[uri] = tree - } + loader.rememberOriginTree(doc, tree) doc.url = copyURI(location) @@ -532,7 +542,7 @@ func (loader *Loader) resolveComponent(doc *T, ref string, path *url.URL, resolv // The value came from a generic map in T.Extensions (a $ref to an // arbitrary top-level key), so the json round-trip above stripped its // origins. Re-attach them from the file, best-effort. - loader.attachOriginToResolved(resolved, componentPath, fragment) + loader.attachOriginToResolved(resolved, componentDoc, fragment) return componentDoc, componentPath, nil default: @@ -544,15 +554,15 @@ func (loader *Loader) resolveComponent(doc *T, ref string, path *url.URL, resolv // through the generic-map path: a $ref to a schema under an arbitrary top-level // key lands in T.Extensions, and the json round-trip in resolveComponent strips // its origin. It walks the document's retained origin tree (see originTrees, -// populated when the file was first unmarshaled: no re-read, no re-parse) down -// to the ref fragment and applies that subtree, so the object carries the same -// origins a typed resolution would, with the original file's line numbers. +// populated when the document was first unmarshaled: no re-read, no re-parse) +// down to the ref fragment and applies that subtree, so the object carries the +// same origins a typed resolution would, with the original file's line numbers. // Best-effort: a missing tree or fragment leaves the object without origins. -func (loader *Loader) attachOriginToResolved(resolved any, componentPath *url.URL, fragment string) { - if !loader.IncludeOrigin || componentPath == nil { +func (loader *Loader) attachOriginToResolved(resolved any, componentDoc *T, fragment string) { + if !loader.IncludeOrigin { return } - tree := loader.originTrees[componentPath.String()] + tree := loader.originTrees[componentDoc] if tree == nil { return } diff --git a/openapi3/origin_external_ref_test.go b/openapi3/origin_external_ref_test.go index cdfbeff3f..94c20ede7 100644 --- a/openapi3/origin_external_ref_test.go +++ b/openapi3/origin_external_ref_test.go @@ -53,3 +53,35 @@ func TestOrigin_ExternalRefToArbitraryTopLevelKey_NoRereads(t *testing.T) { require.Equalf(t, 1, n, "%s must be read exactly once", location) } } + +// Keying the retained trees by document also serves documents loaded from +// memory (no location): a $ref to an arbitrary top-level key in the same +// document gets its origin attached too. +func TestOrigin_InternalRefToArbitraryTopLevelKey_FromData(t *testing.T) { + const spec = ` +openapi: 3.0.0 +info: { title: t, version: "1" } +paths: + /users: + get: + responses: + "200": + description: OK + content: + application/json: + schema: + $ref: '#/User' +User: + type: object + properties: + id: { type: string } +` + loader := NewLoader() + loader.IncludeOrigin = true + doc, err := loader.LoadFromData([]byte(spec)) + require.NoError(t, err) + + user := doc.Paths.Value("/users").Get.Responses.Value("200").Value.Content["application/json"].Schema.Value + require.NotNil(t, user.Origin, "a same-document arbitrary-key $ref carries an origin") + require.NotZero(t, user.Origin.Key.Line) +} From 828b2ccfe6a0f7be366033370b4814eb7d6e31d5 Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Wed, 15 Jul 2026 11:44:39 +0100 Subject: [PATCH 7/7] openapi3: assert exact origin values in the arbitrary-key tests Pin the key location (line, column, name, end span), a field origin, and a nested property origin against the fixture layout, instead of non-zero checks. --- openapi3/origin_external_ref_test.go | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/openapi3/origin_external_ref_test.go b/openapi3/origin_external_ref_test.go index 94c20ede7..83f7ea019 100644 --- a/openapi3/origin_external_ref_test.go +++ b/openapi3/origin_external_ref_test.go @@ -28,7 +28,21 @@ func TestOrigin_ExternalRefToArbitraryTopLevelKey(t *testing.T) { require.NotNil(t, user.Origin.Key, "the origin has a key location") require.Equal(t, "arbitrary_key_schemas.yaml", filepath.Base(user.Origin.Key.File), "origin points at the file the schema lives in, not the referencing document") - require.NotZero(t, user.Origin.Key.Line, "the origin has a source line") + require.Equal(t, Location{ + File: user.Origin.Key.File, + Line: 1, + Column: 1, + Name: "User", + EndLine: 7, EndColumn: 19, + }, *user.Origin.Key, "the key origin spans the whole User block in arbitrary_key_schemas.yaml") + require.Equal(t, 2, user.Origin.Fields["type"].Line, "field origins are attached too") + + // the subtree gets origins as well, with the same file + id := user.Properties["id"].Value + require.NotNil(t, id.Origin) + require.Equal(t, user.Origin.Key.File, id.Origin.Key.File) + require.Equal(t, 4, id.Origin.Key.Line, "the id property's own line") + require.Equal(t, 5, id.Origin.Fields["type"].Line) } // Re-attaching origins reuses the origin tree retained at load time: resolving @@ -83,5 +97,10 @@ User: user := doc.Paths.Value("/users").Get.Responses.Value("200").Value.Content["application/json"].Schema.Value require.NotNil(t, user.Origin, "a same-document arbitrary-key $ref carries an origin") - require.NotZero(t, user.Origin.Key.Line) + require.NotNil(t, user.Origin.Key) + require.Equal(t, "User", user.Origin.Key.Name) + require.Equal(t, 14, user.Origin.Key.Line, "the User: line in the document above") + require.Equal(t, 17, user.Origin.Key.EndLine, "the block's last line") + require.Equal(t, 15, user.Origin.Fields["type"].Line) + require.Empty(t, user.Origin.Key.File, "a document loaded from data has no file") }