Repository: getkin/kin-openapi
Pull request #375 (2021-06-22 09:17:59)

Compile pattern on validate


Patch:
diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml
index 6233f67..be19f94 100644
--- a/.github/workflows/go.yml
+++ b/.github/workflows/go.yml
@@ -65,10 +65,14 @@ 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 ./...
+      env:
+        CGO_ENABLED: '1'
     - run: |
         cd openapi3/testdata
         go get -u -v github.com/getkin/kin-openapi
         go test -tags with_embed ./...
+        git --no-pager diff && git checkout -- .
         cd -
       if: matrix.go != '1.14'
     - if: runner.os == 'Linux'
diff --git a/openapi3/loader.go b/openapi3/loader.go
index ff93182..da34797 100644
--- a/openapi3/loader.go
+++ b/openapi3/loader.go
@@ -305,6 +305,9 @@ func (loader *Loader) resolveComponent(
 	}
 	var cursor interface{}
 	if cursor, err = drill(doc); err != nil {
+		if path == nil {
+			return nil, err
+		}
 		var err2 error
 		data, err2 := loader.readURL(path)
 		if err2 != nil {
@@ -346,6 +349,14 @@ func (loader *Loader) resolveComponent(
 }
 
 func drillIntoField(cursor interface{}, fieldName string) (interface{}, error) {
+	// Special case due to multijson
+	if s, ok := cursor.(*SchemaRef); ok && fieldName == "additionalProperties" {
+		if ap := s.Value.AdditionalPropertiesAllowed; ap != nil {
+			return *ap, nil
+		}
+		return s.Value.AdditionalProperties, nil
+	}
+
 	switch val := reflect.Indirect(reflect.ValueOf(cursor)); val.Kind() {
 	case reflect.Map:
 		elementValue := val.MapIndex(reflect.ValueOf(fieldName))
@@ -372,6 +383,10 @@ func drillIntoField(cursor interface{}, fieldName string) (interface{}, error) {
 			field := val.Type().Field(i)
 			tagValue := field.Tag.Get("yaml")
 			yamlKey := strings.Split(tagValue, ",")[0]
+			if yamlKey == "-" {
+				tagValue := field.Tag.Get("multijson")
+				yamlKey = strings.Split(tagValue, ",")[0]
+			}
 			if yamlKey == fieldName {
 				return val.Field(i).Interface(), nil
 			}
diff --git a/openapi3/schema.go b/openapi3/schema.go
index 8c59f3c..0010627 100644
--- a/openapi3/schema.go
+++ b/openapi3/schema.go
@@ -109,8 +109,6 @@ type Schema struct {
 	Example      interface{}   `json:"example,omitempty" yaml:"example,omitempty"`
 	ExternalDocs *ExternalDocs `json:"externalDocs,omitempty" yaml:"externalDocs,omitempty"`
 
-	// Object-related, here for struct compactness
-	AdditionalPropertiesAllowed *bool `json:"-" multijson:"additionalProperties,omitempty" yaml:"-"`
 	// Array-related, here for struct compactness
 	UniqueItems bool `json:"uniqueItems,omitempty" yaml:"uniqueItems,omitempty"`
 	// Number-related, here for struct compactness
@@ -141,12 +139,13 @@ type Schema struct {
 	Items    *SchemaRef `json:"items,omitempty" yaml:"items,omitempty"`
 
 	// Object
-	Required             []string       `json:"required,omitempty" yaml:"required,omitempty"`
-	Properties           Schemas        `json:"properties,omitempty" yaml:"properties,omitempty"`
-	MinProps             uint64         `json:"minProperties,omitempty" yaml:"minProperties,omitempty"`
-	MaxProps             *uint64        `json:"maxProperties,omitempty" yaml:"maxProperties,omitempty"`
-	AdditionalProperties *SchemaRef     `json:"-" multijson:"additionalProperties,omitempty" yaml:"-"`
-	Discriminator        *Discriminator `json:"discriminator,omitempty" yaml:"discriminator,omitempty"`
+	Required                    []string       `json:"required,omitempty" yaml:"required,omitempty"`
+	Properties                  Schemas        `json:"properties,omitempty" yaml:"properties,omitempty"`
+	MinProps                    uint64         `json:"minProperties,omitempty" yaml:"minProperties,omitempty"`
+	MaxProps                    *uint64        `json:"maxProperties,omitempty" yaml:"maxProperties,omitempty"`
+	AdditionalPropertiesAllowed *bool          `multijson:"additionalProperties,omitempty" json:"-" yaml:"-"` // In this order...
+	AdditionalProperties        *SchemaRef     `multijson:"additionalProperties,omitempty" json:"-" yaml:"-"` // ...for multijson
+	Discriminator               *Discriminator `json:"discriminator,omitempty" yaml:"discriminator,omitempty"`
 }
 
 var _ jsonpointer.JSONPointable = (*Schema)(nil)
@@ -677,6 +676,11 @@ func (schema *Schema) validate(ctx context.Context, stack []*Schema) (err error)
 				}
 			}
 		}
+		if schema.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")
@@ -1139,15 +1143,9 @@ 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 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 +1459,17 @@ func (schema *Schema) expectedType(settings *schemaValidationSettings, typ strin
 	}
 }
 
+func (schema *Schema) compilePattern() (err error) {
+	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 nil
+}
+
 type SchemaError struct {
 	Value       interface{}
 	reversePath []string


Test patch:
diff --git a/openapi3/issue376_test.go b/openapi3/issue376_test.go
new file mode 100644
index 0000000..22aa7fb
--- /dev/null
+++ b/openapi3/issue376_test.go
@@ -0,0 +1,102 @@
+package openapi3
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestIssue376(t *testing.T) {
+	spec := []byte(`
+openapi: 3.0.0
+components:
+  schemas:
+    schema1:
+      type: object
+      additionalProperties:
+        type: string
+    schema2:
+      type: object
+      properties:
+        prop:
+          $ref: '#/components/schemas/schema1/additionalProperties'
+paths: {}
+info:
+  title: An API
+  version: 1.2.3.4
+`)
+
+	loader := NewLoader()
+
+	doc, err := loader.LoadFromData(spec)
+	require.NoError(t, err)
+
+	err = doc.Validate(loader.Context)
+	require.NoError(t, err)
+
+	require.Equal(t, "An API", doc.Info.Title)
+	require.Equal(t, 2, len(doc.Components.Schemas))
+	require.Equal(t, 0, len(doc.Paths))
+
+	require.Equal(t, "string", doc.Components.Schemas["schema2"].Value.Properties["prop"].Value.Type)
+}
+
+func TestMultijsonTagSerialization(t *testing.T) {
+	spec := []byte(`
+openapi: 3.0.0
+components:
+  schemas:
+    unset:
+      type: number
+    #empty-object:
+    # TODO additionalProperties: {}
+    object:
+      additionalProperties: {type: string}
+    boolean:
+      additionalProperties: false
+paths: {}
+info:
+  title: An API
+  version: 1.2.3.4
+`)
+
+	loader := NewLoader()
+
+	doc, err := loader.LoadFromData(spec)
+	require.NoError(t, err)
+
+	err = doc.Validate(loader.Context)
+	require.NoError(t, err)
+
+	for propName, propSchema := range doc.Components.Schemas {
+		ap := propSchema.Value.AdditionalProperties
+		apa := propSchema.Value.AdditionalPropertiesAllowed
+
+		encoded, err := propSchema.MarshalJSON()
+		require.NoError(t, err)
+		require.Equal(t, string(encoded), map[string]string{
+			"unset": `{"type":"number"}`,
+			// TODO: "empty-object":`{"additionalProperties":{}}`,
+			"object":  `{"additionalProperties":{"type":"string"}}`,
+			"boolean": `{"additionalProperties":false}`,
+		}[propName])
+
+		if propName == "unset" {
+			require.True(t, ap == nil && apa == nil)
+			continue
+		}
+
+		apStr := ""
+		if ap != nil {
+			apStr = fmt.Sprintf("{Ref:%s Value.Type:%v}", (*ap).Ref, (*ap).Value.Type)
+		}
+		apaStr := ""
+		if apa != nil {
+			apaStr = fmt.Sprintf("%v", *apa)
+		}
+
+		require.Truef(t, (ap != nil && apa == nil) || (ap == nil && apa != nil),
+			"%s: isnil(%s) xor isnil(%s)", propName, apaStr, apStr)
+	}
+}
diff --git a/openapi3/race_test.go b/openapi3/race_test.go
new file mode 100644
index 0000000..4ac31c3
--- /dev/null
+++ b/openapi3/race_test.go
@@ -0,0 +1,27 @@
+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$",
+		Type:    "string",
+	}
+
+	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_test.go b/openapi3/schema_test.go
index f621ca7..f724f08 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)
+}