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= 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 fea2bc0..1337523 100644 --- a/spec.go +++ b/spec.go @@ -4,8 +4,6 @@ package validate import ( - "bytes" - "encoding/gob" "encoding/json" "fmt" "slices" @@ -99,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 @@ -113,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) @@ -951,14 +960,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 } 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"}}} + } +}