From f89b55f895d8d14902055ed0da770956e4dd4a56 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Mon, 21 Jun 2021 16:21:44 +0200 Subject: [PATCH 1/8] reproduce #371 Signed-off-by: Pierre Fenoll --- .github/workflows/go.yml | 1 + openapi3/race_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 openapi3/race_test.go diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 6233f67e8..499ab730b 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -65,6 +65,7 @@ jobs: run: git --no-pager diff && [[ $(git --no-pager diff --name-only | wc -l) = 0 ]] - run: go test ./... + - run: go test -v -run TestRaceyPatternSchema -race ./... - run: | cd openapi3/testdata go get -u -v github.com/getkin/kin-openapi diff --git a/openapi3/race_test.go b/openapi3/race_test.go new file mode 100644 index 000000000..8bd47c007 --- /dev/null +++ b/openapi3/race_test.go @@ -0,0 +1,26 @@ +package openapi3_test + +import ( + "context" + "testing" + + "github.com/getkin/kin-openapi/openapi3" + "github.com/stretchr/testify/require" +) + +func TestRaceyPatternSchema(t *testing.T) { + schema := openapi3.Schema{ + Pattern: "^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() +} From a436bcaa3bcb5520b61b58a2a77ec3f0440d13f1 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Mon, 21 Jun 2021 16:32:01 +0200 Subject: [PATCH 2/8] go test: -race requires cgo; enable cgo by setting CGO_ENABLED=1 Signed-off-by: Pierre Fenoll --- .github/workflows/go.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 499ab730b..04c631983 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -66,6 +66,8 @@ jobs: - run: go test ./... - run: go test -v -run TestRaceyPatternSchema -race ./... + env: + CGO_ENABLED: '1' - run: | cd openapi3/testdata go get -u -v github.com/getkin/kin-openapi From 722e32c8aa2cd4332c2f771a64aa26a392d27283 Mon Sep 17 00:00:00 2001 From: alexanderbolgov Date: Tue, 22 Jun 2021 11:04:59 +0200 Subject: [PATCH 3/8] compile regex pattern on validate schema --- openapi3/schema.go | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/openapi3/schema.go b/openapi3/schema.go index 8c59f3c04..c5aecd52d 100644 --- a/openapi3/schema.go +++ b/openapi3/schema.go @@ -666,6 +666,11 @@ func (schema *Schema) validate(ctx context.Context, stack []*Schema) (err error) case "byte", "binary", "date", "date-time", "password": // In JSON Draft-07 (not validated yet though): case "regex": + if !SchemaErrorDetailsDisabled { + if err = schema.compilePattern(); err != nil { + return err + } + } case "time", "email", "idn-email": case "hostname", "idn-hostname", "ipv4", "ipv6": case "uri", "uri-reference", "iri", "iri-reference", "uri-template": @@ -1141,13 +1146,7 @@ func (schema *Schema) visitJSONString(settings *schemaValidationSettings, value // "pattern" if pattern := schema.Pattern; pattern != "" && schema.compiledPattern == nil { var err error - if schema.compiledPattern, err = regexp.Compile(pattern); err != nil { - err = &SchemaError{ - Value: value, - Schema: schema, - SchemaField: "pattern", - Reason: fmt.Sprintf("cannot compile pattern %q: %v", pattern, err), - } + if err = schema.compilePattern(); err != nil { if !settings.multiError { return err } @@ -1461,6 +1460,20 @@ func (schema *Schema) expectedType(settings *schemaValidationSettings, typ strin } } +func (schema *Schema) compilePattern() (err error) { + if schema.Pattern == "" { + return nil + } + if schema.compiledPattern, err = regexp.Compile(schema.Pattern); err != nil { + return &SchemaError{ + Schema: schema, + SchemaField: "pattern", + Reason: fmt.Sprintf("cannot compile pattern %q: %v", schema.Pattern, err), + } + } + return err +} + type SchemaError struct { Value interface{} reversePath []string From e435878f553fc436842d59968fa256e807e58278 Mon Sep 17 00:00:00 2001 From: alexanderbolgov Date: Tue, 22 Jun 2021 11:10:26 +0200 Subject: [PATCH 4/8] fix --- openapi3/schema.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openapi3/schema.go b/openapi3/schema.go index c5aecd52d..095d0c671 100644 --- a/openapi3/schema.go +++ b/openapi3/schema.go @@ -666,7 +666,7 @@ func (schema *Schema) validate(ctx context.Context, stack []*Schema) (err error) case "byte", "binary", "date", "date-time", "password": // In JSON Draft-07 (not validated yet though): case "regex": - if !SchemaErrorDetailsDisabled { + if !SchemaFormatValidationDisabled { if err = schema.compilePattern(); err != nil { return err } From f3e30efd3c8255fa3bf820df3f222ac7860654f6 Mon Sep 17 00:00:00 2001 From: alexanderbolgov Date: Tue, 22 Jun 2021 11:20:21 +0200 Subject: [PATCH 5/8] fix --- openapi3/schema.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openapi3/schema.go b/openapi3/schema.go index 095d0c671..726567737 100644 --- a/openapi3/schema.go +++ b/openapi3/schema.go @@ -1471,7 +1471,7 @@ func (schema *Schema) compilePattern() (err error) { Reason: fmt.Sprintf("cannot compile pattern %q: %v", schema.Pattern, err), } } - return err + return nil } type SchemaError struct { From 7be1d5a7725c41247b9ffca5f7e954ec0e917bcb Mon Sep 17 00:00:00 2001 From: alexanderbolgov Date: Thu, 24 Jun 2021 10:03:03 +0200 Subject: [PATCH 6/8] PR comments + test --- openapi3/race_test.go | 1 + openapi3/schema.go | 13 +++++-------- openapi3/schema_test.go | 10 ++++++++++ 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/openapi3/race_test.go b/openapi3/race_test.go index 8bd47c007..4ac31c38e 100644 --- a/openapi3/race_test.go +++ b/openapi3/race_test.go @@ -11,6 +11,7 @@ import ( func TestRaceyPatternSchema(t *testing.T) { schema := openapi3.Schema{ Pattern: "^test|for|race|condition$", + Type: "string", } err := schema.Validate(context.Background()) diff --git a/openapi3/schema.go b/openapi3/schema.go index 726567737..389028e15 100644 --- a/openapi3/schema.go +++ b/openapi3/schema.go @@ -666,11 +666,6 @@ func (schema *Schema) validate(ctx context.Context, stack []*Schema) (err error) case "byte", "binary", "date", "date-time", "password": // In JSON Draft-07 (not validated yet though): case "regex": - if !SchemaFormatValidationDisabled { - if err = schema.compilePattern(); err != nil { - return err - } - } case "time", "email", "idn-email": case "hostname", "idn-hostname", "ipv4", "ipv6": case "uri", "uri-reference", "iri", "iri-reference", "uri-template": @@ -682,6 +677,11 @@ func (schema *Schema) validate(ctx context.Context, stack []*Schema) (err error) } } } + if pattern := schema.Pattern; pattern != "" { + if err = schema.compilePattern(); err != nil { + return err + } + } case "array": if schema.Items == nil { return errors.New("when schema type is 'array', schema 'items' must be non-null") @@ -1461,9 +1461,6 @@ func (schema *Schema) expectedType(settings *schemaValidationSettings, typ strin } func (schema *Schema) compilePattern() (err error) { - if schema.Pattern == "" { - return nil - } if schema.compiledPattern, err = regexp.Compile(schema.Pattern); err != nil { return &SchemaError{ Schema: schema, diff --git a/openapi3/schema_test.go b/openapi3/schema_test.go index f621ca7c9..f724f08e2 100644 --- a/openapi3/schema_test.go +++ b/openapi3/schema_test.go @@ -1219,3 +1219,13 @@ components: require.NotEqual(t, errSchema, err) require.Contains(t, err.Error(), `Error at "/ownerName": Doesn't match schema "not"`) } + +func TestValidationFailsOnInvalidPattern(t *testing.T) { + schema := Schema{ + Pattern: "[", + Type: "string", + } + + var err = schema.Validate(context.Background()) + require.Error(t, err) +} From fcd4b4abc1ab964fcaaa838dc56be8642510b821 Mon Sep 17 00:00:00 2001 From: alexanderbolgov Date: Thu, 24 Jun 2021 10:11:19 +0200 Subject: [PATCH 7/8] simplify --- openapi3/schema.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openapi3/schema.go b/openapi3/schema.go index 389028e15..cd90c6096 100644 --- a/openapi3/schema.go +++ b/openapi3/schema.go @@ -677,7 +677,7 @@ func (schema *Schema) validate(ctx context.Context, stack []*Schema) (err error) } } } - if pattern := schema.Pattern; pattern != "" { + if schema.Pattern != "" { if err = schema.compilePattern(); err != nil { return err } From 917611095f7b4d5acbc20abb231ebff5b161d760 Mon Sep 17 00:00:00 2001 From: alexanderbolgov Date: Thu, 24 Jun 2021 10:12:19 +0200 Subject: [PATCH 8/8] simplify --- openapi3/schema.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openapi3/schema.go b/openapi3/schema.go index cd90c6096..832ce3b3f 100644 --- a/openapi3/schema.go +++ b/openapi3/schema.go @@ -1144,7 +1144,7 @@ func (schema *Schema) visitJSONString(settings *schemaValidationSettings, value } // "pattern" - if pattern := schema.Pattern; pattern != "" && schema.compiledPattern == nil { + if schema.Pattern != "" && schema.compiledPattern == nil { var err error if err = schema.compilePattern(); err != nil { if !settings.multiError {