From 79ac3c4d3609fcc1f94e9588533dd5c3d86070a5 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Mon, 19 Oct 2026 21:10:37 +0200 Subject: [PATCH 01/21] openapi3: ignore the fields next to a Reference Object's $ref in every version The Reference Object of OpenAPI 3.0, 3.1 and 3.2 alike says "any properties added SHALL be ignored". From 3.1 on only summary and description mean something there, and the loader already applies them; anything else was still reported as extra sibling fields, failing documents the specification calls valid. So validateExtras now only reports the x- extensions next to a $ref, under ProhibitExtensionsWithRef. That option was disabled for OpenAPI 3.1 schemas, only because their sibling keywords were reported together with those extensions: it now applies in every version, as documented. ExtraSiblingFieldsError is still returned for the unknown fields of inline objects. validateExtras also reported the fields next to a $ref once the $ref was gone: InternalizeRefs replaces a component that is a $ref by what it refers to, which leaves those fields out of the document, but not out of what Validate checked. Signed-off-by: Pierre Fenoll --- .github/docs/openapi3.txt | 9 +- README.md | 1 + openapi3/reference_object_siblings_test.go | 97 +++++ openapi3/refs.go | 410 ++++++--------------- openapi3/refs.tmpl | 54 +-- openapi3/refs_test.go | 34 +- openapi3/refs_test.tmpl | 6 +- openapi3/validation_options.go | 6 +- 8 files changed, 247 insertions(+), 370 deletions(-) create mode 100644 openapi3/reference_object_siblings_test.go diff --git a/.github/docs/openapi3.txt b/.github/docs/openapi3.txt index 66f9acf65..1c77fe090 100644 --- a/.github/docs/openapi3.txt +++ b/.github/docs/openapi3.txt @@ -4207,7 +4207,10 @@ func AllowExtensionsWithRef() ValidationOption func AllowExtraSiblingFields(fields ...string) ValidationOption AllowExtraSiblingFields called as AllowExtraSiblingFields("description") - makes Validate not return an error when said field appears next to a $ref. + makes Validate not return an error when said field appears where it is + not expected: as an unknown field of an object, as an OpenAPI 3.1 schema + keyword in an OpenAPI 3.0 document, or as an extension next to a $ref under + ProhibitExtensionsWithRef. Other fields next to a $ref are never an error. func DisableExamplesValidation() ValidationOption DisableExamplesValidation disables all example schema validation. @@ -4269,8 +4272,8 @@ func IsOpenAPI32OrLater() ValidationOption func ProhibitExtensionsWithRef() ValidationOption ProhibitExtensionsWithRef causes the validation to return an error if extensions (fields starting with 'x-') are found as siblings for $ref - fields, even in OpenAPI 3.0. Use AllowExtraSiblingFields to allow specific - extensions. + fields, in every OpenAPI version. Use AllowExtraSiblingFields to allow + specific extensions. func SetRegexCompiler(c RegexCompilerFunc) ValidationOption SetRegexCompiler allows to override the regex implementation used to diff --git a/README.md b/README.md index 0ba04f93d..2f2da6e6d 100644 --- a/README.md +++ b/README.md @@ -337,6 +337,7 @@ for _, path := range doc.Paths.InMatchingOrder() { * The error for a header with neither or both of `content` and `schema` reads `header must contain exactly one of content and schema`. It used to say `parameter ...` followed by a `%v` dump of the `Header` struct, which printed heap addresses: the message differed between two runs over the same document, and leaked addresses into anything that logged it. * `openapi3filter` now parses `integer` values of parameters and `application/x-www-form-urlencoded`/`multipart/form-data` bodies as base 10 only, so `010` decodes to `10` rather than `8`, and Go integer literals such as `0x10`, `0b10`, `0o10` or `1_000` are now rejected as invalid integers. * `openapi3filter.ValidateResponse` now decodes and validates response headers described with `content` rather than `schema`, with the body decoder registered for the header's media type (see `RegisterBodyDecoder`). Such a header is no longer reported missing when it is required and present, and a value that does not match its schema is now rejected. As with a response body, a media type with no registered decoder fails with `unsupported content type`. +* `Validate` no longer reports `extra sibling fields` for fields written next to a `$ref`: every version's Reference Object says they "SHALL be ignored". From OpenAPI 3.1 on, `summary` and `description` still override the referenced component's. `ExtraSiblingFieldsError` is still returned for unknown fields of inline objects, and for `x-` extensions next to a `$ref` under `ProhibitExtensionsWithRef`, which now applies to OpenAPI 3.1+ schemas too. It is no longer returned after `InternalizeRefs` for the fields that were next to a component's `$ref` it replaced, and which it leaves out. ### v0.150.0 * `openapi2.Operation.Responses` field type changed from `map[string]*Response` to `*Responses`, which keeps the Responses Object's `x-` extensions in `Responses.Extensions` instead of failing to decode them as responses. diff --git a/openapi3/reference_object_siblings_test.go b/openapi3/reference_object_siblings_test.go new file mode 100644 index 000000000..3f4941d19 --- /dev/null +++ b/openapi3/reference_object_siblings_test.go @@ -0,0 +1,97 @@ +package openapi3_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" +) + +// Every version's Reference Object says "any properties added SHALL be +// ignored". From 3.1 on summary and description are fixed fields that override +// the referenced component's. +func TestReferenceObjectSiblingsAreIgnored(t *testing.T) { + t.Parallel() + for _, version := range []string{"3.0.3", "3.1.0", "3.2.0"} { + t.Run(version, func(t *testing.T) { + t.Parallel() + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData(fmt.Appendf(nil, `{ + "openapi": %q, + "info": {"title": "t", "version": "1"}, + "paths": {"/x": {"get": {"responses": {"200": { + "$ref": "#/components/responses/R", + "description": "local", + "unknown": 1 + }}}}}, + "components": {"responses": {"R": {"description": "target"}}} +}`, version)) + require.NoError(t, err) + require.NoError(t, doc.Validate(t.Context())) + + want := "local" + if doc.IsOpenAPI30() { + want = "target" + } + response := doc.Paths.Value("/x").Get.Responses.Value("200").Value + require.Equal(t, want, *response.Description) + }) + } +} + +// InternalizeRefs replaces a component that is a $ref by what it refers to, +// which leaves out the fields that were next to the $ref. +func TestReferenceObjectSiblingsAreGoneOnceInternalized(t *testing.T) { + t.Parallel() + for _, version := range []string{"3.0.3", "3.1.0"} { + t.Run(version, func(t *testing.T) { + t.Parallel() + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData(fmt.Appendf(nil, `{ + "openapi": %q, + "info": {"title": "t", "version": "1"}, + "paths": {}, + "components": { + "schemas": { + "A": {"type": "string"}, + "B": {"$ref": "#/components/schemas/A", "minLength": 2} + }, + "parameters": { + "P": {"name": "p", "in": "query", "schema": {"type": "string"}}, + "Q": {"$ref": "#/components/parameters/P", "description": "local", "unknown": 1} + } + } +}`, version)) + require.NoError(t, err) + require.NoError(t, doc.Validate(t.Context())) + + doc.InternalizeRefs(t.Context(), nil) + require.NoError(t, doc.Validate(t.Context())) + }) + } +} + +func TestProhibitExtensionsWithRefInEveryVersion(t *testing.T) { + t.Parallel() + for _, version := range []string{"3.0.3", "3.1.0"} { + t.Run(version, func(t *testing.T) { + t.Parallel() + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData(fmt.Appendf(nil, `{ + "openapi": %q, + "info": {"title": "t", "version": "1"}, + "paths": {}, + "components": {"schemas": { + "A": {"type": "string"}, + "B": {"$ref": "#/components/schemas/A", "x-order": 1} + }} +}`, version)) + require.NoError(t, err) + require.NoError(t, doc.Validate(t.Context())) + require.ErrorContains(t, doc.Validate(t.Context(), openapi3.ProhibitExtensionsWithRef()), "extra sibling fields: [x-order]") + require.NoError(t, doc.Validate(t.Context(), openapi3.ProhibitExtensionsWithRef(), openapi3.AllowExtraSiblingFields("x-order"))) + }) + } +} diff --git a/openapi3/refs.go b/openapi3/refs.go index 44a0645c7..3f2c22706 100644 --- a/openapi3/refs.go +++ b/openapi3/refs.go @@ -102,46 +102,24 @@ func (x *CallbackRef) UnmarshalJSON(data []byte) error { return unmarshalJSON(data, &x.Value) } -// validateExtras returns an error if CallbackRef has sibling fields -// alongside $ref that are not allowed by the validation options. +// validateExtras returns an error if CallbackRef has `x-` extensions +// next to its $ref while ProhibitExtensionsWithRef is set. Every other field +// next to a $ref is not an error: every version's Reference Object says "any +// properties added SHALL be ignored", from 3.1 on summary and description +// override the referenced component's, and an OpenAPI 3.1 schema applies its +// sibling keywords alongside the referenced schema. func (x *CallbackRef) validateExtras(ctx context.Context) error { validationOpts := getValidationOptions(ctx) - var extras []string - - allowed := validationOpts.extraSiblingFieldsAllowed - if allowed == nil { - allowed = make(map[string]struct{}) - } - if x.Ref == "" && !validationOpts.isOpenAPI31OrLater { - if _, ok := allowed["description"]; x.Description != nil && !ok { - extras = append(extras, "description") - } - if _, ok := allowed["summary"]; x.Summary != nil && !ok { - extras = append(extras, "summary") - } - } - // OpenAPI 3.0 Reference Objects ignore added properties. Keep the - // explicit extension policy below and validation of inline objects. - if x.Ref == "" || validationOpts.isOpenAPI31OrLater { - for _, ex := range x.extra { - if _, ok := allowed[ex]; !ok { - if _, ok := x.Extensions[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below - } - } + if x.Ref == "" || !validationOpts.schemaExtensionsInRefProhibited { + return nil } - if validationOpts.schemaExtensionsInRefProhibited { - for _, ex := range componentNames(x.Extensions) { - if _, ok := allowed[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below + var extras []string + for _, ex := range componentNames(x.Extensions) { + if _, ok := validationOpts.extraSiblingFieldsAllowed[ex]; !ok { + extras = append(extras, ex) } } - if len(extras) != 0 { return newExtraSiblingFields(extras, x.Origin) } @@ -271,46 +249,24 @@ func (x *ExampleRef) UnmarshalJSON(data []byte) error { return unmarshalJSON(data, &x.Value) } -// validateExtras returns an error if ExampleRef has sibling fields -// alongside $ref that are not allowed by the validation options. +// validateExtras returns an error if ExampleRef has `x-` extensions +// next to its $ref while ProhibitExtensionsWithRef is set. Every other field +// next to a $ref is not an error: every version's Reference Object says "any +// properties added SHALL be ignored", from 3.1 on summary and description +// override the referenced component's, and an OpenAPI 3.1 schema applies its +// sibling keywords alongside the referenced schema. func (x *ExampleRef) validateExtras(ctx context.Context) error { validationOpts := getValidationOptions(ctx) - var extras []string - - allowed := validationOpts.extraSiblingFieldsAllowed - if allowed == nil { - allowed = make(map[string]struct{}) - } - if x.Ref == "" && !validationOpts.isOpenAPI31OrLater { - if _, ok := allowed["description"]; x.Description != nil && !ok { - extras = append(extras, "description") - } - if _, ok := allowed["summary"]; x.Summary != nil && !ok { - extras = append(extras, "summary") - } - } - // OpenAPI 3.0 Reference Objects ignore added properties. Keep the - // explicit extension policy below and validation of inline objects. - if x.Ref == "" || validationOpts.isOpenAPI31OrLater { - for _, ex := range x.extra { - if _, ok := allowed[ex]; !ok { - if _, ok := x.Extensions[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below - } - } + if x.Ref == "" || !validationOpts.schemaExtensionsInRefProhibited { + return nil } - if validationOpts.schemaExtensionsInRefProhibited { - for _, ex := range componentNames(x.Extensions) { - if _, ok := allowed[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below + var extras []string + for _, ex := range componentNames(x.Extensions) { + if _, ok := validationOpts.extraSiblingFieldsAllowed[ex]; !ok { + extras = append(extras, ex) } } - if len(extras) != 0 { return newExtraSiblingFields(extras, x.Origin) } @@ -440,46 +396,24 @@ func (x *HeaderRef) UnmarshalJSON(data []byte) error { return unmarshalJSON(data, &x.Value) } -// validateExtras returns an error if HeaderRef has sibling fields -// alongside $ref that are not allowed by the validation options. +// validateExtras returns an error if HeaderRef has `x-` extensions +// next to its $ref while ProhibitExtensionsWithRef is set. Every other field +// next to a $ref is not an error: every version's Reference Object says "any +// properties added SHALL be ignored", from 3.1 on summary and description +// override the referenced component's, and an OpenAPI 3.1 schema applies its +// sibling keywords alongside the referenced schema. func (x *HeaderRef) validateExtras(ctx context.Context) error { validationOpts := getValidationOptions(ctx) - var extras []string - - allowed := validationOpts.extraSiblingFieldsAllowed - if allowed == nil { - allowed = make(map[string]struct{}) - } - if x.Ref == "" && !validationOpts.isOpenAPI31OrLater { - if _, ok := allowed["description"]; x.Description != nil && !ok { - extras = append(extras, "description") - } - if _, ok := allowed["summary"]; x.Summary != nil && !ok { - extras = append(extras, "summary") - } - } - // OpenAPI 3.0 Reference Objects ignore added properties. Keep the - // explicit extension policy below and validation of inline objects. - if x.Ref == "" || validationOpts.isOpenAPI31OrLater { - for _, ex := range x.extra { - if _, ok := allowed[ex]; !ok { - if _, ok := x.Extensions[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below - } - } + if x.Ref == "" || !validationOpts.schemaExtensionsInRefProhibited { + return nil } - if validationOpts.schemaExtensionsInRefProhibited { - for _, ex := range componentNames(x.Extensions) { - if _, ok := allowed[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below + var extras []string + for _, ex := range componentNames(x.Extensions) { + if _, ok := validationOpts.extraSiblingFieldsAllowed[ex]; !ok { + extras = append(extras, ex) } } - if len(extras) != 0 { return newExtraSiblingFields(extras, x.Origin) } @@ -609,46 +543,24 @@ func (x *LinkRef) UnmarshalJSON(data []byte) error { return unmarshalJSON(data, &x.Value) } -// validateExtras returns an error if LinkRef has sibling fields -// alongside $ref that are not allowed by the validation options. +// validateExtras returns an error if LinkRef has `x-` extensions +// next to its $ref while ProhibitExtensionsWithRef is set. Every other field +// next to a $ref is not an error: every version's Reference Object says "any +// properties added SHALL be ignored", from 3.1 on summary and description +// override the referenced component's, and an OpenAPI 3.1 schema applies its +// sibling keywords alongside the referenced schema. func (x *LinkRef) validateExtras(ctx context.Context) error { validationOpts := getValidationOptions(ctx) - var extras []string - - allowed := validationOpts.extraSiblingFieldsAllowed - if allowed == nil { - allowed = make(map[string]struct{}) - } - if x.Ref == "" && !validationOpts.isOpenAPI31OrLater { - if _, ok := allowed["description"]; x.Description != nil && !ok { - extras = append(extras, "description") - } - if _, ok := allowed["summary"]; x.Summary != nil && !ok { - extras = append(extras, "summary") - } - } - // OpenAPI 3.0 Reference Objects ignore added properties. Keep the - // explicit extension policy below and validation of inline objects. - if x.Ref == "" || validationOpts.isOpenAPI31OrLater { - for _, ex := range x.extra { - if _, ok := allowed[ex]; !ok { - if _, ok := x.Extensions[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below - } - } + if x.Ref == "" || !validationOpts.schemaExtensionsInRefProhibited { + return nil } - if validationOpts.schemaExtensionsInRefProhibited { - for _, ex := range componentNames(x.Extensions) { - if _, ok := allowed[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below + var extras []string + for _, ex := range componentNames(x.Extensions) { + if _, ok := validationOpts.extraSiblingFieldsAllowed[ex]; !ok { + extras = append(extras, ex) } } - if len(extras) != 0 { return newExtraSiblingFields(extras, x.Origin) } @@ -778,46 +690,24 @@ func (x *ParameterRef) UnmarshalJSON(data []byte) error { return unmarshalJSON(data, &x.Value) } -// validateExtras returns an error if ParameterRef has sibling fields -// alongside $ref that are not allowed by the validation options. +// validateExtras returns an error if ParameterRef has `x-` extensions +// next to its $ref while ProhibitExtensionsWithRef is set. Every other field +// next to a $ref is not an error: every version's Reference Object says "any +// properties added SHALL be ignored", from 3.1 on summary and description +// override the referenced component's, and an OpenAPI 3.1 schema applies its +// sibling keywords alongside the referenced schema. func (x *ParameterRef) validateExtras(ctx context.Context) error { validationOpts := getValidationOptions(ctx) - var extras []string - - allowed := validationOpts.extraSiblingFieldsAllowed - if allowed == nil { - allowed = make(map[string]struct{}) - } - if x.Ref == "" && !validationOpts.isOpenAPI31OrLater { - if _, ok := allowed["description"]; x.Description != nil && !ok { - extras = append(extras, "description") - } - if _, ok := allowed["summary"]; x.Summary != nil && !ok { - extras = append(extras, "summary") - } - } - // OpenAPI 3.0 Reference Objects ignore added properties. Keep the - // explicit extension policy below and validation of inline objects. - if x.Ref == "" || validationOpts.isOpenAPI31OrLater { - for _, ex := range x.extra { - if _, ok := allowed[ex]; !ok { - if _, ok := x.Extensions[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below - } - } + if x.Ref == "" || !validationOpts.schemaExtensionsInRefProhibited { + return nil } - if validationOpts.schemaExtensionsInRefProhibited { - for _, ex := range componentNames(x.Extensions) { - if _, ok := allowed[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below + var extras []string + for _, ex := range componentNames(x.Extensions) { + if _, ok := validationOpts.extraSiblingFieldsAllowed[ex]; !ok { + extras = append(extras, ex) } } - if len(extras) != 0 { return newExtraSiblingFields(extras, x.Origin) } @@ -947,46 +837,24 @@ func (x *RequestBodyRef) UnmarshalJSON(data []byte) error { return unmarshalJSON(data, &x.Value) } -// validateExtras returns an error if RequestBodyRef has sibling fields -// alongside $ref that are not allowed by the validation options. +// validateExtras returns an error if RequestBodyRef has `x-` extensions +// next to its $ref while ProhibitExtensionsWithRef is set. Every other field +// next to a $ref is not an error: every version's Reference Object says "any +// properties added SHALL be ignored", from 3.1 on summary and description +// override the referenced component's, and an OpenAPI 3.1 schema applies its +// sibling keywords alongside the referenced schema. func (x *RequestBodyRef) validateExtras(ctx context.Context) error { validationOpts := getValidationOptions(ctx) - var extras []string - - allowed := validationOpts.extraSiblingFieldsAllowed - if allowed == nil { - allowed = make(map[string]struct{}) - } - if x.Ref == "" && !validationOpts.isOpenAPI31OrLater { - if _, ok := allowed["description"]; x.Description != nil && !ok { - extras = append(extras, "description") - } - if _, ok := allowed["summary"]; x.Summary != nil && !ok { - extras = append(extras, "summary") - } - } - // OpenAPI 3.0 Reference Objects ignore added properties. Keep the - // explicit extension policy below and validation of inline objects. - if x.Ref == "" || validationOpts.isOpenAPI31OrLater { - for _, ex := range x.extra { - if _, ok := allowed[ex]; !ok { - if _, ok := x.Extensions[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below - } - } + if x.Ref == "" || !validationOpts.schemaExtensionsInRefProhibited { + return nil } - if validationOpts.schemaExtensionsInRefProhibited { - for _, ex := range componentNames(x.Extensions) { - if _, ok := allowed[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below + var extras []string + for _, ex := range componentNames(x.Extensions) { + if _, ok := validationOpts.extraSiblingFieldsAllowed[ex]; !ok { + extras = append(extras, ex) } } - if len(extras) != 0 { return newExtraSiblingFields(extras, x.Origin) } @@ -1116,46 +984,24 @@ func (x *ResponseRef) UnmarshalJSON(data []byte) error { return unmarshalJSON(data, &x.Value) } -// validateExtras returns an error if ResponseRef has sibling fields -// alongside $ref that are not allowed by the validation options. +// validateExtras returns an error if ResponseRef has `x-` extensions +// next to its $ref while ProhibitExtensionsWithRef is set. Every other field +// next to a $ref is not an error: every version's Reference Object says "any +// properties added SHALL be ignored", from 3.1 on summary and description +// override the referenced component's, and an OpenAPI 3.1 schema applies its +// sibling keywords alongside the referenced schema. func (x *ResponseRef) validateExtras(ctx context.Context) error { validationOpts := getValidationOptions(ctx) - var extras []string - - allowed := validationOpts.extraSiblingFieldsAllowed - if allowed == nil { - allowed = make(map[string]struct{}) - } - if x.Ref == "" && !validationOpts.isOpenAPI31OrLater { - if _, ok := allowed["description"]; x.Description != nil && !ok { - extras = append(extras, "description") - } - if _, ok := allowed["summary"]; x.Summary != nil && !ok { - extras = append(extras, "summary") - } - } - // OpenAPI 3.0 Reference Objects ignore added properties. Keep the - // explicit extension policy below and validation of inline objects. - if x.Ref == "" || validationOpts.isOpenAPI31OrLater { - for _, ex := range x.extra { - if _, ok := allowed[ex]; !ok { - if _, ok := x.Extensions[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below - } - } + if x.Ref == "" || !validationOpts.schemaExtensionsInRefProhibited { + return nil } - if validationOpts.schemaExtensionsInRefProhibited { - for _, ex := range componentNames(x.Extensions) { - if _, ok := allowed[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below + var extras []string + for _, ex := range componentNames(x.Extensions) { + if _, ok := validationOpts.extraSiblingFieldsAllowed[ex]; !ok { + extras = append(extras, ex) } } - if len(extras) != 0 { return newExtraSiblingFields(extras, x.Origin) } @@ -1295,42 +1141,26 @@ func (x *SchemaRef) UnmarshalJSON(data []byte) error { return unmarshalJSON(data, &x.Value) } -// validateExtras returns an error if SchemaRef has sibling fields -// alongside $ref that are not allowed by the validation options. +// validateExtras returns an error if SchemaRef has `x-` extensions +// next to its $ref while ProhibitExtensionsWithRef is set. Every other field +// next to a $ref is not an error: every version's Reference Object says "any +// properties added SHALL be ignored", from 3.1 on summary and description +// override the referenced component's, and an OpenAPI 3.1 schema applies its +// sibling keywords alongside the referenced schema. func (x *SchemaRef) validateExtras(ctx context.Context) error { validationOpts := getValidationOptions(ctx) - var extras []string - - allowed := validationOpts.extraSiblingFieldsAllowed - if allowed == nil { - allowed = make(map[string]struct{}) - } - // OpenAPI 3.0 Reference Objects ignore added properties. Keep the - // explicit extension policy below and validation of inline objects. - if x.Ref == "" || validationOpts.isOpenAPI31OrLater { - for _, ex := range x.extra { - if _, ok := allowed[ex]; !ok { - if _, ok := x.Extensions[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below - } - } + if x.Ref == "" || !validationOpts.schemaExtensionsInRefProhibited { + return nil } - if validationOpts.schemaExtensionsInRefProhibited { - for _, ex := range componentNames(x.Extensions) { - if _, ok := allowed[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below + var extras []string + for _, ex := range componentNames(x.Extensions) { + if _, ok := validationOpts.extraSiblingFieldsAllowed[ex]; !ok { + extras = append(extras, ex) } } - if len(extras) != 0 { - if !validationOpts.isOpenAPI31OrLater { - return newExtraSiblingFields(extras, x.Origin) - } + return newExtraSiblingFields(extras, x.Origin) } return nil } @@ -1458,46 +1288,24 @@ func (x *SecuritySchemeRef) UnmarshalJSON(data []byte) error { return unmarshalJSON(data, &x.Value) } -// validateExtras returns an error if SecuritySchemeRef has sibling fields -// alongside $ref that are not allowed by the validation options. +// validateExtras returns an error if SecuritySchemeRef has `x-` extensions +// next to its $ref while ProhibitExtensionsWithRef is set. Every other field +// next to a $ref is not an error: every version's Reference Object says "any +// properties added SHALL be ignored", from 3.1 on summary and description +// override the referenced component's, and an OpenAPI 3.1 schema applies its +// sibling keywords alongside the referenced schema. func (x *SecuritySchemeRef) validateExtras(ctx context.Context) error { validationOpts := getValidationOptions(ctx) - var extras []string - - allowed := validationOpts.extraSiblingFieldsAllowed - if allowed == nil { - allowed = make(map[string]struct{}) - } - if x.Ref == "" && !validationOpts.isOpenAPI31OrLater { - if _, ok := allowed["description"]; x.Description != nil && !ok { - extras = append(extras, "description") - } - if _, ok := allowed["summary"]; x.Summary != nil && !ok { - extras = append(extras, "summary") - } - } - // OpenAPI 3.0 Reference Objects ignore added properties. Keep the - // explicit extension policy below and validation of inline objects. - if x.Ref == "" || validationOpts.isOpenAPI31OrLater { - for _, ex := range x.extra { - if _, ok := allowed[ex]; !ok { - if _, ok := x.Extensions[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below - } - } + if x.Ref == "" || !validationOpts.schemaExtensionsInRefProhibited { + return nil } - if validationOpts.schemaExtensionsInRefProhibited { - for _, ex := range componentNames(x.Extensions) { - if _, ok := allowed[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below + var extras []string + for _, ex := range componentNames(x.Extensions) { + if _, ok := validationOpts.extraSiblingFieldsAllowed[ex]; !ok { + extras = append(extras, ex) } } - if len(extras) != 0 { return newExtraSiblingFields(extras, x.Origin) } diff --git a/openapi3/refs.tmpl b/openapi3/refs.tmpl index e9b3aa642..e02a67591 100644 --- a/openapi3/refs.tmpl +++ b/openapi3/refs.tmpl @@ -132,56 +132,26 @@ func (x *{{ $type.Name }}Ref) UnmarshalJSON(data []byte) error { return unmarshalJSON(data, &x.Value) } -// validateExtras returns an error if {{ $type.Name }}Ref has sibling fields -// alongside $ref that are not allowed by the validation options. +// validateExtras returns an error if {{ $type.Name }}Ref has `x-` extensions +// next to its $ref while ProhibitExtensionsWithRef is set. Every other field +// next to a $ref is not an error: every version's Reference Object says "any +// properties added SHALL be ignored", from 3.1 on summary and description +// override the referenced component's, and an OpenAPI 3.1 schema applies its +// sibling keywords alongside the referenced schema. func (x *{{ $type.Name }}Ref) validateExtras(ctx context.Context) error { validationOpts := getValidationOptions(ctx) - var extras []string - - allowed := validationOpts.extraSiblingFieldsAllowed - if allowed == nil { - allowed = make(map[string]struct{}) - } - {{- if ne $type.Name "Schema" }} - if x.Ref == "" && !validationOpts.isOpenAPI31OrLater { - if _, ok := allowed["description"]; x.Description != nil && !ok { - extras = append(extras, "description") - } - if _, ok := allowed["summary"]; x.Summary != nil && !ok { - extras = append(extras, "summary") - } - } - {{- end }} - // OpenAPI 3.0 Reference Objects ignore added properties. Keep the - // explicit extension policy below and validation of inline objects. - if x.Ref == "" || validationOpts.isOpenAPI31OrLater { - for _, ex := range x.extra { - if _, ok := allowed[ex]; !ok { - if _, ok := x.Extensions[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below - } - } + if x.Ref == "" || !validationOpts.schemaExtensionsInRefProhibited { + return nil } - if validationOpts.schemaExtensionsInRefProhibited { - for _, ex := range componentNames(x.Extensions) { - if _, ok := allowed[ex]; !ok { - extras = append(extras, ex) - } - // extras in the Extensions checked below + var extras []string + for _, ex := range componentNames(x.Extensions) { + if _, ok := validationOpts.extraSiblingFieldsAllowed[ex]; !ok { + extras = append(extras, ex) } } - if len(extras) != 0 { -{{- if eq $type.Name "Schema" }} - if !validationOpts.isOpenAPI31OrLater { - return newExtraSiblingFields(extras, x.Origin) - } -{{- else }} return newExtraSiblingFields(extras, x.Origin) -{{- end }} } return nil } diff --git a/openapi3/refs_test.go b/openapi3/refs_test.go index 2dbf6aa4c..749195504 100644 --- a/openapi3/refs_test.go +++ b/openapi3/refs_test.go @@ -41,10 +41,10 @@ func TestCallbackRef_Extensions(t *testing.T) { err = ref.Validate(t.Context(), openapi3.ProhibitExtensionsWithRef(), openapi3.AllowExtraSiblingFields("x-order")) require.ErrorContains(t, err, "found unresolved ref") - // OpenAPI 3.1 Reference Objects still reject unknown siblings; Schema + // OpenAPI 3.1 Reference Objects ignore unknown siblings too; Schema // Objects allow sibling keywords under JSON Schema semantics. err = ref.Validate(t.Context(), openapi3.IsOpenAPI31OrLater()) - require.EqualError(t, err, "extra sibling fields: [something]") + require.ErrorContains(t, err, "found unresolved ref") // Verify round trip JSON // Compare as string to make error message more readable if different @@ -98,10 +98,10 @@ func TestExampleRef_Extensions(t *testing.T) { err = ref.Validate(t.Context(), openapi3.ProhibitExtensionsWithRef(), openapi3.AllowExtraSiblingFields("x-order")) require.ErrorContains(t, err, "found unresolved ref") - // OpenAPI 3.1 Reference Objects still reject unknown siblings; Schema + // OpenAPI 3.1 Reference Objects ignore unknown siblings too; Schema // Objects allow sibling keywords under JSON Schema semantics. err = ref.Validate(t.Context(), openapi3.IsOpenAPI31OrLater()) - require.EqualError(t, err, "extra sibling fields: [something]") + require.ErrorContains(t, err, "found unresolved ref") // Verify round trip JSON // Compare as string to make error message more readable if different @@ -155,10 +155,10 @@ func TestHeaderRef_Extensions(t *testing.T) { err = ref.Validate(t.Context(), openapi3.ProhibitExtensionsWithRef(), openapi3.AllowExtraSiblingFields("x-order")) require.ErrorContains(t, err, "found unresolved ref") - // OpenAPI 3.1 Reference Objects still reject unknown siblings; Schema + // OpenAPI 3.1 Reference Objects ignore unknown siblings too; Schema // Objects allow sibling keywords under JSON Schema semantics. err = ref.Validate(t.Context(), openapi3.IsOpenAPI31OrLater()) - require.EqualError(t, err, "extra sibling fields: [something]") + require.ErrorContains(t, err, "found unresolved ref") // Verify round trip JSON // Compare as string to make error message more readable if different @@ -203,10 +203,10 @@ func TestLinkRef_Extensions(t *testing.T) { err = ref.Validate(t.Context(), openapi3.ProhibitExtensionsWithRef(), openapi3.AllowExtraSiblingFields("x-order")) require.ErrorContains(t, err, "found unresolved ref") - // OpenAPI 3.1 Reference Objects still reject unknown siblings; Schema + // OpenAPI 3.1 Reference Objects ignore unknown siblings too; Schema // Objects allow sibling keywords under JSON Schema semantics. err = ref.Validate(t.Context(), openapi3.IsOpenAPI31OrLater()) - require.EqualError(t, err, "extra sibling fields: [something]") + require.ErrorContains(t, err, "found unresolved ref") // Verify round trip JSON // Compare as string to make error message more readable if different @@ -260,10 +260,10 @@ func TestParameterRef_Extensions(t *testing.T) { err = ref.Validate(t.Context(), openapi3.ProhibitExtensionsWithRef(), openapi3.AllowExtraSiblingFields("x-order")) require.ErrorContains(t, err, "found unresolved ref") - // OpenAPI 3.1 Reference Objects still reject unknown siblings; Schema + // OpenAPI 3.1 Reference Objects ignore unknown siblings too; Schema // Objects allow sibling keywords under JSON Schema semantics. err = ref.Validate(t.Context(), openapi3.IsOpenAPI31OrLater()) - require.EqualError(t, err, "extra sibling fields: [something]") + require.ErrorContains(t, err, "found unresolved ref") // Verify round trip JSON // Compare as string to make error message more readable if different @@ -317,10 +317,10 @@ func TestRequestBodyRef_Extensions(t *testing.T) { err = ref.Validate(t.Context(), openapi3.ProhibitExtensionsWithRef(), openapi3.AllowExtraSiblingFields("x-order")) require.ErrorContains(t, err, "found unresolved ref") - // OpenAPI 3.1 Reference Objects still reject unknown siblings; Schema + // OpenAPI 3.1 Reference Objects ignore unknown siblings too; Schema // Objects allow sibling keywords under JSON Schema semantics. err = ref.Validate(t.Context(), openapi3.IsOpenAPI31OrLater()) - require.EqualError(t, err, "extra sibling fields: [something]") + require.ErrorContains(t, err, "found unresolved ref") // Verify round trip JSON // Compare as string to make error message more readable if different @@ -374,10 +374,10 @@ func TestResponseRef_Extensions(t *testing.T) { err = ref.Validate(t.Context(), openapi3.ProhibitExtensionsWithRef(), openapi3.AllowExtraSiblingFields("x-order")) require.ErrorContains(t, err, "found unresolved ref") - // OpenAPI 3.1 Reference Objects still reject unknown siblings; Schema + // OpenAPI 3.1 Reference Objects ignore unknown siblings too; Schema // Objects allow sibling keywords under JSON Schema semantics. err = ref.Validate(t.Context(), openapi3.IsOpenAPI31OrLater()) - require.EqualError(t, err, "extra sibling fields: [something]") + require.ErrorContains(t, err, "found unresolved ref") // Verify round trip JSON // Compare as string to make error message more readable if different @@ -431,7 +431,7 @@ func TestSchemaRef_Extensions(t *testing.T) { err = ref.Validate(t.Context(), openapi3.ProhibitExtensionsWithRef(), openapi3.AllowExtraSiblingFields("x-order")) require.ErrorContains(t, err, "found unresolved ref") - // OpenAPI 3.1 Reference Objects still reject unknown siblings; Schema + // OpenAPI 3.1 Reference Objects ignore unknown siblings too; Schema // Objects allow sibling keywords under JSON Schema semantics. err = ref.Validate(t.Context(), openapi3.IsOpenAPI31OrLater()) require.ErrorContains(t, err, "found unresolved ref") @@ -488,10 +488,10 @@ func TestSecuritySchemeRef_Extensions(t *testing.T) { err = ref.Validate(t.Context(), openapi3.ProhibitExtensionsWithRef(), openapi3.AllowExtraSiblingFields("x-order")) require.ErrorContains(t, err, "found unresolved ref") - // OpenAPI 3.1 Reference Objects still reject unknown siblings; Schema + // OpenAPI 3.1 Reference Objects ignore unknown siblings too; Schema // Objects allow sibling keywords under JSON Schema semantics. err = ref.Validate(t.Context(), openapi3.IsOpenAPI31OrLater()) - require.EqualError(t, err, "extra sibling fields: [something]") + require.ErrorContains(t, err, "found unresolved ref") // Verify round trip JSON // Compare as string to make error message more readable if different diff --git a/openapi3/refs_test.tmpl b/openapi3/refs_test.tmpl index 083e52207..fd97c7b15 100644 --- a/openapi3/refs_test.tmpl +++ b/openapi3/refs_test.tmpl @@ -41,14 +41,10 @@ func Test{{ $type.Name }}Ref_Extensions(t *testing.T) { err = ref.Validate(t.Context(), openapi3.ProhibitExtensionsWithRef(), openapi3.AllowExtraSiblingFields("x-order")) require.ErrorContains(t, err, "found unresolved ref") - // OpenAPI 3.1 Reference Objects still reject unknown siblings; Schema + // OpenAPI 3.1 Reference Objects ignore unknown siblings too; Schema // Objects allow sibling keywords under JSON Schema semantics. err = ref.Validate(t.Context(), openapi3.IsOpenAPI31OrLater()) -{{- if eq $type.Name "Schema" }} require.ErrorContains(t, err, "found unresolved ref") -{{- else }} - require.EqualError(t, err, "extra sibling fields: [something]") -{{- end }} // Verify round trip JSON // Compare as string to make error message more readable if different diff --git a/openapi3/validation_options.go b/openapi3/validation_options.go index 6d20d4c6f..6bb4e723f 100644 --- a/openapi3/validation_options.go +++ b/openapi3/validation_options.go @@ -23,7 +23,9 @@ type ValidationOptions struct { type validationOptionsKey struct{} -// AllowExtraSiblingFields called as AllowExtraSiblingFields("description") makes Validate not return an error when said field appears next to a $ref. +// AllowExtraSiblingFields called as AllowExtraSiblingFields("description") makes Validate not return an error when said field appears +// where it is not expected: as an unknown field of an object, as an OpenAPI 3.1 schema keyword in an OpenAPI 3.0 document, +// or as an extension next to a $ref under ProhibitExtensionsWithRef. Other fields next to a $ref are never an error. func AllowExtraSiblingFields(fields ...string) ValidationOption { return func(options *ValidationOptions) { if options.extraSiblingFieldsAllowed == nil && len(fields) != 0 { @@ -126,7 +128,7 @@ func AllowExtensionsWithRef() ValidationOption { // ProhibitExtensionsWithRef causes the validation to return an // error if extensions (fields starting with 'x-') are found as -// siblings for $ref fields, even in OpenAPI 3.0. Use AllowExtraSiblingFields +// siblings for $ref fields, in every OpenAPI version. Use AllowExtraSiblingFields // to allow specific extensions. func ProhibitExtensionsWithRef() ValidationOption { return func(options *ValidationOptions) { From 0375a5d3d59461e735dea0caf5ca30e8268bd6cb Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Mon, 19 Oct 2026 21:22:25 +0200 Subject: [PATCH 02/21] openapi3: apply a 3.1 schema's $ref siblings alongside the referenced schema In JSON Schema 2020-12, on which OpenAPI 3.1 bases its Schema Object, the keywords next to a $ref apply alongside it: an instance must satisfy both the referenced schema and its siblings. They were instead copied over the referenced schema's keywords of the same name, so a sibling maxLength: 10 relaxed a referenced maxLength: 3, a sibling required list dropped the referenced one, and sibling properties replaced the referenced ones. Annotations, which describe rather than constrain, still override the referenced ones. Every other sibling keyword now goes into one more allOf entry of the copy the loader makes, so Value keeps the shape of the referenced schema. #1271 reaches the same validation results by making the siblings the Value and the referenced schema an allOf entry, which leaves Value without the type or properties of the referenced schema whenever a $ref has even just a description next to it. The $refs within the siblings are now resolved before they are applied, since applyRefSiblings also runs from the callback that ends a cycle. Signed-off-by: Pierre Fenoll --- README.md | 1 + openapi3/loader.go | 70 +++++++++++++-- openapi3/schema_ref_siblings_test.go | 129 +++++++++++++++++++++++++++ 3 files changed, 193 insertions(+), 7 deletions(-) create mode 100644 openapi3/schema_ref_siblings_test.go diff --git a/README.md b/README.md index 2f2da6e6d..4bc9252e3 100644 --- a/README.md +++ b/README.md @@ -338,6 +338,7 @@ for _, path := range doc.Paths.InMatchingOrder() { * `openapi3filter` now parses `integer` values of parameters and `application/x-www-form-urlencoded`/`multipart/form-data` bodies as base 10 only, so `010` decodes to `10` rather than `8`, and Go integer literals such as `0x10`, `0b10`, `0o10` or `1_000` are now rejected as invalid integers. * `openapi3filter.ValidateResponse` now decodes and validates response headers described with `content` rather than `schema`, with the body decoder registered for the header's media type (see `RegisterBodyDecoder`). Such a header is no longer reported missing when it is required and present, and a value that does not match its schema is now rejected. As with a response body, a media type with no registered decoder fails with `unsupported content type`. * `Validate` no longer reports `extra sibling fields` for fields written next to a `$ref`: every version's Reference Object says they "SHALL be ignored". From OpenAPI 3.1 on, `summary` and `description` still override the referenced component's. `ExtraSiblingFieldsError` is still returned for unknown fields of inline objects, and for `x-` extensions next to a `$ref` under `ProhibitExtensionsWithRef`, which now applies to OpenAPI 3.1+ schemas too. It is no longer returned after `InternalizeRefs` for the fields that were next to a component's `$ref` it replaced, and which it leaves out. +* In OpenAPI 3.1+ documents, the keywords next to a schema's `$ref` now apply alongside the referenced schema, as JSON Schema 2020-12 has it, instead of replacing its keywords of the same name: `{"$ref": ..., "maxLength": 10}` next to a referenced `maxLength: 3` still rejects `"abcd"`, and sibling `required` or `properties` add to the referenced ones. The resolved `SchemaRef.Value` keeps the referenced schema's shape: annotations such as `description`, `readOnly` or `default` replace the referenced ones there, and the other sibling keywords are appended to its `AllOf`. ### v0.150.0 * `openapi2.Operation.Responses` field type changed from `map[string]*Response` to `*Responses`, which keeps the Responses Object's `x-` extensions in `Responses.Extensions` instead of failing to decode them as responses. diff --git a/openapi3/loader.go b/openapi3/loader.go index fdcf6ed84..4d612abd4 100644 --- a/openapi3/loader.go +++ b/openapi3/loader.go @@ -1053,6 +1053,14 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat if component.Value != nil { return nil } + if doc.IsOpenAPI31OrLater() && component.sibling != nil { + // Resolve the $refs within the siblings themselves, such as those of + // the properties they add, before applyRefSiblings shares them with + // Value: it may run from the callback below, which resolves nothing. + if err := loader.resolveSchemaRef(doc, &SchemaRef{Value: component.sibling}, documentPath, visited); err != nil { + return err + } + } if !loader.shouldVisitRef(ref, func(value any) { component.Value = value.(*Schema) refPath, _ := loader.resolveRefPath(ref, documentPath) @@ -1530,15 +1538,63 @@ func (loader *Loader) resolvePathItemRef(doc *T, pathItem *PathItem, documentPat return } -// applyRefSiblings overlays the sibling keywords of an OAS 3.1 $ref onto its -// resolved value. It works on a copy so a schema shared by other references is -// not mutated, and does nothing for OAS 3.0, where $ref replaces its object. +// refSiblingAnnotations are the keywords that, written next to a $ref, replace +// the referenced schema's own. They describe the instance rather than constrain +// it, so the one closest to where it is used wins, as summary and description +// do on a Reference Object. +var refSiblingAnnotations = map[string]struct{}{ + "$comment": {}, + "allowEmptyValue": {}, + "default": {}, + "deprecated": {}, + "description": {}, + "discriminator": {}, + "example": {}, + "examples": {}, + "externalDocs": {}, + "nullable": {}, + "readOnly": {}, + "title": {}, + "writeOnly": {}, + "xml": {}, +} + +// applyRefSiblings applies the sibling keywords of an OAS 3.1 $ref to its +// resolved value, and does nothing for OAS 3.0, where $ref replaces its object. +// +// In JSON Schema 2020-12 the keywords next to a $ref apply alongside it: an +// instance must satisfy both the referenced schema and its siblings. So the +// annotations above override the referenced schema's, while every other +// keyword is added as one more allOf entry rather than replacing the +// referenced schema's own: a sibling maxLength: 10 next to a $ref to a schema +// with maxLength: 3 still rejects "abcd". Value thus keeps the shape of the +// referenced schema, with its properties, type and so on. +// +// It works on a copy so a schema shared by other references is not mutated. func (component *SchemaRef) applyRefSiblings(doc *T) { - if doc.IsOpenAPI31OrLater() && component.sibling != nil && component.Value != nil { - schemaCopy := *component.Value - applySiblingSchemaFields(&schemaCopy, component.sibling, component.extra) - component.Value = &schemaCopy + if !doc.IsOpenAPI31OrLater() || component.sibling == nil || component.Value == nil { + return + } + var annotations, assertions []string + for _, field := range component.extra { + if _, ok := refSiblingAnnotations[field]; ok { + annotations = append(annotations, field) + } else { + assertions = append(assertions, field) + } + } + schemaCopy := *component.Value + applySiblingSchemaFields(&schemaCopy, component.sibling, annotations) + if len(assertions) != 0 { + var conjunct Schema + applySiblingSchemaFields(&conjunct, component.sibling, assertions) + if !conjunct.IsEmpty() { + allOf := make(SchemaRefs, 0, len(schemaCopy.AllOf)+1) + allOf = append(allOf, schemaCopy.AllOf...) + schemaCopy.AllOf = append(allOf, &SchemaRef{Value: &conjunct}) + } } + component.Value = &schemaCopy } func unescapeRefString(ref string) string { diff --git a/openapi3/schema_ref_siblings_test.go b/openapi3/schema_ref_siblings_test.go new file mode 100644 index 000000000..5cc07e2ea --- /dev/null +++ b/openapi3/schema_ref_siblings_test.go @@ -0,0 +1,129 @@ +package openapi3_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" +) + +func loadSchemaRefSiblings(t *testing.T, version, schemas string) *openapi3.T { + t.Helper() + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData(fmt.Appendf(nil, `{ + "openapi": %q, + "info": {"title": "t", "version": "1"}, + "paths": {}, + "components": {"schemas": %s} +}`, version, schemas)) + require.NoError(t, err) + require.NoError(t, doc.Validate(t.Context())) + return doc +} + +// In OpenAPI 3.1 a Schema Object is a JSON Schema 2020-12 schema, in which the +// keywords next to $ref apply alongside it: a value must satisfy both. +func TestSchemaRefSiblingsApplyAlongsideTheRef(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + sibling string + valid []any + invalid []any + }{{ + name: "a looser bound does not relax the referenced one", + sibling: `"maxLength": 10`, + valid: []any{"abc"}, + invalid: []any{"abcd", "abcdefghijk"}, + }, { + name: "a tighter bound applies", + sibling: `"minLength": 2`, + valid: []any{"ab"}, + invalid: []any{"a", "abcd"}, + }, { + name: "a different type matches nothing", + sibling: `"type": "integer"`, + invalid: []any{"a", 1.0}, + }} { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + doc := loadSchemaRefSiblings(t, "3.1.0", `{ + "Base": {"type": "string", "maxLength": 3}, + "S": {"properties": {"p": {"$ref": "#/components/schemas/Base", `+tc.sibling+`}}} +}`) + value := doc.Components.Schemas["S"].Value.Properties["p"].Value + for _, v := range tc.valid { + require.NoError(t, value.VisitJSON(v), "%v", v) + } + for _, v := range tc.invalid { + require.Error(t, value.VisitJSON(v), "%v", v) + } + // The referenced schema keeps its own shape, and is not changed. + require.True(t, value.Type.Is("string")) + require.Equal(t, uint64(3), *value.MaxLength) + require.Len(t, doc.Components.Schemas["Base"].Value.AllOf, 0) + }) + } + + t.Run("properties and required add to the referenced ones", func(t *testing.T) { + t.Parallel() + doc := loadSchemaRefSiblings(t, "3.1.0", `{ + "Base": {"type": "object", "properties": {"a": {"type": "string"}}, "required": ["a"]}, + "S": {"properties": {"p": { + "$ref": "#/components/schemas/Base", + "properties": {"b": {"$ref": "#/components/schemas/Int"}}, + "required": ["b"] + }}}, + "Int": {"type": "integer"} +}`) + value := doc.Components.Schemas["S"].Value.Properties["p"].Value + require.NoError(t, value.VisitJSON(map[string]any{"a": "x", "b": 1.0})) + require.Error(t, value.VisitJSON(map[string]any{"a": "x"})) + require.Error(t, value.VisitJSON(map[string]any{"b": 1.0})) + require.Error(t, value.VisitJSON(map[string]any{"a": "x", "b": "y"})) + require.Contains(t, value.Properties, "a") + }) + + t.Run("annotations override the referenced ones", func(t *testing.T) { + t.Parallel() + doc := loadSchemaRefSiblings(t, "3.1.0", `{ + "Base": {"type": "object", "description": "base", "properties": {"a": {"type": "string"}}}, + "S": {"properties": {"p": {"$ref": "#/components/schemas/Base", "description": "local", "readOnly": true}}} +}`) + value := doc.Components.Schemas["S"].Value.Properties["p"].Value + require.Equal(t, "local", value.Description) + require.True(t, value.ReadOnly) + require.Contains(t, value.Properties, "a") + require.Empty(t, value.AllOf) + require.Equal(t, "base", doc.Components.Schemas["Base"].Value.Description) + }) + + t.Run("through a cycle", func(t *testing.T) { + t.Parallel() + doc := loadSchemaRefSiblings(t, "3.1.0", `{ + "Author": {"type": "object", "properties": {"name": {"type": "string"}, "book": {"$ref": "#/components/schemas/Book", "required": ["title"]}}}, + "Book": {"type": "object", "properties": {"title": {"type": "string"}, "author": {"$ref": "#/components/schemas/Author", "required": ["name"]}}} +}`) + book := doc.Components.Schemas["Author"].Value.Properties["book"].Value + require.Error(t, book.VisitJSON(map[string]any{})) + require.NoError(t, book.VisitJSON(map[string]any{"title": "t"})) + author := doc.Components.Schemas["Book"].Value.Properties["author"].Value + require.Error(t, author.VisitJSON(map[string]any{})) + require.NoError(t, author.VisitJSON(map[string]any{"name": "n"})) + }) +} + +// In OpenAPI 3.0 a $ref is a Reference Object, whose added properties are +// ignored. +func TestSchemaRefSiblingsAreIgnoredIn30(t *testing.T) { + t.Parallel() + doc := loadSchemaRefSiblings(t, "3.0.3", `{ + "Base": {"type": "string", "maxLength": 3, "description": "base"}, + "S": {"properties": {"p": {"$ref": "#/components/schemas/Base", "maxLength": 1, "description": "local"}}} +}`) + value := doc.Components.Schemas["S"].Value.Properties["p"].Value + require.Same(t, doc.Components.Schemas["Base"].Value, value) + require.NoError(t, value.VisitJSON("abc")) +} From cb1464646ed473e9054be72c0a67101467e9fba8 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Thu, 15 Oct 2026 10:17:13 +0200 Subject: [PATCH 03/21] openapi3: marshal the fields written next to a $ref They were dropped. An OpenAPI 3.1 schema's sibling keywords, which constrain the referenced schema, were lost that way, and so was every other field next to a $ref: a document did not survive a round trip. Signed-off-by: Pierre Fenoll --- README.md | 1 + openapi3/ref_siblings_marshal_test.go | 121 +++++++++++ openapi3/refs.go | 190 +++++++++++++++--- openapi3/refs.tmpl | 22 +- openapi3/refs_test.go | 72 +++++-- openapi3/refs_test.tmpl | 8 +- ...egistry_1_3_2_Final_openapi_yaml__validate | 9 +- ...ocal_registry_2_4_x_openapi_yaml__validate | 30 ++- openapi3filter/validate_response.go | 2 +- 9 files changed, 391 insertions(+), 64 deletions(-) create mode 100644 openapi3/ref_siblings_marshal_test.go diff --git a/README.md b/README.md index 4bc9252e3..6e3e66764 100644 --- a/README.md +++ b/README.md @@ -339,6 +339,7 @@ for _, path := range doc.Paths.InMatchingOrder() { * `openapi3filter.ValidateResponse` now decodes and validates response headers described with `content` rather than `schema`, with the body decoder registered for the header's media type (see `RegisterBodyDecoder`). Such a header is no longer reported missing when it is required and present, and a value that does not match its schema is now rejected. As with a response body, a media type with no registered decoder fails with `unsupported content type`. * `Validate` no longer reports `extra sibling fields` for fields written next to a `$ref`: every version's Reference Object says they "SHALL be ignored". From OpenAPI 3.1 on, `summary` and `description` still override the referenced component's. `ExtraSiblingFieldsError` is still returned for unknown fields of inline objects, and for `x-` extensions next to a `$ref` under `ProhibitExtensionsWithRef`, which now applies to OpenAPI 3.1+ schemas too. It is no longer returned after `InternalizeRefs` for the fields that were next to a component's `$ref` it replaced, and which it leaves out. * In OpenAPI 3.1+ documents, the keywords next to a schema's `$ref` now apply alongside the referenced schema, as JSON Schema 2020-12 has it, instead of replacing its keywords of the same name: `{"$ref": ..., "maxLength": 10}` next to a referenced `maxLength: 3` still rejects `"abcd"`, and sibling `required` or `properties` add to the referenced ones. The resolved `SchemaRef.Value` keeps the referenced schema's shape: annotations such as `description`, `readOnly` or `default` replace the referenced ones there, and the other sibling keywords are appended to its `AllOf`. +* Marshaling an `openapi3.*Ref` with a `$ref` now writes back the fields that were written next to it, instead of emitting a lone `{"$ref": ...}`. An OpenAPI 3.1 schema's sibling keywords, which constrain the referenced schema, were lost that way, and so was every other field next to a `$ref`. ### v0.150.0 * `openapi2.Operation.Responses` field type changed from `map[string]*Response` to `*Responses`, which keeps the Responses Object's `x-` extensions in `Responses.Extensions` instead of failing to decode them as responses. diff --git a/openapi3/ref_siblings_marshal_test.go b/openapi3/ref_siblings_marshal_test.go new file mode 100644 index 000000000..7480d370b --- /dev/null +++ b/openapi3/ref_siblings_marshal_test.go @@ -0,0 +1,121 @@ +package openapi3_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" +) + +// TestRefSiblingsSurviveMarshal covers what a $ref carries next to it: it has +// to be written back out. +// +// For 3.1+ the loader overlays sibling keywords onto the referenced schema, +// which is what JSON Schema 2020-12 prescribes. Marshaling used to throw the +// siblings away and emit a bare {"$ref": ...}, so a document meant one thing +// before being written out and another after being read back, with nothing to +// report the change. +// +// The generated *Ref types now keep their non-extension siblings verbatim and +// copy them back over the $ref. They do not need to know the document version: +// the version decision was already made at load. +func TestRefSiblingsSurviveMarshal(t *testing.T) { + t.Parallel() + + t.Run("schema", func(t *testing.T) { + t.Parallel() + + const spec = ` +openapi: 3.1.0 +info: + title: schema sibling demo + version: "1" +paths: {} +components: + schemas: + Base: + type: string + Alias: + $ref: "#/components/schemas/Base" + description: what Alias means +` + + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData([]byte(spec)) + require.NoError(t, err) + + // Alias resolves to Base's type with the sibling description layered on + // top, and Base itself is left alone. + alias := doc.Components.Schemas["Alias"] + require.Equal(t, `#/components/schemas/Base`, alias.Ref) + require.Equal(t, &openapi3.Types{"string"}, alias.Value.Type) + require.Equal(t, "what Alias means", alias.Value.Description) + require.Empty(t, doc.Components.Schemas["Base"].Value.Description, + "the overlay must not leak into the referenced schema") + + once, err := doc.MarshalJSON() + require.NoError(t, err) + require.JSONEq(t, `{ + "openapi": "3.1.0", + "info": {"title": "schema sibling demo", "version": "1"}, + "paths": {}, + "components": { + "schemas": { + "Base": {"type": "string"}, + "Alias": { + "$ref": "#/components/schemas/Base", + "description": "what Alias means" + } + } + } + }`, string(once)) + + // So the meaning of Alias survives a save/load cycle. + loader = openapi3.NewLoader() + reloaded, err := loader.LoadFromData(once) + require.NoError(t, err) + require.Equal(t, "what Alias means", + reloaded.Components.Schemas["Alias"].Value.Description) + }) + + t.Run("a sibling that is not a schema keyword is kept too", func(t *testing.T) { + t.Parallel() + + // Preserving only recognized keywords would still lose data. This is + // the shape the fuzzer reduced to; see + // testdata/fuzz/FuzzLoadFromData/schema-ref-junk-sibling. + const spec = ` +openapi: 3.0.0 +info: + title: junk sibling + version: "1" +paths: {} +components: + schemas: + Base: + type: string + Alias: + $ref: "#/components/schemas/Base" + nonsense: 1 +` + + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData([]byte(spec)) + require.NoError(t, err) + + // A Reference Object ignores it. + before := doc.Validate(t.Context()) + require.NoError(t, before) + + once, err := doc.MarshalJSON() + require.NoError(t, err) + require.Contains(t, string(once), `"nonsense":1`) + + // The same document validates the same way after a round trip. + loader = openapi3.NewLoader() + reloaded, err := loader.LoadFromData(once) + require.NoError(t, err) + require.Equal(t, before, reloaded.Validate(t.Context())) + }) +} diff --git a/openapi3/refs.go b/openapi3/refs.go index 3f2c22706..83a1fb9b1 100644 --- a/openapi3/refs.go +++ b/openapi3/refs.go @@ -4,6 +4,7 @@ package openapi3 import ( "context" "encoding/json" + "maps" "net/url" "strings" @@ -24,6 +25,10 @@ type CallbackRef struct { Ref string Value *Callback extra []string + // siblingExtras holds, verbatim, the non-`x-` fields written next to $ref. + // Marshaling writes them back out, so a document survives a round trip: + // an OpenAPI 3.1 schema's sibling keywords constrain the referenced one. + siblingExtras map[string]any refPath *url.URL } @@ -54,12 +59,19 @@ func (x *CallbackRef) setRefPath(u *url.URL) { // MarshalYAML returns the YAML encoding of CallbackRef. func (x CallbackRef) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { - return &Ref{ + out, err := Ref{ Ref: ref, Extensions: x.Extensions, Summary: x.Summary, Description: x.Description, - }, nil + }.MarshalYAML() + if err != nil { + return nil, err + } + if m, ok := out.(map[string]any); ok { + maps.Copy(m, x.siblingExtras) + } + return out, nil } return x.Value.MarshalYAML() } @@ -88,8 +100,12 @@ func (x *CallbackRef) UnmarshalJSON(data []byte) error { delete(extra, "description") if len(extra) != 0 { x.extra = componentNames(extra) - for k := range extra { + for k, v := range extra { if !strings.HasPrefix(k, "x-") { + if x.siblingExtras == nil { + x.siblingExtras = make(map[string]any, len(extra)) + } + x.siblingExtras[k] = v delete(extra, k) } } @@ -171,6 +187,10 @@ type ExampleRef struct { Ref string Value *Example extra []string + // siblingExtras holds, verbatim, the non-`x-` fields written next to $ref. + // Marshaling writes them back out, so a document survives a round trip: + // an OpenAPI 3.1 schema's sibling keywords constrain the referenced one. + siblingExtras map[string]any refPath *url.URL } @@ -201,12 +221,19 @@ func (x *ExampleRef) setRefPath(u *url.URL) { // MarshalYAML returns the YAML encoding of ExampleRef. func (x ExampleRef) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { - return &Ref{ + out, err := Ref{ Ref: ref, Extensions: x.Extensions, Summary: x.Summary, Description: x.Description, - }, nil + }.MarshalYAML() + if err != nil { + return nil, err + } + if m, ok := out.(map[string]any); ok { + maps.Copy(m, x.siblingExtras) + } + return out, nil } return x.Value.MarshalYAML() } @@ -235,8 +262,12 @@ func (x *ExampleRef) UnmarshalJSON(data []byte) error { delete(extra, "description") if len(extra) != 0 { x.extra = componentNames(extra) - for k := range extra { + for k, v := range extra { if !strings.HasPrefix(k, "x-") { + if x.siblingExtras == nil { + x.siblingExtras = make(map[string]any, len(extra)) + } + x.siblingExtras[k] = v delete(extra, k) } } @@ -318,6 +349,10 @@ type HeaderRef struct { Ref string Value *Header extra []string + // siblingExtras holds, verbatim, the non-`x-` fields written next to $ref. + // Marshaling writes them back out, so a document survives a round trip: + // an OpenAPI 3.1 schema's sibling keywords constrain the referenced one. + siblingExtras map[string]any refPath *url.URL } @@ -348,12 +383,19 @@ func (x *HeaderRef) setRefPath(u *url.URL) { // MarshalYAML returns the YAML encoding of HeaderRef. func (x HeaderRef) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { - return &Ref{ + out, err := Ref{ Ref: ref, Extensions: x.Extensions, Summary: x.Summary, Description: x.Description, - }, nil + }.MarshalYAML() + if err != nil { + return nil, err + } + if m, ok := out.(map[string]any); ok { + maps.Copy(m, x.siblingExtras) + } + return out, nil } return x.Value.MarshalYAML() } @@ -382,8 +424,12 @@ func (x *HeaderRef) UnmarshalJSON(data []byte) error { delete(extra, "description") if len(extra) != 0 { x.extra = componentNames(extra) - for k := range extra { + for k, v := range extra { if !strings.HasPrefix(k, "x-") { + if x.siblingExtras == nil { + x.siblingExtras = make(map[string]any, len(extra)) + } + x.siblingExtras[k] = v delete(extra, k) } } @@ -465,6 +511,10 @@ type LinkRef struct { Ref string Value *Link extra []string + // siblingExtras holds, verbatim, the non-`x-` fields written next to $ref. + // Marshaling writes them back out, so a document survives a round trip: + // an OpenAPI 3.1 schema's sibling keywords constrain the referenced one. + siblingExtras map[string]any refPath *url.URL } @@ -495,12 +545,19 @@ func (x *LinkRef) setRefPath(u *url.URL) { // MarshalYAML returns the YAML encoding of LinkRef. func (x LinkRef) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { - return &Ref{ + out, err := Ref{ Ref: ref, Extensions: x.Extensions, Summary: x.Summary, Description: x.Description, - }, nil + }.MarshalYAML() + if err != nil { + return nil, err + } + if m, ok := out.(map[string]any); ok { + maps.Copy(m, x.siblingExtras) + } + return out, nil } return x.Value.MarshalYAML() } @@ -529,8 +586,12 @@ func (x *LinkRef) UnmarshalJSON(data []byte) error { delete(extra, "description") if len(extra) != 0 { x.extra = componentNames(extra) - for k := range extra { + for k, v := range extra { if !strings.HasPrefix(k, "x-") { + if x.siblingExtras == nil { + x.siblingExtras = make(map[string]any, len(extra)) + } + x.siblingExtras[k] = v delete(extra, k) } } @@ -612,6 +673,10 @@ type ParameterRef struct { Ref string Value *Parameter extra []string + // siblingExtras holds, verbatim, the non-`x-` fields written next to $ref. + // Marshaling writes them back out, so a document survives a round trip: + // an OpenAPI 3.1 schema's sibling keywords constrain the referenced one. + siblingExtras map[string]any refPath *url.URL } @@ -642,12 +707,19 @@ func (x *ParameterRef) setRefPath(u *url.URL) { // MarshalYAML returns the YAML encoding of ParameterRef. func (x ParameterRef) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { - return &Ref{ + out, err := Ref{ Ref: ref, Extensions: x.Extensions, Summary: x.Summary, Description: x.Description, - }, nil + }.MarshalYAML() + if err != nil { + return nil, err + } + if m, ok := out.(map[string]any); ok { + maps.Copy(m, x.siblingExtras) + } + return out, nil } return x.Value.MarshalYAML() } @@ -676,8 +748,12 @@ func (x *ParameterRef) UnmarshalJSON(data []byte) error { delete(extra, "description") if len(extra) != 0 { x.extra = componentNames(extra) - for k := range extra { + for k, v := range extra { if !strings.HasPrefix(k, "x-") { + if x.siblingExtras == nil { + x.siblingExtras = make(map[string]any, len(extra)) + } + x.siblingExtras[k] = v delete(extra, k) } } @@ -759,6 +835,10 @@ type RequestBodyRef struct { Ref string Value *RequestBody extra []string + // siblingExtras holds, verbatim, the non-`x-` fields written next to $ref. + // Marshaling writes them back out, so a document survives a round trip: + // an OpenAPI 3.1 schema's sibling keywords constrain the referenced one. + siblingExtras map[string]any refPath *url.URL } @@ -789,12 +869,19 @@ func (x *RequestBodyRef) setRefPath(u *url.URL) { // MarshalYAML returns the YAML encoding of RequestBodyRef. func (x RequestBodyRef) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { - return &Ref{ + out, err := Ref{ Ref: ref, Extensions: x.Extensions, Summary: x.Summary, Description: x.Description, - }, nil + }.MarshalYAML() + if err != nil { + return nil, err + } + if m, ok := out.(map[string]any); ok { + maps.Copy(m, x.siblingExtras) + } + return out, nil } return x.Value.MarshalYAML() } @@ -823,8 +910,12 @@ func (x *RequestBodyRef) UnmarshalJSON(data []byte) error { delete(extra, "description") if len(extra) != 0 { x.extra = componentNames(extra) - for k := range extra { + for k, v := range extra { if !strings.HasPrefix(k, "x-") { + if x.siblingExtras == nil { + x.siblingExtras = make(map[string]any, len(extra)) + } + x.siblingExtras[k] = v delete(extra, k) } } @@ -906,6 +997,10 @@ type ResponseRef struct { Ref string Value *Response extra []string + // siblingExtras holds, verbatim, the non-`x-` fields written next to $ref. + // Marshaling writes them back out, so a document survives a round trip: + // an OpenAPI 3.1 schema's sibling keywords constrain the referenced one. + siblingExtras map[string]any refPath *url.URL } @@ -936,12 +1031,19 @@ func (x *ResponseRef) setRefPath(u *url.URL) { // MarshalYAML returns the YAML encoding of ResponseRef. func (x ResponseRef) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { - return &Ref{ + out, err := Ref{ Ref: ref, Extensions: x.Extensions, Summary: x.Summary, Description: x.Description, - }, nil + }.MarshalYAML() + if err != nil { + return nil, err + } + if m, ok := out.(map[string]any); ok { + maps.Copy(m, x.siblingExtras) + } + return out, nil } return x.Value.MarshalYAML() } @@ -970,8 +1072,12 @@ func (x *ResponseRef) UnmarshalJSON(data []byte) error { delete(extra, "description") if len(extra) != 0 { x.extra = componentNames(extra) - for k := range extra { + for k, v := range extra { if !strings.HasPrefix(k, "x-") { + if x.siblingExtras == nil { + x.siblingExtras = make(map[string]any, len(extra)) + } + x.siblingExtras[k] = v delete(extra, k) } } @@ -1050,6 +1156,10 @@ type SchemaRef struct { Ref string Value *Schema extra []string + // siblingExtras holds, verbatim, the non-`x-` fields written next to $ref. + // Marshaling writes them back out, so a document survives a round trip: + // an OpenAPI 3.1 schema's sibling keywords constrain the referenced one. + siblingExtras map[string]any // sibling holds keyword siblings of a $ref (OAS 3.1 / JSON Schema 2020-12). // It is populated during unmarshal and applied to Value after $ref resolution. sibling *Schema @@ -1083,10 +1193,17 @@ func (x *SchemaRef) setRefPath(u *url.URL) { // MarshalYAML returns the YAML encoding of SchemaRef. func (x SchemaRef) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { - return &Ref{ + out, err := Ref{ Ref: ref, Extensions: x.Extensions, - }, nil + }.MarshalYAML() + if err != nil { + return nil, err + } + if m, ok := out.(map[string]any); ok { + maps.Copy(m, x.siblingExtras) + } + return out, nil } return x.Value.MarshalYAML() } @@ -1127,8 +1244,12 @@ func (x *SchemaRef) UnmarshalJSON(data []byte) error { x.sibling = &sibling } } - for k := range extra { + for k, v := range extra { if !strings.HasPrefix(k, "x-") { + if x.siblingExtras == nil { + x.siblingExtras = make(map[string]any, len(extra)) + } + x.siblingExtras[k] = v delete(extra, k) } } @@ -1210,6 +1331,10 @@ type SecuritySchemeRef struct { Ref string Value *SecurityScheme extra []string + // siblingExtras holds, verbatim, the non-`x-` fields written next to $ref. + // Marshaling writes them back out, so a document survives a round trip: + // an OpenAPI 3.1 schema's sibling keywords constrain the referenced one. + siblingExtras map[string]any refPath *url.URL } @@ -1240,12 +1365,19 @@ func (x *SecuritySchemeRef) setRefPath(u *url.URL) { // MarshalYAML returns the YAML encoding of SecuritySchemeRef. func (x SecuritySchemeRef) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { - return &Ref{ + out, err := Ref{ Ref: ref, Extensions: x.Extensions, Summary: x.Summary, Description: x.Description, - }, nil + }.MarshalYAML() + if err != nil { + return nil, err + } + if m, ok := out.(map[string]any); ok { + maps.Copy(m, x.siblingExtras) + } + return out, nil } return x.Value.MarshalYAML() } @@ -1274,8 +1406,12 @@ func (x *SecuritySchemeRef) UnmarshalJSON(data []byte) error { delete(extra, "description") if len(extra) != 0 { x.extra = componentNames(extra) - for k := range extra { + for k, v := range extra { if !strings.HasPrefix(k, "x-") { + if x.siblingExtras == nil { + x.siblingExtras = make(map[string]any, len(extra)) + } + x.siblingExtras[k] = v delete(extra, k) } } diff --git a/openapi3/refs.tmpl b/openapi3/refs.tmpl index e02a67591..a00db9614 100644 --- a/openapi3/refs.tmpl +++ b/openapi3/refs.tmpl @@ -4,6 +4,7 @@ package {{ .Package }} import ( "context" "encoding/json" + "maps" "net/url" "strings" @@ -27,6 +28,10 @@ type {{ $type.Name }}Ref struct { Ref string Value *{{ $type.Name }} extra []string + // siblingExtras holds, verbatim, the non-`x-` fields written next to $ref. + // Marshaling writes them back out, so a document survives a round trip: + // an OpenAPI 3.1 schema's sibling keywords constrain the referenced one. + siblingExtras map[string]any {{- if eq $type.Name "Schema" }} // sibling holds keyword siblings of a $ref (OAS 3.1 / JSON Schema 2020-12). // It is populated during unmarshal and applied to Value after $ref resolution. @@ -62,14 +67,21 @@ func (x *{{ $type.Name }}Ref) setRefPath(u *url.URL) { // MarshalYAML returns the YAML encoding of {{ $type.Name }}Ref. func (x {{ $type.Name }}Ref) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { - return &Ref{ + out, err := Ref{ Ref: ref, Extensions: x.Extensions, {{- if ne $type.Name "Schema" }} Summary: x.Summary, Description: x.Description, {{- end }} - }, nil + }.MarshalYAML() + if err != nil { + return nil, err + } + if m, ok := out.(map[string]any); ok { + maps.Copy(m, x.siblingExtras) + } + return out, nil } return x.Value.MarshalYAML() } @@ -118,8 +130,12 @@ func (x *{{ $type.Name }}Ref) UnmarshalJSON(data []byte) error { } } {{- end }} - for k := range extra { + for k, v := range extra { if !strings.HasPrefix(k, "x-") { + if x.siblingExtras == nil { + x.siblingExtras = make(map[string]any, len(extra)) + } + x.siblingExtras[k] = v delete(extra, k) } } diff --git a/openapi3/refs_test.go b/openapi3/refs_test.go index 749195504..44100a537 100644 --- a/openapi3/refs_test.go +++ b/openapi3/refs_test.go @@ -15,7 +15,11 @@ func TestCallbackRef_Extensions(t *testing.T) { t.Parallel() data := []byte(`{"$ref":"#/components/schemas/Pet","something":"integer","x-order":1}`) - expectMarshalJson := []byte(`{"$ref":"#/components/schemas/Pet","x-order":1}`) + // Marshaling is a round trip. `something` is not an extension and Validate + // ignores it below, but it is still written back out: a document that comes + // back missing a field it was written with means something different, and + // nothing would report the change. + expectMarshalJson := data ref := openapi3.CallbackRef{} err := json.Unmarshal(data, &ref) @@ -50,7 +54,7 @@ func TestCallbackRef_Extensions(t *testing.T) { // Compare as string to make error message more readable if different outJson, err := ref.MarshalJSON() assert.NoError(t, err) - assert.Equal(t, string(outJson), string(expectMarshalJson), "MarshalJSON output is not the same as input data") + assert.Equal(t, string(expectMarshalJson), string(outJson), "MarshalJSON output is not the same as input data") // non-extension not json lookable _, err = ref.JSONLookup("something") @@ -72,7 +76,11 @@ func TestExampleRef_Extensions(t *testing.T) { t.Parallel() data := []byte(`{"$ref":"#/components/schemas/Pet","something":"integer","x-order":1}`) - expectMarshalJson := []byte(`{"$ref":"#/components/schemas/Pet","x-order":1}`) + // Marshaling is a round trip. `something` is not an extension and Validate + // ignores it below, but it is still written back out: a document that comes + // back missing a field it was written with means something different, and + // nothing would report the change. + expectMarshalJson := data ref := openapi3.ExampleRef{} err := json.Unmarshal(data, &ref) @@ -107,7 +115,7 @@ func TestExampleRef_Extensions(t *testing.T) { // Compare as string to make error message more readable if different outJson, err := ref.MarshalJSON() assert.NoError(t, err) - assert.Equal(t, string(outJson), string(expectMarshalJson), "MarshalJSON output is not the same as input data") + assert.Equal(t, string(expectMarshalJson), string(outJson), "MarshalJSON output is not the same as input data") // non-extension not json lookable _, err = ref.JSONLookup("something") @@ -129,7 +137,11 @@ func TestHeaderRef_Extensions(t *testing.T) { t.Parallel() data := []byte(`{"$ref":"#/components/schemas/Pet","something":"integer","x-order":1}`) - expectMarshalJson := []byte(`{"$ref":"#/components/schemas/Pet","x-order":1}`) + // Marshaling is a round trip. `something` is not an extension and Validate + // ignores it below, but it is still written back out: a document that comes + // back missing a field it was written with means something different, and + // nothing would report the change. + expectMarshalJson := data ref := openapi3.HeaderRef{} err := json.Unmarshal(data, &ref) @@ -164,7 +176,7 @@ func TestHeaderRef_Extensions(t *testing.T) { // Compare as string to make error message more readable if different outJson, err := ref.MarshalJSON() assert.NoError(t, err) - assert.Equal(t, string(outJson), string(expectMarshalJson), "MarshalJSON output is not the same as input data") + assert.Equal(t, string(expectMarshalJson), string(outJson), "MarshalJSON output is not the same as input data") // non-extension not json lookable _, err = ref.JSONLookup("something") @@ -177,7 +189,11 @@ func TestLinkRef_Extensions(t *testing.T) { t.Parallel() data := []byte(`{"$ref":"#/components/schemas/Pet","something":"integer","x-order":1}`) - expectMarshalJson := []byte(`{"$ref":"#/components/schemas/Pet","x-order":1}`) + // Marshaling is a round trip. `something` is not an extension and Validate + // ignores it below, but it is still written back out: a document that comes + // back missing a field it was written with means something different, and + // nothing would report the change. + expectMarshalJson := data ref := openapi3.LinkRef{} err := json.Unmarshal(data, &ref) @@ -212,7 +228,7 @@ func TestLinkRef_Extensions(t *testing.T) { // Compare as string to make error message more readable if different outJson, err := ref.MarshalJSON() assert.NoError(t, err) - assert.Equal(t, string(outJson), string(expectMarshalJson), "MarshalJSON output is not the same as input data") + assert.Equal(t, string(expectMarshalJson), string(outJson), "MarshalJSON output is not the same as input data") // non-extension not json lookable _, err = ref.JSONLookup("something") @@ -234,7 +250,11 @@ func TestParameterRef_Extensions(t *testing.T) { t.Parallel() data := []byte(`{"$ref":"#/components/schemas/Pet","something":"integer","x-order":1}`) - expectMarshalJson := []byte(`{"$ref":"#/components/schemas/Pet","x-order":1}`) + // Marshaling is a round trip. `something` is not an extension and Validate + // ignores it below, but it is still written back out: a document that comes + // back missing a field it was written with means something different, and + // nothing would report the change. + expectMarshalJson := data ref := openapi3.ParameterRef{} err := json.Unmarshal(data, &ref) @@ -269,7 +289,7 @@ func TestParameterRef_Extensions(t *testing.T) { // Compare as string to make error message more readable if different outJson, err := ref.MarshalJSON() assert.NoError(t, err) - assert.Equal(t, string(outJson), string(expectMarshalJson), "MarshalJSON output is not the same as input data") + assert.Equal(t, string(expectMarshalJson), string(outJson), "MarshalJSON output is not the same as input data") // non-extension not json lookable _, err = ref.JSONLookup("something") @@ -291,7 +311,11 @@ func TestRequestBodyRef_Extensions(t *testing.T) { t.Parallel() data := []byte(`{"$ref":"#/components/schemas/Pet","something":"integer","x-order":1}`) - expectMarshalJson := []byte(`{"$ref":"#/components/schemas/Pet","x-order":1}`) + // Marshaling is a round trip. `something` is not an extension and Validate + // ignores it below, but it is still written back out: a document that comes + // back missing a field it was written with means something different, and + // nothing would report the change. + expectMarshalJson := data ref := openapi3.RequestBodyRef{} err := json.Unmarshal(data, &ref) @@ -326,7 +350,7 @@ func TestRequestBodyRef_Extensions(t *testing.T) { // Compare as string to make error message more readable if different outJson, err := ref.MarshalJSON() assert.NoError(t, err) - assert.Equal(t, string(outJson), string(expectMarshalJson), "MarshalJSON output is not the same as input data") + assert.Equal(t, string(expectMarshalJson), string(outJson), "MarshalJSON output is not the same as input data") // non-extension not json lookable _, err = ref.JSONLookup("something") @@ -348,7 +372,11 @@ func TestResponseRef_Extensions(t *testing.T) { t.Parallel() data := []byte(`{"$ref":"#/components/schemas/Pet","something":"integer","x-order":1}`) - expectMarshalJson := []byte(`{"$ref":"#/components/schemas/Pet","x-order":1}`) + // Marshaling is a round trip. `something` is not an extension and Validate + // ignores it below, but it is still written back out: a document that comes + // back missing a field it was written with means something different, and + // nothing would report the change. + expectMarshalJson := data ref := openapi3.ResponseRef{} err := json.Unmarshal(data, &ref) @@ -383,7 +411,7 @@ func TestResponseRef_Extensions(t *testing.T) { // Compare as string to make error message more readable if different outJson, err := ref.MarshalJSON() assert.NoError(t, err) - assert.Equal(t, string(outJson), string(expectMarshalJson), "MarshalJSON output is not the same as input data") + assert.Equal(t, string(expectMarshalJson), string(outJson), "MarshalJSON output is not the same as input data") // non-extension not json lookable _, err = ref.JSONLookup("something") @@ -405,7 +433,11 @@ func TestSchemaRef_Extensions(t *testing.T) { t.Parallel() data := []byte(`{"$ref":"#/components/schemas/Pet","something":"integer","x-order":1}`) - expectMarshalJson := []byte(`{"$ref":"#/components/schemas/Pet","x-order":1}`) + // Marshaling is a round trip. `something` is not an extension and Validate + // ignores it below, but it is still written back out: a document that comes + // back missing a field it was written with means something different, and + // nothing would report the change. + expectMarshalJson := data ref := openapi3.SchemaRef{} err := json.Unmarshal(data, &ref) @@ -440,7 +472,7 @@ func TestSchemaRef_Extensions(t *testing.T) { // Compare as string to make error message more readable if different outJson, err := ref.MarshalJSON() assert.NoError(t, err) - assert.Equal(t, string(outJson), string(expectMarshalJson), "MarshalJSON output is not the same as input data") + assert.Equal(t, string(expectMarshalJson), string(outJson), "MarshalJSON output is not the same as input data") // non-extension not json lookable _, err = ref.JSONLookup("something") @@ -462,7 +494,11 @@ func TestSecuritySchemeRef_Extensions(t *testing.T) { t.Parallel() data := []byte(`{"$ref":"#/components/schemas/Pet","something":"integer","x-order":1}`) - expectMarshalJson := []byte(`{"$ref":"#/components/schemas/Pet","x-order":1}`) + // Marshaling is a round trip. `something` is not an extension and Validate + // ignores it below, but it is still written back out: a document that comes + // back missing a field it was written with means something different, and + // nothing would report the change. + expectMarshalJson := data ref := openapi3.SecuritySchemeRef{} err := json.Unmarshal(data, &ref) @@ -497,7 +533,7 @@ func TestSecuritySchemeRef_Extensions(t *testing.T) { // Compare as string to make error message more readable if different outJson, err := ref.MarshalJSON() assert.NoError(t, err) - assert.Equal(t, string(outJson), string(expectMarshalJson), "MarshalJSON output is not the same as input data") + assert.Equal(t, string(expectMarshalJson), string(outJson), "MarshalJSON output is not the same as input data") // non-extension not json lookable _, err = ref.JSONLookup("something") diff --git a/openapi3/refs_test.tmpl b/openapi3/refs_test.tmpl index fd97c7b15..ae1b06788 100644 --- a/openapi3/refs_test.tmpl +++ b/openapi3/refs_test.tmpl @@ -15,7 +15,11 @@ func Test{{ $type.Name }}Ref_Extensions(t *testing.T) { t.Parallel() data := []byte(`{"$ref":"#/components/schemas/Pet","something":"integer","x-order":1}`) - expectMarshalJson := []byte(`{"$ref":"#/components/schemas/Pet","x-order":1}`) + // Marshaling is a round trip. `something` is not an extension and Validate + // ignores it below, but it is still written back out: a document that comes + // back missing a field it was written with means something different, and + // nothing would report the change. + expectMarshalJson := data ref := openapi3.{{ $type.Name }}Ref{} err := json.Unmarshal(data, &ref) @@ -50,7 +54,7 @@ func Test{{ $type.Name }}Ref_Extensions(t *testing.T) { // Compare as string to make error message more readable if different outJson, err := ref.MarshalJSON() assert.NoError(t, err) - assert.Equal(t, string(outJson), string(expectMarshalJson), "MarshalJSON output is not the same as input data") + assert.Equal(t, string(expectMarshalJson), string(outJson), "MarshalJSON output is not the same as input data") // non-extension not json lookable _, err = ref.JSONLookup("something") diff --git a/openapi3/testdata/apis_guru_openapi_directory/apicurio_local_registry_1_3_2_Final_openapi_yaml__validate b/openapi3/testdata/apis_guru_openapi_directory/apicurio_local_registry_1_3_2_Final_openapi_yaml__validate index 18c3ac135..16a559ed7 100644 --- a/openapi3/testdata/apis_guru_openapi_directory/apicurio_local_registry_1_3_2_Final_openapi_yaml__validate +++ b/openapi3/testdata/apis_guru_openapi_directory/apicurio_local_registry_1_3_2_Final_openapi_yaml__validate @@ -56,13 +56,16 @@ Schema: "type": "string" }, "properties": { - "$ref": "#/components/schemas/Properties" + "$ref": "#/components/schemas/Properties", + "description": "" }, "state": { - "$ref": "#/components/schemas/ArtifactState" + "$ref": "#/components/schemas/ArtifactState", + "description": "" }, "type": { - "$ref": "#/components/schemas/ArtifactType" + "$ref": "#/components/schemas/ArtifactType", + "description": "" }, "version": { "format": "int64", diff --git a/openapi3/testdata/apis_guru_openapi_directory/apicurio_local_registry_2_4_x_openapi_yaml__validate b/openapi3/testdata/apis_guru_openapi_directory/apicurio_local_registry_2_4_x_openapi_yaml__validate index 435f52950..a2d94bdd9 100644 --- a/openapi3/testdata/apis_guru_openapi_directory/apicurio_local_registry_2_4_x_openapi_yaml__validate +++ b/openapi3/testdata/apis_guru_openapi_directory/apicurio_local_registry_2_4_x_openapi_yaml__validate @@ -67,10 +67,12 @@ Schema: "type": "integer" }, "groupId": { - "$ref": "#/components/schemas/GroupId" + "$ref": "#/components/schemas/GroupId", + "description": "" }, "id": { - "$ref": "#/components/schemas/ArtifactId" + "$ref": "#/components/schemas/ArtifactId", + "description": "" }, "labels": { "items": { @@ -89,7 +91,8 @@ Schema: "type": "string" }, "properties": { - "$ref": "#/components/schemas/Properties" + "$ref": "#/components/schemas/Properties", + "description": "" }, "references": { "items": { @@ -98,10 +101,12 @@ Schema: "type": "array" }, "state": { - "$ref": "#/components/schemas/ArtifactState" + "$ref": "#/components/schemas/ArtifactState", + "description": "" }, "type": { - "$ref": "#/components/schemas/ArtifactType" + "$ref": "#/components/schemas/ArtifactType", + "description": "" }, "version": { "type": "string" @@ -208,10 +213,12 @@ Schema: "type": "integer" }, "groupId": { - "$ref": "#/components/schemas/GroupId" + "$ref": "#/components/schemas/GroupId", + "description": "" }, "id": { - "$ref": "#/components/schemas/ArtifactId" + "$ref": "#/components/schemas/ArtifactId", + "description": "" }, "labels": { "items": { @@ -230,7 +237,8 @@ Schema: "type": "string" }, "properties": { - "$ref": "#/components/schemas/Properties" + "$ref": "#/components/schemas/Properties", + "description": "" }, "references": { "items": { @@ -239,10 +247,12 @@ Schema: "type": "array" }, "state": { - "$ref": "#/components/schemas/ArtifactState" + "$ref": "#/components/schemas/ArtifactState", + "description": "" }, "type": { - "$ref": "#/components/schemas/ArtifactType" + "$ref": "#/components/schemas/ArtifactType", + "description": "" }, "version": { "type": "string" diff --git a/openapi3filter/validate_response.go b/openapi3filter/validate_response.go index 221d74ae5..eb8a30267 100644 --- a/openapi3filter/validate_response.go +++ b/openapi3filter/validate_response.go @@ -244,7 +244,7 @@ func decodeContentResponseHeader(headerName string, header *openapi3.Header, inp } schema = mt.Schema.Value _, value, err = decodeBody(strings.NewReader(values[0]), http.Header{headerCT: {mediaType}}, mt.Schema, nil) - return + return // Only one pass } return } From 11caa38d7b9067a9c91f2f79b32a1ea663ae6637 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Tue, 20 Oct 2026 00:52:25 +0200 Subject: [PATCH 04/21] openapi3: tell apart a $ref being resolved as different kinds of objects The loader records the $refs it is in the middle of resolving, to close cycles: a $ref met again is filled in with the value once it is resolved. It recorded them by $ref alone, so the same $ref written for a path item and for one of its responses handed the path item to the response, which panicked with "interface conversion: interface {} is *openapi3.PathItem, not *openapi3.Response". They are now recorded by kind of object and $ref. Signed-off-by: Pierre Fenoll --- README.md | 2 + openapi3/loader.go | 89 +++++++++++++++++-------------- openapi3/loader_ref_kinds_test.go | 26 +++++++++ 3 files changed, 77 insertions(+), 40 deletions(-) create mode 100644 openapi3/loader_ref_kinds_test.go diff --git a/README.md b/README.md index 6e3e66764..6a896668e 100644 --- a/README.md +++ b/README.md @@ -340,6 +340,8 @@ for _, path := range doc.Paths.InMatchingOrder() { * `Validate` no longer reports `extra sibling fields` for fields written next to a `$ref`: every version's Reference Object says they "SHALL be ignored". From OpenAPI 3.1 on, `summary` and `description` still override the referenced component's. `ExtraSiblingFieldsError` is still returned for unknown fields of inline objects, and for `x-` extensions next to a `$ref` under `ProhibitExtensionsWithRef`, which now applies to OpenAPI 3.1+ schemas too. It is no longer returned after `InternalizeRefs` for the fields that were next to a component's `$ref` it replaced, and which it leaves out. * In OpenAPI 3.1+ documents, the keywords next to a schema's `$ref` now apply alongside the referenced schema, as JSON Schema 2020-12 has it, instead of replacing its keywords of the same name: `{"$ref": ..., "maxLength": 10}` next to a referenced `maxLength: 3` still rejects `"abcd"`, and sibling `required` or `properties` add to the referenced ones. The resolved `SchemaRef.Value` keeps the referenced schema's shape: annotations such as `description`, `readOnly` or `default` replace the referenced ones there, and the other sibling keywords are appended to its `AllOf`. * Marshaling an `openapi3.*Ref` with a `$ref` now writes back the fields that were written next to it, instead of emitting a lone `{"$ref": ...}`. An OpenAPI 3.1 schema's sibling keywords, which constrain the referenced schema, were lost that way, and so was every other field next to a `$ref`. +* The loader now resolves a Path Item Object's `$ref` even when fields are written next to it, and merges the two: it used to skip the `$ref` then, silently dropping every operation of the referenced path item and never noticing a `$ref` to nothing. The specification leaves the result undefined only for a field set on both sides, which is then taken from next to the `$ref`. Marshaling writes back the `$ref` with the fields that were next to it, which it used to drop. +* The loader no longer panics with `interface conversion` when the same `$ref` is written where two kinds of objects are expected, one nested in the other, such as a path item and one of its responses. ### v0.150.0 * `openapi2.Operation.Responses` field type changed from `map[string]*Response` to `*Responses`, which keeps the Responses Object's `x-` extensions in `Responses.Extensions` instead of failing to decode them as responses. diff --git a/openapi3/loader.go b/openapi3/loader.go index 4d612abd4..86647b3d3 100644 --- a/openapi3/loader.go +++ b/openapi3/loader.go @@ -402,29 +402,38 @@ func isSingleRefElement(ref string) bool { return !strings.Contains(ref, "#") } -func (loader *Loader) visitRef(ref string) { +// visitedRefKey identifies a $ref being resolved as an object of the given +// kind. The same $ref can be written where different kinds of objects are +// expected, as in a path item and in one of its responses: resolving one must +// not hand its value over to the other. +func visitedRefKey(kind, ref string) string { return kind + " " + ref } + +func (loader *Loader) visitRef(kind, ref string) { if loader.visitedRefs == nil { loader.visitedRefs = make(map[string]struct{}) loader.backtrack = make(map[string][]func(value any)) } - loader.visitedPath = append(loader.visitedPath, ref) - loader.visitedRefs[ref] = struct{}{} + key := visitedRefKey(kind, ref) + loader.visitedPath = append(loader.visitedPath, key) + loader.visitedRefs[key] = struct{}{} } -func (loader *Loader) unvisitRef(ref string, value any) { +func (loader *Loader) unvisitRef(kind, ref string, value any) { + key := visitedRefKey(kind, ref) if value != nil { - for _, fn := range loader.backtrack[ref] { + for _, fn := range loader.backtrack[key] { fn(value) } } - delete(loader.visitedRefs, ref) - delete(loader.backtrack, ref) + delete(loader.visitedRefs, key) + delete(loader.backtrack, key) loader.visitedPath = loader.visitedPath[:len(loader.visitedPath)-1] } -func (loader *Loader) shouldVisitRef(ref string, fn func(value any)) bool { - if _, ok := loader.visitedRefs[ref]; ok { - loader.backtrack[ref] = append(loader.backtrack[ref], fn) +func (loader *Loader) shouldVisitRef(kind, ref string, fn func(value any)) bool { + key := visitedRefKey(kind, ref) + if _, ok := loader.visitedRefs[key]; ok { + loader.backtrack[key] = append(loader.backtrack[key], fn) return false } return true @@ -745,7 +754,7 @@ func (loader *Loader) resolveHeaderRef(doc *T, component *HeaderRef, documentPat if component.Value != nil { return nil } - if !loader.shouldVisitRef(ref, func(value any) { + if !loader.shouldVisitRef("Header", ref, func(value any) { component.Value = value.(*Header) applyHeaderRefMetadata(component.Value, component, isOpenAPI31OrLater) refPath, _ := loader.resolveRefPath(ref, documentPath) @@ -753,7 +762,7 @@ func (loader *Loader) resolveHeaderRef(doc *T, component *HeaderRef, documentPat }) { return nil } - loader.visitRef(ref) + loader.visitRef("Header", ref) if isSingleRefElement(ref) { var header Header if documentPath, err = loader.loadSingleElementFromURI(ref, documentPath, &header); err != nil { @@ -776,7 +785,7 @@ func (loader *Loader) resolveHeaderRef(doc *T, component *HeaderRef, documentPat component.Value = resolved.Value component.setRefPath(resolved.RefPath()) } - defer loader.unvisitRef(ref, component.Value) + defer loader.unvisitRef("Header", ref, component.Value) applyHeaderRefMetadata(component.Value, component, isOpenAPI31OrLater) } value := component.Value @@ -816,7 +825,7 @@ func (loader *Loader) resolveParameterRef(doc *T, component *ParameterRef, docum if component.Value != nil { return nil } - if !loader.shouldVisitRef(ref, func(value any) { + if !loader.shouldVisitRef("Parameter", ref, func(value any) { component.Value = value.(*Parameter) applyParameterRefMetadata(component.Value, component, isOpenAPI31OrLater) refPath, _ := loader.resolveRefPath(ref, documentPath) @@ -824,7 +833,7 @@ func (loader *Loader) resolveParameterRef(doc *T, component *ParameterRef, docum }) { return nil } - loader.visitRef(ref) + loader.visitRef("Parameter", ref) if isSingleRefElement(ref) { var param Parameter if documentPath, err = loader.loadSingleElementFromURI(ref, documentPath, ¶m); err != nil { @@ -847,7 +856,7 @@ func (loader *Loader) resolveParameterRef(doc *T, component *ParameterRef, docum component.Value = resolved.Value component.setRefPath(resolved.RefPath()) } - defer loader.unvisitRef(ref, component.Value) + defer loader.unvisitRef("Parameter", ref, component.Value) applyParameterRefMetadata(component.Value, component, isOpenAPI31OrLater) } value := component.Value @@ -895,7 +904,7 @@ func (loader *Loader) resolveRequestBodyRef(doc *T, component *RequestBodyRef, d if component.Value != nil { return nil } - if !loader.shouldVisitRef(ref, func(value any) { + if !loader.shouldVisitRef("RequestBody", ref, func(value any) { component.Value = value.(*RequestBody) applyRequestBodyRefMetadata(component.Value, component, isOpenAPI31OrLater) refPath, _ := loader.resolveRefPath(ref, documentPath) @@ -903,7 +912,7 @@ func (loader *Loader) resolveRequestBodyRef(doc *T, component *RequestBodyRef, d }) { return nil } - loader.visitRef(ref) + loader.visitRef("RequestBody", ref) if isSingleRefElement(ref) { var requestBody RequestBody if documentPath, err = loader.loadSingleElementFromURI(ref, documentPath, &requestBody); err != nil { @@ -926,7 +935,7 @@ func (loader *Loader) resolveRequestBodyRef(doc *T, component *RequestBodyRef, d component.Value = resolved.Value component.setRefPath(resolved.RefPath()) } - defer loader.unvisitRef(ref, component.Value) + defer loader.unvisitRef("RequestBody", ref, component.Value) applyRequestBodyRefMetadata(component.Value, component, isOpenAPI31OrLater) } value := component.Value @@ -961,7 +970,7 @@ func (loader *Loader) resolveResponseRef(doc *T, component *ResponseRef, documen if component.Value != nil { return nil } - if !loader.shouldVisitRef(ref, func(value any) { + if !loader.shouldVisitRef("Response", ref, func(value any) { component.Value = value.(*Response) applyResponseRefMetadata(component.Value, component, isOpenAPI31OrLater) refPath, _ := loader.resolveRefPath(ref, documentPath) @@ -969,7 +978,7 @@ func (loader *Loader) resolveResponseRef(doc *T, component *ResponseRef, documen }) { return nil } - loader.visitRef(ref) + loader.visitRef("Response", ref) if isSingleRefElement(ref) { var resp Response if documentPath, err = loader.loadSingleElementFromURI(ref, documentPath, &resp); err != nil { @@ -992,7 +1001,7 @@ func (loader *Loader) resolveResponseRef(doc *T, component *ResponseRef, documen component.Value = resolved.Value component.setRefPath(resolved.RefPath()) } - defer loader.unvisitRef(ref, component.Value) + defer loader.unvisitRef("Response", ref, component.Value) applyResponseRefMetadata(component.Value, component, isOpenAPI31OrLater) } value := component.Value @@ -1061,7 +1070,7 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat return err } } - if !loader.shouldVisitRef(ref, func(value any) { + if !loader.shouldVisitRef("Schema", ref, func(value any) { component.Value = value.(*Schema) refPath, _ := loader.resolveRefPath(ref, documentPath) component.setRefPath(refPath) @@ -1072,7 +1081,7 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat }) { return nil } - loader.visitRef(ref) + loader.visitRef("Schema", ref) if isSingleRefElement(ref) { var schema Schema if documentPath, err = loader.loadSingleElementFromURI(ref, documentPath, &schema); err != nil { @@ -1095,7 +1104,7 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat component.Value = resolved.Value component.setRefPath(resolved.RefPath()) } - defer loader.unvisitRef(ref, component.Value) + defer loader.unvisitRef("Schema", ref, component.Value) // OAS 3.1 / JSON Schema 2020-12: apply sibling keywords from the original schema // object on top of the resolved $ref value. In 3.1, siblings are not ignored — @@ -1257,7 +1266,7 @@ func (loader *Loader) resolveSecuritySchemeRef(doc *T, component *SecurityScheme if component.Value != nil { return nil } - if !loader.shouldVisitRef(ref, func(value any) { + if !loader.shouldVisitRef("SecurityScheme", ref, func(value any) { component.Value = value.(*SecurityScheme) applySecuritySchemeRefMetadata(component.Value, component, isOpenAPI31OrLater) refPath, _ := loader.resolveRefPath(ref, documentPath) @@ -1265,7 +1274,7 @@ func (loader *Loader) resolveSecuritySchemeRef(doc *T, component *SecurityScheme }) { return nil } - loader.visitRef(ref) + loader.visitRef("SecurityScheme", ref) if isSingleRefElement(ref) { var scheme SecurityScheme if _, err = loader.loadSingleElementFromURI(ref, documentPath, &scheme); err != nil { @@ -1288,7 +1297,7 @@ func (loader *Loader) resolveSecuritySchemeRef(doc *T, component *SecurityScheme component.Value = resolved.Value component.setRefPath(resolved.RefPath()) } - defer loader.unvisitRef(ref, component.Value) + defer loader.unvisitRef("SecurityScheme", ref, component.Value) applySecuritySchemeRefMetadata(component.Value, component, isOpenAPI31OrLater) } return nil @@ -1318,7 +1327,7 @@ func (loader *Loader) resolveExampleRef(doc *T, component *ExampleRef, documentP if component.Value != nil { return nil } - if !loader.shouldVisitRef(ref, func(value any) { + if !loader.shouldVisitRef("Example", ref, func(value any) { component.Value = value.(*Example) applyExampleRefMetadata(component.Value, component, isOpenAPI31OrLater) refPath, _ := loader.resolveRefPath(ref, documentPath) @@ -1326,7 +1335,7 @@ func (loader *Loader) resolveExampleRef(doc *T, component *ExampleRef, documentP }) { return nil } - loader.visitRef(ref) + loader.visitRef("Example", ref) if isSingleRefElement(ref) { var example Example if _, err = loader.loadSingleElementFromURI(ref, documentPath, &example); err != nil { @@ -1349,7 +1358,7 @@ func (loader *Loader) resolveExampleRef(doc *T, component *ExampleRef, documentP component.Value = resolved.Value component.setRefPath(resolved.RefPath()) } - defer loader.unvisitRef(ref, component.Value) + defer loader.unvisitRef("Example", ref, component.Value) applyExampleRefMetadata(component.Value, component, isOpenAPI31OrLater) } return nil @@ -1364,14 +1373,14 @@ func (loader *Loader) resolveCallbackRef(doc *T, component *CallbackRef, documen if component.Value != nil { return nil } - if !loader.shouldVisitRef(ref, func(value any) { + if !loader.shouldVisitRef("Callback", ref, func(value any) { component.Value = value.(*Callback) refPath, _ := loader.resolveRefPath(ref, documentPath) component.setRefPath(refPath) }) { return nil } - loader.visitRef(ref) + loader.visitRef("Callback", ref) if isSingleRefElement(ref) { var resolved Callback if documentPath, err = loader.loadSingleElementFromURI(ref, documentPath, &resolved); err != nil { @@ -1394,7 +1403,7 @@ func (loader *Loader) resolveCallbackRef(doc *T, component *CallbackRef, documen component.Value = resolved.Value component.setRefPath(resolved.RefPath()) } - defer loader.unvisitRef(ref, component.Value) + defer loader.unvisitRef("Callback", ref, component.Value) } value := component.Value if value == nil { @@ -1430,7 +1439,7 @@ func (loader *Loader) resolveLinkRef(doc *T, component *LinkRef, documentPath *u if component.Value != nil { return nil } - if !loader.shouldVisitRef(ref, func(value any) { + if !loader.shouldVisitRef("Link", ref, func(value any) { component.Value = value.(*Link) applyLinkRefMetadata(component.Value, component, isOpenAPI31OrLater) refPath, _ := loader.resolveRefPath(ref, documentPath) @@ -1438,7 +1447,7 @@ func (loader *Loader) resolveLinkRef(doc *T, component *LinkRef, documentPath *u }) { return nil } - loader.visitRef(ref) + loader.visitRef("Link", ref) if isSingleRefElement(ref) { var link Link if _, err = loader.loadSingleElementFromURI(ref, documentPath, &link); err != nil { @@ -1461,7 +1470,7 @@ func (loader *Loader) resolveLinkRef(doc *T, component *LinkRef, documentPath *u component.Value = resolved.Value component.setRefPath(resolved.RefPath()) } - defer loader.unvisitRef(ref, component.Value) + defer loader.unvisitRef("Link", ref, component.Value) applyLinkRefMetadata(component.Value, component, isOpenAPI31OrLater) } return nil @@ -1477,12 +1486,12 @@ func (loader *Loader) resolvePathItemRef(doc *T, pathItem *PathItem, documentPat if !pathItem.isEmpty() { return } - if !loader.shouldVisitRef(ref, func(value any) { + if !loader.shouldVisitRef("PathItem", ref, func(value any) { *pathItem = *value.(*PathItem) }) { return nil } - loader.visitRef(ref) + loader.visitRef("PathItem", ref) if isSingleRefElement(ref) { var p PathItem if documentPath, err = loader.loadSingleElementFromURI(ref, documentPath, &p); err != nil { @@ -1500,7 +1509,7 @@ func (loader *Loader) resolvePathItemRef(doc *T, pathItem *PathItem, documentPat *pathItem = resolved } pathItem.Ref = ref - defer loader.unvisitRef(ref, pathItem) + defer loader.unvisitRef("PathItem", ref, pathItem) } for _, parameter := range pathItem.Parameters { diff --git a/openapi3/loader_ref_kinds_test.go b/openapi3/loader_ref_kinds_test.go new file mode 100644 index 000000000..8c48ce728 --- /dev/null +++ b/openapi3/loader_ref_kinds_test.go @@ -0,0 +1,26 @@ +package openapi3_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" +) + +// The same $ref written where two kinds of objects are expected, one within +// the other, must not hand the value resolved for one to the other. +func TestSameRefForDifferentKindsOfObjects(t *testing.T) { + t.Parallel() + loader := openapi3.NewLoader() + require.NotPanics(t, func() { + _, _ = loader.LoadFromData([]byte(`{ + "openapi": "3.0.3", + "info": {"title": "t", "version": "1"}, + "paths": { + "/x": {"$ref": "#/paths/~1y"}, + "/y": {"get": {"responses": {"200": {"$ref": "#/paths/~1y"}}}} + } +}`)) + }) +} From c59eda0a25da1a208c408dabb2d37224fdd62632 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Mon, 19 Oct 2026 21:26:30 +0200 Subject: [PATCH 05/21] openapi3: merge the fields next to a path item's $ref with the item it points to A Path Item Object's $ref is no Reference Object: the fields written next to it belong to the same path item, and every version of the specification leaves the result undefined only "in case a Path Item Object field appears both in the defined object and the referenced object". Any such field made the loader skip the $ref, silently dropping all the operations of the referenced path item, and letting a $ref to nothing go unnoticed. Both are now merged; a field set on both sides is taken from next to the $ref, as a Reference Object's summary and description are, which is what OpenAPI 3.1 says this $ref is likely to be aligned with. Marshaling writes back the $ref with the fields that were next to it, instead of the $ref alone. Signed-off-by: Pierre Fenoll --- .github/docs/openapi3.txt | 2 + openapi3/loader.go | 26 +++++-- openapi3/path_item.go | 86 +++++++++++++++++++++- openapi3/path_item_ref_siblings_test.go | 96 +++++++++++++++++++++++++ 4 files changed, 204 insertions(+), 6 deletions(-) create mode 100644 openapi3/path_item_ref_siblings_test.go diff --git a/.github/docs/openapi3.txt b/.github/docs/openapi3.txt index 1c77fe090..a77c3e95e 100644 --- a/.github/docs/openapi3.txt +++ b/.github/docs/openapi3.txt @@ -2282,6 +2282,8 @@ type PathItem struct { // Keys are case-sensitive HTTP method tokens and must not duplicate // the methods covered by the fixed fields above. AdditionalOperations map[string]*Operation `json:"additionalOperations,omitempty,case:strict" yaml:"additionalOperations,omitempty"` // OpenAPI >=3.2 + + // Has unexported fields. } PathItem is specified by OpenAPI/Swagger standard version 3. See https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#path-item-object diff --git a/openapi3/loader.go b/openapi3/loader.go index 86647b3d3..72850311a 100644 --- a/openapi3/loader.go +++ b/openapi3/loader.go @@ -1483,11 +1483,28 @@ func (loader *Loader) resolvePathItemRef(doc *T, pathItem *PathItem, documentPat } if ref := pathItem.Ref; ref != "" { - if !pathItem.isEmpty() { + if pathItem.refResolved { return } + // The fields written next to $ref used to stop it from being resolved. + // They are merged with the item it points to instead; see mergeRefSiblings. + var siblings *PathItem + if !pathItem.isEmpty() || len(pathItem.Extensions) != 0 { + local := *pathItem + local.Ref = "" + siblings = &local + } + resolvedTo := func(resolved PathItem) { + *pathItem = resolved + pathItem.Ref = ref + pathItem.refResolved = true + if siblings != nil { + pathItem.mergeRefSiblings(siblings) + pathItem.refSiblings = siblings + } + } if !loader.shouldVisitRef("PathItem", ref, func(value any) { - *pathItem = *value.(*PathItem) + resolvedTo(*value.(*PathItem)) }) { return nil } @@ -1497,7 +1514,7 @@ func (loader *Loader) resolvePathItemRef(doc *T, pathItem *PathItem, documentPat if documentPath, err = loader.loadSingleElementFromURI(ref, documentPath, &p); err != nil { return } - *pathItem = p + resolvedTo(p) } else { var resolved PathItem if doc, documentPath, err = loader.resolveComponent(doc, ref, documentPath, &resolved); err != nil { @@ -1506,9 +1523,8 @@ func (loader *Loader) resolvePathItemRef(doc *T, pathItem *PathItem, documentPat } return } - *pathItem = resolved + resolvedTo(resolved) } - pathItem.Ref = ref defer loader.unvisitRef("PathItem", ref, pathItem) } diff --git a/openapi3/path_item.go b/openapi3/path_item.go index 2ce2f5233..2f1c6edcf 100644 --- a/openapi3/path_item.go +++ b/openapi3/path_item.go @@ -35,6 +35,14 @@ type PathItem struct { // Keys are case-sensitive HTTP method tokens and must not duplicate // the methods covered by the fixed fields above. AdditionalOperations map[string]*Operation `json:"additionalOperations,omitempty,case:strict" yaml:"additionalOperations,omitempty"` // OpenAPI >=3.2 + + // refSiblings holds the fields written next to Ref, as the document had + // them: once the loader has merged the referenced path item in, they can no + // longer be told apart from it, and marshaling writes back only these. + refSiblings *PathItem + // refResolved records that the loader has merged the referenced path item + // in, so that it does not do it twice. + refResolved bool } // MethodQuery is the HTTP QUERY method, introduced as a fixed Path Item @@ -80,7 +88,19 @@ func (pathItem PathItem) MarshalJSON() ([]byte, error) { // MarshalYAML returns the YAML encoding of PathItem. func (pathItem PathItem) MarshalYAML() (any, error) { if ref := pathItem.Ref; ref != "" { - return Ref{Ref: ref}, nil + if pathItem.refResolved { + if pathItem.refSiblings == nil { + return Ref{Ref: ref}, nil + } + pathItem = *pathItem.refSiblings + } + pathItem.Ref = "" + m, err := pathItem.MarshalYAML() + if err != nil { + return nil, err + } + m.(map[string]any)["$ref"] = ref + return m, nil } m := make(map[string]any, 15+len(pathItem.Extensions)) @@ -381,6 +401,70 @@ func (pathItem *PathItem) Validate(ctx context.Context, opts ...ValidationOption } // isEmpty's introduced in 546590b1 +// mergeRefSiblings sets on pathItem, a copy of the item its $ref points to, +// the fields that siblings, the item holding that $ref, sets. +// +// The specification only defines the result when no field is set on both: "In +// case a Path Item Object field appears both in the defined object and the +// referenced object, the behavior is undefined". The sibling then wins, like +// the summary and description of a Reference Object do over those of the +// object it points to, which is what OpenAPI 3.1 says this $ref is likely to +// be aligned with. It never modifies the maps of the referenced item, which +// other path items may share. +func (pathItem *PathItem) mergeRefSiblings(siblings *PathItem) { + if len(siblings.Extensions) != 0 { + extensions := make(map[string]any, len(pathItem.Extensions)+len(siblings.Extensions)) + maps.Copy(extensions, pathItem.Extensions) + maps.Copy(extensions, siblings.Extensions) + pathItem.Extensions = extensions + } + if x := siblings.Summary; x != "" { + pathItem.Summary = x + } + if x := siblings.Description; x != "" { + pathItem.Description = x + } + if x := siblings.Connect; x != nil { + pathItem.Connect = x + } + if x := siblings.Delete; x != nil { + pathItem.Delete = x + } + if x := siblings.Get; x != nil { + pathItem.Get = x + } + if x := siblings.Head; x != nil { + pathItem.Head = x + } + if x := siblings.Options; x != nil { + pathItem.Options = x + } + if x := siblings.Patch; x != nil { + pathItem.Patch = x + } + if x := siblings.Post; x != nil { + pathItem.Post = x + } + if x := siblings.Put; x != nil { + pathItem.Put = x + } + if x := siblings.Trace; x != nil { + pathItem.Trace = x + } + if x := siblings.Query; x != nil { + pathItem.Query = x + } + if x := siblings.AdditionalOperations; len(x) != 0 { + pathItem.AdditionalOperations = x + } + if x := siblings.Servers; len(x) != 0 { + pathItem.Servers = x + } + if x := siblings.Parameters; len(x) != 0 { + pathItem.Parameters = x + } +} + func (pathItem *PathItem) isEmpty() bool { // NOTE: ignores pathItem.Extensions // NOTE: ignores pathItem.Ref diff --git a/openapi3/path_item_ref_siblings_test.go b/openapi3/path_item_ref_siblings_test.go new file mode 100644 index 000000000..aea0a7cd6 --- /dev/null +++ b/openapi3/path_item_ref_siblings_test.go @@ -0,0 +1,96 @@ +package openapi3_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" +) + +func loadPathItemRefSiblings(t *testing.T, version, siblings string) (*openapi3.T, error) { + t.Helper() + loader := openapi3.NewLoader() + return loader.LoadFromData(fmt.Appendf(nil, `{ + "openapi": %q, + "info": {"title": "t", "version": "1"}, + "paths": { + "/x": {"$ref": "#/paths/~1y", %s}, + "/y": { + "summary": "target", + "get": {"responses": {"200": {"description": "target get"}}} + } + } +}`, version, siblings)) +} + +// A Path Item Object's $ref is not a Reference Object: the fields written next +// to it are part of the same path item. The specification leaves only the +// result for a field set on both sides undefined. +func TestPathItemRefSiblings(t *testing.T) { + t.Parallel() + for _, version := range []string{"3.0.3", "3.1.0", "3.2.0"} { + t.Run(version, func(t *testing.T) { + t.Parallel() + t.Run("fields set on one side only are merged", func(t *testing.T) { + t.Parallel() + doc, err := loadPathItemRefSiblings(t, version, `"post": {"responses": {"200": {"description": "local post"}}}`) + require.NoError(t, err) + require.NoError(t, doc.Validate(t.Context())) + + x := doc.Paths.Value("/x") + require.Equal(t, "target", x.Summary) + require.Equal(t, "target get", *x.Get.Responses.Value("200").Value.Description) + require.Equal(t, "local post", *x.Post.Responses.Value("200").Value.Description) + + // The referenced path item is not changed. + require.Nil(t, doc.Paths.Value("/y").Post) + }) + + t.Run("a field set on both sides is taken from next to the $ref", func(t *testing.T) { + t.Parallel() + doc, err := loadPathItemRefSiblings(t, version, `"summary": "local", "get": {"responses": {"200": {"description": "local get"}}}`) + require.NoError(t, err) + require.NoError(t, doc.Validate(t.Context())) + + x := doc.Paths.Value("/x") + require.Equal(t, "local", x.Summary) + require.Equal(t, "local get", *x.Get.Responses.Value("200").Value.Description) + + y := doc.Paths.Value("/y") + require.Equal(t, "target", y.Summary) + require.Equal(t, "target get", *y.Get.Responses.Value("200").Value.Description) + }) + + t.Run("only the fields next to the $ref are marshaled back", func(t *testing.T) { + t.Parallel() + doc, err := loadPathItemRefSiblings(t, version, `"summary": "local", "x-local": 1`) + require.NoError(t, err) + + data, err := doc.Paths.Value("/x").MarshalJSON() + require.NoError(t, err) + require.JSONEq(t, `{"$ref": "#/paths/~1y", "summary": "local", "x-local": 1}`, string(data)) + + data, err = doc.MarshalJSON() + require.NoError(t, err) + loader := openapi3.NewLoader() + reloaded, err := loader.LoadFromData(data) + require.NoError(t, err) + require.Equal(t, "local", reloaded.Paths.Value("/x").Summary) + require.NotNil(t, reloaded.Paths.Value("/x").Get) + }) + + t.Run("a $ref to nothing is reported despite the siblings", func(t *testing.T) { + t.Parallel() + loader := openapi3.NewLoader() + _, err := loader.LoadFromData(fmt.Appendf(nil, `{ + "openapi": %q, + "info": {"title": "t", "version": "1"}, + "paths": {"/x": {"$ref": "#/paths/~1nowhere", "summary": "local"}} +}`, version)) + require.Error(t, err) + }) + }) + } +} From 98fd8d4745636cef0043b00895c8ede0690f1b9a Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Tue, 20 Oct 2026 21:46:54 +0200 Subject: [PATCH 06/21] openapi3: internalize, version and locate the keywords next to a schema $ref Running the test cases of #1271 against the previous commits shows four ways the keywords written next to an OpenAPI 3.1 schema's $ref still go wrong: * InternalizeRefs made a component of a $ref's Value, which is a copy of the referenced schema with the siblings applied, so they leaked into every other use of that component. It now uses the referenced schema, which SchemaRef keeps for this. And the external $refs within the siblings were rewritten in memory but marshaled as they were first read, so the internalized document still pointed outside. Marshaling now takes each sibling from what the parsed siblings now hold, and falls back to what was read for the fields a Schema leaves out, such as pattern: "". * A file of bare schemas has no openapi field, so the siblings of the $refs within it were ignored as in OpenAPI 3.0, even when an OpenAPI 3.1 document referred to it. Such a file now follows the version of the document whose refs the loader resolves. * Siblings that do not form a schema, such as allOf: {type: integer}, were silently dropped. OpenAPI 3.1 reads them as a Schema Object, so loading now fails there. OpenAPI 3.0 still ignores them. * The allOf entry made of the assertion siblings had no Origin, so the errors it raised, such as for an invalid pattern, could not be located. It now shares the $ref's. Signed-off-by: Pierre Fenoll --- README.md | 4 ++++ openapi3/internalize_refs.go | 8 +++++++- openapi3/loader.go | 39 ++++++++++++++++++++++++++++++------ openapi3/refs.go | 25 +++++++++++++++++++++++ openapi3/refs.tmpl | 27 +++++++++++++++++++++++++ 5 files changed, 96 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 6a896668e..5f058ba95 100644 --- a/README.md +++ b/README.md @@ -342,6 +342,10 @@ for _, path := range doc.Paths.InMatchingOrder() { * Marshaling an `openapi3.*Ref` with a `$ref` now writes back the fields that were written next to it, instead of emitting a lone `{"$ref": ...}`. An OpenAPI 3.1 schema's sibling keywords, which constrain the referenced schema, were lost that way, and so was every other field next to a `$ref`. * The loader now resolves a Path Item Object's `$ref` even when fields are written next to it, and merges the two: it used to skip the `$ref` then, silently dropping every operation of the referenced path item and never noticing a `$ref` to nothing. The specification leaves the result undefined only for a field set on both sides, which is then taken from next to the `$ref`. Marshaling writes back the `$ref` with the fields that were next to it, which it used to drop. * The loader no longer panics with `interface conversion` when the same `$ref` is written where two kinds of objects are expected, one nested in the other, such as a path item and one of its responses. +* `InternalizeRefs` now makes a component of the schema an external `$ref` points to, rather than of that schema with the OpenAPI 3.1 keywords next to the `$ref` applied to it, which leaked them into every other use of the component. The external `$ref`s within those keywords are now rewritten in the marshaled document too. +* The keywords next to a schema's `$ref` in a file of bare schemas, which has no `openapi` field, now follow the version of the document that refers to the file: they were ignored, as in OpenAPI 3.0, even from an OpenAPI 3.1 document. +* In OpenAPI 3.1+ documents, loading now fails when the keywords next to a schema's `$ref` do not form a schema, such as `allOf: {type: integer}`, instead of silently ignoring them all. +* The errors raised by the keywords next to an OpenAPI 3.1 schema's `$ref` now carry their `Origin`. ### v0.150.0 * `openapi2.Operation.Responses` field type changed from `map[string]*Response` to `*Responses`, which keeps the Responses Object's `x-` extensions in `Responses.Extensions` instead of failing to decode them as responses. diff --git a/openapi3/internalize_refs.go b/openapi3/internalize_refs.go index 3dd99aca7..dbed548a2 100644 --- a/openapi3/internalize_refs.go +++ b/openapi3/internalize_refs.go @@ -148,7 +148,13 @@ func (doc *T) addSchemaToSpec(s *SchemaRef, refNameResolver RefNameResolver, par if doc.Components.Schemas == nil { doc.Components.Schemas = make(Schemas) } - doc.Components.Schemas[name] = s.Value.NewRef() + // A $ref with siblings has a copy of its target as Value, with the siblings + // applied: the component is the target itself. + value := s.Value + if s.target != nil { + value = s.target + } + doc.Components.Schemas[name] = value.NewRef() s.Ref = "#/components/schemas/" + name return true } diff --git a/openapi3/loader.go b/openapi3/loader.go index 72850311a..e7e422cdc 100644 --- a/openapi3/loader.go +++ b/openapi3/loader.go @@ -67,6 +67,12 @@ type Loader struct { visitedRefs map[string]struct{} visitedPath []string backtrack map[string][]func(value any) + + // rootDoc is the document whose refs are being resolved, as opposed to + // the documents it refers to, and resolvingDepth how deep in these the + // loader currently is. + rootDoc *T + resolvingDepth int } // NewLoader returns an empty Loader @@ -250,6 +256,12 @@ func (loader *Loader) ResolveRefsIn(doc *T, location *url.URL) (err error) { loader.Context = context.Background() } + if loader.resolvingDepth == 0 { + loader.rootDoc = doc + } + loader.resolvingDepth++ + defer func() { loader.resolvingDepth-- }() + if loader.visitedPathItemRefs == nil { loader.resetVisitedPathItemRefs() } @@ -1062,7 +1074,10 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat if component.Value != nil { return nil } - if doc.IsOpenAPI31OrLater() && component.sibling != nil { + if loader.isOpenAPI31OrLater(doc) && component.siblingErr != nil { + return fmt.Errorf("the fields next to $ref %q are not a schema: %w", ref, component.siblingErr) + } + if loader.isOpenAPI31OrLater(doc) && component.sibling != nil { // Resolve the $refs within the siblings themselves, such as those of // the properties they add, before applyRefSiblings shares them with // Value: it may run from the callback below, which resolves nothing. @@ -1077,7 +1092,7 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat // A $ref reached again while its target is still being resolved is // filled in here when the cycle unwinds, which can overwrite a value // that already had its siblings applied, so apply them here as well. - component.applyRefSiblings(doc) + component.applyRefSiblings(loader.isOpenAPI31OrLater(doc)) }) { return nil } @@ -1111,7 +1126,7 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat // they augment the referenced schema (e.g. deprecated:true alongside $ref). // Only apply for OAS 3.1+ — in 3.0 $ref replaces its entire object and siblings // are (validly) ignored. - component.applyRefSiblings(doc) + component.applyRefSiblings(loader.isOpenAPI31OrLater(doc)) } value := component.Value if value == nil { @@ -1596,10 +1611,11 @@ var refSiblingAnnotations = map[string]struct{}{ // referenced schema, with its properties, type and so on. // // It works on a copy so a schema shared by other references is not mutated. -func (component *SchemaRef) applyRefSiblings(doc *T) { - if !doc.IsOpenAPI31OrLater() || component.sibling == nil || component.Value == nil { +func (component *SchemaRef) applyRefSiblings(isOpenAPI31OrLater bool) { + if !isOpenAPI31OrLater || component.sibling == nil || component.Value == nil { return } + component.target = component.Value var annotations, assertions []string for _, field := range component.extra { if _, ok := refSiblingAnnotations[field]; ok { @@ -1611,7 +1627,8 @@ func (component *SchemaRef) applyRefSiblings(doc *T) { schemaCopy := *component.Value applySiblingSchemaFields(&schemaCopy, component.sibling, annotations) if len(assertions) != 0 { - var conjunct Schema + // The siblings sit in the same object as the $ref, so they share its origin. + conjunct := Schema{Origin: component.Origin} applySiblingSchemaFields(&conjunct, component.sibling, assertions) if !conjunct.IsEmpty() { allOf := make(SchemaRefs, 0, len(schemaCopy.AllOf)+1) @@ -1622,6 +1639,16 @@ func (component *SchemaRef) applyRefSiblings(doc *T) { component.Value = &schemaCopy } +// isOpenAPI31OrLater tells whether doc follows OAS 3.1 or later. A document +// that does not say, such as a file of bare schemas a $ref points into, follows +// the version of the document that refers to it. +func (loader *Loader) isOpenAPI31OrLater(doc *T) bool { + if doc.OpenAPI == "" && loader.rootDoc != nil { + doc = loader.rootDoc + } + return doc.IsOpenAPI31OrLater() +} + func unescapeRefString(ref string) string { return strings.ReplaceAll(strings.ReplaceAll(ref, "~1", "/"), "~0", "~") } diff --git a/openapi3/refs.go b/openapi3/refs.go index 83a1fb9b1..9fce3febf 100644 --- a/openapi3/refs.go +++ b/openapi3/refs.go @@ -1163,6 +1163,12 @@ type SchemaRef struct { // sibling holds keyword siblings of a $ref (OAS 3.1 / JSON Schema 2020-12). // It is populated during unmarshal and applied to Value after $ref resolution. sibling *Schema + // siblingErr is why the siblings could not be read as a Schema. OAS 3.1 + // reports it, while OAS 3.0 ignores the fields next to a $ref anyway. + siblingErr error + // target is the referenced schema, which Value is a copy of once siblings + // are applied to it. + target *Schema refPath *url.URL } @@ -1202,6 +1208,23 @@ func (x SchemaRef) MarshalYAML() (any, error) { } if m, ok := out.(map[string]any); ok { maps.Copy(m, x.siblingExtras) + if x.sibling != nil && len(x.siblingExtras) != 0 { + // The $refs within the siblings may have been rewritten since, as + // InternalizeRefs does, so write out what the siblings now hold. A + // field the Schema leaves out, such as an empty pattern, is still + // written as it was. + current, err := x.sibling.MarshalYAML() + if err != nil { + return nil, err + } + if current, ok := current.(map[string]any); ok { + for k := range x.siblingExtras { + if v, ok := current[k]; ok { + m[k] = v + } + } + } + } } return out, nil } @@ -1242,6 +1265,8 @@ func (x *SchemaRef) UnmarshalJSON(data []byte) error { var sibling Schema if err := unmarshalJSON(data, &sibling); err == nil { x.sibling = &sibling + } else { + x.siblingErr = err } } for k, v := range extra { diff --git a/openapi3/refs.tmpl b/openapi3/refs.tmpl index a00db9614..8398c302b 100644 --- a/openapi3/refs.tmpl +++ b/openapi3/refs.tmpl @@ -36,6 +36,12 @@ type {{ $type.Name }}Ref struct { // sibling holds keyword siblings of a $ref (OAS 3.1 / JSON Schema 2020-12). // It is populated during unmarshal and applied to Value after $ref resolution. sibling *Schema + // siblingErr is why the siblings could not be read as a Schema. OAS 3.1 + // reports it, while OAS 3.0 ignores the fields next to a $ref anyway. + siblingErr error + // target is the referenced schema, which Value is a copy of once siblings + // are applied to it. + target *Schema {{- end }} refPath *url.URL @@ -80,6 +86,25 @@ func (x {{ $type.Name }}Ref) MarshalYAML() (any, error) { } if m, ok := out.(map[string]any); ok { maps.Copy(m, x.siblingExtras) + {{- if eq $type.Name "Schema" }} + if x.sibling != nil && len(x.siblingExtras) != 0 { + // The $refs within the siblings may have been rewritten since, as + // InternalizeRefs does, so write out what the siblings now hold. A + // field the Schema leaves out, such as an empty pattern, is still + // written as it was. + current, err := x.sibling.MarshalYAML() + if err != nil { + return nil, err + } + if current, ok := current.(map[string]any); ok { + for k := range x.siblingExtras { + if v, ok := current[k]; ok { + m[k] = v + } + } + } + } + {{- end }} } return out, nil } @@ -127,6 +152,8 @@ func (x *{{ $type.Name }}Ref) UnmarshalJSON(data []byte) error { var sibling Schema if err := unmarshalJSON(data, &sibling); err == nil { x.sibling = &sibling + } else { + x.siblingErr = err } } {{- end }} From 52aade8175050135d50006822c01a95fbd2f92ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=94=D0=BC=D0=B8=D1=82=D1=80=D0=B8=D0=B9=20=D0=90=D0=B1?= =?UTF-8?q?=D1=80=D0=B0=D0=BC=D0=BE=D0=B2?= Date: Tue, 20 Oct 2026 21:47:02 +0200 Subject: [PATCH 07/21] openapi3: test $ref siblings with the cases of #1271 These are the test cases of #1271, which makes the keywords next to an OpenAPI 3.1 schema's $ref the Value and the referenced schema an allOf entry of it. Here Value keeps the shape of the referenced schema, so the assertions on where each keyword ends up now check what the schema accepts, or the shape it has here, instead: * Fields next to a $ref round-trip as written, where #1271 writes them out as an ordinary Schema would and so drops their zero values. * In OpenAPI 3.0, where these fields are ignored, the loader does not follow the $refs within them, so InternalizeRefs leaves them as written. #1271 internalizes them. * A cyclic Go value is no longer validated: it overflows the stack against any recursive schema, siblings or not, and decoding JSON cannot produce one. The four cases of $refs that only lead to one another are skipped. Their Value is nil, with or without siblings, as on master: JSON Schema 2020-12 leaves such infinite recursion undefined, and #1271 resolves them to the conjunction of their siblings. Adapted by Pierre Fenoll from https://github.com/getkin/kin-openapi/pull/1271 Signed-off-by: Pierre Fenoll --- openapi3/issue1271_test.go | 744 +++++++++++++++++++++++++++++++++++++ 1 file changed, 744 insertions(+) create mode 100644 openapi3/issue1271_test.go diff --git a/openapi3/issue1271_test.go b/openapi3/issue1271_test.go new file mode 100644 index 000000000..d5b700975 --- /dev/null +++ b/openapi3/issue1271_test.go @@ -0,0 +1,744 @@ +package openapi3_test + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" +) + +// These cases come from https://github.com/getkin/kin-openapi/pull/1271. +// There, the keywords next to a 3.1 schema $ref become the Value and the +// referenced schema an allOf entry of it. Here Value keeps the shape of the +// referenced schema, so the cases check what a schema accepts and how it is +// written back out rather than where each keyword ends up. +// +// The skipped cases are $refs that only lead to one another, which leaves +// their Value nil, with or without siblings. JSON Schema 2020-12 leaves such +// infinite recursion undefined (§9.4.1); #1271 resolves them to the +// conjunction of their siblings. + +func TestSchemaRefSiblingIsSerializedAndAppliedAsConjunct(t *testing.T) { + t.Parallel() + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData([]byte(` +openapi: 3.1.0 +info: + title: ref sibling + version: "1" +paths: {} +components: + schemas: + Base: + type: string + maxLength: 5 + Use: + $ref: '#/components/schemas/Base' + minLength: 3 + x-use-site: true +`)) + require.NoError(t, err) + + useRef := doc.Components.Schemas["Use"] + require.Equal(t, "string", useRef.Value.Type.Slice()[0]) + require.Equal(t, uint64(5), *useRef.Value.MaxLength) + require.NotContains(t, doc.Components.Schemas["Base"].Value.Extensions, "x-use-site") + require.Error(t, useRef.Value.VisitJSON("xx", openapi3.EnableJSONSchema2020())) + require.Error(t, useRef.Value.VisitJSON("xxxxxx", openapi3.EnableJSONSchema2020())) + require.NoError(t, useRef.Value.VisitJSON("xxxx", openapi3.EnableJSONSchema2020())) + + data, err := json.Marshal(useRef) + require.NoError(t, err) + require.JSONEq(t, `{ + "$ref":"#/components/schemas/Base", + "minLength":3, + "x-use-site":true +}`, string(data)) + documentData, err := json.Marshal(doc) + require.NoError(t, err) + roundTripLoader := openapi3.NewLoader() + roundTripped, err := roundTripLoader.LoadFromData(documentData) + require.NoError(t, err) + roundTrippedUse := roundTripped.Components.Schemas["Use"].Value + require.Error(t, roundTrippedUse.VisitJSON("xx", openapi3.EnableJSONSchema2020())) + require.Error(t, roundTrippedUse.VisitJSON("xxxxxx", openapi3.EnableJSONSchema2020())) + require.NoError(t, roundTrippedUse.VisitJSON("xxxx", openapi3.EnableJSONSchema2020())) + + useRef.Extensions["x-use-site"] = "changed" + useRef.Extensions["x-added"] = true + data, err = json.Marshal(useRef) + require.NoError(t, err) + require.JSONEq(t, `{ + "$ref":"#/components/schemas/Base", + "minLength":3, + "x-use-site":"changed", + "x-added":true +}`, string(data)) + delete(useRef.Extensions, "x-use-site") + data, err = json.Marshal(useRef) + require.NoError(t, err) + require.NotContains(t, string(data), "x-use-site") + + value := useRef.Value + allOf := len(value.AllOf) + for range 3 { + require.NoError(t, loader.ResolveRefsIn(doc, nil)) + require.Same(t, value, useRef.Value) + require.Len(t, useRef.Value.AllOf, allOf) + } +} + +func TestSchemaRefSiblingRoundTripsAsWritten(t *testing.T) { + t.Parallel() + data := []byte(`{ + "$ref":"#/components/schemas/Base", + "pattern":"", + "minLength":0, + "uniqueItems":false, + "required":[], + "properties":{}, + "default":null, + "maximum":0, + "x-use-site":true +}`) + var ref openapi3.SchemaRef + require.NoError(t, json.Unmarshal(data, &ref)) + + // Unlike #1271, which writes them out as an ordinary Schema would, dropping + // the zero values, the fields next to the $ref come back as written. + out, err := json.Marshal(ref) + require.NoError(t, err) + require.JSONEq(t, string(data), string(out)) +} + +func TestSchemaRefSiblingPreservesBothDynamicRefs(t *testing.T) { + t.Parallel() + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData([]byte(` +openapi: 3.1.0 +info: + title: dynamic ref conjunction + version: "1" +paths: {} +components: + schemas: + Base: + $dynamicRef: '#base' + Use: + $ref: '#/components/schemas/Base' + $dynamicRef: '#use' +`)) + require.NoError(t, err) + + use := doc.Components.Schemas["Use"].Value + require.Equal(t, "#base", use.DynamicRef) + require.Len(t, use.AllOf, 1) + require.Equal(t, "#use", use.AllOf[0].Value.DynamicRef) + require.Equal(t, "#base", doc.Components.Schemas["Base"].Value.DynamicRef) +} + +func TestSchemaRefSiblingOnSelfReference(t *testing.T) { + t.Parallel() + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData([]byte(` +openapi: 3.1.0 +info: + title: recursive ref sibling + version: "1" +paths: {} +components: + schemas: + Node: + type: object + required: [name] + properties: + name: + type: string + parent: + $ref: '#/components/schemas/Node' + minProperties: 2 +`)) + require.NoError(t, err) + + node := doc.Components.Schemas["Node"].Value + parent := node.Properties["parent"] + require.Error(t, parent.Value.VisitJSON(map[string]any{"name": "child"}, openapi3.EnableJSONSchema2020())) + require.NoError(t, parent.Value.VisitJSON(map[string]any{"name": "child", "extra": true}, openapi3.EnableJSONSchema2020())) + + invalidRecursiveValue := map[string]any{ + "name": "child", + "parent": map[string]any{ + "first": true, + "second": true, + }, + } + validRecursiveValue := map[string]any{ + "name": "child", + "parent": map[string]any{ + "name": "parent", + "extra": true, + }, + } + for _, opts := range [][]openapi3.SchemaValidationOption{ + nil, + {openapi3.EnableJSONSchema2020()}, + } { + require.Error(t, node.VisitJSON(invalidRecursiveValue, opts...), "the recursive target must still require name") + require.NoError(t, node.VisitJSON(validRecursiveValue, opts...)) + } + + data, err := json.Marshal(parent) + require.NoError(t, err) + require.JSONEq(t, `{ + "$ref":"#/components/schemas/Node", + "minProperties":2 +}`, string(data)) +} + +func TestSchemaRefSiblingOnSelfReferenceOutsideComponents(t *testing.T) { + t.Skip("TODO: give a value to the schemas of a cycle of $refs that never reaches a schema") + t.Parallel() + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData([]byte(` +openapi: 3.1.0 +info: + title: path-local recursive ref sibling + version: "1" +paths: + /x: + post: + requestBody: + content: + application/json: + schema: + $ref: '#/paths/~1x/post/requestBody/content/application~1json/schema' + minProperties: 1 + responses: + "200": + description: ok +`)) + require.NoError(t, err) + + schemaRef := doc.Paths.Value("/x").Post.RequestBody.Value.Content["application/json"].Schema + require.NotNil(t, schemaRef.Value) + require.Error(t, schemaRef.Value.VisitJSON(map[string]any{})) + require.NoError(t, schemaRef.Value.VisitJSON(map[string]any{"value": true})) + + data, err := json.Marshal(schemaRef) + require.NoError(t, err) + require.JSONEq(t, `{ + "$ref":"#/paths/~1x/post/requestBody/content/application~1json/schema", + "minProperties":1 +}`, string(data)) +} + +func TestSchemaRefSiblingsOnLongerCycle(t *testing.T) { + t.Skip("TODO: give a value to the schemas of a cycle of $refs that never reaches a schema") + t.Parallel() + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData([]byte(` +openapi: 3.1.0 +info: + title: ref-only cycle with siblings + version: "1" +paths: {} +components: + schemas: + A: + $ref: '#/components/schemas/B' + minLength: 2 + B: + $ref: '#/components/schemas/A' + maxLength: 4 +`)) + require.NoError(t, err) + + a := doc.Components.Schemas["A"].Value + b := doc.Components.Schemas["B"].Value + require.NotNil(t, a) + require.NotNil(t, b) + validationErr := a.VisitJSON("x", openapi3.EnableJSONSchema2020()) + require.Error(t, validationErr) + getErr := func() { _ = validationErr.Error() } + require.NotPanics(t, getErr, "cyclic effective schemas must remain serializable in errors") + require.Error(t, a.VisitJSON("12345", openapi3.EnableJSONSchema2020())) + require.Error(t, b.VisitJSON("x", openapi3.EnableJSONSchema2020())) + require.Error(t, b.VisitJSON("12345", openapi3.EnableJSONSchema2020())) + require.NoError(t, a.VisitJSON("1234", openapi3.EnableJSONSchema2020())) + require.NoError(t, b.VisitJSON("1234", openapi3.EnableJSONSchema2020())) + + aValue, bValue := a, b + for range 3 { + require.NoError(t, loader.ResolveRefsIn(doc, nil)) + require.Same(t, aValue, doc.Components.Schemas["A"].Value) + require.Same(t, bValue, doc.Components.Schemas["B"].Value) + } + for name, expected := range map[string]string{ + "A": `{"$ref":"#/components/schemas/B","minLength":2}`, + "B": `{"$ref":"#/components/schemas/A","maxLength":4}`, + } { + data, err := json.Marshal(doc.Components.Schemas[name]) + require.NoError(t, err) + require.JSONEq(t, expected, string(data)) + } +} + +func TestSchemaRefSiblingAnchorsPartiallyAnnotatedAliasCycle(t *testing.T) { + t.Skip("TODO: give a value to the schemas of a cycle of $refs that never reaches a schema") + t.Parallel() + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData([]byte(` +openapi: 3.1.0 +info: + title: partially annotated alias cycle + version: "1" +paths: {} +components: + schemas: + A: + $ref: '#/components/schemas/B' + minLength: 2 + B: + $ref: '#/components/schemas/A' +`)) + require.NoError(t, err) + + for _, name := range []string{"A", "B"} { + value := doc.Components.Schemas[name].Value + require.NotNil(t, value) + require.Error(t, value.VisitJSON("x", openapi3.EnableJSONSchema2020())) + require.NoError(t, value.VisitJSON("xx", openapi3.EnableJSONSchema2020())) + } +} + +func TestSchemaRefSiblingsAreIgnoredOnOpenAPI30AliasCycle(t *testing.T) { + t.Skip("TODO: give a value to the schemas of a cycle of $refs that never reaches a schema") + t.Parallel() + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData([]byte(` +openapi: 3.0.3 +info: + title: ref-only cycle with ignored siblings + version: "1" +paths: {} +components: + schemas: + A: + $ref: '#/components/schemas/B' + minLength: 2 + B: + $ref: '#/components/schemas/A' + maxLength: 4 +`)) + require.NoError(t, err) + + for _, name := range []string{"A", "B"} { + ref := doc.Components.Schemas[name] + require.NotNil(t, ref.Value) + require.Zero(t, ref.Value.MinLength) + require.Nil(t, ref.Value.MaxLength) + require.NoError(t, ref.Value.VisitJSON("x")) + require.NoError(t, ref.Value.VisitJSON("12345")) + } + + a, err := json.Marshal(doc.Components.Schemas["A"]) + require.NoError(t, err) + require.JSONEq(t, `{"$ref":"#/components/schemas/B","minLength":2}`, string(a)) + b, err := json.Marshal(doc.Components.Schemas["B"]) + require.NoError(t, err) + require.JSONEq(t, `{"$ref":"#/components/schemas/A","maxLength":4}`, string(b)) +} + +func TestSchemaRefSiblingErrorRetainsUseSiteOrigin(t *testing.T) { + t.Parallel() + loader := openapi3.NewLoader() + loader.IncludeOrigin = true + doc, err := loader.LoadFromData([]byte(` +openapi: 3.1.0 +info: + title: sibling origin + version: "1" +paths: {} +components: + schemas: + Base: + type: string + Use: + $ref: '#/components/schemas/Base' + pattern: '(?invalid' +`)) + require.NoError(t, err) + + use := doc.Components.Schemas["Use"] + validationErr := use.Value.VisitJSON("value", openapi3.EnableJSONSchema2020()) + var patternErr *openapi3.SchemaPatternRegexError + require.True(t, errors.As(validationErr, &patternErr)) + require.NotNil(t, patternErr.Origin) + require.NotNil(t, patternErr.Origin.Key) + require.Equal(t, 11, patternErr.Origin.Key.Line) + require.Equal(t, 7, patternErr.Origin.Fields.Get("pattern").Column) +} + +func TestSchemaRefSiblingStructuralCycleKeepsAliasTarget(t *testing.T) { + t.Parallel() + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData([]byte(` +openapi: 3.1.0 +info: + title: structural sibling cycle + version: "1" +paths: {} +components: + schemas: + A: + $ref: '#/components/schemas/B' + properties: + child: + $ref: '#/components/schemas/A' + B: + type: object + required: [name] + properties: + name: + type: string +`)) + require.NoError(t, err) + + a := doc.Components.Schemas["A"].Value + require.Error(t, a.VisitJSON(map[string]any{ + "name": "root", + "child": map[string]any{"other": true}, + })) + require.NoError(t, a.VisitJSON(map[string]any{ + "name": "root", + "child": map[string]any{"name": "nested"}, + })) +} + +func TestSchemaRefSiblingInRawExternalSchemaUsesRootDialect(t *testing.T) { + t.Parallel() + dir := t.TempDir() + rootPath := filepath.Join(dir, "openapi.yaml") + externalPath := filepath.Join(dir, "schemas.yaml") + + require.NoError(t, os.WriteFile(rootPath, []byte(` +openapi: 3.1.0 +info: + title: raw external schema sibling + version: "1" +paths: {} +components: + schemas: + Use: + $ref: 'schemas.yaml#/$defs/Alias' +`), 0o600)) + require.NoError(t, os.WriteFile(externalPath, []byte(` +$defs: + Base: + type: string + Alias: + $ref: '#/$defs/Base' + minLength: 3 +`), 0o600)) + + loader := openapi3.NewLoader() + loader.IsExternalRefsAllowed = true + doc, err := loader.LoadFromFile(rootPath) + require.NoError(t, err) + + use := doc.Components.Schemas["Use"].Value + require.Error(t, use.VisitJSON("xx", openapi3.EnableJSONSchema2020())) + require.NoError(t, use.VisitJSON("xxx", openapi3.EnableJSONSchema2020())) +} + +func TestSchemaRefSiblingDoesNotMutateSharedTarget(t *testing.T) { + t.Parallel() + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData([]byte(` +openapi: 3.1.0 +info: + title: independent ref siblings + version: "1" +paths: {} +components: + schemas: + Base: + type: string + maxLength: 5 + Short: + $ref: '#/components/schemas/Base' + maxLength: 3 + Long: + $ref: '#/components/schemas/Base' + minLength: 4 +`)) + require.NoError(t, err) + + base := doc.Components.Schemas["Base"].Value + short := doc.Components.Schemas["Short"].Value + long := doc.Components.Schemas["Long"].Value + require.Equal(t, uint64(5), *base.MaxLength) + require.Zero(t, base.MinLength) + require.Empty(t, base.AllOf) + require.NotSame(t, base, short) + require.NotSame(t, base, long) + require.NoError(t, base.VisitJSON("xxxxx", openapi3.EnableJSONSchema2020())) + require.Error(t, short.VisitJSON("xxxx", openapi3.EnableJSONSchema2020())) + require.Error(t, long.VisitJSON("xxx", openapi3.EnableJSONSchema2020())) + require.NoError(t, long.VisitJSON("xxxx", openapi3.EnableJSONSchema2020())) +} + +func TestSchemaRefSiblingSerializationIsIndependentOfOASVersion(t *testing.T) { + t.Parallel() + const spec = ` +openapi: %s +info: + title: versioned sibling semantics + version: "1" +paths: {} +components: + schemas: + Base: + type: string + Use: + $ref: '#/components/schemas/Base' + minLength: 3 +` + + for _, version := range []string{"3.0.3", "3.1.0"} { + t.Run(version, func(t *testing.T) { + t.Parallel() + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData(fmt.Appendf(nil, spec, version)) + require.NoError(t, err) + use := doc.Components.Schemas["Use"] + data, err := json.Marshal(use) + require.NoError(t, err) + require.JSONEq(t, `{ + "$ref":"#/components/schemas/Base", + "minLength":3 +}`, string(data)) + if version == "3.0.3" { + require.NoError(t, use.Value.VisitJSON("xx")) + } else { + require.Error(t, use.Value.VisitJSON("xx")) + } + }) + } +} + +func TestMalformedSchemaRefSiblingUsesRootDialect(t *testing.T) { + t.Parallel() + const spec = ` +openapi: %s +info: + title: malformed sibling + version: "1" +paths: {} +components: + schemas: + Base: + type: string + Use: + $ref: '#/components/schemas/Base' + allOf: + type: integer +` + + loader30 := openapi3.NewLoader() + doc, err := loader30.LoadFromData(fmt.Appendf(nil, spec, "3.0.3")) + require.NoError(t, err, "OpenAPI 3.0 ignores malformed fields next to $ref") + require.Equal(t, "string", doc.Components.Schemas["Use"].Value.Type.Slice()[0]) + + loader31 := openapi3.NewLoader() + _, err = loader31.LoadFromData(fmt.Appendf(nil, spec, "3.1.0")) + require.Error(t, err, "OpenAPI 3.1 treats the adjacent fields as a Schema Object") +} + +func TestInternalizeRefsUpdatesSchemaRefSiblingFields(t *testing.T) { + t.Parallel() + dir := t.TempDir() + rootPath := filepath.Join(dir, "root.yaml") + externalPath := filepath.Join(dir, "external.yaml") + + require.NoError(t, os.WriteFile(rootPath, []byte(` +openapi: 3.1.0 +info: + title: Internalized sibling refs + version: "1.0" +paths: {} +components: + schemas: + Root: + type: object + properties: + use: + $ref: external.yaml#/Base + items: + $ref: external.yaml#/Item +`), 0o600)) + require.NoError(t, os.WriteFile(externalPath, []byte(` +Base: + type: array +Item: + type: string +`), 0o600)) + + loader := openapi3.NewLoader() + loader.IsExternalRefsAllowed = true + doc, err := loader.LoadFromFile(rootPath) + require.NoError(t, err) + + doc.InternalizeRefs(t.Context(), nil) + data, err := doc.MarshalJSON() + require.NoError(t, err) + require.NotContains(t, string(data), "external.yaml") + + use := doc.Components.Schemas["Root"].Value.Properties["use"] + require.True(t, strings.HasPrefix(use.Ref, "#/components/schemas/")) + targetName := strings.TrimPrefix(use.Ref, "#/components/schemas/") + target := doc.Components.Schemas[targetName].Value + require.Nil(t, target.Items, + "internalization must componentize the referenced target, not the schema with the siblings applied") + require.Empty(t, target.AllOf, + "internalization must componentize the referenced target, not the schema with the siblings applied") + var marshaledUse map[string]any + require.NoError(t, json.Unmarshal(mustMarshalJSON(t, use), &marshaledUse)) + items := marshaledUse["items"].(map[string]any) + require.True(t, strings.HasPrefix(items["$ref"].(string), "#/components/schemas/")) + + loader = openapi3.NewLoader() + roundTripped, err := loader.LoadFromData(data) + require.NoError(t, err, "the internalized document must not require external access") + roundTrippedUse := roundTripped.Components.Schemas["Root"].Value.Properties["use"].Value + require.NoError(t, roundTrippedUse.VisitJSON([]any{"value"}, openapi3.EnableJSONSchema2020())) + require.Error(t, roundTrippedUse.VisitJSON([]any{1}, openapi3.EnableJSONSchema2020())) +} + +// In OpenAPI 3.0 the fields next to a $ref are ignored, so the loader never +// follows the $refs within them and internalization leaves them as written. +func TestInternalizeRefsLeavesIgnoredSchemaRefSiblingFieldsInOpenAPI30(t *testing.T) { + t.Parallel() + dir := t.TempDir() + rootPath := filepath.Join(dir, "root.yaml") + externalPath := filepath.Join(dir, "external.yaml") + + require.NoError(t, os.WriteFile(rootPath, []byte(` +openapi: 3.0.3 +info: + title: Internalized OpenAPI 3.0 sibling refs + version: "1.0" +paths: {} +components: + schemas: + Root: + type: object + properties: + use: + $ref: external.yaml#/Base + items: + $ref: external.yaml#/Item +`), 0o600)) + require.NoError(t, os.WriteFile(externalPath, []byte(` +Base: + type: array +Item: + type: string +`), 0o600)) + + loader := openapi3.NewLoader() + loader.IsExternalRefsAllowed = true + doc, err := loader.LoadFromFile(rootPath) + require.NoError(t, err) + + use := doc.Components.Schemas["Root"].Value.Properties["use"] + require.Nil(t, use.Value.Items, "an OpenAPI 3.0 sibling must not affect the resolved target") + before, err := json.Marshal(use) + require.NoError(t, err) + require.Contains(t, string(before), `external.yaml#/Item`) + + doc.InternalizeRefs(t.Context(), nil) + data, err := doc.MarshalJSON() + require.NoError(t, err) + require.Nil(t, use.Value.Items, "internalization must not change OpenAPI 3.0 sibling semantics") + + var marshaledUse map[string]any + require.NoError(t, json.Unmarshal(mustMarshalJSON(t, use), &marshaledUse)) + require.True(t, strings.HasPrefix(marshaledUse["$ref"].(string), "#/components/schemas/")) + items := marshaledUse["items"].(map[string]any) + require.Equal(t, "external.yaml#/Item", items["$ref"]) + + roundTripLoader := openapi3.NewLoader() + roundTripped, err := roundTripLoader.LoadFromData(data) + require.NoError(t, err, "the internalized document must not require external access") + roundTrippedUse := roundTripped.Components.Schemas["Root"].Value.Properties["use"] + require.Nil(t, roundTrippedUse.Value.Items) +} + +func TestInternalizeRefsUpdatesSiblingFieldsWithFalseTarget(t *testing.T) { + t.Parallel() + dir := t.TempDir() + rootPath := filepath.Join(dir, "root.yaml") + externalPath := filepath.Join(dir, "external.yaml") + + require.NoError(t, os.WriteFile(rootPath, []byte(` +openapi: 3.1.0 +info: + title: Internalized false-target sibling refs + version: "1.0" +paths: {} +components: + schemas: + RejectEverything: false + Root: + type: object + properties: + use: + $ref: '#/components/schemas/RejectEverything' + items: + $ref: external.yaml#/Item +`), 0o600)) + require.NoError(t, os.WriteFile(externalPath, []byte(` +Item: + type: string +`), 0o600)) + + loader := openapi3.NewLoader() + loader.IsExternalRefsAllowed = true + doc, err := loader.LoadFromFile(rootPath) + require.NoError(t, err) + + use := doc.Components.Schemas["Root"].Value.Properties["use"] + require.Error(t, use.Value.VisitJSON([]any{"value"}, openapi3.EnableJSONSchema2020())) + + doc.InternalizeRefs(t.Context(), nil) + data, err := doc.MarshalJSON() + require.NoError(t, err) + require.NotContains(t, string(data), "external.yaml") + require.Error(t, use.Value.VisitJSON([]any{"value"}, openapi3.EnableJSONSchema2020())) +} + +func TestRecursiveSchemaDistinguishesOverlappingSlices(t *testing.T) { + t.Parallel() + schema := openapi3.NewArraySchema().WithMinItems(1) + schema.Items = &openapi3.SchemaRef{Value: schema} + + value := make([]any /*,*/, 1) + value[0] = value[:0] + require.Error(t, schema.VisitJSON(value), + "a shorter subslice with the same backing address is a distinct JSON instance") +} + +func mustMarshalJSON(t *testing.T, value any) []byte { + t.Helper() + data, err := json.Marshal(value) + require.NoError(t, err) + return data +} From 2420538c110afc6f49044be370f12b667151029a Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Thu, 22 Oct 2026 13:27:08 +0200 Subject: [PATCH 08/21] changelog: fixup rebased messes (#1372) Signed-off-by: Pierre Fenoll --- README.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 5f058ba95..21e4bcfba 100644 --- a/README.md +++ b/README.md @@ -329,10 +329,9 @@ for _, path := range doc.Paths.InMatchingOrder() { ## CHANGELOG: Sub-v1 breaking API changes -### v0.151.0 -* Removed the deprecated package-global `openapi3.IncludeOrigin` variable. Set the `IncludeOrigin` field of `openapi3.Loader` instead. -* `openapi3filter.UrlencodedBodyDecoder` (hence `application/x-www-form-urlencoded` request body validation) now returns a `*openapi3filter.ParseError` for a value that cannot be parsed into its property's schema type, such as `n=abc` for an `integer` property, instead of silently dropping it from the decoded body. -* `openapi3filter` now writes a query parameter's array `default` into the request URL following the parameter's `style` and `explode` (including their defaults), so an array without `explode` set is written as `?ids=1&ids=2` rather than `?ids=1,2`, and `pipeDelimited`/`spaceDelimited` arrays use their own delimiter. +### v0.152.0 +* JSON keys of OpenAPI documents only fill the field of the same case (`json:",case:strict"`) when `encoding/json` is implemented with `encoding/json/v2` (by default from Go 1.27, and on Go 1.25 and 1.26 with `GOEXPERIMENT=jsonv2`): a key such as `"Components"` (not `"components"`) is kept as an extension instead of filling `openapi3.T.Components`. Other builds still match keys case-insensitively, as `encoding/json` v1 does. +* On Go 1.27+, unless built with `GOEXPERIMENT=nojsonv2`, `openapi2` and `openapi3` decode documents with `encoding/json/v2`: objects with duplicate member names and strings with invalid UTF-8 are rejected, and decoding errors are worded differently and locate the faulty value from the root of the document, keys of paths, responses and callbacks included (e.g. `.paths.~1pets.get.responses.200.description`). On these builds `openapi2.Responses`, `openapi3.Callback`, `openapi3.Paths` and `openapi3.Responses` also implement `encoding/json/v2.UnmarshalerFrom`. * A schema with an empty `type: []` no longer marshals as `"type": null` which did not survive being read back. * The error for a header with neither or both of `content` and `schema` reads `header must contain exactly one of content and schema`. It used to say `parameter ...` followed by a `%v` dump of the `Header` struct, which printed heap addresses: the message differed between two runs over the same document, and leaked addresses into anything that logged it. * `openapi3filter` now parses `integer` values of parameters and `application/x-www-form-urlencoded`/`multipart/form-data` bodies as base 10 only, so `010` decodes to `10` rather than `8`, and Go integer literals such as `0x10`, `0b10`, `0o10` or `1_000` are now rejected as invalid integers. @@ -347,6 +346,11 @@ for _, path := range doc.Paths.InMatchingOrder() { * In OpenAPI 3.1+ documents, loading now fails when the keywords next to a schema's `$ref` do not form a schema, such as `allOf: {type: integer}`, instead of silently ignoring them all. * The errors raised by the keywords next to an OpenAPI 3.1 schema's `$ref` now carry their `Origin`. +### v0.151.0 +* Removed the deprecated package-global `openapi3.IncludeOrigin` variable. Set the `IncludeOrigin` field of `openapi3.Loader` instead. +* `openapi3filter.UrlencodedBodyDecoder` (hence `application/x-www-form-urlencoded` request body validation) now returns a `*openapi3filter.ParseError` for a value that cannot be parsed into its property's schema type, such as `n=abc` for an `integer` property, instead of silently dropping it from the decoded body. +* `openapi3filter` now writes a query parameter's array `default` into the request URL following the parameter's `style` and `explode` (including their defaults), so an array without `explode` set is written as `?ids=1&ids=2` rather than `?ids=1,2`, and `pipeDelimited`/`spaceDelimited` arrays use their own delimiter. + ### v0.150.0 * `openapi2.Operation.Responses` field type changed from `map[string]*Response` to `*Responses`, which keeps the Responses Object's `x-` extensions in `Responses.Extensions` instead of failing to decode them as responses. * `(*openapi3.Schema).VisitJSON(..)` without the `openapi3.EnableJSONSchema2020()` option no longer validates the OpenAPI 3.1 `const` and `prefixItems` keywords. Like the other 3.1-only keywords, they are validated by the JSON Schema 2020-12 validator, which `openapi3filter` enables for 3.1 documents. `items` still applies only past the `prefixItems` positions. @@ -362,8 +366,6 @@ for _, path := range doc.Paths.InMatchingOrder() { * `(*openapi3.Parameter).SerializationMethod()` (for query parameters) and `(*openapi3.Encoding).SerializationMethod()` now default `explode` to `false` for `style: spaceDelimited` and `style: pipeDelimited`, as the spec says, so `openapi3filter` splits `?ids=1|2|3` and `application/x-www-form-urlencoded` body properties such as `ids=1|2|3` instead of expecting `ids=1&ids=2&ids=3`. Set `explode: true` to keep the previous behavior. * Removed the package-global `openapi3.SchemaErrorDetailsDisabled` variable. Pass the new `openapi3.DisableSchemaErrorDetails()` option to `(*openapi3.Schema).VisitJSON(..)` instead, or add it to `openapi3filter.Options.SchemaValidationOptions`, which `openapi3filter` now also applies when validating parameters. Combined with `openapi3.EnableJSONSchema2020()` (hence with `openapi3filter` on OpenAPI 3.1+ documents), validation fails with `openapi3.ErrSchemaErrorDetailsWithJSONSchema2020`, as that validator's error messages include values. * The message customizer set with `openapi3.SetSchemaErrorMessageCustomizer(..)` (hence `(*openapi3filter.Options).WithCustomSchemaErrorFunc(..)`) now also applies to discriminator errors, to errors of patterns that fail to compile and to errors of the JSON Schema 2020-12 validator (`openapi3.EnableJSONSchema2020()`). -* JSON keys of OpenAPI documents only fill the field of the same case (`json:",case:strict"`) when `encoding/json` is implemented with `encoding/json/v2` (by default from Go 1.27, and on Go 1.25 and 1.26 with `GOEXPERIMENT=jsonv2`): a key such as `"Components"` (not `"components"`) is kept as an extension instead of filling `openapi3.T.Components`. Other builds still match keys case-insensitively, as `encoding/json` v1 does. -* On Go 1.27+, unless built with `GOEXPERIMENT=nojsonv2`, `openapi2` and `openapi3` decode documents with `encoding/json/v2`: objects with duplicate member names and strings with invalid UTF-8 are rejected, and decoding errors are worded differently and locate the faulty value from the root of the document, keys of paths, responses and callbacks included (e.g. `.paths.~1pets.get.responses.200.description`). On these builds `openapi2.Responses`, `openapi3.Callback`, `openapi3.Paths` and `openapi3.Responses` also implement `encoding/json/v2.UnmarshalerFrom`. ### v0.147.0 * `(*openapi3.PathItem).SetOperation(string, *Operation)` no longer panics on unhandled HTTP methods: these are now stored in the new `openapi3.PathItem.AdditionalOperations` field (passing a nil operation deletes the entry). From 15948ef260a97b47e1f72c877393ccc73afbda1d Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Thu, 22 Oct 2026 15:36:44 +0200 Subject: [PATCH 09/21] Add fuzz targets (#1373) Signed-off-by: Pierre Fenoll --- .github/workflows/fuzz.yml | 89 ++++++++++ Makefile | 37 +++-- openapi2conv/fuzz_test.go | 84 ++++++++++ openapi3/fuzz_test.go | 112 +++++++++++++ .../fuzz/FuzzLoadFromData/56b819f42562dabd | 2 + .../fuzz/FuzzLoadFromData/e9783ed365c6dc4c | 2 + openapi3filter/fuzz_test.go | 152 ++++++++++++++++++ 7 files changed, 464 insertions(+), 14 deletions(-) create mode 100644 .github/workflows/fuzz.yml create mode 100644 openapi2conv/fuzz_test.go create mode 100644 openapi3/fuzz_test.go create mode 100644 openapi3/testdata/fuzz/FuzzLoadFromData/56b819f42562dabd create mode 100644 openapi3/testdata/fuzz/FuzzLoadFromData/e9783ed365c6dc4c create mode 100644 openapi3filter/fuzz_test.go diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml new file mode 100644 index 000000000..4c920d2ac --- /dev/null +++ b/.github/workflows/fuzz.yml @@ -0,0 +1,89 @@ +name: fuzz +on: + pull_request: + push: + branches: + - master + schedule: + # Nightly, so a long run has a chance to reach code a 90s PR run never does. + - cron: '17 3 * * *' + workflow_dispatch: + inputs: + fuzztime: + description: 'How long to fuzz each target (go test -fuzztime syntax)' + required: false + default: 30m + +permissions: + contents: read + +jobs: + fuzz: + name: ${{ matrix.target }} + runs-on: ubuntu-latest + # A budget of 15m plus corpus minimization has to fit, and a hung target + # must not hold a runner forever. + timeout-minutes: 45 + env: + GO111MODULE: 'on' + CGO_ENABLED: '0' + strategy: + # Every target reports independently: one crasher must not hide the rest. + fail-fast: false + matrix: + include: + - {package: ./openapi3, target: FuzzLoadFromData} + - {package: ./openapi3, target: FuzzSchemaVisitJSON} + - {package: ./openapi2conv, target: FuzzToV3} + - {package: ./openapi3filter, target: FuzzValidateRequest} + steps: + + - uses: actions/setup-go@v6 + with: + go-version: stable + + - uses: actions/checkout@v6 + + # The generated corpus lives under $GOCACHE/fuzz and is what makes fuzzing + # cumulative: without it every run restarts from the seed corpus. The key + # carries the run id so each run writes a fresh entry, and restore-keys + # falls back to the most recent one for this target. + - name: Fuzzing corpus cache + uses: actions/cache@v3 + with: + path: ~/.cache/go-build/fuzz + key: fuzz-${{ matrix.target }}-${{ github.run_id }} + restore-keys: fuzz-${{ matrix.target }}- + + - name: Pick a budget + id: budget + run: | + if [[ -n '${{ inputs.fuzztime }}' ]]; then + fuzztime='${{ inputs.fuzztime }}' + elif [[ '${{ github.event_name }}' = schedule ]]; then + fuzztime=15m + else + fuzztime=90s + fi + echo "fuzztime=$fuzztime" >>"$GITHUB_OUTPUT" + + # -run='^$' skips the ordinary tests: the seed corpus is already replayed by + # `go test ./...` in the go workflow, so this job only spends time fuzzing. + - name: Fuzz ${{ matrix.target }} + run: | + go test \ + -run='^$' \ + -fuzz='^${{ matrix.target }}$' \ + -fuzztime='${{ steps.budget.outputs.fuzztime }}' \ + -fuzzminimizetime=60s \ + ${{ matrix.package }} + + # A failing input is written next to the package it broke. Upload it so the + # crasher can be committed to the seed corpus, where it becomes a permanent + # regression test replayed by `go test ./...`. + - if: failure() + uses: actions/upload-artifact@v4 + with: + name: crashers-${{ matrix.target }} + path: '**/testdata/fuzz/**' + if-no-files-found: ignore diff --git a/Makefile b/Makefile index 57bab901a..8fc32f5e5 100644 --- a/Makefile +++ b/Makefile @@ -1,28 +1,37 @@ -.DEFAULT_GOAL := prepare +.DEFAULT_GOAL := all +.PHONY: help generate format test test-fast fuzz all -.PHONY: help generate format test prepare +# How long to spend on each fuzz target. Override: make fuzz FUZZTIME=10m +FUZZTIME ?= 60s -help: - @printf '%s\n' \ - 'Usage: make ' \ - '' \ - ' generate Regenerate Go code, map helpers, and API documentation.' \ - ' format Apply the repository formatters and import cleanup.' \ - ' test Run the full Go package test suite.' \ - ' prepare Run generation, formatting, and tests in sequence.' +## all: Run generation, formatting, and tests in sequence +all: generate format test +## generate: Regenerate Go code, map helpers, and API documentation generate: go generate ./... ./maps.sh ./docs.sh +## format: Apply the repository formatters and import cleanup format: go fmt ./... +## test: Run the full Go package test suite test: go test ./... -prepare: - $(MAKE) generate - $(MAKE) format - $(MAKE) test +## test-fast: Run tests skipping the slow ones +test-fast: + go test -short ./... + +## fuzz: Fuzz each target for FUZZTIME (default 60s) +fuzz: + go test -run='^$$' -fuzz='^FuzzLoadFromData$$' -fuzztime=$(FUZZTIME) ./openapi3 + go test -run='^$$' -fuzz='^FuzzSchemaVisitJSON$$' -fuzztime=$(FUZZTIME) ./openapi3 + go test -run='^$$' -fuzz='^FuzzToV3$$' -fuzztime=$(FUZZTIME) ./openapi2conv + go test -run='^$$' -fuzz='^FuzzValidateRequest$$' -fuzztime=$(FUZZTIME) ./openapi3filter + +## help: list targets +help: + @grep -E '^## ' $(MAKEFILE_LIST) | sed 's/## / make /' diff --git a/openapi2conv/fuzz_test.go b/openapi2conv/fuzz_test.go new file mode 100644 index 000000000..8443c7dd4 --- /dev/null +++ b/openapi2conv/fuzz_test.go @@ -0,0 +1,84 @@ +package openapi2conv_test + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi2" + "github.com/getkin/kin-openapi/openapi2conv" + "github.com/getkin/kin-openapi/openapi3" +) + +// maxFuzzInput bounds fuzzer inputs so the fuzzer spends its budget on +// conversion logic rather than on allocating huge documents. +const maxFuzzInput = 32 << 10 + +// FuzzToV3 converts an arbitrary OpenAPI 2 document to OpenAPI 3 and back. +func FuzzToV3(f *testing.F) { + paths, err := filepath.Glob("../openapi2/testdata/*.json") + require.NoError(f, err) + for _, path := range paths { + data, err := os.ReadFile(path) + require.NoError(f, err) + if len(data) <= maxFuzzInput { + f.Add(data) + } + } + f.Add([]byte(`{"swagger":"2.0","info":{"title":"t","version":"1"},"paths":{}}`)) + f.Add([]byte(`{"swagger":"2.0","paths":{"/x/{id}":{"get":{"parameters":[{"name":"id","in":"path","type":"string","required":true}],"responses":{"200":{"description":"ok"}}}}}}`)) + f.Add([]byte(`{"swagger":"2.0","definitions":{"A":{"type":"object","properties":{"b":{"$ref":"#/definitions/A"}}}},"paths":{}}`)) + f.Add([]byte(`{"swagger":"2.0","consumes":["multipart/form-data"],"paths":{"/f":{"post":{"parameters":[{"name":"file","in":"formData","type":"file"}],"responses":{"200":{"description":"ok"}}}}}}`)) + + f.Fuzz(func(t *testing.T, data []byte) { + if len(data) > maxFuzzInput { + t.Skip() + } + + var doc2 openapi2.T + if err := json.Unmarshal(data, &doc2); err != nil { + t.Skip() + } + + doc3, err := openapi2conv.ToV3(&doc2) + if err != nil { + return + } + require.NotNil(t, doc3) + + roundTrip := func(doc *openapi3.T) *openapi3.T { + back2, err := openapi2conv.FromV3(doc) + if err != nil { + return nil + } + next3, err := openapi2conv.ToV3(back2) + if err != nil { + return nil + } + return next3 + } + + once := roundTrip(doc3) + if once == nil { + return + } + twice := roundTrip(once) + if twice == nil { + return + } + + if err := once.Validate(t.Context()); err != nil { + return + } + + onceJSON, err := json.Marshal(once) + require.NoError(t, err) + twiceJSON, err := json.Marshal(twice) + require.NoError(t, err) + + require.JSONEq(t, string(onceJSON), string(twiceJSON)) + }) +} diff --git a/openapi3/fuzz_test.go b/openapi3/fuzz_test.go new file mode 100644 index 000000000..2e2923417 --- /dev/null +++ b/openapi3/fuzz_test.go @@ -0,0 +1,112 @@ +package openapi3_test + +import ( + "encoding/json" + "net/url" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" +) + +// maxFuzzInput bounds fuzzer inputs so the fuzzer spends its budget on +// conversion logic rather than on allocating huge documents. +const maxFuzzInput = 32 << 10 + +// maxFuzzSeed keeps the seed corpus small enough that `go test ./...` still +// replays it on every run without noticeably slowing the suite down. +const maxFuzzSeed = 16 << 10 + +func newFuzzLoader() *openapi3.Loader { + loader := openapi3.NewLoader() + loader.IsExternalRefsAllowed = true + loader.ReadFromURIFunc = func(*openapi3.Loader, *url.URL) ([]byte, error) { return []byte(`{}`), nil } // deterministic + return loader +} + +func seedFromTestdata(f *testing.F, globs ...string) { + f.Helper() + for _, glob := range globs { + paths, err := filepath.Glob(glob) + require.NoError(f, err) + for _, path := range paths { + data, err := os.ReadFile(path) + require.NoError(f, err) + if len(data) <= maxFuzzSeed { + f.Add(data) + } + } + } +} + +// enc(dec) == enc(dec(enc(dec))) +func FuzzLoadFromData(f *testing.F) { + seedFromTestdata(f, "testdata/*.yml", "testdata/*.yaml", "testdata/*.json") + + f.Fuzz(func(t *testing.T, data []byte) { + if len(data) > maxFuzzInput { + t.Skip() + } + + loader := newFuzzLoader() + doc, err := loader.LoadFromData(data) + if err != nil { + return + } + require.NotNil(t, doc) + + // Validation runs over attacker-shaped documents: it may reject them but + // must never panic nor fail to terminate. + valid := doc.Validate(t.Context()) + + once, err := doc.MarshalJSON() + require.NoError(t, err) + + loader = newFuzzLoader() + // The loader must accept its own output + reloaded, err := loader.LoadFromData(once) + require.NoError(t, err, "\nonce: %s\nvalid: %+v\ndata:\n%s", string(once), valid, string(data)) + + twice, err := reloaded.MarshalJSON() + require.NoError(t, err) + + require.JSONEq(t, string(once), string(twice)) + + // Validation should be deterministic + require.Equal(t, valid, reloaded.Validate(t.Context())) + }) +} + +// No crash when validating values against schemas +func FuzzSchemaVisitJSON(f *testing.F) { + f.Add([]byte(`{"type":"string","minLength":1,"pattern":"^a+$"}`), []byte(`"aaa"`)) + f.Add([]byte(`{"type":"integer","multipleOf":0.01,"maximum":10}`), []byte(`3`)) + f.Add([]byte(`{"type":"array","items":{"type":"number"},"uniqueItems":true}`), []byte(`[1,2,1]`)) + f.Add([]byte(`{"type":"object","required":["a"],"additionalProperties":false}`), []byte(`{"a":null,"b":1}`)) + f.Add([]byte(`{"allOf":[{"type":"string"},{"maxLength":2}]}`), []byte(`"abc"`)) + f.Add([]byte(`{"oneOf":[{"type":"string"},{"type":"number"}],"nullable":true}`), []byte(`null`)) + f.Add([]byte(`{"type":"string","format":"date-time"}`), []byte(`"not a date"`)) + + f.Fuzz(func(t *testing.T, schemaJSON, valueJSON []byte) { + if len(schemaJSON) > maxFuzzInput || len(valueJSON) > maxFuzzInput { + t.Skip() + } + + var schema openapi3.Schema + if err := schema.UnmarshalJSON(schemaJSON); err != nil { + t.Skip() + } + + var value any + if err := json.Unmarshal(valueJSON, &value); err != nil { + t.Skip() + } + + _ = schema.VisitJSON(value) + _ = schema.VisitJSON(value, openapi3.EnableFormatValidation()) + _ = schema.VisitJSON(value, openapi3.EnableJSONSchema2020()) + }) +} diff --git a/openapi3/testdata/fuzz/FuzzLoadFromData/56b819f42562dabd b/openapi3/testdata/fuzz/FuzzLoadFromData/56b819f42562dabd new file mode 100644 index 000000000..5abb93b83 --- /dev/null +++ b/openapi3/testdata/fuzz/FuzzLoadFromData/56b819f42562dabd @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("openapi: \"3.1\"\ncomponents:\n schemas: \n 0:\n if:\n 0:\n $ref: A") diff --git a/openapi3/testdata/fuzz/FuzzLoadFromData/e9783ed365c6dc4c b/openapi3/testdata/fuzz/FuzzLoadFromData/e9783ed365c6dc4c new file mode 100644 index 000000000..25a7f3ad2 --- /dev/null +++ b/openapi3/testdata/fuzz/FuzzLoadFromData/e9783ed365c6dc4c @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("openapi: A\ncomponents:\n schemas: \n 00: \n 0:\n $ref: 00A") diff --git a/openapi3filter/fuzz_test.go b/openapi3filter/fuzz_test.go new file mode 100644 index 000000000..4c728b114 --- /dev/null +++ b/openapi3filter/fuzz_test.go @@ -0,0 +1,152 @@ +package openapi3filter_test + +import ( + "bytes" + "net/http" + "net/url" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" + "github.com/getkin/kin-openapi/openapi3filter" + "github.com/getkin/kin-openapi/routers" + "github.com/getkin/kin-openapi/routers/gorillamux" +) + +// maxFuzzInput bounds fuzzer inputs so the fuzzer spends its budget on +// conversion logic rather than on allocating huge documents. +const maxFuzzInput = 32 << 10 + +func newFuzzLoader() *openapi3.Loader { + loader := openapi3.NewLoader() + loader.IsExternalRefsAllowed = true + loader.ReadFromURIFunc = func(*openapi3.Loader, *url.URL) ([]byte, error) { return []byte(`{}`), nil } // deterministic + return loader +} + +func FuzzValidateRequest(f *testing.F) { + paths, err := filepath.Glob("../openapi3/testdata/*.yml") + require.NoError(f, err) + for _, path := range paths { + data, err := os.ReadFile(path) + require.NoError(f, err) + if len(data) <= 16<<10 { + f.Add(data, "GET", "/", "application/json", []byte(`{}`)) + } + } + + const spec = ` +openapi: 3.0.0 +info: {title: t, version: "1"} +paths: + /items/{id}: + post: + parameters: + - {name: id, in: path, required: true, schema: {type: integer}} + - {name: q, in: query, schema: {type: array, items: {type: string}}} + requestBody: + content: + application/json: {schema: {type: object, required: [n], properties: {n: {type: number}}}} + multipart/form-data: {schema: {type: object, properties: {f: {type: string, format: binary}}}} + application/x-www-form-urlencoded: {schema: {type: object, properties: {a: {type: string}}}} + responses: {"200": {description: ok}} +` + f.Add([]byte(spec), "POST", "/items/1?q=a&q=b", "application/json", []byte(`{"n":1}`)) + f.Add([]byte(spec), "POST", "/items/x", "application/json", []byte(`{"n":"nope"}`)) + f.Add([]byte(spec), "POST", "/items/1", "application/x-www-form-urlencoded", []byte(`a=b&a=c`)) + f.Add([]byte(spec), "POST", "/items/1", "multipart/form-data; boundary=b", []byte("--b\r\nContent-Disposition: form-data; name=\"f\"\r\n\r\nx\r\n--b--\r\n")) + + f.Fuzz(func(t *testing.T, data []byte, method, target, contentType string, body []byte) { + if len(data) > maxFuzzInput || len(body) > maxFuzzInput { + t.Skip() + } + if len(method) > 64 || len(target) > 4<<10 || len(contentType) > 1<<10 { + t.Skip() + } + + loader := newFuzzLoader() + doc, err := loader.LoadFromData(data) + if err != nil { + t.Skip() + } + if err = doc.Validate(t.Context()); err != nil { + t.Skip() + } + + router, err := gorillamux.NewRouter(doc) + if err != nil { + return + } + + req, err := http.NewRequest(method, target, bytes.NewReader(body)) + if err != nil { + t.Skip() + } + req.Header.Set("Content-Type", contentType) + + route, pathParams, err := router.FindRoute(req) + if err != nil { + return + } + + // A matched route must be usable by callers: every field they dereference + // has to be populated, and every path parameter of the matched template + // has to be present. + require.NotNil(t, route) + require.NotNil(t, route.Spec) + require.NotNil(t, route.PathItem) + require.NotNil(t, route.Operation) + require.Equal(t, req.Method, route.Method) + requirePathParamsCoverTemplate(t, route, pathParams) + + // No crash in here + _ = openapi3filter.ValidateRequest(t.Context(), &openapi3filter.RequestValidationInput{ + Request: req, + PathParams: pathParams, + Route: route, + Options: &openapi3filter.Options{ + MultiError: true, + AuthenticationFunc: openapi3filter.NoopAuthenticationFunc, + }, + }) + }) +} + +func requirePathParamsCoverTemplate(t *testing.T, route *routers.Route, pathParams map[string]string) { + t.Helper() + for _, name := range muxVarNames(route.Path) { + require.Contains(t, pathParams, name, "path %q", route.Path) + } +} + +// muxVarNames extracts the variable names gorilla/mux reads out of a route +// template. It must stay in sync with mux's own parsing: braces nest, so +// "{{a}}" holds the single variable "{a}", and everything after the first +// colon is a match pattern rather than part of the name. +func muxVarNames(tpl string) (names []string) { + level, idx := 0, 0 + for i := 0; i < len(tpl); i++ { + switch tpl[i] { + case '{': + if level++; level == 1 { + idx = i + } + case '}': + if level--; level == 0 { + name, _, _ := strings.Cut(tpl[idx+1:i], ":") + names = append(names, name) + } else if level < 0 { + names = nil + return + } + } + } + if level != 0 { + names = nil + } + return +} From 454df67def35700151b6a39a75a9738a65995314 Mon Sep 17 00:00:00 2001 From: Frode Aannevik Date: Fri, 23 Oct 2026 10:20:13 +0200 Subject: [PATCH 10/21] openapi3: set the failing schema on JSON Schema 2020-12 validation errors (#1378) --- openapi3/issue741_test.go | 3 + openapi3/loader_http_error_test.go | 2 + openapi3/schema_jsonschema_validator.go | 66 +++++++--- ...om_AccountService_3_openapi_yaml__validate | 5 +- ...om_AccountService_4_openapi_yaml__validate | 5 +- ...om_AccountService_5_openapi_yaml__validate | 6 +- ...om_AccountService_6_openapi_yaml__validate | 6 +- ...n_com_FundService_3_openapi_yaml__validate | 5 +- ...egalEntityService_1_openapi_yaml__validate | 4 +- ...egalEntityService_2_openapi_yaml__validate | 4 +- ...egalEntityService_3_openapi_yaml__validate | 4 +- ...tificationService_3_openapi_yaml__validate | 10 +- ...tificationService_4_openapi_yaml__validate | 10 +- ...tificationService_5_openapi_yaml__validate | 10 +- ...tificationService_6_openapi_yaml__validate | 10 +- .../exoapi_dev_1_0_0_openapi_yaml__validate | 9 +- .../rentcast_io_1_0_openapi_yaml__validate | 124 +++++++++++++++++- .../vercel_com_0_0_1_openapi_yaml__validate | 15 ++- ...ebscraping_ai_3_0_0_openapi_yaml__validate | 7 +- openapi3filter/issue1377_test.go | 53 ++++++++ 20 files changed, 326 insertions(+), 32 deletions(-) create mode 100644 openapi3filter/issue1377_test.go diff --git a/openapi3/issue741_test.go b/openapi3/issue741_test.go index f6cd672e0..21863bfe3 100644 --- a/openapi3/issue741_test.go +++ b/openapi3/issue741_test.go @@ -29,11 +29,14 @@ func TestIssue741(t *testing.T) { ts.URL, ) + readFromURI := openapi3.URIMapCache(openapi3.ReadFromHTTP(ts.Client())) + var wg sync.WaitGroup for range 10 { wg.Go(func() { loader := openapi3.NewLoader() loader.IsExternalRefsAllowed = true + loader.ReadFromURIFunc = readFromURI doc, err := loader.LoadFromData(rootSpec) require.NoError(t, err) require.NotNil(t, doc) diff --git a/openapi3/loader_http_error_test.go b/openapi3/loader_http_error_test.go index a1e51b7a2..0f886cdc6 100644 --- a/openapi3/loader_http_error_test.go +++ b/openapi3/loader_http_error_test.go @@ -48,6 +48,7 @@ func TestLoadReferenceFromRemoteURLFailsWithHttpError(t *testing.T) { loader := openapi3.NewLoader() loader.IsExternalRefsAllowed = true + loader.ReadFromURIFunc = openapi3.ReadFromHTTP(ts.Client()) _, err := loader.LoadFromDataWithPath(spec, &url.URL{Path: "testdata/testfilename.openapi.json"}) require.EqualError(t, err, fmt.Sprintf(`error resolving reference "%s/components.openapi.json#/components/headers/CustomTestHeader": error loading "%s/components.openapi.json": request returned status code 400`, ts.URL, ts.URL)) @@ -91,6 +92,7 @@ func TestLoadFromRemoteURLFailsWithHttpError(t *testing.T) { loader := openapi3.NewLoader() loader.IsExternalRefsAllowed = true + loader.ReadFromURIFunc = openapi3.ReadFromHTTP(ts.Client()) doc, err := loader.LoadFromDataWithPath(spec, &url.URL{Path: "testdata/testfilename.openapi.json"}) require.Nil(t, doc) diff --git a/openapi3/schema_jsonschema_validator.go b/openapi3/schema_jsonschema_validator.go index 515477eb7..c52480d0d 100644 --- a/openapi3/schema_jsonschema_validator.go +++ b/openapi3/schema_jsonschema_validator.go @@ -9,6 +9,7 @@ import ( "strconv" "strings" + "github.com/go-openapi/jsonpointer" "github.com/santhosh-tekuri/jsonschema/v6" "github.com/santhosh-tekuri/jsonschema/v6/kind" ) @@ -18,6 +19,8 @@ type jsonSchemaValidator struct { compiler *jsonschema.Compiler schema *jsonschema.Schema hasInternalRefs bool + root *Schema + defs map[string]*Schema } // newJSONSchemaValidator creates a new validator using JSON Schema 2020-12 @@ -36,7 +39,8 @@ func newJSONSchemaValidator(schema *Schema, settings *schemaValidationSettings) // Boolean schemas are valid JSON Schema resources as well. schemaMap, _ := schemaDocument.(map[string]any) hasInternalRefs := containsInternalSchemaRef(schemaDocument) - if err := addInternalSchemaRefs(schemaMap, schema); err != nil { + defs, err := addInternalSchemaRefs(schemaMap, schema) + if err != nil { return nil, fmt.Errorf("failed to prepare schema references: %w", err) } @@ -63,6 +67,8 @@ func newJSONSchemaValidator(schema *Schema, settings *schemaValidationSettings) compiler: compiler, schema: compiledSchema, hasInternalRefs: hasInternalRefs, + root: schema, + defs: defs, }, nil } @@ -89,11 +95,12 @@ func containsInternalSchemaRef(node any) bool { // resolved Schema. VisitJSON receives a subtree rather than the containing // OpenAPI document, so copy each resolved target into local $defs and rewrite // its reference. This also handles references to a nested property or item. -func addInternalSchemaRefs(schemaMap map[string]any, schema *Schema) error { +func addInternalSchemaRefs(schemaMap map[string]any, schema *Schema) (map[string]*Schema, error) { if schemaMap == nil || schema == nil { - return nil + return nil, nil } refs := make(map[string]any) + values := make(map[string]*Schema) // Recursive schemas reference an already-visited target, which WalkSubtree would not report. err := NewSchemaRef("", schema).walkSubtreeRefs(func(_ string, ref *SchemaRef) error { if ref.Ref == "" || !strings.HasPrefix(ref.Ref, "#/components/schemas/") || ref.Value == nil { @@ -111,25 +118,28 @@ func addInternalSchemaRefs(schemaMap map[string]any, schema *Schema) error { return err } refs[ref.Ref] = target + values[ref.Ref] = ref.Value return nil }) if err != nil { - return err + return nil, err } if len(refs) == 0 { - return nil + return nil, nil } defs := make(map[string]any, len(refs)) ids := make(map[string]string, len(refs)) + defValues := make(map[string]*Schema, len(refs)) for ref := range refs { ids[ref] = fmt.Sprintf("ref%d", len(ids)) + defValues[ids[ref]] = values[ref] } rewriteInternalSchemaRefs(schemaMap, refs, ids, defs) if len(defs) > 0 { schemaMap["$defs"] = defs rewriteInternalSchemaRefs(defs, refs, ids, defs) } - return nil + return defValues, nil } // rewriteInternalSchemaRefs rewrites document-relative component references to @@ -252,30 +262,30 @@ func validateNumberFormat(format string, settings *schemaValidationSettings, val func (v *jsonSchemaValidator) validate(settings *schemaValidationSettings, value any) error { if err := v.schema.Validate(value); err != nil { // Convert jsonschema error to SchemaError - return convertJSONSchemaError(settings, err, value) + return v.convertJSONSchemaError(settings, err, value) } return nil } // convertJSONSchemaError converts a jsonschema validation error to OpenAPI SchemaError format. // root is the validated value, from which the failing value is looked up. -func convertJSONSchemaError(settings *schemaValidationSettings, err error, root any) error { +func (v *jsonSchemaValidator) convertJSONSchemaError(settings *schemaValidationSettings, err error, root any) error { // TODO: Go 1.26 // if err, ok := errors.AsType[*jsonschema.ValidationError](err); ok { - // return formatValidationError(settings, err, "", root) + // return v.formatValidationError(settings, err, "", root) var validationErr *jsonschema.ValidationError if errors.As(err, &validationErr) { - return formatValidationError(settings, validationErr, "", root) + return v.formatValidationError(settings, validationErr, "", root) } return err } // formatValidationError recursively formats validation errors into SchemaErrors // that carry the same structure as the ones of the built-in validator: the path -// of the failing value (JSONPointer), the failing keyword (SchemaField) and the -// failing value (Value). The jsonschema error itself stays reachable with -// errors.As. -func formatValidationError(settings *schemaValidationSettings, verr *jsonschema.ValidationError, parentPath string, root any) error { +// of the failing value (JSONPointer), the failing keyword (SchemaField), the +// failing schema (Schema) and the failing value (Value). The jsonschema error +// itself stays reachable with errors.As. +func (v *jsonSchemaValidator) formatValidationError(settings *schemaValidationSettings, verr *jsonschema.ValidationError, parentPath string, root any) error { // Build the path from InstanceLocation slice path := "/" + strings.Join(verr.InstanceLocation, "/") if parentPath != "" && path != "/" { @@ -299,7 +309,7 @@ func formatValidationError(settings *schemaValidationSettings, verr *jsonschema. } value, _ := valueAt(root, failing.InstanceLocation) - schemaErr := settings.newSchemaError(nil, value). + schemaErr := settings.newSchemaError(v.schemaAt(failing.SchemaURL), value). withReason(msg.String()). withCause(verr) for _, key := range slices.Backward(failing.InstanceLocation) { @@ -313,7 +323,7 @@ func formatValidationError(settings *schemaValidationSettings, verr *jsonschema. if len(verr.Causes) > 0 { var subErrors MultiError for _, cause := range verr.Causes { - if subErr := formatValidationError(settings, cause, path, root); subErr != nil { + if subErr := v.formatValidationError(settings, cause, path, root); subErr != nil { subErrors = append(subErrors, subErr) } } @@ -325,6 +335,30 @@ func formatValidationError(settings *schemaValidationSettings, verr *jsonschema. return schemaErr } +// schemaAt is the schema at schemaURL, the location of a schema of the +// compiled document, or nil when it is not one of v's schemas. +func (v *jsonSchemaValidator) schemaAt(schemaURL string) *Schema { + _, fragment, _ := strings.Cut(schemaURL, "#") + root := v.root + if rest, ok := strings.CutPrefix(fragment, "/$defs/"); ok { + id, rest, _ := strings.Cut(rest, "/") + if root = v.defs[id]; root == nil { + return nil + } + fragment = strings.TrimSuffix("/"+rest, "/") + } + ptr, err := jsonpointer.New(fragment) + if err != nil { + return nil + } + found, _, err := ptr.Get(root) + if err != nil { + return nil + } + schema, _ := found.(*Schema) + return schema +} + // isWrapperErrorKind reports whether k only groups the errors of its causes. func isWrapperErrorKind(k jsonschema.ErrorKind) bool { switch k.(type) { diff --git a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_AccountService_3_openapi_yaml__validate b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_AccountService_3_openapi_yaml__validate index 9e1d679e8..a85b907dd 100644 --- a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_AccountService_3_openapi_yaml__validate +++ b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_AccountService_3_openapi_yaml__validate @@ -1,6 +1,9 @@ invalid paths: invalid path /closeAccount: invalid operation POST: invalid example: example generic: validation failed due to: error at "/errorCode": at '/errorCode': got number, want string Schema: - null + { + "description": "The error code mapped to the error message.", + "type": "string" + } Value: 10003 diff --git a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_AccountService_4_openapi_yaml__validate b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_AccountService_4_openapi_yaml__validate index 9e1d679e8..a85b907dd 100644 --- a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_AccountService_4_openapi_yaml__validate +++ b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_AccountService_4_openapi_yaml__validate @@ -1,6 +1,9 @@ invalid paths: invalid path /closeAccount: invalid operation POST: invalid example: example generic: validation failed due to: error at "/errorCode": at '/errorCode': got number, want string Schema: - null + { + "description": "The error code mapped to the error message.", + "type": "string" + } Value: 10003 diff --git a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_AccountService_5_openapi_yaml__validate b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_AccountService_5_openapi_yaml__validate index aca52ea9d..919417f97 100644 --- a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_AccountService_5_openapi_yaml__validate +++ b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_AccountService_5_openapi_yaml__validate @@ -1,6 +1,10 @@ invalid paths: invalid path /checkAccountHolder: invalid operation POST: invalid example: example basic: validation failed due to: error at "/tier": at '/tier': got string, want integer Schema: - null + { + "description": "The tier required for the account holder.", + "format": "int32", + "type": "integer" + } Value: "2" diff --git a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_AccountService_6_openapi_yaml__validate b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_AccountService_6_openapi_yaml__validate index aca52ea9d..919417f97 100644 --- a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_AccountService_6_openapi_yaml__validate +++ b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_AccountService_6_openapi_yaml__validate @@ -1,6 +1,10 @@ invalid paths: invalid path /checkAccountHolder: invalid operation POST: invalid example: example basic: validation failed due to: error at "/tier": at '/tier': got string, want integer Schema: - null + { + "description": "The tier required for the account holder.", + "format": "int32", + "type": "integer" + } Value: "2" diff --git a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_FundService_3_openapi_yaml__validate b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_FundService_3_openapi_yaml__validate index d4a06941f..231dd5261 100644 --- a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_FundService_3_openapi_yaml__validate +++ b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_FundService_3_openapi_yaml__validate @@ -1,6 +1,9 @@ invalid paths: invalid path /debitAccountHolder: invalid operation POST: invalid example: example debit-account-holder: validation failed due to: error at "/submittedAsync": at '/submittedAsync': got string, want boolean Schema: - null + { + "description": "Indicates whether the request is processed asynchronously. Depending on the request's platform settings, the following scenarios may be applied:\n* **true**: The request is queued and will be executed when the providing service is available in the order in which the requests are received.\n* **false**: The processing of the request is immediately attempted; it may result in an error if the providing service is unavailable.", + "type": "boolean" + } Value: "false" diff --git a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_LegalEntityService_1_openapi_yaml__validate b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_LegalEntityService_1_openapi_yaml__validate index dca7a33dc..2e436140e 100644 --- a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_LegalEntityService_1_openapi_yaml__validate +++ b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_LegalEntityService_1_openapi_yaml__validate @@ -1,6 +1,8 @@ invalid paths: invalid path /documents: invalid operation POST: invalid example: validation failed due to: at '': got number, want string Schema: - null + { + "type": "string" + } Value: 13008 diff --git a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_LegalEntityService_2_openapi_yaml__validate b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_LegalEntityService_2_openapi_yaml__validate index dca7a33dc..2e436140e 100644 --- a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_LegalEntityService_2_openapi_yaml__validate +++ b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_LegalEntityService_2_openapi_yaml__validate @@ -1,6 +1,8 @@ invalid paths: invalid path /documents: invalid operation POST: invalid example: validation failed due to: at '': got number, want string Schema: - null + { + "type": "string" + } Value: 13008 diff --git a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_LegalEntityService_3_openapi_yaml__validate b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_LegalEntityService_3_openapi_yaml__validate index dca7a33dc..2e436140e 100644 --- a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_LegalEntityService_3_openapi_yaml__validate +++ b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_LegalEntityService_3_openapi_yaml__validate @@ -1,6 +1,8 @@ invalid paths: invalid path /documents: invalid operation POST: invalid example: validation failed due to: at '': got number, want string Schema: - null + { + "type": "string" + } Value: 13008 diff --git a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_MarketPayNotificationService_3_openapi_yaml__validate b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_MarketPayNotificationService_3_openapi_yaml__validate index 77e3029b9..5c83785e4 100644 --- a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_MarketPayNotificationService_3_openapi_yaml__validate +++ b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_MarketPayNotificationService_3_openapi_yaml__validate @@ -1,6 +1,14 @@ invalid webhooks: webhook "/ACCOUNT_CLOSED": invalid operation POST: invalid example: example accountClosed: validation failed due to: at '': got string, want object Schema: - null + { + "properties": { + "notificationResponse": { + "description": "Set this parameter to **[accepted]** to acknowledge that you received a notification from Adyen.", + "type": "string" + } + }, + "type": "object" + } Value: "[accepted]" diff --git a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_MarketPayNotificationService_4_openapi_yaml__validate b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_MarketPayNotificationService_4_openapi_yaml__validate index 77e3029b9..5c83785e4 100644 --- a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_MarketPayNotificationService_4_openapi_yaml__validate +++ b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_MarketPayNotificationService_4_openapi_yaml__validate @@ -1,6 +1,14 @@ invalid webhooks: webhook "/ACCOUNT_CLOSED": invalid operation POST: invalid example: example accountClosed: validation failed due to: at '': got string, want object Schema: - null + { + "properties": { + "notificationResponse": { + "description": "Set this parameter to **[accepted]** to acknowledge that you received a notification from Adyen.", + "type": "string" + } + }, + "type": "object" + } Value: "[accepted]" diff --git a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_MarketPayNotificationService_5_openapi_yaml__validate b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_MarketPayNotificationService_5_openapi_yaml__validate index 77e3029b9..5c83785e4 100644 --- a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_MarketPayNotificationService_5_openapi_yaml__validate +++ b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_MarketPayNotificationService_5_openapi_yaml__validate @@ -1,6 +1,14 @@ invalid webhooks: webhook "/ACCOUNT_CLOSED": invalid operation POST: invalid example: example accountClosed: validation failed due to: at '': got string, want object Schema: - null + { + "properties": { + "notificationResponse": { + "description": "Set this parameter to **[accepted]** to acknowledge that you received a notification from Adyen.", + "type": "string" + } + }, + "type": "object" + } Value: "[accepted]" diff --git a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_MarketPayNotificationService_6_openapi_yaml__validate b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_MarketPayNotificationService_6_openapi_yaml__validate index 77e3029b9..5c83785e4 100644 --- a/openapi3/testdata/apis_guru_openapi_directory/adyen_com_MarketPayNotificationService_6_openapi_yaml__validate +++ b/openapi3/testdata/apis_guru_openapi_directory/adyen_com_MarketPayNotificationService_6_openapi_yaml__validate @@ -1,6 +1,14 @@ invalid webhooks: webhook "/ACCOUNT_CLOSED": invalid operation POST: invalid example: example accountClosed: validation failed due to: at '': got string, want object Schema: - null + { + "properties": { + "notificationResponse": { + "description": "Set this parameter to **[accepted]** to acknowledge that you received a notification from Adyen.", + "type": "string" + } + }, + "type": "object" + } Value: "[accepted]" diff --git a/openapi3/testdata/apis_guru_openapi_directory/exoapi_dev_1_0_0_openapi_yaml__validate b/openapi3/testdata/apis_guru_openapi_directory/exoapi_dev_1_0_0_openapi_yaml__validate index 3c74aa999..c701924a1 100644 --- a/openapi3/testdata/apis_guru_openapi_directory/exoapi_dev_1_0_0_openapi_yaml__validate +++ b/openapi3/testdata/apis_guru_openapi_directory/exoapi_dev_1_0_0_openapi_yaml__validate @@ -1,6 +1,13 @@ invalid paths: invalid path /html-renderer: invalid operation POST: invalid default: validation failed due to: at '': got number, want string Schema: - null + { + "default": 0, + "description": "PDF page margin (in px, mm, in or cm)", + "examples": [ + "1in" + ], + "type": "string" + } Value: 0 diff --git a/openapi3/testdata/apis_guru_openapi_directory/rentcast_io_1_0_openapi_yaml__validate b/openapi3/testdata/apis_guru_openapi_directory/rentcast_io_1_0_openapi_yaml__validate index ee3b991c4..4ba23441f 100644 --- a/openapi3/testdata/apis_guru_openapi_directory/rentcast_io_1_0_openapi_yaml__validate +++ b/openapi3/testdata/apis_guru_openapi_directory/rentcast_io_1_0_openapi_yaml__validate @@ -1,6 +1,128 @@ invalid paths: invalid path /avm/rent/long-term: invalid operation GET: invalid example: example Response: validation failed due to: at '': got string, want object Schema: - null + { + "properties": { + "comparables": { + "items": { + "properties": { + "addressLine1": { + "example": "7410 Adaja Riv", + "type": "string" + }, + "bathrooms": { + "default": 0, + "example": 2, + "type": "integer" + }, + "bedrooms": { + "default": 0, + "example": 4, + "type": "integer" + }, + "city": { + "example": "San Antonio", + "type": "string" + }, + "correlation": { + "default": 0, + "example": 0.988, + "type": "number" + }, + "county": { + "example": "Bexar", + "type": "string" + }, + "daysOld": { + "default": 0, + "example": 11, + "type": "integer" + }, + "distance": { + "default": 0, + "example": 0.5282, + "type": "number" + }, + "formattedAddress": { + "example": "7410 Adaja Riv, San Antonio, TX 78244", + "type": "string" + }, + "id": { + "example": "7410-Adaja-Riv,-San-Antonio,-TX-78244", + "type": "string" + }, + "lastSeenDate": { + "example": "2023-02-17T00:00:00.000Z", + "type": "string" + }, + "latitude": { + "default": 0, + "example": 29.481005, + "type": "number" + }, + "listedDate": { + "example": "2023-01-22T00:00:00.000Z", + "type": "string" + }, + "longitude": { + "default": 0, + "example": -98.344856, + "type": "number" + }, + "price": { + "default": 0, + "example": 1850, + "type": "integer" + }, + "propertyType": { + "example": "Single Family", + "type": "string" + }, + "squareFootage": { + "default": 0, + "example": 1627, + "type": "integer" + }, + "state": { + "example": "TX", + "type": "string" + }, + "zipCode": { + "example": "78244", + "type": "string" + } + }, + "type": "object" + }, + "type": "array" + }, + "latitude": { + "default": 0, + "example": 29.475962, + "type": "number" + }, + "longitude": { + "default": 0, + "example": -98.351442, + "type": "number" + }, + "rent": { + "default": 0, + "example": 1800, + "type": "integer" + }, + "rentRangeHigh": { + "default": 0, + "example": 1860, + "type": "integer" + }, + "rentRangeLow": { + "default": 0, + "example": 1740, + "type": "integer" + } + }, + "type": "object" + } Value: "{\"rent\":1800,\"rentRangeLow\":1740,\"rentRangeHigh\":1860,\"longitude\":-98.351442,\"latitude\":29.475962,\"comparables\":[{\"id\":\"7410-Adaja-Riv,-San-Antonio,-TX-78244\",\"formattedAddress\":\"7410 Adaja Riv, San Antonio, TX 78244\",\"addressLine1\":\"7410 Adaja Riv\",\"city\":\"San Antonio\",\"state\":\"TX\",\"zipCode\":\"78244\",\"county\":\"Bexar\",\"longitude\":-98.344856,\"latitude\":29.481005,\"propertyType\":\"Single Family\",\"bedrooms\":4,\"bathrooms\":2,\"squareFootage\":1627,\"price\":1850,\"listedDate\":\"2023-01-22T00:00:00.000Z\",\"lastSeenDate\":\"2023-02-17T00:00:00.000Z\",\"daysOld\":11,\"distance\":0.5282,\"correlation\":0.988},{\"id\":\"7303-Avila-Vis,-San-Antonio,-TX-78244\",\"formattedAddress\":\"7303 Avila Vis, San Antonio, TX 78244\",\"addressLine1\":\"7303 Avila Vis\",\"city\":\"San Antonio\",\"state\":\"TX\",\"zipCode\":\"78244\",\"county\":\"Bexar\",\"longitude\":-98.34565,\"latitude\":29.483196,\"propertyType\":\"Single Family\",\"bedrooms\":4,\"bathrooms\":2,\"squareFootage\":1627,\"price\":1850,\"listedDate\":\"2023-02-03T00:00:00.000Z\",\"lastSeenDate\":\"2023-02-17T00:00:00.000Z\",\"daysOld\":11,\"distance\":0.6099,\"correlation\":0.9867},{\"id\":\"7331-Avila-Vis,-San-Antonio,-TX-78244\",\"formattedAddress\":\"7331 Avila Vis, San Antonio, TX 78244\",\"addressLine1\":\"7331 Avila Vis\",\"city\":\"San Antonio\",\"state\":\"TX\",\"zipCode\":\"78244\",\"county\":\"Bexar\",\"longitude\":-98.34481,\"latitude\":29.482922,\"propertyType\":\"Single Family\",\"bedrooms\":4,\"bathrooms\":2,\"squareFootage\":1627,\"price\":1810,\"listedDate\":\"2023-02-21T00:00:00.000Z\",\"lastSeenDate\":\"2023-02-27T00:00:00.000Z\",\"daysOld\":1,\"distance\":0.6255,\"correlation\":0.9865},{\"id\":\"7411-Avila-Vis,-San-Antonio,-TX-78244\",\"formattedAddress\":\"7411 Avila Vis, San Antonio, TX 78244\",\"addressLine1\":\"7411 Avila Vis\",\"city\":\"San Antonio\",\"state\":\"TX\",\"zipCode\":\"78244\",\"county\":\"Bexar\",\"longitude\":-98.344398,\"latitude\":29.482676,\"propertyType\":\"Single Family\",\"bedrooms\":4,\"bathrooms\":2,\"squareFootage\":1627,\"price\":1815,\"listedDate\":\"2023-01-16T00:00:00.000Z\",\"lastSeenDate\":\"2023-02-10T00:00:00.000Z\",\"daysOld\":18,\"distance\":0.629,\"correlation\":0.9864},{\"id\":\"7327-Plata-Cir,-San-Antonio,-TX-78244\",\"formattedAddress\":\"7327 Plata Cir, San Antonio, TX 78244\",\"addressLine1\":\"7327 Plata Cir\",\"city\":\"San Antonio\",\"state\":\"TX\",\"zipCode\":\"78244\",\"county\":\"Bexar\",\"longitude\":-98.344406,\"latitude\":29.480621,\"propertyType\":\"Single Family\",\"bedrooms\":4,\"bathrooms\":2,\"squareFootage\":1492,\"price\":1680,\"listedDate\":\"2023-01-12T00:00:00.000Z\",\"lastSeenDate\":\"2023-02-08T00:00:00.000Z\",\"daysOld\":20,\"distance\":0.5323,\"correlation\":0.9758}]}" diff --git a/openapi3/testdata/apis_guru_openapi_directory/vercel_com_0_0_1_openapi_yaml__validate b/openapi3/testdata/apis_guru_openapi_directory/vercel_com_0_0_1_openapi_yaml__validate index 95868ff6c..0b2671730 100644 --- a/openapi3/testdata/apis_guru_openapi_directory/vercel_com_0_0_1_openapi_yaml__validate +++ b/openapi3/testdata/apis_guru_openapi_directory/vercel_com_0_0_1_openapi_yaml__validate @@ -1,6 +1,19 @@ invalid paths: invalid path /v1/teams/{teamId}/members/{uid}: invalid operation PATCH: invalid default: validation failed due to: at '': got array, want string Schema: - null + { + "default": [ + "MEMBER", + "VIEWER" + ], + "description": "The role in the team of the member.", + "examples": [ + [ + "MEMBER", + "VIEWER" + ] + ], + "type": "string" + } Value: [ diff --git a/openapi3/testdata/apis_guru_openapi_directory/webscraping_ai_3_0_0_openapi_yaml__validate b/openapi3/testdata/apis_guru_openapi_directory/webscraping_ai_3_0_0_openapi_yaml__validate index ce3debc9b..871764345 100644 --- a/openapi3/testdata/apis_guru_openapi_directory/webscraping_ai_3_0_0_openapi_yaml__validate +++ b/openapi3/testdata/apis_guru_openapi_directory/webscraping_ai_3_0_0_openapi_yaml__validate @@ -1,6 +1,11 @@ invalid components: parameter "headers": invalid example: validation failed due to: at '': got string, want object Schema: - null + { + "additionalProperties": { + "type": "string" + }, + "type": "object" + } Value: "{\"Cookie\":\"session=some_id\"}" diff --git a/openapi3filter/issue1377_test.go b/openapi3filter/issue1377_test.go new file mode 100644 index 000000000..b1aebb183 --- /dev/null +++ b/openapi3filter/issue1377_test.go @@ -0,0 +1,53 @@ +package openapi3filter + +import ( + "net/http" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestIssue1377(t *testing.T) { + t.Parallel() + + const spec = ` +openapi: 3.1.0 +info: {title: repro, version: "1"} +paths: + /config: + put: + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + interval: + type: string + enum: [day, week] + responses: + "204": {description: ok} +` + + req, err := http.NewRequest(http.MethodPut, "/config", strings.NewReader(`{"interval":"hour"}`)) + require.NoError(t, err) + req.Header.Set(headerCT, "application/json") + + router := setupTestRouter(t, spec) + route, pathParams, err := router.FindRoute(req) + require.NoError(t, err) + + err = ValidateRequest(t.Context(), &RequestValidationInput{ + Request: req, + PathParams: pathParams, + Route: route, + }) + require.Error(t, err) + + verr := ConvertErrors(err).(*ValidationError) + require.Equal(t, http.StatusUnprocessableEntity, verr.Status) + require.Equal(t, &ValidationErrorSource{Pointer: "/interval"}, verr.Source) + require.Equal(t, "value hour at /interval must be one of: day, week", verr.Detail) +} From 7dd0f6c49979f1e646f83fcf6d888990da6d17b4 Mon Sep 17 00:00:00 2001 From: Luis Figueroa <62305361+LuisFigueroaG@users.noreply.github.com> Date: Sat, 24 Oct 2026 20:15:14 -0300 Subject: [PATCH 11/21] openapi3filter: parse number values as decimal only (#1366) --- README.md | 3 +++ openapi3filter/req_resp_decoder.go | 6 +++++ openapi3filter/req_resp_decoder_test.go | 35 +++++++++++++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/README.md b/README.md index 21e4bcfba..661a1e767 100644 --- a/README.md +++ b/README.md @@ -329,6 +329,9 @@ for _, path := range doc.Paths.InMatchingOrder() { ## CHANGELOG: Sub-v1 breaking API changes +### v0.153.0 +* `openapi3filter` now parses `number` values of parameters and `application/x-www-form-urlencoded`/`multipart/form-data` bodies as decimal numbers only, so Go float literals such as `0x1p4`, `1_000`, `Inf`, `-infinity` or `NaN` are now rejected as invalid numbers. + ### v0.152.0 * JSON keys of OpenAPI documents only fill the field of the same case (`json:",case:strict"`) when `encoding/json` is implemented with `encoding/json/v2` (by default from Go 1.27, and on Go 1.25 and 1.26 with `GOEXPERIMENT=jsonv2`): a key such as `"Components"` (not `"components"`) is kept as an extension instead of filling `openapi3.T.Components`. Other builds still match keys case-insensitively, as `encoding/json` v1 does. * On Go 1.27+, unless built with `GOEXPERIMENT=nojsonv2`, `openapi2` and `openapi3` decode documents with `encoding/json/v2`: objects with duplicate member names and strings with invalid UTF-8 are rejected, and decoding errors are worded differently and locate the faulty value from the root of the document, keys of paths, responses and callbacks included (e.g. `.paths.~1pets.get.responses.200.description`). On these builds `openapi2.Responses`, `openapi3.Callback`, `openapi3.Paths` and `openapi3.Responses` also implement `encoding/json/v2.UnmarshalerFrom`. diff --git a/openapi3filter/req_resp_decoder.go b/openapi3filter/req_resp_decoder.go index 3bcfd4931..d734c6c4d 100644 --- a/openapi3filter/req_resp_decoder.go +++ b/openapi3filter/req_resp_decoder.go @@ -1259,6 +1259,12 @@ func parsePrimitiveCase(raw string, schema *openapi3.SchemaRef, typ string) (any } return v, nil case "number": + // strconv.ParseFloat also accepts Go float literals that are not decimal + // numbers: hexadecimal (0x1p4), digits separated by underscores (1_000), + // infinities and NaN. + if strings.ContainsFunc(raw, func(r rune) bool { return !strings.ContainsRune("+-.0123456789eE", r) }) { + return nil, &ParseError{Kind: KindInvalidFormat, Value: raw, Reason: "an invalid " + typ, Cause: strconv.ErrSyntax} + } v, err := strconv.ParseFloat(raw, 64) if err != nil { return nil, &ParseError{Kind: KindInvalidFormat, Value: raw, Reason: "an invalid " + typ, Cause: err.(*strconv.NumError).Err} diff --git a/openapi3filter/req_resp_decoder_test.go b/openapi3filter/req_resp_decoder_test.go index c62a9dfb3..26eac74a9 100644 --- a/openapi3filter/req_resp_decoder_test.go +++ b/openapi3filter/req_resp_decoder_test.go @@ -767,6 +767,41 @@ func TestDecodeParameter(t *testing.T) { found: true, err: &ParseError{Kind: KindInvalidFormat, Value: "foo"}, }, + { + name: "number exponent", + param: &openapi3.Parameter{Name: "param", In: "query", Schema: numberSchema}, + query: "param=-1.5E+3", + want: -1500.0, + found: true, + }, + { + name: "number hexadecimal", + param: &openapi3.Parameter{Name: "param", In: "query", Schema: numberSchema}, + query: "param=0x1p4", + found: true, + err: &ParseError{Kind: KindInvalidFormat, Value: "0x1p4"}, + }, + { + name: "number underscores", + param: &openapi3.Parameter{Name: "param", In: "query", Schema: numberSchema}, + query: "param=1_000.5", + found: true, + err: &ParseError{Kind: KindInvalidFormat, Value: "1_000.5"}, + }, + { + name: "number infinity", + param: &openapi3.Parameter{Name: "param", In: "query", Schema: numberSchema}, + query: "param=-Inf", + found: true, + err: &ParseError{Kind: KindInvalidFormat, Value: "-Inf"}, + }, + { + name: "number NaN", + param: &openapi3.Parameter{Name: "param", In: "query", Schema: numberSchema}, + query: "param=NaN", + found: true, + err: &ParseError{Kind: KindInvalidFormat, Value: "NaN"}, + }, { name: "boolean", param: &openapi3.Parameter{Name: "param", In: "query", Schema: booleanSchema}, From 5c5819ec17df42dd7c525bdaf593fe6239c24d78 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Sun, 25 Oct 2026 23:05:29 +0100 Subject: [PATCH 12/21] openapi3: marshal a ref holding neither $ref nor a value as null --- openapi3/null_ref_test.go | 37 +++++++++++++++++++++++++++++++++++++ openapi3/refs.go | 27 +++++++++++++++++++++++++++ openapi3/refs.tmpl | 3 +++ 3 files changed, 67 insertions(+) create mode 100644 openapi3/null_ref_test.go diff --git a/openapi3/null_ref_test.go b/openapi3/null_ref_test.go new file mode 100644 index 000000000..2383164bf --- /dev/null +++ b/openapi3/null_ref_test.go @@ -0,0 +1,37 @@ +package openapi3_test + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" +) + +// A null where a component belongs decodes to a ref with neither $ref nor a +// value. The loader rejects it, but a document unmarshaled directly holds it, +// and it must marshal back to null rather than dereference the missing value. +func TestRefs_NullValueMarshalsAsNull(t *testing.T) { + spec := []byte(`{ + "openapi": "3.0.0", + "info": {"title": "t", "version": "1"}, + "paths": {"/a": {"get": {"parameters": [null], "responses": {"200": null}}}}, + "components": { + "schemas": {"A": null}, + "responses": {"R": null}, + "parameters": {"P": null} + } + }`) + var doc openapi3.T + require.NoError(t, json.Unmarshal(spec, &doc)) + + once, err := json.Marshal(&doc) + require.NoError(t, err) + + var reloaded openapi3.T + require.NoError(t, json.Unmarshal(once, &reloaded)) + twice, err := json.Marshal(&reloaded) + require.NoError(t, err) + require.JSONEq(t, string(once), string(twice)) +} diff --git a/openapi3/refs.go b/openapi3/refs.go index 9fce3febf..085f85c1f 100644 --- a/openapi3/refs.go +++ b/openapi3/refs.go @@ -73,6 +73,9 @@ func (x CallbackRef) MarshalYAML() (any, error) { } return out, nil } + if x.Value == nil { + return nil, nil + } return x.Value.MarshalYAML() } @@ -235,6 +238,9 @@ func (x ExampleRef) MarshalYAML() (any, error) { } return out, nil } + if x.Value == nil { + return nil, nil + } return x.Value.MarshalYAML() } @@ -397,6 +403,9 @@ func (x HeaderRef) MarshalYAML() (any, error) { } return out, nil } + if x.Value == nil { + return nil, nil + } return x.Value.MarshalYAML() } @@ -559,6 +568,9 @@ func (x LinkRef) MarshalYAML() (any, error) { } return out, nil } + if x.Value == nil { + return nil, nil + } return x.Value.MarshalYAML() } @@ -721,6 +733,9 @@ func (x ParameterRef) MarshalYAML() (any, error) { } return out, nil } + if x.Value == nil { + return nil, nil + } return x.Value.MarshalYAML() } @@ -883,6 +898,9 @@ func (x RequestBodyRef) MarshalYAML() (any, error) { } return out, nil } + if x.Value == nil { + return nil, nil + } return x.Value.MarshalYAML() } @@ -1045,6 +1063,9 @@ func (x ResponseRef) MarshalYAML() (any, error) { } return out, nil } + if x.Value == nil { + return nil, nil + } return x.Value.MarshalYAML() } @@ -1228,6 +1249,9 @@ func (x SchemaRef) MarshalYAML() (any, error) { } return out, nil } + if x.Value == nil { + return nil, nil + } return x.Value.MarshalYAML() } @@ -1404,6 +1428,9 @@ func (x SecuritySchemeRef) MarshalYAML() (any, error) { } return out, nil } + if x.Value == nil { + return nil, nil + } return x.Value.MarshalYAML() } diff --git a/openapi3/refs.tmpl b/openapi3/refs.tmpl index 8398c302b..84076b991 100644 --- a/openapi3/refs.tmpl +++ b/openapi3/refs.tmpl @@ -108,6 +108,9 @@ func (x {{ $type.Name }}Ref) MarshalYAML() (any, error) { } return out, nil } + if x.Value == nil { + return nil, nil + } return x.Value.MarshalYAML() } From c5fa9405c759e1a0f46c6d9b517cbf72bc4774e9 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Sun, 25 Oct 2026 23:05:29 +0100 Subject: [PATCH 13/21] openapi2conv: fix nil dereferences and input mutation in FromV3 Found by seeding FuzzToV3 with the APIs-guru corpus. --- openapi2conv/openapi2_conv.go | 34 +++++++++++-------- openapi2conv/schema_conv_test.go | 56 ++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 13 deletions(-) create mode 100644 openapi2conv/schema_conv_test.go diff --git a/openapi2conv/openapi2_conv.go b/openapi2conv/openapi2_conv.go index c77072af0..38fee0cf5 100644 --- a/openapi2conv/openapi2_conv.go +++ b/openapi2conv/openapi2_conv.go @@ -565,7 +565,7 @@ func ToV3SchemaRef(schema *openapi2.SchemaRef) *openapi3.SchemaRef { } v3Schema := &openapi3.Schema{ - Extensions: schema.Value.Extensions, + Extensions: maps.Clone(schema.Value.Extensions), Type: schema.Value.Type, Title: schema.Value.Title, Format: schema.Value.Format, @@ -746,11 +746,16 @@ func ToV3SecurityScheme(securityScheme *openapi2.SecurityScheme) (*openapi3.Secu // FromV3 converts an OpenAPIv3 spec to an OpenAPIv2 spec func FromV3(doc3 *openapi3.T) (*openapi2.T, error) { - doc2Responses, err := FromV3Responses(doc3.Components.Responses, doc3.Components) + // A document need not declare components. + components := doc3.Components + if components == nil { + components = &openapi3.Components{} + } + doc2Responses, err := FromV3Responses(components.Responses, components) if err != nil { return nil, err } - schemas, parameters := FromV3Schemas(doc3.Components.Schemas, doc3.Components) + schemas, parameters := FromV3Schemas(components.Schemas, components) doc2 := &openapi2.T{ Swagger: "2.0", Info: *doc3.Info, @@ -809,7 +814,7 @@ func FromV3(doc3 *openapi3.T) (*openapi2.T, error) { } params := openapi2.Parameters{} for _, param := range pathItem.Parameters { - p, err := FromV3Parameter(param, doc3.Components) + p, err := FromV3Parameter(param, components) if err != nil { return nil, err } @@ -819,14 +824,14 @@ func FromV3(doc3 *openapi3.T) (*openapi2.T, error) { doc2.Paths[path].Parameters = params } - for name, param := range doc3.Components.Parameters { - if doc2.Parameters[name], err = FromV3Parameter(param, doc3.Components); err != nil { + for name, param := range components.Parameters { + if doc2.Parameters[name], err = FromV3Parameter(param, components); err != nil { return nil, err } } - for name, requestBodyRef := range doc3.Components.RequestBodies { - bodyOrRefParameters, formDataParameters, consumes, err := fromV3RequestBodies(name, requestBodyRef, doc3.Components) + for name, requestBodyRef := range components.RequestBodies { + bodyOrRefParameters, formDataParameters, consumes, err := fromV3RequestBodies(name, requestBodyRef, components) if err != nil { return nil, err } @@ -845,7 +850,7 @@ func FromV3(doc3 *openapi3.T) (*openapi2.T, error) { } } - if m := doc3.Components.SecuritySchemes; m != nil { + if m := components.SecuritySchemes; m != nil { doc2SecuritySchemes := make(map[string]*openapi2.SecurityScheme) for id, securityScheme := range m { v, err := FromV3SecurityScheme(securityScheme) @@ -927,6 +932,10 @@ func FromV3Schemas(schemas map[string]*openapi3.SchemaRef, components *openapi3. } func FromV3SchemaRef(schema *openapi3.SchemaRef, components *openapi3.Components) (*openapi2.SchemaRef, *openapi2.Parameter) { + // A media type need not carry a schema. + if schema == nil { + return nil, nil + } if ref := schema.Ref; ref != "" { // FromV3RequestBodyFormData (and other recursive call sites in // this file) pass components=nil when recursing into array @@ -938,7 +947,7 @@ func FromV3SchemaRef(schema *openapi3.SchemaRef, components *openapi3.Components // schema is not known locally': emit a plain $ref and move on. name := getParameterNameFromNewRef(ref) if components != nil { - if val, ok := components.Schemas[name]; ok { + if val, ok := components.Schemas[name]; ok && val != nil && val.Value != nil { if val.Value.Format == "binary" { v2Ref := strings.Replace(ref, "#/components/schemas/", "#/parameters/", 1) return nil, &openapi2.Parameter{Ref: v2Ref} @@ -987,7 +996,7 @@ func FromV3SchemaRef(schema *openapi3.SchemaRef, components *openapi3.Components } v2Schema := &openapi2.Schema{ - Extensions: schema.Value.Extensions, + Extensions: maps.Clone(schema.Value.Extensions), Type: schema.Value.Type, Title: schema.Value.Title, Format: schema.Value.Format, @@ -1040,8 +1049,7 @@ func FromV3SchemaRef(schema *openapi3.SchemaRef, components *openapi3.Components v2Schema.AllOf[i], _ = FromV3SchemaRef(v, components) } if schema.Value.PermitsNull() { - schema.Value.Nullable = false - if schema.Value.Extensions == nil { + if v2Schema.Extensions == nil { v2Schema.Extensions = make(map[string]any) } v2Schema.Extensions["x-nullable"] = true diff --git a/openapi2conv/schema_conv_test.go b/openapi2conv/schema_conv_test.go new file mode 100644 index 000000000..b215f7fba --- /dev/null +++ b/openapi2conv/schema_conv_test.go @@ -0,0 +1,56 @@ +package openapi2conv_test + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi2" + "github.com/getkin/kin-openapi/openapi2conv" + "github.com/getkin/kin-openapi/openapi3" +) + +// A media type need not carry a schema. +func TestFromV3_MediaTypeWithoutSchema(t *testing.T) { + var doc3 openapi3.T + require.NoError(t, json.Unmarshal([]byte(`{ + "openapi": "3.0.0", + "info": {"title": "t", "version": "1"}, + "paths": {"/a": {"get": {"responses": {"200": { + "description": "ok", + "content": {"application/json": {}} + }}}}} + }`), &doc3)) + + doc2, err := openapi2conv.FromV3(&doc3) + require.NoError(t, err) + require.Nil(t, doc2.Paths["/a"].Get.Responses.Value("200").Schema) +} + +// Converting a nullable schema either way leaves the source document as it +// was: converting it again gives the same result. +func TestSchemaConversion_LeavesSourceUnchanged(t *testing.T) { + var doc2 openapi2.T + require.NoError(t, json.Unmarshal([]byte(`{ + "swagger": "2.0", + "info": {"title": "t", "version": "1"}, + "paths": {}, + "definitions": {"A": {"type": "string", "x-nullable": true, "x-other": 1}} + }`), &doc2)) + + doc3, err := openapi2conv.ToV3(&doc2) + require.NoError(t, err) + require.Equal(t, true, doc2.Definitions["A"].Value.Extensions["x-nullable"]) + require.True(t, doc3.Components.Schemas["A"].Value.Nullable) + + back, err := openapi2conv.FromV3(doc3) + require.NoError(t, err) + require.Equal(t, true, back.Definitions["A"].Value.Extensions["x-nullable"]) + require.True(t, doc3.Components.Schemas["A"].Value.Nullable) + require.NotContains(t, doc3.Components.Schemas["A"].Value.Extensions, "x-nullable") + + again, err := openapi2conv.FromV3(doc3) + require.NoError(t, err) + require.Equal(t, back.Definitions, again.Definitions) +} From 5a3af7827b759e81bfc62a3ee580f21754e34930 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Sun, 25 Oct 2026 23:05:30 +0100 Subject: [PATCH 14/21] fuzz: seed from all of testdata and the APIs-guru corpus, cover the YAML path Seeds: - Walk testdata trees rather than globbing their top level, so specs in origin/, issue*/, relativeDocs/ and the like seed the fuzzers too: FuzzLoadFromData goes from 51 seeds to 161. - Add the APIs-guru corpus documents under 16 KiB (about 280 OpenAPI 3 and 550 Swagger 2) when the TestV{2,3}ApisGuruOpenapiDirectory tests have downloaded it, outside -short. FuzzLoadFromData reaches 449. - Add YAML-specific seeds: anchors and aliases, merge keys, tags, block scalars, YAML 1.1 implicit resolution, flow style and CRLF. Targets: - FuzzLoadWithOrigins: asking for origins changes nothing about the document, and every recorded position lies inside the input. - FuzzYAMLMatchesJSON: a JSON document decodes the same through the YAML path, and any document survives being re-encoded as YAML. - FuzzUnmarshalFromData: an openapi2 document loads back to itself from its own JSON and YAML forms. internal/fuzzing holds what targets in several packages share: finding seeds, and telling which documents the YAML emitter cannot write back. The fuzz workflow runs the new targets and restores the corpus from the go workflow's cache, fetching it when that is missing. --- .github/workflows/fuzz.yml | 23 ++++ Makefile | 3 + internal/fuzzing/fuzzing.go | 119 ++++++++++++++++++++ openapi2/fuzz_test.go | 78 +++++++++++++ openapi2conv/fuzz_test.go | 9 ++ openapi3/fuzz_test.go | 141 ++++++++++++++++++++--- openapi3/yaml_fuzz_test.go | 218 ++++++++++++++++++++++++++++++++++++ openapi3filter/fuzz_test.go | 15 +-- 8 files changed, 578 insertions(+), 28 deletions(-) create mode 100644 internal/fuzzing/fuzzing.go create mode 100644 openapi2/fuzz_test.go create mode 100644 openapi3/yaml_fuzz_test.go diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml index 4c920d2ac..c9202215a 100644 --- a/.github/workflows/fuzz.yml +++ b/.github/workflows/fuzz.yml @@ -25,6 +25,8 @@ jobs: # must not hold a runner forever. timeout-minutes: 45 env: + # Keep in sync with the go workflow, whose cache this restores. + APISGURU_COMMIT: f7207cf0a5c56081d275ebae4cf615249323385d GO111MODULE: 'on' CGO_ENABLED: '0' strategy: @@ -33,7 +35,10 @@ jobs: matrix: include: - {package: ./openapi3, target: FuzzLoadFromData} + - {package: ./openapi3, target: FuzzLoadWithOrigins} + - {package: ./openapi3, target: FuzzYAMLMatchesJSON} - {package: ./openapi3, target: FuzzSchemaVisitJSON} + - {package: ./openapi2, target: FuzzUnmarshalFromData} - {package: ./openapi2conv, target: FuzzToV3} - {package: ./openapi3filter, target: FuzzValidateRequest} steps: @@ -44,6 +49,24 @@ jobs: - uses: actions/checkout@v6 + # The APIs-guru corpus seeds the fuzzers alongside testdata. The go + # workflow caches it, so fetch it only when that cache is missing. + - name: APIs-guru corpus cache + id: apisguru + uses: actions/cache/restore@v4 + with: + path: | + openapi2/testdata/APIs-guru-openapi-directory-* + openapi3/testdata/APIs-guru-openapi-directory-* + key: suite-${{ env.APISGURU_COMMIT }} + + - name: Fetch APIs-guru corpus + if: steps.apisguru.outputs.cache-hit != 'true' + run: | + dir=openapi3/testdata/APIs-guru-openapi-directory-${APISGURU_COMMIT:0:7} + mkdir -p "$dir" + curl -fsSL "https://github.com/APIs-guru/openapi-directory/tarball/$APISGURU_COMMIT" | tar -xz -C "$dir" + # The generated corpus lives under $GOCACHE/fuzz and is what makes fuzzing # cumulative: without it every run restarts from the seed corpus. The key # carries the run id so each run writes a fresh entry, and restore-keys diff --git a/Makefile b/Makefile index 8fc32f5e5..2978ca959 100644 --- a/Makefile +++ b/Makefile @@ -28,7 +28,10 @@ test-fast: ## fuzz: Fuzz each target for FUZZTIME (default 60s) fuzz: go test -run='^$$' -fuzz='^FuzzLoadFromData$$' -fuzztime=$(FUZZTIME) ./openapi3 + go test -run='^$$' -fuzz='^FuzzLoadWithOrigins$$' -fuzztime=$(FUZZTIME) ./openapi3 + go test -run='^$$' -fuzz='^FuzzYAMLMatchesJSON$$' -fuzztime=$(FUZZTIME) ./openapi3 go test -run='^$$' -fuzz='^FuzzSchemaVisitJSON$$' -fuzztime=$(FUZZTIME) ./openapi3 + go test -run='^$$' -fuzz='^FuzzUnmarshalFromData$$' -fuzztime=$(FUZZTIME) ./openapi2 go test -run='^$$' -fuzz='^FuzzToV3$$' -fuzztime=$(FUZZTIME) ./openapi2conv go test -run='^$$' -fuzz='^FuzzValidateRequest$$' -fuzztime=$(FUZZTIME) ./openapi3filter diff --git a/internal/fuzzing/fuzzing.go b/internal/fuzzing/fuzzing.go new file mode 100644 index 000000000..ad8bf19d9 --- /dev/null +++ b/internal/fuzzing/fuzzing.go @@ -0,0 +1,119 @@ +// Package fuzzing holds what the fuzz targets share: finding the documents +// they seed their corpus with, and telling which documents survive YAML. +package fuzzing + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +// MaxSize keeps the seed corpus small enough that `go test ./...` still +// replays it on every run without noticeably slowing the suite down. +const MaxSize = 16 << 10 + +// Testdata returns every spec under the given testdata roots, however deep, +// so documents kept beside the tests that load them seed the fuzzers too. +// Fuzz corpora are skipped, and so is the downloaded APIs-guru directory, +// which APIsGuru reads. +func Testdata(tb testing.TB, roots ...string) [][]byte { + tb.Helper() + var seeds [][]byte + for _, root := range roots { + seeds = append(seeds, walk(tb, root, func(name string) bool { + return name == "fuzz" || strings.HasPrefix(name, apisGuruPrefix) + }, nil)...) + } + return seeds +} + +const apisGuruPrefix = "APIs-guru-openapi-directory-" + +// APIsGuru returns the documents of the APIs-guru corpus under testdata whose +// top-level keys include marker, e.g. "openapi:" or "swagger:". The corpus is +// downloaded by the TestV{2,3}ApisGuruOpenapiDirectory tests, and when it is +// absent this returns nothing. Under -short, which skips those tests, it +// returns nothing either, keeping the short suite short. +func APIsGuru(tb testing.TB, testdata, marker string) [][]byte { + tb.Helper() + if testing.Short() { + return nil + } + dirs, err := filepath.Glob(filepath.Join(testdata, apisGuruPrefix+"*")) + if err != nil { + tb.Fatal(err) + } + var seeds [][]byte + for _, dir := range dirs { + // The tarball's own top-level directory carries the same name. + root := filepath.Join(dir, filepath.Base(dir), "APIs") + seeds = append(seeds, walk(tb, root, nil, func(data []byte) bool { + return bytes.HasPrefix(data, []byte(marker)) || bytes.Contains(data, []byte("\n"+marker)) + })...) + } + return seeds +} + +func walk(tb testing.TB, root string, skipDir func(string) bool, keep func([]byte) bool) [][]byte { + tb.Helper() + var seeds [][]byte + if err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + if path != root && skipDir != nil && skipDir(d.Name()) { + return filepath.SkipDir + } + return nil + } + switch filepath.Ext(path) { + case ".yml", ".yaml", ".json": + default: + return nil + } + if info, err := d.Info(); err != nil || info.Size() > MaxSize { + return err + } + data, err := os.ReadFile(path) + if err != nil { + return err + } + if keep == nil || keep(data) { + seeds = append(seeds, data) + } + return nil + }); err != nil { + tb.Fatal(err) + } + return seeds +} + +// LosesInYAML reports whether v, decoded from JSON, holds something the YAML +// emitter does not write back faithfully, so a round trip through YAML text +// cannot be expected to preserve it: +// +// - a "<<" key, which is written unquoted and so reads back as a merge; +// - a string starting with a line break, which is written as a block +// scalar that drops it. +func LosesInYAML(v any) bool { + switch v := v.(type) { + case string: + return strings.HasPrefix(v, "\n") || strings.HasPrefix(v, "\r") + case map[string]any: + for k, e := range v { + if k == "<<" || LosesInYAML(k) || LosesInYAML(e) { + return true + } + } + case []any: + for _, e := range v { + if LosesInYAML(e) { + return true + } + } + } + return false +} diff --git a/openapi2/fuzz_test.go b/openapi2/fuzz_test.go new file mode 100644 index 000000000..764a961ce --- /dev/null +++ b/openapi2/fuzz_test.go @@ -0,0 +1,78 @@ +package openapi2 + +import ( + "encoding/json" + "testing" + + "github.com/oasdiff/yaml" + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/internal/fuzzing" +) + +// maxFuzzInput bounds fuzzer inputs so the fuzzer spends its budget on +// decoding rather than on allocating huge documents. +const maxFuzzInput = 32 << 10 + +// A swagger document, JSON or YAML, loads back to itself from its own JSON and +// YAML forms. +func FuzzUnmarshalFromData(f *testing.F) { + for _, seed := range fuzzing.Testdata(f, "testdata") { + f.Add(seed) + } + // Read from openapi3, the one copy the fuzz workflow fetches. + for _, seed := range fuzzing.APIsGuru(f, "../openapi3/testdata", "swagger:") { + f.Add(seed) + } + f.Add([]byte(`swagger: "2.0" +info: {title: t, version: "1.0"} +x-date: 2020-06-11 +paths: + /a/{id}: + get: + parameters: + - &id {name: id, in: path, required: true, type: string} + responses: + 200: &ok {description: ok, schema: {type: integer, enum: [0x2A, 4_2]}} + 201: *ok + /b: + <<: {get: {responses: {default: {description: d}}}} +definitions: + A: + type: object + x-keys: {18_24: a, 1.5: b} + properties: {b: {$ref: '#/definitions/A'}} +`)) + + f.Fuzz(func(t *testing.T, data []byte) { + if len(data) > maxFuzzInput { + t.Skip() + } + + var doc T + if unmarshal(data, &doc) != nil { + return + } + once, err := json.Marshal(&doc) + require.NoError(t, err) + + var fromJSON T + require.NoError(t, unmarshal(once, &fromJSON), "json:\n%s", once) + twice, err := json.Marshal(&fromJSON) + require.NoError(t, err) + require.JSONEq(t, string(once), string(twice)) + + var generic any + require.NoError(t, json.Unmarshal(once, &generic)) + if fuzzing.LosesInYAML(generic) { + return + } + y, err := yaml.Marshal(&doc) + require.NoError(t, err) + var fromYAML T + require.NoError(t, unmarshal(y, &fromYAML), "yaml:\n%s", y) + thrice, err := json.Marshal(&fromYAML) + require.NoError(t, err) + require.JSONEq(t, string(once), string(thrice), "yaml:\n%s", y) + }) +} diff --git a/openapi2conv/fuzz_test.go b/openapi2conv/fuzz_test.go index 8443c7dd4..3d214d312 100644 --- a/openapi2conv/fuzz_test.go +++ b/openapi2conv/fuzz_test.go @@ -6,8 +6,10 @@ import ( "path/filepath" "testing" + "github.com/oasdiff/yaml" "github.com/stretchr/testify/require" + "github.com/getkin/kin-openapi/internal/fuzzing" "github.com/getkin/kin-openapi/openapi2" "github.com/getkin/kin-openapi/openapi2conv" "github.com/getkin/kin-openapi/openapi3" @@ -28,6 +30,13 @@ func FuzzToV3(f *testing.F) { f.Add(data) } } + // The corpus is YAML, which this target does not decode, so it goes in as + // JSON. It is read from openapi3, the one copy the fuzz workflow fetches. + for _, seed := range fuzzing.APIsGuru(f, "../openapi3/testdata", "swagger:") { + if data, err := yaml.YAMLToJSON(seed); err == nil && len(data) <= maxFuzzInput { + f.Add(data) + } + } f.Add([]byte(`{"swagger":"2.0","info":{"title":"t","version":"1"},"paths":{}}`)) f.Add([]byte(`{"swagger":"2.0","paths":{"/x/{id}":{"get":{"parameters":[{"name":"id","in":"path","type":"string","required":true}],"responses":{"200":{"description":"ok"}}}}}}`)) f.Add([]byte(`{"swagger":"2.0","definitions":{"A":{"type":"object","properties":{"b":{"$ref":"#/definitions/A"}}}},"paths":{}}`)) diff --git a/openapi3/fuzz_test.go b/openapi3/fuzz_test.go index 2e2923417..c87c875c7 100644 --- a/openapi3/fuzz_test.go +++ b/openapi3/fuzz_test.go @@ -3,12 +3,13 @@ package openapi3_test import ( "encoding/json" "net/url" - "os" - "path/filepath" + "reflect" + "strings" "testing" "github.com/stretchr/testify/require" + "github.com/getkin/kin-openapi/internal/fuzzing" "github.com/getkin/kin-openapi/openapi3" ) @@ -16,10 +17,6 @@ import ( // conversion logic rather than on allocating huge documents. const maxFuzzInput = 32 << 10 -// maxFuzzSeed keeps the seed corpus small enough that `go test ./...` still -// replays it on every run without noticeably slowing the suite down. -const maxFuzzSeed = 16 << 10 - func newFuzzLoader() *openapi3.Loader { loader := openapi3.NewLoader() loader.IsExternalRefsAllowed = true @@ -27,24 +24,24 @@ func newFuzzLoader() *openapi3.Loader { return loader } -func seedFromTestdata(f *testing.F, globs ...string) { +// seedFromTestdata adds the YAML-specific seeds and the specs under testdata, +// then those of the APIs-guru corpus when it has been downloaded. +func seedFromTestdata(f *testing.F) { f.Helper() - for _, glob := range globs { - paths, err := filepath.Glob(glob) - require.NoError(f, err) - for _, path := range paths { - data, err := os.ReadFile(path) - require.NoError(f, err) - if len(data) <= maxFuzzSeed { - f.Add(data) - } - } + for _, seed := range openapi3.YAMLFuzzSeeds { + f.Add([]byte(seed)) + } + for _, seed := range fuzzing.Testdata(f, "testdata") { + f.Add(seed) + } + for _, seed := range fuzzing.APIsGuru(f, "testdata", "openapi:") { + f.Add(seed) } } // enc(dec) == enc(dec(enc(dec))) func FuzzLoadFromData(f *testing.F) { - seedFromTestdata(f, "testdata/*.yml", "testdata/*.yaml", "testdata/*.json") + seedFromTestdata(f) f.Fuzz(func(t *testing.T, data []byte) { if len(data) > maxFuzzInput { @@ -80,6 +77,114 @@ func FuzzLoadFromData(f *testing.F) { }) } +// Asking for origins changes nothing about the document, and every recorded +// position lies inside the input. +func FuzzLoadWithOrigins(f *testing.F) { + seedFromTestdata(f) + + f.Fuzz(func(t *testing.T, data []byte) { + if len(data) > maxFuzzInput { + t.Skip() + } + + plain, plainErr := newFuzzLoader().LoadFromData(data) + + loader := newFuzzLoader() + loader.IncludeOrigin = true + withOrigins, err := loader.LoadFromData(data) + require.Equal(t, plainErr == nil, err == nil, "without origins: %v\nwith origins: %v", plainErr, err) + if err != nil { + return + } + + a, err := plain.MarshalJSON() + require.NoError(t, err) + b, err := withOrigins.MarshalJSON() + require.NoError(t, err) + require.JSONEq(t, string(a), string(b)) + + lines := yamlLines(data) + checkLocation := func(what string, loc openapi3.Location) { + if loc.File != "" || loc.Line == 0 { + return // another document, or not recorded + } + require.GreaterOrEqual(t, loc.Line, 1, what) + require.LessOrEqual(t, loc.Line, lines, what) + require.GreaterOrEqual(t, loc.Column, 1, what) + if loc.EndLine != 0 { + require.GreaterOrEqual(t, loc.EndLine, loc.Line, "%s: %+v", what, loc) + require.LessOrEqual(t, loc.EndLine, lines, "%s: %+v", what, loc) + require.GreaterOrEqual(t, loc.EndColumn, 1, "%s: %+v", what, loc) + } + } + walkOrigins(withOrigins, func(o *openapi3.Origin) { + if o.Key != nil { + checkLocation("key", *o.Key) + } + for _, loc := range o.Fields { + checkLocation("field "+loc.Name, loc) + } + for name, locs := range o.Sequences { + for _, loc := range locs { + checkLocation("sequence "+name, loc) + } + } + }) + }) +} + +// yamlLines counts lines the way the YAML parser numbers them, which breaks +// on CR, LF, CRLF, NEL, LS and PS alike. +func yamlLines(data []byte) int { + s := strings.ReplaceAll(string(data), "\r\n", "\n") + return 1 + strings.Count(s, "\n") + strings.Count(s, "\r") + + strings.Count(s, "\u0085") + strings.Count(s, "\u2028") + strings.Count(s, "\u2029") +} + +var originType = reflect.TypeFor[openapi3.Origin]() + +// walkOrigins calls fn on every *Origin reachable from v, unexported fields +// included since that is where Paths and Responses keep their entries. A +// resolved document is a graph, so each pointer is visited once. +func walkOrigins(v any, fn func(*openapi3.Origin)) { + seen := make(map[uintptr]struct{}) + var walk func(reflect.Value) + walk = func(v reflect.Value) { + switch v.Kind() { + case reflect.Pointer: + if v.IsNil() { + return + } + if _, ok := seen[v.Pointer()]; ok { + return + } + seen[v.Pointer()] = struct{}{} + if v.Type().Elem() == originType { + fn((*openapi3.Origin)(v.UnsafePointer())) + return + } + walk(v.Elem()) + case reflect.Interface: + if !v.IsNil() { + walk(v.Elem()) + } + case reflect.Struct: + for i := range v.NumField() { + walk(v.Field(i)) + } + case reflect.Slice, reflect.Array: + for i := range v.Len() { + walk(v.Index(i)) + } + case reflect.Map: + for it := v.MapRange(); it.Next(); { + walk(it.Value()) + } + } + } + walk(reflect.ValueOf(v)) +} + // No crash when validating values against schemas func FuzzSchemaVisitJSON(f *testing.F) { f.Add([]byte(`{"type":"string","minLength":1,"pattern":"^a+$"}`), []byte(`"aaa"`)) diff --git a/openapi3/yaml_fuzz_test.go b/openapi3/yaml_fuzz_test.go new file mode 100644 index 000000000..d3c5f5a2f --- /dev/null +++ b/openapi3/yaml_fuzz_test.go @@ -0,0 +1,218 @@ +package openapi3 + +import ( + "encoding/json" + "testing" + + "github.com/oasdiff/yaml" + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/internal/fuzzing" +) + +// maxYAMLFuzzInput bounds fuzzer inputs, see maxFuzzInput. +const maxYAMLFuzzInput = 32 << 10 + +// yamlFuzzSeeds exercise what only the YAML decode path sees: anchors and +// aliases, merge keys, tags, block scalars, non-string keys and YAML 1.1 +// implicit resolution. Shared by every fuzz target that loads documents. +var yamlFuzzSeeds = []string{ + // Anchors and aliases, on a schema, a parameter and a $ref. + `openapi: 3.0.0 +info: {title: t, version: "1"} +paths: + /a/{id}: + parameters: + - &id {name: id, in: path, required: true, schema: {type: string}} + get: + parameters: [*id] + responses: + 200: &ok + description: ok + content: + application/json: + schema: &s {type: object, properties: {a: {type: integer}}} + 201: *ok +components: + schemas: + A: *s + B: {$ref: &r '#/components/schemas/A'} + C: {$ref: *r} +`, + // Merge keys: in a path item, in Paths itself, from a sequence of sources. + `openapi: 3.0.0 +info: {title: t, version: "1"} +x-base: &base + description: base + responses: {"200": {description: ok}} +x-more: &more + summary: more + description: overridden +paths: + /m: + get: + <<: [*base, *more] + operationId: m + <<: {/n: {get: {responses: {default: {description: d}}}}} +components: + schemas: + S: + <<: {type: object, required: [a]} + properties: {a: {type: string}} +`, + // Block scalars, including one ending the block, and comments. + `openapi: 3.0.0 +info: + title: t + version: "1" + description: | + line one + + line three +paths: + /b: + get: + responses: + "200": + description: >- + folded + text # not a comment + # trailing comment +`, + // YAML 1.1 implicit resolution: timestamps, hex, underscores, bools, + // non-string keys and a tagged scalar. + `openapi: 3.0.0 +info: {title: t, version: 1.0} +paths: {} +components: + schemas: + T: + type: string + format: date-time + example: 2020-06-11T16:32:50-03:00 + x-ts: !!timestamp 2001-01-01 + x-keys: {18_24: a, 0x2A: b, 1.5: c, true: d} + I: + type: integer + enum: [0x2A, 4_2, 052, 42] + example: 42 + default: 0x2A + nullable: yes + maximum: 1e3 +`, + // OpenAPI 3.1: boolean schemas, type lists, exclusive bounds as numbers, + // explicitly null responses, webhooks. + `openapi: 3.1.0 +info: {title: t, version: "1"} +paths: + /w: + get: + responses: ~ +webhooks: + hook: + post: + requestBody: + content: + application/json: + schema: + type: [object, "null"] + additionalProperties: false + unevaluatedProperties: true + properties: + n: {type: number, exclusiveMinimum: 0, exclusiveMaximum: 10} + any: true + none: false +`, + // Flow style, CRLF line endings, a document marker. + "---\r\n{openapi: 3.0.0, info: {title: t, version: \"1\"},\r\n paths: {/f: {get: {responses: {\"200\": {description: ok}}}}}}\r\n", + // $ref siblings and extensions on maplike collections. + `openapi: 3.1.0 +info: {title: t, version: "1"} +paths: + x-p: 1 + /r: + $ref: '#/components/pathItems/R' + summary: s +components: + pathItems: + R: + get: + responses: + x-r: 2 + "200": {$ref: '#/components/responses/OK', description: d} + callbacks: + cb: + x-c: 3 + '{$request.body#/u}': {post: {responses: {"200": {description: ok}}}} + responses: + OK: {description: ok} +`, +} + +// decodeYAML decodes data the way unmarshal does once JSON has refused it. +func decodeYAML(data []byte, v any) error { + _, err := yaml.Unmarshal(data, v, yaml.DecodeOpts{DisableTimestamps: true}) + return err +} + +func requireSameDoc(t *testing.T, expected, actual *T, msgAndArgs ...any) { + t.Helper() + e, err := expected.MarshalJSON() + require.NoError(t, err) + a, err := actual.MarshalJSON() + require.NoError(t, err) + require.JSONEq(t, string(e), string(a), msgAndArgs...) +} + +// The YAML decode path must agree with the JSON one: +// - a JSON document both paths accept decodes to the same thing, and +// - any document, re-encoded as block YAML, decodes back to itself. +func FuzzYAMLMatchesJSON(f *testing.F) { + for _, seed := range yamlFuzzSeeds { + f.Add([]byte(seed)) + } + for _, seed := range fuzzing.Testdata(f, "testdata") { + f.Add(seed) + } + for _, seed := range fuzzing.APIsGuru(f, "testdata", "openapi:") { + f.Add(seed) + } + f.Add([]byte(`{"openapi":"3.0.0","info":{"title":"t","version":"1"},"paths":{"/a":{"get":{"responses":{"200":{"description":"ok"}}}}}}`)) + f.Add([]byte(`{"openapi":"3.0.0","paths":{"x-a":{"<<":1},"/x":{}},"components":{"schemas":{"<<":{"type":"string"}}}}`)) + f.Add([]byte(`{"openapi":"3.1.0","components":{"schemas":{"a":{"enum":[1,1.5,"1",true,null],"example":12345678901234567890,"x-e":{"1":[{}]}}}}}`)) + f.Add([]byte(`{"openapi":"3.0.0","info":{"title":"é\/\t","version":"2020-01-01"},"paths":{}}`)) + + f.Fuzz(func(t *testing.T, data []byte) { + if len(data) > maxYAMLFuzzInput { + t.Skip() + } + + if json.Valid(data) { + var viaJSON, viaYAML T + if unmarshalJSON(data, &viaJSON) == nil && decodeYAML(data, &viaYAML) == nil { + requireSameDoc(t, &viaJSON, &viaYAML, "data:\n%s", data) + } + } + + var doc T + if decodeYAML(data, &doc) != nil { + return + } + j, err := doc.MarshalJSON() + require.NoError(t, err) + var generic any + require.NoError(t, json.Unmarshal(j, &generic)) + if fuzzing.LosesInYAML(generic) { + return + } + y, err := yaml.JSONToYAML(j) + require.NoError(t, err) + + var back T + require.NoError(t, decodeYAML(y, &back), "yaml:\n%s", y) + requireSameDoc(t, &doc, &back, "yaml:\n%s\ndata:\n%s", y, data) + }) +} + +// YAMLFuzzSeeds exposes yamlFuzzSeeds to the external test package. +var YAMLFuzzSeeds = yamlFuzzSeeds diff --git a/openapi3filter/fuzz_test.go b/openapi3filter/fuzz_test.go index 4c728b114..5f257dde8 100644 --- a/openapi3filter/fuzz_test.go +++ b/openapi3filter/fuzz_test.go @@ -4,13 +4,12 @@ import ( "bytes" "net/http" "net/url" - "os" - "path/filepath" "strings" "testing" "github.com/stretchr/testify/require" + "github.com/getkin/kin-openapi/internal/fuzzing" "github.com/getkin/kin-openapi/openapi3" "github.com/getkin/kin-openapi/openapi3filter" "github.com/getkin/kin-openapi/routers" @@ -29,14 +28,10 @@ func newFuzzLoader() *openapi3.Loader { } func FuzzValidateRequest(f *testing.F) { - paths, err := filepath.Glob("../openapi3/testdata/*.yml") - require.NoError(f, err) - for _, path := range paths { - data, err := os.ReadFile(path) - require.NoError(f, err) - if len(data) <= 16<<10 { - f.Add(data, "GET", "/", "application/json", []byte(`{}`)) - } + seeds := fuzzing.Testdata(f, "../openapi3/testdata", "testdata") + seeds = append(seeds, fuzzing.APIsGuru(f, "../openapi3/testdata", "openapi:")...) + for _, seed := range seeds { + f.Add(seed, "GET", "/", "application/json", []byte(`{}`)) } const spec = ` From 8424c1606adf2b5b044b45787606e08a8d1ba825 Mon Sep 17 00:00:00 2001 From: Yi-111-a Date: Thu, 22 Oct 2026 12:47:08 +0800 Subject: [PATCH 15/21] openapi3filter: decode query, path, header and cookie params with typeless const/enum schemas Closes #1367 --- openapi3filter/issue1367_test.go | 137 +++++++++++++++++++++++++++++ openapi3filter/req_resp_decoder.go | 59 ++++++++++++- 2 files changed, 195 insertions(+), 1 deletion(-) create mode 100644 openapi3filter/issue1367_test.go diff --git a/openapi3filter/issue1367_test.go b/openapi3filter/issue1367_test.go new file mode 100644 index 000000000..e30ba877e --- /dev/null +++ b/openapi3filter/issue1367_test.go @@ -0,0 +1,137 @@ +package openapi3filter + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestIssue1367TypelessConstAndEnumPrimitives(t *testing.T) { + t.Parallel() + + const spec = ` +openapi: 3.1.0 +info: + title: repro + version: 1.0.0 +paths: + /items: + get: + parameters: + - name: kind + in: query + required: true + schema: + const: widget + - name: withenum + in: query + schema: + enum: [widget] + responses: + '200': + description: ok +` + + testcases := []struct { + url string + fail bool + }{{ + url: "?kind=widget", + }, { + url: "?kind=widget&withenum=widget", + }, { + url: "?kind=widget&withenum=gadget", + fail: true, + }, { + url: "?kind=gadget", + fail: true, + }, { + url: "?kind=widget&withenum=", + fail: true, + }} + for _, tc := range testcases { + t.Run(tc.url, func(t *testing.T) { + req, err := http.NewRequest(http.MethodGet, "/items"+tc.url, nil) + require.NoError(t, err) + + router := setupTestRouter(t, spec) + require.NoError(t, err) + route, pathParams, err := router.FindRoute(req) + require.NoError(t, err) + + err = ValidateRequest(t.Context(), &RequestValidationInput{ + Request: req, + PathParams: pathParams, + Route: route, + }) + if tc.fail { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestIssue1367TypelessConstAndEnumNumberAndBoolean(t *testing.T) { + t.Parallel() + + const spec = ` +openapi: 3.1.0 +info: + title: repro + version: 1.0.0 +paths: + /picked: + get: + parameters: + - name: rank + in: query + required: true + schema: + enum: [1, 2, 3] + - name: flag + in: query + schema: + const: true + responses: + '200': + description: ok +` + + testcases := []struct { + url string + fail bool + }{{ + url: "?rank=2&flag=true", + }, { + url: "?rank=2&flag=false", + fail: true, + }, { + url: "?rank=9&flag=true", + fail: true, + }} + for _, tc := range testcases { + t.Run(tc.url, func(t *testing.T) { + req, err := http.NewRequest(http.MethodGet, "/picked"+tc.url, nil) + require.NoError(t, err) + + router := setupTestRouter(t, spec) + require.NoError(t, err) + route, pathParams, err := router.FindRoute(req) + require.NoError(t, err) + + err = ValidateRequest(t.Context(), &RequestValidationInput{ + Request: req, + PathParams: pathParams, + Route: route, + }) + if tc.fail { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/openapi3filter/req_resp_decoder.go b/openapi3filter/req_resp_decoder.go index d734c6c4d..51b2ff050 100644 --- a/openapi3filter/req_resp_decoder.go +++ b/openapi3filter/req_resp_decoder.go @@ -338,6 +338,14 @@ func decodeValueGuarded(dec valueDecoder, param string, sm *openapi3.Serializati return nil, found, errors.New("not implemented: decoding 'not'") } + if schema.Value.Type == nil && (schema.Value.Const != nil || len(schema.Value.Enum) > 0) { + // A schema without `type` is valid in OpenAPI 3.0 and 3.1; with + // only `const`/`enum` the schema types the value on its own (see + // #1367). Parameter values are strings, so decode as a primitive + // and let schema validation apply the constrained values. + return dec.DecodePrimitive(param, sm, schema) + } + if schema.Value.Type != nil { var decodeFn func(param string, sm *openapi3.SerializationMethod, schema *openapi3.SchemaRef) (any, bool, error) switch { @@ -1232,7 +1240,17 @@ func parseArray(raw []string, schemaRef *openapi3.SchemaRef) ([]any, error) { // that is specified by a schema. The function returns nil when the source string is empty and the type is not "string". // The function panics when a schema has a non-primitive type. func parsePrimitive(raw string, schema *openapi3.SchemaRef) (v any, err error) { - for _, typ := range schema.Value.Type.Slice() { + typs := schema.Value.Type.Slice() + constrainedTypeless := len(typs) == 0 && (schema.Value.Const != nil || len(schema.Value.Enum) > 0) + if constrainedTypeless { + // A typeless schema constrained by `const`/`enum` types the value + // on its own (the raw string must still be parsed so that a + // numeric/boolean constant is compared to the same Go type). + if t := constraintPrimitiveType(schema); t != "" { + typs = []string{t} + } + } + for _, typ := range typs { if raw == "" && typ != openapi3.TypeString { return nil, nil } @@ -1240,9 +1258,48 @@ func parsePrimitive(raw string, schema *openapi3.SchemaRef) (v any, err error) { return } } + if constrainedTypeless { + // Values of mixed kinds cannot be parsed to a single primitive + // type; keep the raw string and let schema validation decide. + return raw, nil + } return } +// constraintPrimitiveType returns the primitive type implied by the +// `const`/`enum` values of a typeless schema, or "" when that does not +// resolve to a single primitive type (mixed kinds, nested values). +func constraintPrimitiveType(schema *openapi3.SchemaRef) string { + var values []any + if schema.Value.Const != nil { + values = append(values, schema.Value.Const) + } + values = append(values, schema.Value.Enum...) + if len(values) == 0 { + return "" + } + typ := "" + for _, value := range values { + var t string + switch value.(type) { + case bool: + t = openapi3.TypeBoolean + case float64, float32, int, int32, int64: + t = openapi3.TypeNumber + case string: + t = openapi3.TypeString + default: + return "" + } + if typ == "" { + typ = t + } else if typ != t { + return "" + } + } + return typ +} + func parsePrimitiveCase(raw string, schema *openapi3.SchemaRef, typ string) (any, error) { switch typ { case "integer": From 44f4af5890137c52cf0efd75e0b574f166b35007 Mon Sep 17 00:00:00 2001 From: Yi-111-a Date: Thu, 22 Oct 2026 22:57:04 +0800 Subject: [PATCH 16/21] openapi3filter: typeless const/enum decode with a slice of types, external _test pkg --- openapi3filter/issue1367_test.go | 131 +++++++++++++++++++++-------- openapi3filter/req_resp_decoder.go | 64 ++++++++------ 2 files changed, 132 insertions(+), 63 deletions(-) diff --git a/openapi3filter/issue1367_test.go b/openapi3filter/issue1367_test.go index e30ba877e..ba618a087 100644 --- a/openapi3filter/issue1367_test.go +++ b/openapi3filter/issue1367_test.go @@ -1,12 +1,54 @@ -package openapi3filter +package openapi3filter_test import ( "net/http" "testing" "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" + "github.com/getkin/kin-openapi/openapi3filter" + "github.com/getkin/kin-openapi/routers" + "github.com/getkin/kin-openapi/routers/gorillamux" ) +func setupTestRouter(t *testing.T, spec string) routers.Router { + t.Helper() + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData([]byte(spec)) + require.NoError(t, err) + + err = doc.Validate(loader.Context) + require.NoError(t, err) + + router, err := gorillamux.NewRouter(doc) + require.NoError(t, err) + + return router +} + +func testIssue1367Request(t *testing.T, spec, url string, fail bool) { + t.Helper() + + req, err := http.NewRequest(http.MethodGet, url, nil) + require.NoError(t, err) + + router := setupTestRouter(t, spec) + route, pathParams, err := router.FindRoute(req) + require.NoError(t, err) + + err = openapi3filter.ValidateRequest(t.Context(), &openapi3filter.RequestValidationInput{ + Request: req, + PathParams: pathParams, + Route: route, + }) + if fail { + require.Error(t, err) + } else { + require.NoError(t, err) + } +} + func TestIssue1367TypelessConstAndEnumPrimitives(t *testing.T) { t.Parallel() @@ -32,6 +74,7 @@ paths: '200': description: ok ` + const rootURL = "http://example.com/items" testcases := []struct { url string @@ -52,24 +95,7 @@ paths: }} for _, tc := range testcases { t.Run(tc.url, func(t *testing.T) { - req, err := http.NewRequest(http.MethodGet, "/items"+tc.url, nil) - require.NoError(t, err) - - router := setupTestRouter(t, spec) - require.NoError(t, err) - route, pathParams, err := router.FindRoute(req) - require.NoError(t, err) - - err = ValidateRequest(t.Context(), &RequestValidationInput{ - Request: req, - PathParams: pathParams, - Route: route, - }) - if tc.fail { - require.Error(t, err) - } else { - require.NoError(t, err) - } + testIssue1367Request(t, spec, rootURL+tc.url, tc.fail) }) } } @@ -99,6 +125,7 @@ paths: '200': description: ok ` + const rootURL = "http://example.com/picked" testcases := []struct { url string @@ -114,24 +141,54 @@ paths: }} for _, tc := range testcases { t.Run(tc.url, func(t *testing.T) { - req, err := http.NewRequest(http.MethodGet, "/picked"+tc.url, nil) - require.NoError(t, err) - - router := setupTestRouter(t, spec) - require.NoError(t, err) - route, pathParams, err := router.FindRoute(req) - require.NoError(t, err) - - err = ValidateRequest(t.Context(), &RequestValidationInput{ - Request: req, - PathParams: pathParams, - Route: route, - }) - if tc.fail { - require.Error(t, err) - } else { - require.NoError(t, err) - } + testIssue1367Request(t, spec, rootURL+tc.url, tc.fail) + }) + } +} + +// The accept set of a typeless `enum`/`const` may mix kinds, so the +// decoder works with a slice of types rather than a single one; a +// numeric-looking raw string still decodes to the enum's number even +// when a string member exists, and an array member can never be +// produced from a parameter value. +func TestIssue1367TypelessEnumMixedKinds(t *testing.T) { + t.Parallel() + + const spec = ` +openapi: 3.1.0 +info: + title: repro + version: 1.0.0 +paths: + /mid: + get: + parameters: + - name: mid + in: query + required: true + schema: + enum: [42, "42", [42]] + responses: + '200': + description: ok +` + const rootURL = "http://example.com/mid" + + testcases := []struct { + url string + fail bool + }{{ + url: "?mid=42", + }, { + url: "?mid=gadget", + fail: true, + }, { + url: "?mid=%5B42%5D", + fail: true, + }} + for _, tc := range testcases { + t.Run(tc.url, func(t *testing.T) { + testIssue1367Request(t, spec, rootURL+tc.url, tc.fail) }) } } diff --git a/openapi3filter/req_resp_decoder.go b/openapi3filter/req_resp_decoder.go index 51b2ff050..01832db9b 100644 --- a/openapi3filter/req_resp_decoder.go +++ b/openapi3filter/req_resp_decoder.go @@ -1245,10 +1245,10 @@ func parsePrimitive(raw string, schema *openapi3.SchemaRef) (v any, err error) { if constrainedTypeless { // A typeless schema constrained by `const`/`enum` types the value // on its own (the raw string must still be parsed so that a - // numeric/boolean constant is compared to the same Go type). - if t := constraintPrimitiveType(schema); t != "" { - typs = []string{t} - } + // numeric/boolean constant is compared to the same Go type, see + // #1367). Its accept set may mix kinds, so decode with a slice of + // types, one per present member kind. + typs = constraintTypes(schema) } for _, typ := range typs { if raw == "" && typ != openapi3.TypeString { @@ -1259,45 +1259,57 @@ func parsePrimitive(raw string, schema *openapi3.SchemaRef) (v any, err error) { } } if constrainedTypeless { - // Values of mixed kinds cannot be parsed to a single primitive - // type; keep the raw string and let schema validation decide. + // No member kind parses a raw string (only array/object members + // are present); keep the raw string and let schema validation + // decide. return raw, nil } return } -// constraintPrimitiveType returns the primitive type implied by the -// `const`/`enum` values of a typeless schema, or "" when that does not -// resolve to a single primitive type (mixed kinds, nested values). -func constraintPrimitiveType(schema *openapi3.SchemaRef) string { +// constraintTypes returns the types implied by the `const`/`enum` values +// of a typeless schema, in a fixed priority order (boolean before number +// before string, so that a numeric raw string is not shadowed by the +// string case, and string last, so that `"42"` in +// `enum: [42, "42", [42]]` does not hide `42`), or an empty slice when +// the schema has no such values. +func constraintTypes(schema *openapi3.SchemaRef) []string { var values []any if schema.Value.Const != nil { values = append(values, schema.Value.Const) } values = append(values, schema.Value.Enum...) if len(values) == 0 { - return "" + return nil } - typ := "" + present := make(map[string]bool, len(values)) for _, value := range values { - var t string switch value.(type) { case bool: - t = openapi3.TypeBoolean + present[openapi3.TypeBoolean] = true case float64, float32, int, int32, int64: - t = openapi3.TypeNumber + present[openapi3.TypeNumber] = true case string: - t = openapi3.TypeString - default: - return "" - } - if typ == "" { - typ = t - } else if typ != t { - return "" - } - } - return typ + present[openapi3.TypeString] = true + case []any: + present[openapi3.TypeArray] = true + case map[string]any: + present[openapi3.TypeObject] = true + } + } + typs := make([]string, 0, len(present)) + for _, typ := range []string{ + openapi3.TypeBoolean, + openapi3.TypeNumber, + openapi3.TypeString, + openapi3.TypeArray, + openapi3.TypeObject, + } { + if present[typ] { + typs = append(typs, typ) + } + } + return typs } func parsePrimitiveCase(raw string, schema *openapi3.SchemaRef, typ string) (any, error) { From 0d96dffb29ba3014eb68dec5de06d7e3c0e7dc85 Mon Sep 17 00:00:00 2001 From: Yi-111-a Date: Mon, 26 Oct 2026 12:47:50 +0800 Subject: [PATCH 17/21] openapi3filter: simplify typeless const/enum type derivation, name subtests and assert errors --- openapi3filter/issue1367_test.go | 86 +++++++++++++++++------------- openapi3filter/req_resp_decoder.go | 70 ++++++++++-------------- 2 files changed, 76 insertions(+), 80 deletions(-) diff --git a/openapi3filter/issue1367_test.go b/openapi3filter/issue1367_test.go index ba618a087..14ad5d6c6 100644 --- a/openapi3filter/issue1367_test.go +++ b/openapi3filter/issue1367_test.go @@ -27,7 +27,7 @@ func setupTestRouter(t *testing.T, spec string) routers.Router { return router } -func testIssue1367Request(t *testing.T, spec, url string, fail bool) { +func testIssue1367Request(t *testing.T, spec, url, errContains string) { t.Helper() req, err := http.NewRequest(http.MethodGet, url, nil) @@ -42,8 +42,8 @@ func testIssue1367Request(t *testing.T, spec, url string, fail bool) { PathParams: pathParams, Route: route, }) - if fail { - require.Error(t, err) + if errContains != "" { + require.ErrorContains(t, err, errContains) } else { require.NoError(t, err) } @@ -77,25 +77,31 @@ paths: const rootURL = "http://example.com/items" testcases := []struct { - url string - fail bool + name string + url string + errContains string }{{ - url: "?kind=widget", + name: "const matches", + url: "?kind=widget", }, { - url: "?kind=widget&withenum=widget", + name: "const and enum match", + url: "?kind=widget&withenum=widget", }, { - url: "?kind=widget&withenum=gadget", - fail: true, + name: "enum mismatch", + url: "?kind=widget&withenum=gadget", + errContains: "parameter \"withenum\" in query has an error: validation failed due to: at '': value must be 'widget'", }, { - url: "?kind=gadget", - fail: true, + name: "const mismatch", + url: "?kind=gadget", + errContains: "parameter \"kind\" in query has an error: validation failed due to: at '': value must be 'widget'", }, { - url: "?kind=widget&withenum=", - fail: true, + name: "empty enum value", + url: "?kind=widget&withenum=", + errContains: "parameter \"withenum\" in query has an error: validation failed due to: at '': value must be 'widget'", }} for _, tc := range testcases { - t.Run(tc.url, func(t *testing.T) { - testIssue1367Request(t, spec, rootURL+tc.url, tc.fail) + t.Run(tc.name, func(t *testing.T) { + testIssue1367Request(t, spec, rootURL+tc.url, tc.errContains) }) } } @@ -128,29 +134,29 @@ paths: const rootURL = "http://example.com/picked" testcases := []struct { - url string - fail bool + name string + url string + errContains string }{{ - url: "?rank=2&flag=true", + name: "number enum and boolean const match", + url: "?rank=2&flag=true", }, { - url: "?rank=2&flag=false", - fail: true, + name: "boolean const mismatch", + url: "?rank=2&flag=false", + errContains: "parameter \"flag\" in query has an error: validation failed due to: at '': value must be true", }, { - url: "?rank=9&flag=true", - fail: true, + name: "number enum mismatch", + url: "?rank=9&flag=true", + errContains: "parameter \"rank\" in query has an error: validation failed due to: at '': value must be one of 1, 2, 3", }} for _, tc := range testcases { - t.Run(tc.url, func(t *testing.T) { - testIssue1367Request(t, spec, rootURL+tc.url, tc.fail) + t.Run(tc.name, func(t *testing.T) { + testIssue1367Request(t, spec, rootURL+tc.url, tc.errContains) }) } } -// The accept set of a typeless `enum`/`const` may mix kinds, so the -// decoder works with a slice of types rather than a single one; a -// numeric-looking raw string still decodes to the enum's number even -// when a string member exists, and an array member can never be -// produced from a parameter value. +// A typeless enum may mix kinds; a numeric string still decodes to the number. func TestIssue1367TypelessEnumMixedKinds(t *testing.T) { t.Parallel() @@ -175,20 +181,24 @@ paths: const rootURL = "http://example.com/mid" testcases := []struct { - url string - fail bool + name string + url string + errContains string }{{ - url: "?mid=42", + name: "numeric string decodes to number", + url: "?mid=42", }, { - url: "?mid=gadget", - fail: true, + name: "no member matches", + url: "?mid=gadget", + errContains: "parameter \"mid\" in query has an error: validation failed due to: at '': 'enum' failed", }, { - url: "?mid=%5B42%5D", - fail: true, + name: "array member never matches", + url: "?mid=%5B42%5D", + errContains: "parameter \"mid\" in query has an error: validation failed due to: at '': 'enum' failed", }} for _, tc := range testcases { - t.Run(tc.url, func(t *testing.T) { - testIssue1367Request(t, spec, rootURL+tc.url, tc.fail) + t.Run(tc.name, func(t *testing.T) { + testIssue1367Request(t, spec, rootURL+tc.url, tc.errContains) }) } } diff --git a/openapi3filter/req_resp_decoder.go b/openapi3filter/req_resp_decoder.go index 01832db9b..8e0ad765b 100644 --- a/openapi3filter/req_resp_decoder.go +++ b/openapi3filter/req_resp_decoder.go @@ -339,10 +339,7 @@ func decodeValueGuarded(dec valueDecoder, param string, sm *openapi3.Serializati } if schema.Value.Type == nil && (schema.Value.Const != nil || len(schema.Value.Enum) > 0) { - // A schema without `type` is valid in OpenAPI 3.0 and 3.1; with - // only `const`/`enum` the schema types the value on its own (see - // #1367). Parameter values are strings, so decode as a primitive - // and let schema validation apply the constrained values. + // Typeless const/enum schema (#1367): decode as a primitive. return dec.DecodePrimitive(param, sm, schema) } @@ -1243,11 +1240,7 @@ func parsePrimitive(raw string, schema *openapi3.SchemaRef) (v any, err error) { typs := schema.Value.Type.Slice() constrainedTypeless := len(typs) == 0 && (schema.Value.Const != nil || len(schema.Value.Enum) > 0) if constrainedTypeless { - // A typeless schema constrained by `const`/`enum` types the value - // on its own (the raw string must still be parsed so that a - // numeric/boolean constant is compared to the same Go type, see - // #1367). Its accept set may mix kinds, so decode with a slice of - // types, one per present member kind. + // No type: derive the candidate types from the const/enum values (#1367). typs = constraintTypes(schema) } for _, typ := range typs { @@ -1259,45 +1252,19 @@ func parsePrimitive(raw string, schema *openapi3.SchemaRef) (v any, err error) { } } if constrainedTypeless { - // No member kind parses a raw string (only array/object members - // are present); keep the raw string and let schema validation - // decide. return raw, nil } return } -// constraintTypes returns the types implied by the `const`/`enum` values -// of a typeless schema, in a fixed priority order (boolean before number -// before string, so that a numeric raw string is not shadowed by the -// string case, and string last, so that `"42"` in -// `enum: [42, "42", [42]]` does not hide `42`), or an empty slice when -// the schema has no such values. +// constraintTypes returns the types of the const/enum values of a schema, +// with boolean and number before string so that "42" decodes to 42. func constraintTypes(schema *openapi3.SchemaRef) []string { - var values []any + values := schema.Value.Enum if schema.Value.Const != nil { - values = append(values, schema.Value.Const) + values = append([]any{schema.Value.Const}, values...) } - values = append(values, schema.Value.Enum...) - if len(values) == 0 { - return nil - } - present := make(map[string]bool, len(values)) - for _, value := range values { - switch value.(type) { - case bool: - present[openapi3.TypeBoolean] = true - case float64, float32, int, int32, int64: - present[openapi3.TypeNumber] = true - case string: - present[openapi3.TypeString] = true - case []any: - present[openapi3.TypeArray] = true - case map[string]any: - present[openapi3.TypeObject] = true - } - } - typs := make([]string, 0, len(present)) + var typs []string for _, typ := range []string{ openapi3.TypeBoolean, openapi3.TypeNumber, @@ -1305,13 +1272,32 @@ func constraintTypes(schema *openapi3.SchemaRef) []string { openapi3.TypeArray, openapi3.TypeObject, } { - if present[typ] { - typs = append(typs, typ) + for _, value := range values { + if valueType(value) == typ { + typs = append(typs, typ) + break + } } } return typs } +func valueType(value any) string { + switch value.(type) { + case bool: + return openapi3.TypeBoolean + case float32, float64, int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64, json.Number: + return openapi3.TypeNumber + case string: + return openapi3.TypeString + case []any: + return openapi3.TypeArray + case map[string]any: + return openapi3.TypeObject + } + return "" +} + func parsePrimitiveCase(raw string, schema *openapi3.SchemaRef, typ string) (any, error) { switch typ { case "integer": From b8b4f18c653391c235f5155a6bbc8e9fa98c046e Mon Sep 17 00:00:00 2001 From: Yi-111-a <153097222+Yi-111-a@users.noreply.github.com> Date: Mon, 26 Oct 2026 19:01:44 +0800 Subject: [PATCH 18/21] openapi3filter: derive typeless const/enum candidate types in O(values) --- openapi3filter/req_resp_decoder.go | 70 ++++++++++++++++-------------- 1 file changed, 37 insertions(+), 33 deletions(-) diff --git a/openapi3filter/req_resp_decoder.go b/openapi3filter/req_resp_decoder.go index 8e0ad765b..0cbb99b4b 100644 --- a/openapi3filter/req_resp_decoder.go +++ b/openapi3filter/req_resp_decoder.go @@ -1259,45 +1259,49 @@ func parsePrimitive(raw string, schema *openapi3.SchemaRef) (v any, err error) { // constraintTypes returns the types of the const/enum values of a schema, // with boolean and number before string so that "42" decodes to 42. +// It makes a single pass over the values, then keeps the seen types only, +// reducing the worst case from O(5 x values) hash lookups to O(values). func constraintTypes(schema *openapi3.SchemaRef) []string { - values := schema.Value.Enum + seen := make(map[string]struct{}, 5) + consider := func(value any) { + switch value.(type) { + case bool: + seen[openapi3.TypeBoolean] = struct{}{} + case float32, float64, int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64, json.Number: + seen[openapi3.TypeNumber] = struct{}{} + case string: + seen[openapi3.TypeString] = struct{}{} + case []any: + seen[openapi3.TypeArray] = struct{}{} + case map[string]any: + seen[openapi3.TypeObject] = struct{}{} + } + } + for _, value := range schema.Value.Enum { + consider(value) + } if schema.Value.Const != nil { - values = append([]any{schema.Value.Const}, values...) - } - var typs []string - for _, typ := range []string{ - openapi3.TypeBoolean, - openapi3.TypeNumber, - openapi3.TypeString, - openapi3.TypeArray, - openapi3.TypeObject, - } { - for _, value := range values { - if valueType(value) == typ { - typs = append(typs, typ) - break - } - } + consider(schema.Value.Const) + } + typs := make([]string, 0, 5) + if _, ok := seen[openapi3.TypeBoolean]; ok { + typs = append(typs, openapi3.TypeBoolean) + } + if _, ok := seen[openapi3.TypeNumber]; ok { + typs = append(typs, openapi3.TypeNumber) + } + if _, ok := seen[openapi3.TypeString]; ok { + typs = append(typs, openapi3.TypeString) + } + if _, ok := seen[openapi3.TypeArray]; ok { + typs = append(typs, openapi3.TypeArray) + } + if _, ok := seen[openapi3.TypeObject]; ok { + typs = append(typs, openapi3.TypeObject) } return typs } -func valueType(value any) string { - switch value.(type) { - case bool: - return openapi3.TypeBoolean - case float32, float64, int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64, json.Number: - return openapi3.TypeNumber - case string: - return openapi3.TypeString - case []any: - return openapi3.TypeArray - case map[string]any: - return openapi3.TypeObject - } - return "" -} - func parsePrimitiveCase(raw string, schema *openapi3.SchemaRef, typ string) (any, error) { switch typ { case "integer": From 21622f0888b57a703b7f1d868c85191fe3cff1cd Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Mon, 26 Oct 2026 16:50:02 +0200 Subject: [PATCH 19/21] openapi2conv: handle some nil derefs (#1384) Co-authored-by: Claude Opus 5 --- openapi2conv/nil_collections_test.go | 64 +++++++++++++++++++ openapi2conv/openapi2_conv.go | 40 +++++++++--- .../testdata/fuzz/FuzzToV3/2915761dc93996a1 | 2 + 3 files changed, 98 insertions(+), 8 deletions(-) create mode 100644 openapi2conv/nil_collections_test.go create mode 100644 openapi2conv/testdata/fuzz/FuzzToV3/2915761dc93996a1 diff --git a/openapi2conv/nil_collections_test.go b/openapi2conv/nil_collections_test.go new file mode 100644 index 000000000..0793695d4 --- /dev/null +++ b/openapi2conv/nil_collections_test.go @@ -0,0 +1,64 @@ +package openapi2conv_test + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi2" + "github.com/getkin/kin-openapi/openapi2conv" +) + +// A Swagger document can write null wherever an object belongs, and every +// collection that accepts one reached a dereference. Converting such a +// document reports it rather than panicking: the member stays nil and the +// resulting document says so when validated. +func TestToV3NullMembers(t *testing.T) { + t.Parallel() + + for _, doc2 := range []string{ + `{"paths":{"0":null}}`, + `{"paths":{"0":{"get":{"responses":{"":null}}}}}`, + `{"paths":{"0":{"get":{"responses":{"200":{"description":"d","headers":{"h":null}}}}}}}`, + `{"paths":{"0":{"get":{"parameters":[null]}}}}`, + `{"paths":{"0":{"parameters":[null],"get":{"responses":{"200":{"description":"d"}}}}}}`, + `{"responses":{"x":null}}`, + `{"parameters":{"x":null}}`, + `{"definitions":{"x":null}}`, + `{"securityDefinitions":{"x":null}}`, + } { + t.Run(doc2, func(t *testing.T) { + t.Parallel() + + var doc openapi2.T + require.NoError(t, json.Unmarshal([]byte(doc2), &doc)) + + // Whatever comes back, nothing dereferences a null on the way. + doc3, err := openapi2conv.ToV3(&doc) + if err != nil { + return + } + _, err = doc3.MarshalJSON() + require.NoError(t, err) + }) + } +} + +// Converting a document leaves it as it was: a v3 header drops the in and name +// a v2 header carries, and the copy being converted is where that happens. +func TestToV3HeadersLeavesInputAlone(t *testing.T) { + t.Parallel() + + const spec = `{"paths":{"/a":{"get":{"responses":{"200":{"description":"d","headers":{"h":{"type":"string"}}}}}}}}` + + var doc openapi2.T + require.NoError(t, json.Unmarshal([]byte(spec), &doc)) + header := doc.Paths["/a"].Get.Responses.Value("200").Headers["h"] + header.Name, header.In = "h", "header" + + _, err := openapi2conv.ToV3(&doc) + require.NoError(t, err) + require.Equal(t, "h", header.Name) + require.Equal(t, "header", header.In) +} diff --git a/openapi2conv/openapi2_conv.go b/openapi2conv/openapi2_conv.go index 38fee0cf5..954534f6a 100644 --- a/openapi2conv/openapi2_conv.go +++ b/openapi2conv/openapi2_conv.go @@ -127,6 +127,9 @@ func ToV3WithLoader(doc2 *openapi2.T, loader *openapi3.Loader, location *url.URL } func ToV3PathItem(doc2 *openapi2.T, components *openapi3.Components, pathItem *openapi2.PathItem, consumes []string) (*openapi3.PathItem, error) { + if pathItem == nil { + return nil, nil + } doc3 := &openapi3.PathItem{ Extensions: stripNonExtensions(pathItem.Extensions), } @@ -245,7 +248,11 @@ func inheritedRequestBodyParams(components *openapi3.Components, pathItem *opena // parameterIdentity returns the key a parameter is known by inside a path item: // a reference keeps its reference, an inline parameter its location and name. +// A parameter written as null has no identity and overrides nothing. func parameterIdentity(parameter *openapi2.Parameter) string { + if parameter == nil { + return "" + } if parameter.Ref != "" { return parameter.Ref } @@ -260,6 +267,9 @@ func getParameterNameFromOldRef(ref string) string { } func ToV3Parameter(components *openapi3.Components, parameter *openapi2.Parameter, consumes []string) (*openapi3.ParameterRef, *openapi3.RequestBodyRef, map[string]*openapi3.SchemaRef, error) { + if parameter == nil { + return nil, nil, nil, nil + } if ref := parameter.Ref; ref != "" { if strings.HasPrefix(ref, "#/parameters/") { name := getParameterNameFromOldRef(ref) @@ -487,6 +497,9 @@ func onlyOneReqBodyParam(bodies []*openapi3.RequestBodyRef, formDataSchemas map[ } func ToV3Response(response *openapi2.Response, produces []string) (*openapi3.ResponseRef, error) { + if response == nil { + return nil, nil + } if ref := response.Ref; ref != "" { return &openapi3.ResponseRef{Ref: ToV3Ref(ref)}, nil } @@ -527,16 +540,27 @@ func ToV3Response(response *openapi2.Response, produces []string) (*openapi3.Res func ToV3Headers(defs map[string]*openapi2.Header) openapi3.Headers { headers := make(openapi3.Headers, len(defs)) for name, header := range defs { - header.In = "" - header.Name = "" - if ref := header.Ref; ref != "" { + if header == nil { + headers[name] = nil + continue + } + // On a copy: a header's in and name are dropped on the way to a + // v3 header, and the document being converted keeps its own. + h := *header + h.In = "" + h.Name = "" + if ref := h.Ref; ref != "" { headers[name] = &openapi3.HeaderRef{Ref: ToV3Ref(ref)} - } else { - parameter, _, _, _ := ToV3Parameter(nil, &header.Parameter, nil) - headers[name] = &openapi3.HeaderRef{Value: &openapi3.Header{ - Parameter: *parameter.Value, - }} + continue + } + parameter, _, _, _ := ToV3Parameter(nil, &h.Parameter, nil) + if parameter == nil || parameter.Value == nil { + headers[name] = nil + continue } + headers[name] = &openapi3.HeaderRef{Value: &openapi3.Header{ + Parameter: *parameter.Value, + }} } return headers } diff --git a/openapi2conv/testdata/fuzz/FuzzToV3/2915761dc93996a1 b/openapi2conv/testdata/fuzz/FuzzToV3/2915761dc93996a1 new file mode 100644 index 000000000..c3ae52221 --- /dev/null +++ b/openapi2conv/testdata/fuzz/FuzzToV3/2915761dc93996a1 @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("{\"paths\":{\"0\":{\"get\":{\"responses\":{\"\":null}}}}}") From 4b868cedd0959a90dee6de65ad765435ee62f46c Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Mon, 26 Oct 2026 17:38:32 +0200 Subject: [PATCH 20/21] ci: a minimize budget the minimizer can finish in (#1385) Co-authored-by: Claude Opus 5 --- .github/workflows/fuzz.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml index c9202215a..836f85d10 100644 --- a/.github/workflows/fuzz.yml +++ b/.github/workflows/fuzz.yml @@ -92,13 +92,20 @@ jobs: # -run='^$' skips the ordinary tests: the seed corpus is already replayed by # `go test ./...` in the go workflow, so this job only spends time fuzzing. + # + # The minimize budget is small on purpose. A minimize that cannot finish + # inside it writes the candidate it had and the job fails with "fuzzing + # process hung or terminated unexpectedly while minimizing", whose + # candidate then passes on replay: a red build and nothing to act on. The + # budget only decides how small the saved input gets, so spending less of + # it costs a larger seed and buys a signal that means what it says. - name: Fuzz ${{ matrix.target }} run: | go test \ -run='^$' \ -fuzz='^${{ matrix.target }}$' \ -fuzztime='${{ steps.budget.outputs.fuzztime }}' \ - -fuzzminimizetime=60s \ + -fuzzminimizetime=10s \ ${{ matrix.package }} # A failing input is written next to the package it broke. Upload it so the From caa1af107e250bc3bec4c0f97a80959ff040e8fe Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Thu, 22 Oct 2026 11:28:39 +0300 Subject: [PATCH 21/21] openapi3: return a typed error for a disallowed external $ref The loader refuses a $ref to another document when IsExternalRefsAllowed is false, and reported it with fmt.Errorf, so the message was the only thing telling that refusal apart from a document that failed to load for some other reason. A caller that wants to name the setting as the remedy, or map the refusal to its own error or exit code, had to match the text. Return *DisallowedExternalRefError instead, carrying the offending $ref. The message is unchanged, so a consumer matching the text still works. Both places that apply the setting go through allowsExternalRefs: a $ref naming a whole document (loadSingleElementFromURI) and a $ref with a fragment (resolveRefPath). The test covers each. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017e285CvofYf6joqC98gTXz --- .github/docs/openapi3.txt | 13 ++++++ openapi3/loader.go | 21 ++++++++-- .../loader_disallowed_external_ref_test.go | 42 +++++++++++++++++++ 3 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 openapi3/loader_disallowed_external_ref_test.go diff --git a/.github/docs/openapi3.txt b/.github/docs/openapi3.txt index a77c3e95e..90b98c3db 100644 --- a/.github/docs/openapi3.txt +++ b/.github/docs/openapi3.txt @@ -739,6 +739,19 @@ func (e *DependentSchemasFieldFor31Plus) As(target any) bool func (e *DependentSchemasFieldFor31Plus) Code() string +type DisallowedExternalRefError struct { + // Ref is the $ref value as it appears in the document, e.g. + // "other.yaml#/components/schemas/Thing". + Ref string +} + DisallowedExternalRefError is returned for a $ref to another document + encountered while Loader.IsExternalRefsAllowed is false. A caller that tells + a refused $ref apart from a document that failed to load for another reason, + to report the setting as the remedy or to map it to its own error, matches + this type with errors.As rather than the message text. + +func (e *DisallowedExternalRefError) Error() string + type Discriminator struct { Extensions map[string]any `json:"-" yaml:"-"` Origin *Origin `json:"-" yaml:"-"` diff --git a/openapi3/loader.go b/openapi3/loader.go index e7e422cdc..d62b68059 100644 --- a/openapi3/loader.go +++ b/openapi3/loader.go @@ -109,11 +109,26 @@ func (loader *Loader) loadFromURIInternal(location *url.URL) (*T, error) { return loader.loadFromDataWithPathInternal(data, location) } -func (loader *Loader) allowsExternalRefs(ref string) (err error) { +// DisallowedExternalRefError is returned for a $ref to another document +// encountered while Loader.IsExternalRefsAllowed is false. A caller that tells +// a refused $ref apart from a document that failed to load for another reason, +// to report the setting as the remedy or to map it to its own error, matches +// this type with errors.As rather than the message text. +type DisallowedExternalRefError struct { + // Ref is the $ref value as it appears in the document, e.g. + // "other.yaml#/components/schemas/Thing". + Ref string +} + +func (e *DisallowedExternalRefError) Error() string { + return fmt.Sprintf("encountered disallowed external reference: %q", e.Ref) +} + +func (loader *Loader) allowsExternalRefs(ref string) error { if !loader.IsExternalRefsAllowed { - err = fmt.Errorf("encountered disallowed external reference: %q", ref) + return &DisallowedExternalRefError{Ref: ref} } - return + return nil } func (loader *Loader) loadSingleElementFromURI(ref string, rootPath *url.URL, element any) (*url.URL, error) { diff --git a/openapi3/loader_disallowed_external_ref_test.go b/openapi3/loader_disallowed_external_ref_test.go new file mode 100644 index 000000000..73d1d698e --- /dev/null +++ b/openapi3/loader_disallowed_external_ref_test.go @@ -0,0 +1,42 @@ +package openapi3_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" +) + +// A $ref naming a whole document and a $ref with a fragment take different +// paths through the loader, and each applies IsExternalRefsAllowed on its own. +func TestDisallowedExternalRefError(t *testing.T) { + t.Parallel() + + for _, ref := range []string{"other.yaml", "other.yaml#/components/schemas/Thing"} { + t.Run(ref, func(t *testing.T) { + t.Parallel() + + spec := fmt.Appendf(nil, ` +openapi: 3.0.0 +info: + title: Test + version: '1.0' +paths: {} +components: + schemas: + Thing: + $ref: %q +`, ref) + + loader := openapi3.NewLoader() + _, err := loader.LoadFromData(spec) + require.Error(t, err) + + var refused *openapi3.DisallowedExternalRefError + require.ErrorAs(t, err, &refused) + require.Equal(t, ref, refused.Ref) + }) + } +}