Repository: getkin/kin-openapi (go)

Issue:
Existing valid specs now fail validation because of circular reference check.
Existing specs which used to validate fine with previous version, are now failing to load because of the new circular reference check [https://github.com/getkin/kin-openapi/pull/607].
The hardcoded limit of 3 visits per definition is too aggressive for complex API specs.
I propose increasing the default check limit to a higher number like 100 or 1K. Since the check was introduced to guard against infinite loop, a higher limit would still do the job, but would allow for complex specs to still be validated correctly.
Ideally the limit should be configurable on the loader.


Pull request description:
Introduce package-wide CircularReferenceCounter to work around #615
Superseeds https://github.com/getkin/kin-openapi/pull/616

Fixes #615 

Patch:
diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml
index d34061a..649a148 100644
--- a/.github/workflows/go.yml
+++ b/.github/workflows/go.yml
@@ -62,10 +62,10 @@ jobs:
     - run: git --no-pager diff --exit-code
 
     - if: runner.os == 'Linux'
-      run: go test ./...
+      run: go test -count=10 ./...
       env:
         GOARCH: '386'
-    - run: go test ./...
+    - run: go test -count=10 ./...
     - run: go test -count=2 -covermode=atomic ./...
     - run: go test -v -run TestRaceyPatternSchema -race ./...
       env:
diff --git a/openapi3/loader.go b/openapi3/loader.go
index 637e1ac..0c08331 100644
--- a/openapi3/loader.go
+++ b/openapi3/loader.go
@@ -17,6 +17,7 @@ import (
 )
 
 var CircularReferenceError = "kin-openapi bug found: circular schema reference not handled"
+var CircularReferenceCounter = 3
 
 func foundUnresolvedRef(ref string) error {
 	return fmt.Errorf("found unresolved ref: %q", ref)
@@ -724,7 +725,7 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat
 			}
 			component.Value = &schema
 		} else {
-			if visitedLimit(visited, ref, 3) {
+			if visitedLimit(visited, ref) {
 				visited = append(visited, ref)
 				return fmt.Errorf("%s - %s", CircularReferenceError, strings.Join(visited, " -> "))
 			}
@@ -1088,12 +1089,12 @@ func unescapeRefString(ref string) string {
 	return strings.Replace(strings.Replace(ref, "~1", "/", -1), "~0", "~", -1)
 }
 
-func visitedLimit(visited []string, ref string, limit int) bool {
+func visitedLimit(visited []string, ref string) bool {
 	visitedCount := 0
 	for _, v := range visited {
 		if v == ref {
 			visitedCount++
-			if visitedCount >= limit {
+			if visitedCount >= CircularReferenceCounter {
 				return true
 			}
 		}


Test patch:
diff --git a/openapi3/issue615_test.go b/openapi3/issue615_test.go
new file mode 100644
index 0000000..ceb317a
--- /dev/null
+++ b/openapi3/issue615_test.go
@@ -0,0 +1,33 @@
+package openapi3_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/getkin/kin-openapi/openapi3"
+)
+
+func TestIssue615(t *testing.T) {
+	for {
+		loader := openapi3.NewLoader()
+		loader.IsExternalRefsAllowed = true
+		_, err := loader.LoadFromFile("testdata/recursiveRef/issue615.yml")
+		if err == nil {
+			continue
+		}
+		// Test currently reproduces the issue 615: failure to load a valid spec
+		// Upon issue resolution, this check should be changed to require.NoError
+		require.Error(t, err, openapi3.CircularReferenceError)
+		break
+	}
+
+	var old int
+	old, openapi3.CircularReferenceCounter = openapi3.CircularReferenceCounter, 4
+	defer func() { openapi3.CircularReferenceCounter = old }()
+
+	loader := openapi3.NewLoader()
+	loader.IsExternalRefsAllowed = true
+	_, err := loader.LoadFromFile("testdata/recursiveRef/issue615.yml")
+	require.NoError(t, err)
+}
diff --git a/openapi3/testdata/recursiveRef/issue615.yml b/openapi3/testdata/recursiveRef/issue615.yml
new file mode 100644
index 0000000..d1370e3
--- /dev/null
+++ b/openapi3/testdata/recursiveRef/issue615.yml
@@ -0,0 +1,60 @@
+openapi: "3.0.3"
+info:
+  title: Deep recursive cyclic refs example
+  version: "1.0"
+paths:
+  /foo:
+    $ref: ./paths/foo.yml
+components:
+  schemas:
+    FilterColumnIncludes:
+      type: object
+      properties:
+        $includes:
+          $ref: '#/components/schemas/FilterPredicate'
+      additionalProperties: false
+      maxProperties: 1
+      minProperties: 1
+    FilterPredicate:
+      oneOf:
+        - $ref: '#/components/schemas/FilterValue'
+        - type: array
+          items:
+            $ref: '#/components/schemas/FilterPredicate'
+          minLength: 1
+        - $ref: '#/components/schemas/FilterPredicateOp'
+        - $ref: '#/components/schemas/FilterPredicateRangeOp'
+    FilterPredicateOp:
+      type: object
+      properties:
+        $any:
+          oneOf:
+            - type: array
+              items:
+                $ref: '#/components/schemas/FilterPredicate'
+        $none:
+          oneOf:
+            - $ref: '#/components/schemas/FilterPredicate'
+            - type: array
+              items:
+                $ref: '#/components/schemas/FilterPredicate'
+      additionalProperties: false
+      maxProperties: 1
+      minProperties: 1
+    FilterPredicateRangeOp:
+      type: object
+      properties:
+        $lt:
+          $ref: '#/components/schemas/FilterRangeValue'
+      additionalProperties: false
+      maxProperties: 2
+      minProperties: 2
+    FilterRangeValue:
+      oneOf:
+        - type: number
+        - type: string
+    FilterValue:
+      oneOf:
+        - type: number
+        - type: string
+        - type: boolean
\ No newline at end of file