diff --git a/.github/docs/openapi3.txt b/.github/docs/openapi3.txt index 66f9acf65..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:"-"` @@ -2282,6 +2295,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 @@ -4207,7 +4222,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 +4287,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/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml new file mode 100644 index 000000000..836f85d10 --- /dev/null +++ b/.github/workflows/fuzz.yml @@ -0,0 +1,119 @@ +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: + # Keep in sync with the go workflow, whose cache this restores. + APISGURU_COMMIT: f7207cf0a5c56081d275ebae4cf615249323385d + 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: FuzzLoadWithOrigins} + - {package: ./openapi3, target: FuzzYAMLMatchesJSON} + - {package: ./openapi3, target: FuzzSchemaVisitJSON} + - {package: ./openapi2, target: FuzzUnmarshalFromData} + - {package: ./openapi2conv, target: FuzzToV3} + - {package: ./openapi3filter, target: FuzzValidateRequest} + steps: + + - uses: actions/setup-go@v6 + with: + go-version: stable + + - 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 + # 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. + # + # 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=10s \ + ${{ 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..2978ca959 100644 --- a/Makefile +++ b/Makefile @@ -1,28 +1,40 @@ -.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='^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 + +## help: list targets +help: + @grep -E '^## ' $(MAKEFILE_LIST) | sed 's/## / make /' diff --git a/README.md b/README.md index 0ba04f93d..661a1e767 100644 --- a/README.md +++ b/README.md @@ -329,14 +329,30 @@ 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.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`. * 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. * `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`. +* 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.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. @@ -353,8 +369,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). 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 new file mode 100644 index 000000000..3d214d312 --- /dev/null +++ b/openapi2conv/fuzz_test.go @@ -0,0 +1,93 @@ +package openapi2conv_test + +import ( + "encoding/json" + "os" + "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" +) + +// 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) + } + } + // 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":{}}`)) + 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/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 c77072af0..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 } @@ -565,7 +589,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 +770,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 +838,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 +848,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 +874,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 +956,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 +971,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 +1020,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 +1073,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) +} 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}}}}}") diff --git a/openapi3/fuzz_test.go b/openapi3/fuzz_test.go new file mode 100644 index 000000000..c87c875c7 --- /dev/null +++ b/openapi3/fuzz_test.go @@ -0,0 +1,217 @@ +package openapi3_test + +import ( + "encoding/json" + "net/url" + "reflect" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/internal/fuzzing" + "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 + +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 +} + +// 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 _, 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) + + 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())) + }) +} + +// 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"`)) + 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/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/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 +} 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.go b/openapi3/loader.go index fdcf6ed84..d62b68059 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 @@ -103,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) { @@ -250,6 +271,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() } @@ -402,29 +429,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 +781,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 +789,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 +812,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 +852,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 +860,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 +883,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 +931,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 +939,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 +962,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 +997,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 +1005,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 +1028,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 @@ -1053,18 +1089,29 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat if component.Value != nil { return nil } - if !loader.shouldVisitRef(ref, func(value any) { + 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. + if err := loader.resolveSchemaRef(doc, &SchemaRef{Value: component.sibling}, documentPath, visited); err != nil { + return err + } + } + if !loader.shouldVisitRef("Schema", ref, func(value any) { component.Value = value.(*Schema) refPath, _ := loader.resolveRefPath(ref, documentPath) component.setRefPath(refPath) // 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 } - loader.visitRef(ref) + loader.visitRef("Schema", ref) if isSingleRefElement(ref) { var schema Schema if documentPath, err = loader.loadSingleElementFromURI(ref, documentPath, &schema); err != nil { @@ -1087,14 +1134,14 @@ 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 — // 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 { @@ -1249,7 +1296,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) @@ -1257,7 +1304,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 { @@ -1280,7 +1327,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 @@ -1310,7 +1357,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) @@ -1318,7 +1365,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 { @@ -1341,7 +1388,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 @@ -1356,14 +1403,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 { @@ -1386,7 +1433,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 { @@ -1422,7 +1469,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) @@ -1430,7 +1477,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 { @@ -1453,7 +1500,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 @@ -1466,21 +1513,38 @@ func (loader *Loader) resolvePathItemRef(doc *T, pathItem *PathItem, documentPat } if ref := pathItem.Ref; ref != "" { - if !pathItem.isEmpty() { + if pathItem.refResolved { return } - if !loader.shouldVisitRef(ref, func(value any) { - *pathItem = *value.(*PathItem) + // 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) { + resolvedTo(*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 { return } - *pathItem = p + resolvedTo(p) } else { var resolved PathItem if doc, documentPath, err = loader.resolveComponent(doc, ref, documentPath, &resolved); err != nil { @@ -1489,10 +1553,9 @@ func (loader *Loader) resolvePathItemRef(doc *T, pathItem *PathItem, documentPat } return } - *pathItem = resolved + resolvedTo(resolved) } - pathItem.Ref = ref - defer loader.unvisitRef(ref, pathItem) + defer loader.unvisitRef("PathItem", ref, pathItem) } for _, parameter := range pathItem.Parameters { @@ -1530,15 +1593,75 @@ 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. -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 +// 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(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 { + annotations = append(annotations, field) + } else { + assertions = append(assertions, field) + } + } + schemaCopy := *component.Value + applySiblingSchemaFields(&schemaCopy, component.sibling, annotations) + if len(assertions) != 0 { + // 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) + allOf = append(allOf, schemaCopy.AllOf...) + schemaCopy.AllOf = append(allOf, &SchemaRef{Value: &conjunct}) + } + } + 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 { 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) + }) + } +} 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/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"}}}} + } +}`)) + }) +} 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/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) + }) + }) + } +} 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/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..085f85c1f 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,22 @@ 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 + } + if x.Value == nil { + return nil, nil } return x.Value.MarshalYAML() } @@ -88,8 +103,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) } } @@ -102,46 +121,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) } @@ -193,6 +190,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 } @@ -223,12 +224,22 @@ 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 + } + if x.Value == nil { + return nil, nil } return x.Value.MarshalYAML() } @@ -257,8 +268,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) } } @@ -271,46 +286,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) } @@ -362,6 +355,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 } @@ -392,12 +389,22 @@ 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 + } + if x.Value == nil { + return nil, nil } return x.Value.MarshalYAML() } @@ -426,8 +433,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) } } @@ -440,46 +451,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) } @@ -531,6 +520,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 } @@ -561,12 +554,22 @@ 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 + } + if x.Value == nil { + return nil, nil } return x.Value.MarshalYAML() } @@ -595,8 +598,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) } } @@ -609,46 +616,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) } @@ -700,6 +685,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 } @@ -730,12 +719,22 @@ 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 + } + if x.Value == nil { + return nil, nil } return x.Value.MarshalYAML() } @@ -764,8 +763,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) } } @@ -778,46 +781,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) } @@ -869,6 +850,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 } @@ -899,12 +884,22 @@ 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 + } + if x.Value == nil { + return nil, nil } return x.Value.MarshalYAML() } @@ -933,8 +928,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) } } @@ -947,46 +946,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) } @@ -1038,6 +1015,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 } @@ -1068,12 +1049,22 @@ 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 + } + if x.Value == nil { + return nil, nil } return x.Value.MarshalYAML() } @@ -1102,8 +1093,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) } } @@ -1116,46 +1111,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) } @@ -1204,9 +1177,19 @@ 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 + // 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 } @@ -1237,10 +1220,37 @@ 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) + 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 + } + if x.Value == nil { + return nil, nil } return x.Value.MarshalYAML() } @@ -1279,10 +1289,16 @@ 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 := 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) } } @@ -1295,42 +1311,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 } @@ -1380,6 +1380,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 } @@ -1410,12 +1414,22 @@ 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 + } + if x.Value == nil { + return nil, nil } return x.Value.MarshalYAML() } @@ -1444,8 +1458,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) } } @@ -1458,46 +1476,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..84076b991 100644 --- a/openapi3/refs.tmpl +++ b/openapi3/refs.tmpl @@ -4,6 +4,7 @@ package {{ .Package }} import ( "context" "encoding/json" + "maps" "net/url" "strings" @@ -27,10 +28,20 @@ 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. 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 @@ -62,14 +73,43 @@ 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) + {{- 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 + } + if x.Value == nil { + return nil, nil } return x.Value.MarshalYAML() } @@ -115,11 +155,17 @@ 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 }} - 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) } } @@ -132,56 +178,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..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) @@ -41,16 +45,16 @@ 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 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) @@ -98,16 +106,16 @@ 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 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) @@ -155,16 +167,16 @@ 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 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) @@ -203,16 +219,16 @@ 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 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) @@ -260,16 +280,16 @@ 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 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) @@ -317,16 +341,16 @@ 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 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) @@ -374,16 +402,16 @@ 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 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) @@ -431,7 +463,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") @@ -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) @@ -488,16 +524,16 @@ 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 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 083e52207..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) @@ -41,20 +45,16 @@ 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 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/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/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")) +} 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/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/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/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/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) { 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 new file mode 100644 index 000000000..5f257dde8 --- /dev/null +++ b/openapi3filter/fuzz_test.go @@ -0,0 +1,147 @@ +package openapi3filter_test + +import ( + "bytes" + "net/http" + "net/url" + "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" + "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) { + 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 = ` +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 +} diff --git a/openapi3filter/issue1367_test.go b/openapi3filter/issue1367_test.go new file mode 100644 index 000000000..14ad5d6c6 --- /dev/null +++ b/openapi3filter/issue1367_test.go @@ -0,0 +1,204 @@ +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, errContains string) { + 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 errContains != "" { + require.ErrorContains(t, err, errContains) + } else { + require.NoError(t, err) + } +} + +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 +` + const rootURL = "http://example.com/items" + + testcases := []struct { + name string + url string + errContains string + }{{ + name: "const matches", + url: "?kind=widget", + }, { + name: "const and enum match", + url: "?kind=widget&withenum=widget", + }, { + 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'", + }, { + name: "const mismatch", + url: "?kind=gadget", + errContains: "parameter \"kind\" in query has an error: validation failed due to: at '': value must be 'widget'", + }, { + 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.name, func(t *testing.T) { + testIssue1367Request(t, spec, rootURL+tc.url, tc.errContains) + }) + } +} + +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 +` + const rootURL = "http://example.com/picked" + + testcases := []struct { + name string + url string + errContains string + }{{ + name: "number enum and boolean const match", + url: "?rank=2&flag=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", + }, { + 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.name, func(t *testing.T) { + testIssue1367Request(t, spec, rootURL+tc.url, tc.errContains) + }) + } +} + +// A typeless enum may mix kinds; a numeric string still decodes to the number. +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 { + name string + url string + errContains string + }{{ + name: "numeric string decodes to number", + url: "?mid=42", + }, { + name: "no member matches", + url: "?mid=gadget", + errContains: "parameter \"mid\" in query has an error: validation failed due to: at '': 'enum' failed", + }, { + 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.name, func(t *testing.T) { + testIssue1367Request(t, spec, rootURL+tc.url, tc.errContains) + }) + } +} 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) +} diff --git a/openapi3filter/req_resp_decoder.go b/openapi3filter/req_resp_decoder.go index 3bcfd4931..0cbb99b4b 100644 --- a/openapi3filter/req_resp_decoder.go +++ b/openapi3filter/req_resp_decoder.go @@ -338,6 +338,11 @@ 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) { + // Typeless const/enum schema (#1367): decode as a primitive. + 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 +1237,13 @@ 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 { + // No type: derive the candidate types from the const/enum values (#1367). + typs = constraintTypes(schema) + } + for _, typ := range typs { if raw == "" && typ != openapi3.TypeString { return nil, nil } @@ -1240,9 +1251,57 @@ func parsePrimitive(raw string, schema *openapi3.SchemaRef) (v any, err error) { return } } + if constrainedTypeless { + return raw, nil + } return } +// 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 { + 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 { + 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 parsePrimitiveCase(raw string, schema *openapi3.SchemaRef, typ string) (any, error) { switch typ { case "integer": @@ -1259,6 +1318,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}, 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 }