Fix/no in place expansion of caller schema

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

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=

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)
 }
 

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.

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
 	}
 

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())
+}

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"}}}
+  }
+}