openapi3: fix getkin#775 race on schema pattern regex compilation

.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 ./...
 

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

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=

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

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.