From ea4ea79bec03b9227c4129cf0638701c4d1f3659 Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Mon, 24 Aug 2026 16:35:22 +0200 Subject: [PATCH 1/3] fix(validate): clone schemata through JSON instead of gob deepCloneSchema round-tripped through gob, which omits any field holding its zero value and flattens a pointer to what it points at. A *float64 pointing at 0 therefore travelled as the zero value and came back nil, so "minimum": 0 was dropped - and the JSON Schema meta-schema spells every positiveInteger that way, which is what maxLength, maxItems, minLength and multipleOf resolve to. The one caller clones the meta-schema's #/definitions/parameter before expanding it, so parameter validation has been checking against a meta-schema with those lower bounds missing. The copy now goes through jsonutils.FromDynamicJSON, the form spec.Schema is defined by. On a mid-sized document JSON also round-trips faster than gob and allocates less than half as much, so nothing is traded for the correctness. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Frederic BIDON --- spec.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/spec.go b/spec.go index fea2bc0..7296b48 100644 --- a/spec.go +++ b/spec.go @@ -4,8 +4,6 @@ package validate import ( - "bytes" - "encoding/gob" "encoding/json" "fmt" "slices" @@ -951,14 +949,15 @@ func (s *SpecValidator) expandedAnalyzer() *analysis.Spec { return s.analyzer } +// deepCloneSchema returns a copy of src that shares nothing with it. +// +// The copy goes through JSON, which is the form [spec.Schema] is defined by. gob drops any field +// holding its zero value and flattens a pointer to what it points at, so a *float64 pointing at +// 0 - "minimum": 0, which the JSON Schema meta-schema spells for every positiveInteger - came +// back nil and the bound was lost. JSON is also the faster of the two on this model. func deepCloneSchema(src spec.Schema) (spec.Schema, error) { - var b bytes.Buffer - if err := gob.NewEncoder(&b).Encode(src); err != nil { - return spec.Schema{}, err - } - var dst spec.Schema - if err := gob.NewDecoder(&b).Decode(&dst); err != nil { + if err := jsonutils.FromDynamicJSON(src, &dst); err != nil { return spec.Schema{}, err } From 9e394037a262fbd6cabc01f42a181a9ec2369dfc Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Mon, 24 Aug 2026 16:08:40 +0200 Subject: [PATCH 2/3] fix(validate): stop rewriting the caller's document while validating newSchemaValidator expands a schema that carries a $ref or an id, and spec.ExpandSchemaWithOptions rewrites the schema it is given. Several callers hand it a schema that belongs to the document the caller still holds: the members of allOf/anyOf/oneOf, "not", a dependency schema, additionalProperties, and every schema the default and example validators walk out of the specification. Validating replaced those $ref with their targets, so a caller that went on to flatten the document worked on a different document than the one it loaded - go-swagger reloads the spec after validating for exactly this reason. SpecValidator.Validate now takes one copy of the document and works on that. Expansion below is unchanged, so what validation reports is unchanged too, down to the error paths. Raw() is read before the copy, so the checks that go through the authored bytes still see them. NewSchemaValidator does the same for the single-schema API. Both mark SchemaValidatorOptions.ownSchemata, which tells the validators beneath that the schemata they walk are theirs to rewrite. The copy is taken at the entry point rather than in newSchemaValidator because a copy per schema is paid again at every level of a recursive document: on the kubernetes benchmark that cost 4x the time and 3.3x the memory. Cloning once measures as no change (benchstat p=0.69, n=5). It also has to cover parameters, path items and responses, which expand as well. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Frederic BIDON --- schema.go | 16 ++++++++++++++ schema_option.go | 9 ++++++++ spec.go | 13 ++++++++++- spec_test.go | 30 ++++++++++++++++++++++++++ testdata/bugs/no-mutation/fixture.json | 20 +++++++++++++++++ 5 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 testdata/bugs/no-mutation/fixture.json diff --git a/schema.go b/schema.go index e7af892..9b49007 100644 --- a/schema.go +++ b/schema.go @@ -61,6 +61,22 @@ func NewSchemaValidator(schema *spec.Schema, rootSchema any, root string, format o(opts) } + // the caller still owns this schema, and validation expands what it walks: work on a copy, + // once, here - every validator below this one is then free to expand in place + if !opts.ownSchemata && schema != nil { + cloned, err := deepCloneSchema(*schema) + if err != nil { + panic(invalidSchemaProvidedMsg(err).Error()) + } + + if rootSchema == schema { + rootSchema = &cloned + } + + schema = &cloned + opts.ownSchemata = true + } + return newSchemaValidator(schema, rootSchema, rootPathFromString(root), formats, opts) } diff --git a/schema_option.go b/schema_option.go index 3ca489c..09a03eb 100644 --- a/schema_option.go +++ b/schema_option.go @@ -18,6 +18,15 @@ type SchemaValidatorOptions struct { recycleResult bool skipSchemataResult bool pathLoaderWithOptions func(string, ...loading.Option) (json.RawMessage, error) + + // ownSchemata tells the validators that the schemata they walk are theirs to rewrite. + // + // Validation expands every schema carrying a $ref or an id, and expansion rewrites what it is + // given. The copy that makes this safe is taken once, at the entry point - the whole document + // in [SpecValidator.Validate], the single schema in [NewSchemaValidator] - because a copy per + // schema multiplies with the recursion, and because parameters, path items and responses + // expand too, not only schemata. + ownSchemata bool } // Option sets optional rules for schema validation. diff --git a/spec.go b/spec.go index 7296b48..1337523 100644 --- a/spec.go +++ b/spec.go @@ -97,6 +97,17 @@ func (s *SpecValidator) Validate(data any) (*Result, *Result) { errs.AddErrors(invalidDocumentMsg()) return errs, warnings // no point in continuing } + + // Validation expands what it walks - schemata, but also parameters, path items and responses - + // and expansion rewrites what it is given. Take one copy of the whole document here and work + // on that, so the caller gets back the document it handed over. Raw() still reads the bytes as + // they were authored, so the checks below that go through them are unaffected. + // + // Cloning here rather than per schema is what keeps the cost flat: a copy taken inside + // newSchemaValidator is paid again at every level of a recursive document. + raw := sd.Raw() + sd = sd.Pristine() + s.schemaOptions.ownSchemata = true s.spec = sd s.analyzer = analysis.New(sd.Spec()) // where each $ref sits, as authored: refs are reported against the @@ -111,7 +122,7 @@ func (s *SpecValidator) Validate(data any) (*Result, *Result) { // Raw spec unmarshalling errors var obj any - if err := json.Unmarshal(sd.Raw(), &obj); err != nil { + if err := json.Unmarshal(raw, &obj); err != nil { // NOTE: under normal conditions, the *load.Document has been already unmarshalled // So this one is just a paranoid check on the behavior of the spec package panic(InvalidDocumentError) diff --git a/spec_test.go b/spec_test.go index b9108fb..99dd040 100644 --- a/spec_test.go +++ b/spec_test.go @@ -942,3 +942,33 @@ func Test_2866(t *testing.T) { require.NoError(t, Spec(doc, strfmt.Default)) } + +// TestSpec_DoesNotMutateDocument checks that validation leaves the caller's document as it was. +// +// newSchemaValidator expands a schema carrying a $ref, and several call sites hand it a schema +// that belongs to the document the caller still holds - additionalProperties here, reached while +// the default validator walks the specification. Expanding in place replaced the $ref with its +// target, so callers that went on to flatten the document worked on a different document than the +// one they loaded (go-swagger works around this by reloading the spec after validating). +func TestSpec_DoesNotMutateDocument(t *testing.T) { + path := filepath.Join("testdata", "bugs", "no-mutation", "fixture.json") + + doc, err := loads.Spec(path) + require.NoError(t, err) + + before, err := json.Marshal(doc.Spec()) + require.NoError(t, err) + + require.NoError(t, Spec(doc, strfmt.Default)) + + after, err := json.Marshal(doc.Spec()) + require.NoError(t, err) + + assert.JSONEqf(t, string(before), string(after), "validation rewrote the caller's document") + + // the $ref that used to be inlined + bag := doc.Spec().Definitions["Bag"] + require.NotNil(t, bag.AdditionalProperties) + require.NotNil(t, bag.AdditionalProperties.Schema) + assert.EqualT(t, "#/definitions/Item", bag.AdditionalProperties.Schema.Ref.String()) +} diff --git a/testdata/bugs/no-mutation/fixture.json b/testdata/bugs/no-mutation/fixture.json new file mode 100644 index 0000000..191e86b --- /dev/null +++ b/testdata/bugs/no-mutation/fixture.json @@ -0,0 +1,20 @@ +{ + "swagger": "2.0", + "info": {"title": "validate must not rewrite the caller's document", "version": "1.0.0"}, + "paths": { + "/things": { + "get": { + "parameters": [{"name": "body", "in": "body", "schema": {"$ref": "#/definitions/Bag"}}], + "responses": {"200": {"description": "ok"}} + } + } + }, + "definitions": { + "Bag": { + "type": "object", + "additionalProperties": {"$ref": "#/definitions/Item"}, + "default": {"anything": {"name": "x"}} + }, + "Item": {"type": "object", "properties": {"name": {"type": "string"}}} + } +} From bbce87e5ca161742f33b62e51ff94d53f0cc35a4 Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Mon, 24 Aug 2026 20:42:38 +0200 Subject: [PATCH 3/3] fix: upgrade spec and analysis to get expand fixes Signed-off-by: Frederic BIDON --- go.mod | 24 ++++++++++++------------ go.sum | 52 ++++++++++++++++++++++++++-------------------------- 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/go.mod b/go.mod index e2f1119..7543638 100644 --- a/go.mod +++ b/go.mod @@ -1,27 +1,27 @@ module github.com/go-openapi/validate require ( - github.com/go-openapi/analysis v0.26.0 + github.com/go-openapi/analysis v0.26.1 github.com/go-openapi/errors v0.22.8 github.com/go-openapi/jsonpointer v1.0.0 github.com/go-openapi/loads v0.25.1 - github.com/go-openapi/spec v0.22.9 + github.com/go-openapi/spec v0.22.10 github.com/go-openapi/strfmt v0.27.0 - github.com/go-openapi/swag/conv v0.29.0 - github.com/go-openapi/swag/fileutils v0.29.0 - github.com/go-openapi/swag/jsonutils v0.29.0 - github.com/go-openapi/swag/loading v0.29.0 - github.com/go-openapi/swag/pools v0.29.0 - github.com/go-openapi/swag/stringutils v0.29.0 - github.com/go-openapi/testify/v2 v2.6.1 + github.com/go-openapi/swag/conv v0.29.1 + github.com/go-openapi/swag/fileutils v0.29.1 + github.com/go-openapi/swag/jsonutils v0.29.1 + github.com/go-openapi/swag/loading v0.29.1 + github.com/go-openapi/swag/pools v0.29.1 + github.com/go-openapi/swag/stringutils v0.29.1 + github.com/go-openapi/testify/v2 v2.7.0 go.yaml.in/yaml/v3 v3.0.5 ) require ( github.com/go-openapi/jsonreference v1.0.0 // indirect - github.com/go-openapi/swag/mangling v0.28.0 // indirect - github.com/go-openapi/swag/typeutils v0.29.0 // indirect - github.com/go-openapi/swag/yamlutils v0.29.0 // indirect + github.com/go-openapi/swag/mangling v0.29.1 // indirect + github.com/go-openapi/swag/typeutils v0.29.1 // indirect + github.com/go-openapi/swag/yamlutils v0.29.1 // indirect github.com/go-viper/mapstructure/v2 v2.5.0 // indirect github.com/google/uuid v1.6.0 // indirect github.com/oklog/ulid/v2 v2.1.2 // indirect diff --git a/go.sum b/go.sum index d0b4ec0..0757740 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -github.com/go-openapi/analysis v0.26.0 h1:1xECln1iMMmQnTjgcknC1vi1hA4KISt6IHpSwnqcuwI= -github.com/go-openapi/analysis v0.26.0/go.mod h1:40gERFi/2dyXA1FaqRRLxkv1IlC6X+GPDNd1xrYAjZE= +github.com/go-openapi/analysis v0.26.1 h1:BqYuDaQiFflcgPWGDQc7niUFE2pHTVq1H3iwDsRYMqM= +github.com/go-openapi/analysis v0.26.1/go.mod h1:E2siwFrz00/Z1sifwhy63h+hMkg0USofwD4gYPYeQME= github.com/go-openapi/errors v0.22.8 h1:oP7sW7TWc3wFFjrzzj0nI83H2qMBkNjNfSd+XRejk/I= github.com/go-openapi/errors v0.22.8/go.mod h1:BuUoHcYrU6E7V9gfj1I5wLQqgtIHnup/alXZ8KdgQ0w= github.com/go-openapi/jsonpointer v1.0.0 h1:kR9tHqY0CtZaOPVFm622dPVNhrvYpwr4uCxgL3h1H8s= @@ -8,34 +8,34 @@ github.com/go-openapi/jsonreference v1.0.0 h1:jlmTr6torcd1YgDQvSfNmRtKzYDO4FGBkr github.com/go-openapi/jsonreference v1.0.0/go.mod h1:jtwdyGbJk0Xhe5Y+rwtglQP6Sb1WZST4rT32LWB+sv0= github.com/go-openapi/loads v0.25.1 h1:toKQdIDLxlqfKLLGUUmUsiTd5/X0Chzvde9EGYQP/Ac= github.com/go-openapi/loads v0.25.1/go.mod h1:33Hen4tsKXHL45TyYojvfD5fZUFN4O1y4r/XhsRW2zc= -github.com/go-openapi/spec v0.22.9 h1:/vKIFDcGKp0ktZWGbym/tJEWbk6/XOEmAVU0kqKMH+w= -github.com/go-openapi/spec v0.22.9/go.mod h1:b/mNUYIOQOyIiUzUzXEE8xzyZqf93KvM9hQGP91yfl0= +github.com/go-openapi/spec v0.22.10 h1:5cp1dq++t4U/4WCg6f1wqReZowUrJ5kl8Eri4SMl51s= +github.com/go-openapi/spec v0.22.10/go.mod h1:aWRr+Ntv5tHoMQo0C1slTNLFo1FOYdZXFlUqViCN7yM= github.com/go-openapi/strfmt v0.27.0 h1:kbcTeaD9TXuXD0hhMXzuYa1sdTo6+dWGvwjW93E80IM= github.com/go-openapi/strfmt v0.27.0/go.mod h1:s/qhDqfY72irigXUGJmtgid2Rm+3tnz3k8hZaRmvWYc= -github.com/go-openapi/swag/conv v0.29.0 h1:4+1TogWpOIzMPzVKrvx1BfqBYlApB7D7DW3EAWpwmp4= -github.com/go-openapi/swag/conv v0.29.0/go.mod h1:ch1l7V87F6zQXuLs5s0RFvrro6aFvrVcfVXn2PTZnu8= -github.com/go-openapi/swag/fileutils v0.29.0 h1:meobnn3MsAkF6XmJn6qw3hPeMAOhwf7XD5BOKeFzqWU= -github.com/go-openapi/swag/fileutils v0.29.0/go.mod h1:/wofKYckbtRl2p3+EwQsosie5CT1B38+dQ+PS579BzI= -github.com/go-openapi/swag/jsonutils v0.29.0 h1:Xgnf9g32ycQjQUnDxkhqLraH2FhitcSE3w7ayQB3TgA= -github.com/go-openapi/swag/jsonutils v0.29.0/go.mod h1:5WYmjf6hJcBve+ArzBaUsYy4M1GXsgjIQTmwJKfZHrA= -github.com/go-openapi/swag/jsonutils/fixtures_test v0.29.0 h1:bpSF6LFkJJVtaRtJCzbZADVPVHQYKPwPdKthOQA2/5o= -github.com/go-openapi/swag/jsonutils/fixtures_test v0.29.0/go.mod h1:julgTUKZ9/D0j6O7GKajmRs+812FWxQg/mMpGunWSjg= -github.com/go-openapi/swag/loading v0.29.0 h1:r1lg2DQbT1VgBwgiPYXBM059RNswFI6r36CC0QCcRGw= -github.com/go-openapi/swag/loading v0.29.0/go.mod h1:l/Z4MNbom0jSqzvWJqK2VUUWEceBknGEuVbLHLq4KN0= -github.com/go-openapi/swag/mangling v0.28.0 h1:pH8eyeNO9SLYsTMWJrurnNfKmDa28XrlA+HePVD53VM= -github.com/go-openapi/swag/mangling v0.28.0/go.mod h1:jtBE2+V+3pILxOR7Vgce+Cwp6A2PgZbvVqfNntbVs0w= -github.com/go-openapi/swag/pools v0.29.0 h1:uMQcoJeHJ8fWkdfEXJZMMpqk6hpfW8qTL5Q/IoRFFII= -github.com/go-openapi/swag/pools v0.29.0/go.mod h1:leDcaghjkRAhCuCRv9NfJU5f0mjoU3cT/XZObhMk3pc= -github.com/go-openapi/swag/stringutils v0.29.0 h1:/IEOuZ7PGJi6lqgH83dVt7/A9eHsDGEH1459lm+gpEo= -github.com/go-openapi/swag/stringutils v0.29.0/go.mod h1:7fSqZ+z8Qc0tOfAAK0jVa5qFGrnIlRi6n7NeGGrr1vc= -github.com/go-openapi/swag/typeutils v0.29.0 h1:HrWCYZeXVVNDo/7QQPRaYk33XeIDxksbxpalID3bWR8= -github.com/go-openapi/swag/typeutils v0.29.0/go.mod h1:hxpgDZJVBkBsi/d3MIUosafoFdE5exaQRmVp0zwu3YE= -github.com/go-openapi/swag/yamlutils v0.29.0 h1:JOKKuhMnBx4HYTM+kPEYw8S5YKKU9PnC4Mwb+c69BBA= -github.com/go-openapi/swag/yamlutils v0.29.0/go.mod h1:/+FVozjFWZzku6mRz5U/Qmq5Yk8PLFxBLLWA/jHaxYE= +github.com/go-openapi/swag/conv v0.29.1 h1:AC4Eh/5c/eUDOUCzzsRC9ghmFgOSBHeRMGIngY0ZUGA= +github.com/go-openapi/swag/conv v0.29.1/go.mod h1:S1X7/ZrBEZOC0Wc8AGxjbcGS92l3WEjA7aPtpl+RaqM= +github.com/go-openapi/swag/fileutils v0.29.1 h1:ZcPzMceVhU1WPbK6N1G6sNQKdd1CWJlf3cA08UHuoM0= +github.com/go-openapi/swag/fileutils v0.29.1/go.mod h1:/wofKYckbtRl2p3+EwQsosie5CT1B38+dQ+PS579BzI= +github.com/go-openapi/swag/jsonutils v0.29.1 h1:AFCxs0eQZ24/QyfhVHM2t49rMz7Vv3XCsZQI6yrNy+c= +github.com/go-openapi/swag/jsonutils v0.29.1/go.mod h1:u3+sCfJpttDpcmS5kpm0yxL6GK0eWgODsx8Yw8fcqNM= +github.com/go-openapi/swag/jsonutils/fixtures_test v0.29.1 h1:BiiXE31Bx9SfpsMmOQj5KYpUhTZBpLVriVhJDuLuY2o= +github.com/go-openapi/swag/jsonutils/fixtures_test v0.29.1/go.mod h1:julgTUKZ9/D0j6O7GKajmRs+812FWxQg/mMpGunWSjg= +github.com/go-openapi/swag/loading v0.29.1 h1:FCv5fG8UhTdDJa2R7w+5O9Ekpcbw7tt0nFWvmDKGBjc= +github.com/go-openapi/swag/loading v0.29.1/go.mod h1:N0ESuem4p2oedKal8EJhciqnJ9Q9Wmt83L1CRB3Fouw= +github.com/go-openapi/swag/mangling v0.29.1 h1:lHALtvYCdxVnRl4GrHmFPwfBTZYIObqdGNSKyu/8D6I= +github.com/go-openapi/swag/mangling v0.29.1/go.mod h1:SAop9pB7PUjQ/CGCNf/JmCKTRK+GDO+RqE9UHqC/N6s= +github.com/go-openapi/swag/pools v0.29.1 h1:NRogYxdEW9SjRM4mkAOji9iefO4MRXq3p/ZJcoQbUKg= +github.com/go-openapi/swag/pools v0.29.1/go.mod h1:leDcaghjkRAhCuCRv9NfJU5f0mjoU3cT/XZObhMk3pc= +github.com/go-openapi/swag/stringutils v0.29.1 h1:1ykunK7iJQk1uOO7+oUH1ukbsK85fFCOiCFMOVSY+F0= +github.com/go-openapi/swag/stringutils v0.29.1/go.mod h1:7fSqZ+z8Qc0tOfAAK0jVa5qFGrnIlRi6n7NeGGrr1vc= +github.com/go-openapi/swag/typeutils v0.29.1 h1:Nzv9nhnlLCRBPQqfOX+7lB6Guju370or8StT+lIOf6M= +github.com/go-openapi/swag/typeutils v0.29.1/go.mod h1:hxpgDZJVBkBsi/d3MIUosafoFdE5exaQRmVp0zwu3YE= +github.com/go-openapi/swag/yamlutils v0.29.1 h1:69w3tsBajm7MR/fejLy7HD/3J68Ys1SeeZMEzZ3w2sk= +github.com/go-openapi/swag/yamlutils v0.29.1/go.mod h1:rgsp3vT/QdWzKwn43CigDwjOGIenPyTZMKnxEM8jZOA= github.com/go-openapi/testify/enable/yaml/v2 v2.6.1 h1:Jm+/ze2rMtbD98yen92AhATGLGREDYXG56Xr4gMjEtE= github.com/go-openapi/testify/enable/yaml/v2 v2.6.1/go.mod h1:YDPnwCRDu38/oJBVMBVXOUDiJ9cIeBHWvfImHaXqnv4= -github.com/go-openapi/testify/v2 v2.6.1 h1:6CNJhTjMzgaeaH8WhshcsZNPIvRemiOcFpU7seO/y7Q= -github.com/go-openapi/testify/v2 v2.6.1/go.mod h1:SgsVHtfooshd0tublTtJ50FPKhujf47YRqauXXOUxfw= +github.com/go-openapi/testify/v2 v2.7.0 h1:bycOreEj6wfBvijg3YFogZ/sFjTCDmQnwSodSzHa3X8= +github.com/go-openapi/testify/v2 v2.7.0/go.mod h1:SgsVHtfooshd0tublTtJ50FPKhujf47YRqauXXOUxfw= github.com/go-viper/mapstructure/v2 v2.5.0 h1:vM5IJoUAy3d7zRSVtIwQgBj7BiWtMPfmPEgAXnvj1Ro= github.com/go-viper/mapstructure/v2 v2.5.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=