From 3a2156cb934e6b8481c80b39f3b0c5d9939ef8d6 Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Tue, 28 Jul 2026 01:16:40 +0300 Subject: [PATCH] openapi3: record origins for specs written in JSON Only the yaml path builds the origin tree, and unmarshal tried json first, returning as soon as it succeeded. Every JSON document therefore loaded with no positions at all, whatever IncludeOrigin was set to, so consumers reporting source locations had nothing to report for roughly half the specs in the wild. Make the order depend on IncludeOrigin: without it the json fast path still runs first (#680 is unaffected), with it the yaml path runs first, which is what gives JSON documents origins. yaml is a superset of json, so it parses the same documents, and tab-indented, minified and large-number JSON were all verified to load unchanged. Not all of them, though: json permits duplicate keys and resolves them last-one-wins, while yaml rejects them. So json still runs as a fallback when yaml fails, and such documents keep loading exactly as before, simply without origins. That also keeps the both-parsers-failed error message intact, since both errors are still populated. Tests cover origins on a JSON spec (document, info, path item, operation and response positions), the duplicate-key fallback, and that origins stay absent when IncludeOrigin is off. Co-Authored-By: Claude Opus 5 (1M context) --- openapi3/marsh.go | 26 ++++- openapi3/origin_json_test.go | 104 +++++++++++++++++++ openapi3/testdata/origin/duplicate_keys.json | 9 ++ openapi3/testdata/origin/simple.json | 21 ++++ 4 files changed, 155 insertions(+), 5 deletions(-) create mode 100644 openapi3/origin_json_test.go create mode 100644 openapi3/testdata/origin/duplicate_keys.json create mode 100644 openapi3/testdata/origin/simple.json diff --git a/openapi3/marsh.go b/openapi3/marsh.go index f895e6e40..72ca10930 100644 --- a/openapi3/marsh.go +++ b/openapi3/marsh.go @@ -17,15 +17,22 @@ func unmarshalError(jsonUnmarshalErr error) error { return jsonUnmarshalErr } -// 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). +// unmarshal decodes data into v. It returns the document origin tree when the +// data took the yaml path, so the caller can retain it (see Loader.originTrees). +// +// Only the yaml path records positions, so the order of the two attempts +// depends on includeOrigin: without it the json fast path runs first, with it +// the yaml path does, which is what gives json documents origins. Either way +// both parsers are tried before failing, so anything that loaded before still +// loads. 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, nil + if !includeOrigin { + if jsonErr = json.Unmarshal(data, v); jsonErr == nil { + return nil, nil + } } // UnmarshalStrict(data, v) TODO: investigate how ymlv3 handles duplicate map keys @@ -43,6 +50,15 @@ func unmarshal(data []byte, v any, includeOrigin bool, location *url.URL) (*orig yamlErr = err } + // The yaml path was tried first and failed, so json has not run yet. It + // accepts documents yaml rejects (duplicate keys, which json resolves + // last-one-wins), so this keeps them loading, without origins. + if includeOrigin { + if jsonErr = json.Unmarshal(data, v); jsonErr == nil { + return nil, nil + } + } + // If both unmarshaling attempts fail, return a new error that includes both errors return nil, fmt.Errorf("failed to unmarshal data: json error: %v, yaml error: %v", jsonErr, yamlErr) } diff --git a/openapi3/origin_json_test.go b/openapi3/origin_json_test.go new file mode 100644 index 000000000..46d751578 --- /dev/null +++ b/openapi3/origin_json_test.go @@ -0,0 +1,104 @@ +package openapi3_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" +) + +// A JSON document gets origins like a YAML one: only the yaml parser records +// positions, so the loader runs it first when origins are requested. +func TestOrigin_JSONSpec(t *testing.T) { + loader := openapi3.NewLoader() + loader.IncludeOrigin = true + loader.Context = t.Context() + + doc, err := loader.LoadFromFile("testdata/origin/simple.json") + require.NoError(t, err) + + require.NotNil(t, doc.Origin) + require.Equal(t, + openapi3.Location{ + File: "testdata/origin/simple.json", + Line: 2, + Column: 3, + Name: "openapi", + }, + doc.Origin.Fields["openapi"]) + + require.NotNil(t, doc.Info.Origin) + require.Equal(t, + &openapi3.Location{ + File: "testdata/origin/simple.json", + Line: 3, + Column: 3, + Name: "info", + EndLine: 6, + EndColumn: 4, + }, + doc.Info.Origin.Key) + + pathItem := doc.Paths.Find("/partner-api/test/some-method") + require.NotNil(t, pathItem.Origin) + require.Equal(t, + &openapi3.Location{ + File: "testdata/origin/simple.json", + Line: 8, + Column: 5, + Name: "/partner-api/test/some-method", + EndLine: 19, + EndColumn: 6, + }, + pathItem.Origin.Key) + + require.NotNil(t, pathItem.Get.Origin) + require.Equal(t, + &openapi3.Location{ + File: "testdata/origin/simple.json", + Line: 9, + Column: 7, + Name: "get", + EndLine: 18, + EndColumn: 8, + }, + pathItem.Get.Origin.Key) + + response := pathItem.Get.Responses.Value("200") + require.NotNil(t, response.Value.Origin) + require.Equal(t, + openapi3.Location{ + File: "testdata/origin/simple.json", + Line: 15, + Column: 13, + Name: "description", + }, + response.Value.Origin.Fields["description"]) +} + +// Origins are off, so the json fast path runs first and no positions are +// recorded. Pins that requesting origins is what changes the parser order. +func TestOrigin_JSONSpecWithoutIncludeOrigin(t *testing.T) { + loader := openapi3.NewLoader() + loader.Context = t.Context() + + doc, err := loader.LoadFromFile("testdata/origin/simple.json") + require.NoError(t, err) + require.Nil(t, doc.Origin) + require.Equal(t, "Test API", doc.Info.Title) +} + +// json permits duplicate keys and resolves them last-one-wins; yaml rejects +// them. Such a document must keep loading when origins are requested, since +// the loader falls back to json, and it simply has no origins. +func TestOrigin_JSONSpecDuplicateKeys(t *testing.T) { + loader := openapi3.NewLoader() + loader.IncludeOrigin = true + loader.Context = t.Context() + + doc, err := loader.LoadFromFile("testdata/origin/duplicate_keys.json") + require.NoError(t, err) + require.Equal(t, "Second", doc.Info.Title) + require.Nil(t, doc.Origin) +} diff --git a/openapi3/testdata/origin/duplicate_keys.json b/openapi3/testdata/origin/duplicate_keys.json new file mode 100644 index 000000000..7b4482a90 --- /dev/null +++ b/openapi3/testdata/origin/duplicate_keys.json @@ -0,0 +1,9 @@ +{ + "openapi": "3.0.1", + "info": { + "title": "First", + "title": "Second", + "version": "v1" + }, + "paths": {} +} diff --git a/openapi3/testdata/origin/simple.json b/openapi3/testdata/origin/simple.json new file mode 100644 index 000000000..8b495eb31 --- /dev/null +++ b/openapi3/testdata/origin/simple.json @@ -0,0 +1,21 @@ +{ + "openapi": "3.0.1", + "info": { + "title": "Test API", + "version": "v1" + }, + "paths": { + "/partner-api/test/some-method": { + "get": { + "tags": [ + "Test" + ], + "responses": { + "200": { + "description": "Success" + } + } + } + } + } +}