From 5b696534a1baffabf0b607f4349ff16861eae287 Mon Sep 17 00:00:00 2001 From: radwaretaltr <119794762+radwaretaltr@users.noreply.github.com> Date: Wed, 7 Jun 2023 17:02:03 +0300 Subject: [PATCH 1/2] openapi3: fix #775 race on schema pattern regex compilation --- .github/workflows/go.yml | 11 ++++--- go.mod | 12 ++++++-- go.sum | 4 --- openapi3/race_test.go | 22 ++++++++++---- openapi3/schema.go | 65 +++++++++++++++++++++++----------------- 5 files changed, 71 insertions(+), 43 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index c8755bf40..2df5de06a 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -12,7 +12,7 @@ jobs: strategy: fail-fast: true matrix: - go: ['1.16', '1.x'] + go: ['1.x'] os: - ubuntu-latest - windows-latest @@ -76,10 +76,13 @@ jobs: GOARCH: '386' - run: go test -count=10 ./... - run: go test -count=2 -covermode=atomic ./... - - run: go test -v -run TestRaceyPatternSchema -race ./... + - run: go test -v -count=10 -run TestRaceyPatternSchemaValidateHindersIt -race ./... env: CGO_ENABLED: '1' - - run: go test -v -run TestIssue741 -race ./... + - run: go test -v -count=10 -run TestRaceyPatternSchemaForIssue775 -race ./... + env: + CGO_ENABLED: '1' + - run: go test -v -count=10 -run TestIssue741 -race ./... env: CGO_ENABLED: '1' - run: git --no-pager diff --exit-code @@ -99,7 +102,7 @@ jobs: run: | ! git grep -InE 'json:"' | grep -v _test.go | grep -v yaml: - - if: runner.os == 'Linux' && matrix.go != '1.16' + - if: runner.os == 'Linux' name: nilness run: go run golang.org/x/tools/go/analysis/passes/nilness/cmd/nilness@latest ./... diff --git a/go.mod b/go.mod index 12a2f1af7..b631a24db 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/getkin/kin-openapi -go 1.16 +go 1.18 require ( github.com/go-openapi/jsonpointer v0.19.5 @@ -9,6 +9,14 @@ require ( github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 github.com/perimeterx/marshmallow v1.1.4 github.com/stretchr/testify v1.8.1 - gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 ) + +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/go-openapi/swag v0.19.5 // indirect + github.com/josharian/intern v1.0.0 // indirect + github.com/mailru/easyjson v0.7.7 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + gopkg.in/yaml.v2 v2.4.0 // indirect +) diff --git a/go.sum b/go.sum index 4d05787e4..9c9f4bce9 100644 --- a/go.sum +++ b/go.sum @@ -6,7 +6,6 @@ github.com/go-openapi/jsonpointer v0.19.5/go.mod h1:Pl9vOtqEWErmShwVjC8pYs9cog34 github.com/go-openapi/swag v0.19.5 h1:lTz6Ys4CmqqCQmZPBlbQENR1/GucA2bzYTE12Pw4tFY= github.com/go-openapi/swag v0.19.5/go.mod h1:POnQmlKehdgb5mhVOsnJFsivZCEZ/vjK9gh66Z9tfKk= github.com/go-test/deep v1.0.8 h1:TDsG77qcSprGbC6vTN8OuXp5g+J+b5Pcguhf7Zt61VM= -github.com/go-test/deep v1.0.8/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE= github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI= github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= github.com/invopop/yaml v0.1.0 h1:YW3WGUoJEXYfzWBjn00zIlrw7brGVD0fUKRYDPAPhrc= @@ -36,10 +35,7 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= -github.com/ugorji/go v1.2.7 h1:qYhyWUUd6WbiM+C6JZAUkIJt/1WrjzNHY9+KCIjVqTo= -github.com/ugorji/go v1.2.7/go.mod h1:nF9osbDWLy6bDVv/Rtoh6QgnvNDpmCalQV5urGCCS6M= github.com/ugorji/go/codec v1.2.7 h1:YPXUKf7fYbp/y8xloBqZOw2qaVggbfwMlI8WM3wZUJ0= -github.com/ugorji/go/codec v1.2.7/go.mod h1:WGN1fab3R1fzQlVQTkfxVtIBhWDRqOviHU95kRgeqEY= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/openapi3/race_test.go b/openapi3/race_test.go index c617cfe49..405942520 100644 --- a/openapi3/race_test.go +++ b/openapi3/race_test.go @@ -9,11 +9,8 @@ import ( "github.com/getkin/kin-openapi/openapi3" ) -func TestRaceyPatternSchema(t *testing.T) { - schema := openapi3.Schema{ - Pattern: "^test|for|race|condition$", - Type: "string", - } +func TestRaceyPatternSchemaValidateHindersIt(t *testing.T) { + schema := openapi3.NewStringSchema().WithPattern("^test|for|race|condition$") err := schema.Validate(context.Background()) require.NoError(t, err) @@ -26,3 +23,18 @@ func TestRaceyPatternSchema(t *testing.T) { go visit() visit() } + +func TestRaceyPatternSchemaForIssue775(t *testing.T) { + schema := openapi3.NewStringSchema().WithPattern("^test|for|race|condition$") + + // err := schema.Validate(context.Background()) + // require.NoError(t, err) + + visit := func() { + err := schema.VisitJSONString("test") + require.NoError(t, err) + } + + go visit() + visit() +} diff --git a/openapi3/schema.go b/openapi3/schema.go index 5abf1a83f..a72f5b317 100644 --- a/openapi3/schema.go +++ b/openapi3/schema.go @@ -13,6 +13,7 @@ import ( "sort" "strconv" "strings" + "sync" "unicode/utf16" "github.com/go-openapi/jsonpointer" @@ -47,6 +48,8 @@ var ( ErrSchemaInputNaN = errors.New("floating point NaN is not allowed") // ErrSchemaInputInf may be returned when validating a number ErrSchemaInputInf = errors.New("floating point Inf is not allowed") + + compiledPatterns sync.Map ) // Float64Ptr is a helper for defining OpenAPI schemas. @@ -154,10 +157,9 @@ type Schema struct { MultipleOf *float64 `json:"multipleOf,omitempty" yaml:"multipleOf,omitempty"` // String - MinLength uint64 `json:"minLength,omitempty" yaml:"minLength,omitempty"` - MaxLength *uint64 `json:"maxLength,omitempty" yaml:"maxLength,omitempty"` - Pattern string `json:"pattern,omitempty" yaml:"pattern,omitempty"` - compiledPattern *regexp.Regexp + MinLength uint64 `json:"minLength,omitempty" yaml:"minLength,omitempty"` + MaxLength *uint64 `json:"maxLength,omitempty" yaml:"maxLength,omitempty"` + Pattern string `json:"pattern,omitempty" yaml:"pattern,omitempty"` // Array MinItems uint64 `json:"minItems,omitempty" yaml:"minItems,omitempty"` @@ -712,7 +714,6 @@ func (schema *Schema) WithMaxLengthDecodedBase64(i int64) *Schema { func (schema *Schema) WithPattern(pattern string) *Schema { schema.Pattern = pattern - schema.compiledPattern = nil return schema } @@ -960,8 +961,8 @@ func (schema *Schema) validate(ctx context.Context, stack []*Schema) ([]*Schema, } } } - if schema.Pattern != "" && !validationOpts.schemaPatternValidationDisabled { - if err := schema.compilePattern(); err != nil { + if !validationOpts.schemaPatternValidationDisabled && schema.Pattern != "" { + if _, err := schema.compilePattern(); err != nil { return stack, err } } @@ -1612,28 +1613,32 @@ func (schema *Schema) visitJSONString(settings *schemaValidationSettings, value } // "pattern" - if schema.Pattern != "" && schema.compiledPattern == nil && !settings.patternValidationDisabled { - var err error - if err = schema.compilePattern(); err != nil { + if !settings.patternValidationDisabled && schema.Pattern != "" { + cpiface, _ := compiledPatterns.Load(schema.Pattern) + cp, _ := cpiface.(*regexp.Regexp) + if cp == nil { + var err error + if cp, err = schema.compilePattern(); err != nil { + if !settings.multiError { + return err + } + me = append(me, err) + } + } + if !cp.MatchString(value) { + err := &SchemaError{ + Value: value, + Schema: schema, + SchemaField: "pattern", + Reason: fmt.Sprintf(`string doesn't match the regular expression "%s"`, schema.Pattern), + customizeMessageError: settings.customizeMessageError, + } if !settings.multiError { return err } me = append(me, err) } } - if cp := schema.compiledPattern; cp != nil && !cp.MatchString(value) { - err := &SchemaError{ - Value: value, - Schema: schema, - SchemaField: "pattern", - Reason: fmt.Sprintf(`string doesn't match the regular expression "%s"`, schema.Pattern), - customizeMessageError: settings.customizeMessageError, - } - if !settings.multiError { - return err - } - me = append(me, err) - } // "format" var formatStrErr string @@ -1985,16 +1990,20 @@ func (schema *Schema) expectedType(settings *schemaValidationSettings, value int } } -func (schema *Schema) compilePattern() (err error) { - if schema.compiledPattern, err = regexp.Compile(schema.Pattern); err != nil { - return &SchemaError{ +// NOTE: racey WRT [writes to schema.Pattern] vs [reads schema.Pattern then writes to compiledPatterns] +func (schema *Schema) compilePattern() (cp *regexp.Regexp, err error) { + pattern := schema.Pattern + if cp, err = regexp.Compile(pattern); err != nil { + err = &SchemaError{ Schema: schema, SchemaField: "pattern", Origin: err, - Reason: fmt.Sprintf("cannot compile pattern %q: %v", schema.Pattern, err), + Reason: fmt.Sprintf("cannot compile pattern %q: %v", pattern, err), } + return } - return nil + var _ bool = compiledPatterns.CompareAndSwap(pattern, nil, cp) + return } // SchemaError is an error that occurs during schema validation. From 77c187c194a22b5f630224dab28bbb928daa9d23 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Wed, 21 Jun 2023 10:38:29 +0200 Subject: [PATCH 2/2] ran: go get -u -a -v ./... && go mod tidy && go mod verify Signed-off-by: Pierre Fenoll --- go.mod | 7 +++---- go.sum | 31 ++++++++++++++----------------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/go.mod b/go.mod index b631a24db..1b8855e56 100644 --- a/go.mod +++ b/go.mod @@ -3,9 +3,9 @@ module github.com/getkin/kin-openapi go 1.18 require ( - github.com/go-openapi/jsonpointer v0.19.5 + github.com/go-openapi/jsonpointer v0.19.6 github.com/gorilla/mux v1.8.0 - github.com/invopop/yaml v0.1.0 + github.com/invopop/yaml v0.2.0 github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 github.com/perimeterx/marshmallow v1.1.4 github.com/stretchr/testify v1.8.1 @@ -14,9 +14,8 @@ require ( require ( github.com/davecgh/go-spew v1.1.1 // indirect - github.com/go-openapi/swag v0.19.5 // indirect + github.com/go-openapi/swag v0.22.4 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/mailru/easyjson v0.7.7 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - gopkg.in/yaml.v2 v2.4.0 // indirect ) diff --git a/go.sum b/go.sum index 9c9f4bce9..1ce7d8619 100644 --- a/go.sum +++ b/go.sum @@ -1,24 +1,25 @@ +github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/go-openapi/jsonpointer v0.19.5 h1:gZr+CIYByUqjcgeLXnQu2gHYQC9o73G2XUeOFYEICuY= -github.com/go-openapi/jsonpointer v0.19.5/go.mod h1:Pl9vOtqEWErmShwVjC8pYs9cog34VGT37dQOVbmoatg= -github.com/go-openapi/swag v0.19.5 h1:lTz6Ys4CmqqCQmZPBlbQENR1/GucA2bzYTE12Pw4tFY= -github.com/go-openapi/swag v0.19.5/go.mod h1:POnQmlKehdgb5mhVOsnJFsivZCEZ/vjK9gh66Z9tfKk= +github.com/go-openapi/jsonpointer v0.19.6 h1:eCs3fxoIi3Wh6vtgmLTOjdhSpiqphQ+DaPn38N2ZdrE= +github.com/go-openapi/jsonpointer v0.19.6/go.mod h1:osyAmYz/mB/C3I+WsTTSgw1ONzaLJoLCyoi6/zppojs= +github.com/go-openapi/swag v0.22.3/go.mod h1:UzaqsxGiab7freDnrUUra0MwWfN/q7tE4j+VcZ0yl14= +github.com/go-openapi/swag v0.22.4 h1:QLMzNJnMGPRNDCbySlcj1x01tzU8/9LTTL9hZZZogBU= +github.com/go-openapi/swag v0.22.4/go.mod h1:UzaqsxGiab7freDnrUUra0MwWfN/q7tE4j+VcZ0yl14= github.com/go-test/deep v1.0.8 h1:TDsG77qcSprGbC6vTN8OuXp5g+J+b5Pcguhf7Zt61VM= github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI= github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= -github.com/invopop/yaml v0.1.0 h1:YW3WGUoJEXYfzWBjn00zIlrw7brGVD0fUKRYDPAPhrc= -github.com/invopop/yaml v0.1.0/go.mod h1:2XuRLgs/ouIrW3XNzuNj7J3Nvu/Dig5MXvbCEdiBN3Q= +github.com/invopop/yaml v0.2.0 h1:7zky/qH+O0DwAyoobXUqvVBwgBFRxKoQ/3FjcVpjTMY= +github.com/invopop/yaml v0.2.0/go.mod h1:2XuRLgs/ouIrW3XNzuNj7J3Nvu/Dig5MXvbCEdiBN3Q= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= -github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= -github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= +github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI= +github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= -github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= -github.com/mailru/easyjson v0.0.0-20190614124828-94de47d64c63/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc= -github.com/mailru/easyjson v0.0.0-20190626092158-b2ccc519800e/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc= +github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0= github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 h1:RWengNIwukTxcDr9M+97sNutRR1RKhG96O6jWumTTnw= @@ -30,18 +31,14 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= -github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/ugorji/go/codec v1.2.7 h1:YPXUKf7fYbp/y8xloBqZOw2qaVggbfwMlI8WM3wZUJ0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= -gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= -gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=