Added support for configurable levels of circular reference checking #113 #130

Signed-off-by: quobix <dave@quobix.com>
This commit is contained in:
quobix
2023-09-19 15:03:13 -04:00
parent 53a46f4c60
commit 715bfc3052
5 changed files with 330 additions and 20 deletions

View File

@@ -8,6 +8,8 @@ type CircularReferenceResult struct {
Start *Reference Start *Reference
LoopIndex int LoopIndex int
LoopPoint *Reference LoopPoint *Reference
IsArrayResult bool // if this result comes from an array loop.
PolymorphicType string // which type of polymorphic loop is this? (oneOf, anyOf, allOf)
IsPolymorphicResult bool // if this result comes from a polymorphic loop. IsPolymorphicResult bool // if this result comes from a polymorphic loop.
IsInfiniteLoop bool // if all the definitions in the reference loop are marked as required, this is an infinite circular reference, thus is not allowed. IsInfiniteLoop bool // if all the definitions in the reference loop are marked as required, this is an infinite circular reference, thus is not allowed.
} }

View File

@@ -28,6 +28,7 @@ type Reference struct {
Name string Name string
Node *yaml.Node Node *yaml.Node
ParentNode *yaml.Node ParentNode *yaml.Node
ParentNodeSchemaType string // used to determine if the parent node is an array or not.
Resolved bool Resolved bool
Circular bool Circular bool
Seen bool Seen bool

View File

@@ -42,6 +42,8 @@ type Resolver struct {
indexesVisited int indexesVisited int
journeysTaken int journeysTaken int
relativesSeen int relativesSeen int
ignorePoly bool
ignoreArray bool
} }
// NewResolver will create a new resolver from a *index.SpecIndex // NewResolver will create a new resolver from a *index.SpecIndex
@@ -92,10 +94,21 @@ func (resolver *Resolver) GetNonPolymorphicCircularErrors() []*index.CircularRef
res = append(res, resolver.circularReferences[i]) res = append(res, resolver.circularReferences[i])
} }
} }
return res return res
} }
// IgnorePolymorphicCircularReferences will ignore any circular references that are polymorphic (oneOf, anyOf, allOf)
// This must be set before any resolving is done.
func (resolver *Resolver) IgnorePolymorphicCircularReferences() {
resolver.ignorePoly = true
}
// IgnoreArrayCircularReferences will ignore any circular references that stem from arrays. This must be set before
// any resolving is done.
func (resolver *Resolver) IgnoreArrayCircularReferences() {
resolver.ignoreArray = true
}
// GetJourneysTaken returns the number of journeys taken by the resolver // GetJourneysTaken returns the number of journeys taken by the resolver
func (resolver *Resolver) GetJourneysTaken() int { func (resolver *Resolver) GetJourneysTaken() int {
return resolver.journeysTaken return resolver.journeysTaken
@@ -231,7 +244,7 @@ func (resolver *Resolver) VisitReference(ref *index.Reference, seen map[string]b
} }
journey = append(journey, ref) journey = append(journey, ref)
relatives := resolver.extractRelatives(ref.Node, seen, journey, resolve) relatives := resolver.extractRelatives(ref.Node, nil, seen, journey, resolve)
seen = make(map[string]bool) seen = make(map[string]bool)
@@ -254,11 +267,17 @@ func (resolver *Resolver) VisitReference(ref *index.Reference, seen map[string]b
visitedDefinitions := map[string]bool{} visitedDefinitions := map[string]bool{}
isInfiniteLoop, _ := resolver.isInfiniteCircularDependency(foundDup, visitedDefinitions, nil) isInfiniteLoop, _ := resolver.isInfiniteCircularDependency(foundDup, visitedDefinitions, nil)
isArray := false
if r.ParentNodeSchemaType == "array" {
isArray = true
}
circRef = &index.CircularReferenceResult{ circRef = &index.CircularReferenceResult{
Journey: loop, Journey: loop,
Start: foundDup, Start: foundDup,
LoopIndex: i, LoopIndex: i,
LoopPoint: foundDup, LoopPoint: foundDup,
IsArrayResult: isArray,
IsInfiniteLoop: isInfiniteLoop, IsInfiniteLoop: isInfiniteLoop,
} }
resolver.circularReferences = append(resolver.circularReferences, circRef) resolver.circularReferences = append(resolver.circularReferences, circRef)
@@ -321,7 +340,7 @@ func (resolver *Resolver) isInfiniteCircularDependency(ref *index.Reference, vis
return false, visitedDefinitions return false, visitedDefinitions
} }
func (resolver *Resolver) extractRelatives(node *yaml.Node, func (resolver *Resolver) extractRelatives(node, parent *yaml.Node,
foundRelatives map[string]bool, foundRelatives map[string]bool,
journey []*index.Reference, resolve bool) []*index.Reference { journey []*index.Reference, resolve bool) []*index.Reference {
@@ -333,7 +352,30 @@ func (resolver *Resolver) extractRelatives(node *yaml.Node,
if len(node.Content) > 0 { if len(node.Content) > 0 {
for i, n := range node.Content { for i, n := range node.Content {
if utils.IsNodeMap(n) || utils.IsNodeArray(n) { if utils.IsNodeMap(n) || utils.IsNodeArray(n) {
found = append(found, resolver.extractRelatives(n, foundRelatives, journey, resolve)...)
var anyvn, allvn, onevn, arrayTypevn *yaml.Node
// extract polymorphic references
if len(n.Content) > 1 {
_, anyvn = utils.FindKeyNodeTop("anyOf", n.Content)
_, allvn = utils.FindKeyNodeTop("allOf", n.Content)
_, onevn = utils.FindKeyNodeTop("oneOf", n.Content)
_, arrayTypevn = utils.FindKeyNodeTop("type", n.Content)
}
if anyvn != nil || allvn != nil || onevn != nil {
if resolver.ignorePoly {
continue
}
}
if arrayTypevn != nil {
if arrayTypevn.Value == "array" {
if resolver.ignoreArray {
continue
}
}
}
found = append(found, resolver.extractRelatives(n, node, foundRelatives, journey, resolve)...)
} }
if i%2 == 0 && n.Value == "$ref" { if i%2 == 0 && n.Value == "$ref" {
@@ -357,10 +399,22 @@ func (resolver *Resolver) extractRelatives(node *yaml.Node,
continue continue
} }
schemaType := ""
if parent != nil {
_, arrayTypevn := utils.FindKeyNodeTop("type", parent.Content)
if arrayTypevn != nil {
if arrayTypevn.Value == "array" {
schemaType = "array"
}
}
}
r := &index.Reference{ r := &index.Reference{
Definition: value, Definition: value,
Name: value, Name: value,
Node: node, Node: node,
ParentNode: parent,
ParentNodeSchemaType: schemaType,
} }
found = append(found, r) found = append(found, r)
@@ -398,6 +452,7 @@ func (resolver *Resolver) extractRelatives(node *yaml.Node,
Start: ref, Start: ref,
LoopIndex: i, LoopIndex: i,
LoopPoint: ref, LoopPoint: ref,
PolymorphicType: n.Value,
IsPolymorphicResult: true, IsPolymorphicResult: true,
} }
@@ -435,6 +490,7 @@ func (resolver *Resolver) extractRelatives(node *yaml.Node,
Start: ref, Start: ref,
LoopIndex: i, LoopIndex: i,
LoopPoint: ref, LoopPoint: ref,
PolymorphicType: n.Value,
IsPolymorphicResult: true, IsPolymorphicResult: true,
} }
@@ -449,6 +505,7 @@ func (resolver *Resolver) extractRelatives(node *yaml.Node,
} }
break break
} }
} }
} }
} }

View File

@@ -4,7 +4,6 @@ import (
"errors" "errors"
"fmt" "fmt"
"io/ioutil" "io/ioutil"
"net/url"
"testing" "testing"
"github.com/pb33f/libopenapi/index" "github.com/pb33f/libopenapi/index"
@@ -63,22 +62,68 @@ func TestResolver_CheckForCircularReferences(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
} }
func TestResolver_CheckForCircularReferences_DigitalOcean(t *testing.T) { func TestResolver_CheckForCircularReferences_CatchArray(t *testing.T) {
circular, _ := ioutil.ReadFile("../test_specs/digitalocean.yaml") circular := []byte(`openapi: 3.0.0
components:
schemas:
ProductCategory:
type: "object"
properties:
name:
type: "string"
children:
type: "array"
items:
$ref: "#/components/schemas/ProductCategory"
description: "Array of sub-categories in the same format."
required:
- "name"
- "children"`)
var rootNode yaml.Node var rootNode yaml.Node
yaml.Unmarshal(circular, &rootNode) yaml.Unmarshal(circular, &rootNode)
baseURL, _ := url.Parse("https://raw.githubusercontent.com/digitalocean/openapi/main/specification") index := index.NewSpecIndex(&rootNode)
index := index.NewSpecIndexWithConfig(&rootNode, &index.SpecIndexConfig{
AllowRemoteLookup: true,
AllowFileLookup: true,
BaseURL: baseURL,
})
resolver := NewResolver(index) resolver := NewResolver(index)
assert.NotNil(t, resolver) assert.NotNil(t, resolver)
circ := resolver.CheckForCircularReferences()
assert.Len(t, circ, 1)
assert.Len(t, resolver.GetResolvingErrors(), 1) // infinite loop is a resolving error.
assert.Len(t, resolver.GetCircularErrors(), 1)
assert.True(t, resolver.GetCircularErrors()[0].IsArrayResult)
_, err := yaml.Marshal(resolver.resolvedRoot)
assert.NoError(t, err)
}
func TestResolver_CheckForCircularReferences_IgnoreArray(t *testing.T) {
circular := []byte(`openapi: 3.0.0
components:
schemas:
ProductCategory:
type: "object"
properties:
name:
type: "string"
children:
type: "array"
items:
$ref: "#/components/schemas/ProductCategory"
description: "Array of sub-categories in the same format."
required:
- "name"
- "children"`)
var rootNode yaml.Node
yaml.Unmarshal(circular, &rootNode)
index := index.NewSpecIndex(&rootNode)
resolver := NewResolver(index)
assert.NotNil(t, resolver)
resolver.IgnoreArrayCircularReferences()
circ := resolver.CheckForCircularReferences() circ := resolver.CheckForCircularReferences()
assert.Len(t, circ, 0) assert.Len(t, circ, 0)
assert.Len(t, resolver.GetResolvingErrors(), 0) assert.Len(t, resolver.GetResolvingErrors(), 0)
@@ -88,6 +133,208 @@ func TestResolver_CheckForCircularReferences_DigitalOcean(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
} }
func TestResolver_CheckForCircularReferences_IgnorePoly_Any(t *testing.T) {
circular := []byte(`openapi: 3.0.0
components:
schemas:
ProductCategory:
type: "object"
properties:
name:
type: "string"
children:
type: "object"
anyOf:
- $ref: "#/components/schemas/ProductCategory"
description: "Array of sub-categories in the same format."
required:
- "name"
- "children"`)
var rootNode yaml.Node
yaml.Unmarshal(circular, &rootNode)
index := index.NewSpecIndex(&rootNode)
resolver := NewResolver(index)
assert.NotNil(t, resolver)
resolver.IgnorePolymorphicCircularReferences()
circ := resolver.CheckForCircularReferences()
assert.Len(t, circ, 0)
assert.Len(t, resolver.GetResolvingErrors(), 0)
assert.Len(t, resolver.GetCircularErrors(), 0)
_, err := yaml.Marshal(resolver.resolvedRoot)
assert.NoError(t, err)
}
func TestResolver_CheckForCircularReferences_IgnorePoly_All(t *testing.T) {
circular := []byte(`openapi: 3.0.0
components:
schemas:
ProductCategory:
type: "object"
properties:
name:
type: "string"
children:
type: "object"
allOf:
- $ref: "#/components/schemas/ProductCategory"
description: "Array of sub-categories in the same format."
required:
- "name"
- "children"`)
var rootNode yaml.Node
yaml.Unmarshal(circular, &rootNode)
index := index.NewSpecIndex(&rootNode)
resolver := NewResolver(index)
assert.NotNil(t, resolver)
resolver.IgnorePolymorphicCircularReferences()
circ := resolver.CheckForCircularReferences()
assert.Len(t, circ, 0)
assert.Len(t, resolver.GetResolvingErrors(), 0)
assert.Len(t, resolver.GetCircularErrors(), 0)
_, err := yaml.Marshal(resolver.resolvedRoot)
assert.NoError(t, err)
}
func TestResolver_CheckForCircularReferences_IgnorePoly_One(t *testing.T) {
circular := []byte(`openapi: 3.0.0
components:
schemas:
ProductCategory:
type: "object"
properties:
name:
type: "string"
children:
type: "object"
oneOf:
- $ref: "#/components/schemas/ProductCategory"
description: "Array of sub-categories in the same format."
required:
- "name"
- "children"`)
var rootNode yaml.Node
yaml.Unmarshal(circular, &rootNode)
index := index.NewSpecIndex(&rootNode)
resolver := NewResolver(index)
assert.NotNil(t, resolver)
resolver.IgnorePolymorphicCircularReferences()
circ := resolver.CheckForCircularReferences()
assert.Len(t, circ, 0)
assert.Len(t, resolver.GetResolvingErrors(), 0)
assert.Len(t, resolver.GetCircularErrors(), 0)
_, err := yaml.Marshal(resolver.resolvedRoot)
assert.NoError(t, err)
}
func TestResolver_CheckForCircularReferences_CatchPoly_Any(t *testing.T) {
circular := []byte(`openapi: 3.0.0
components:
schemas:
ProductCategory:
type: "object"
properties:
name:
type: "string"
children:
type: "object"
anyOf:
- $ref: "#/components/schemas/ProductCategory"
description: "Array of sub-categories in the same format."
required:
- "name"
- "children"`)
var rootNode yaml.Node
yaml.Unmarshal(circular, &rootNode)
index := index.NewSpecIndex(&rootNode)
resolver := NewResolver(index)
assert.NotNil(t, resolver)
circ := resolver.CheckForCircularReferences()
assert.Len(t, circ, 0)
assert.Len(t, resolver.GetResolvingErrors(), 0) // not an infinite loop if poly.
assert.Len(t, resolver.GetCircularErrors(), 1)
assert.Equal(t, "anyOf", resolver.GetCircularErrors()[0].PolymorphicType)
_, err := yaml.Marshal(resolver.resolvedRoot)
assert.NoError(t, err)
}
func TestResolver_CheckForCircularReferences_CatchPoly_All(t *testing.T) {
circular := []byte(`openapi: 3.0.0
components:
schemas:
ProductCategory:
type: "object"
properties:
name:
type: "string"
children:
type: "object"
allOf:
- $ref: "#/components/schemas/ProductCategory"
description: "Array of sub-categories in the same format."
required:
- "name"
- "children"`)
var rootNode yaml.Node
yaml.Unmarshal(circular, &rootNode)
index := index.NewSpecIndex(&rootNode)
resolver := NewResolver(index)
assert.NotNil(t, resolver)
circ := resolver.CheckForCircularReferences()
assert.Len(t, circ, 0)
assert.Len(t, resolver.GetResolvingErrors(), 0) // not an infinite loop if poly.
assert.Len(t, resolver.GetCircularErrors(), 1)
assert.Equal(t, "allOf", resolver.GetCircularErrors()[0].PolymorphicType)
assert.True(t, resolver.GetCircularErrors()[0].IsPolymorphicResult)
_, err := yaml.Marshal(resolver.resolvedRoot)
assert.NoError(t, err)
}
//func TestResolver_CheckForCircularReferences_DigitalOcean(t *testing.T) {
// circular, _ := ioutil.ReadFile("../test_specs/digitalocean.yaml")
// var rootNode yaml.Node
// yaml.Unmarshal(circular, &rootNode)
//
// baseURL, _ := url.Parse("https://raw.githubusercontent.com/digitalocean/openapi/main/specification")
//
// index := index.NewSpecIndexWithConfig(&rootNode, &index.SpecIndexConfig{
// AllowRemoteLookup: true,
// AllowFileLookup: true,
// BaseURL: baseURL,
// })
//
// resolver := NewResolver(index)
// assert.NotNil(t, resolver)
//
// circ := resolver.CheckForCircularReferences()
// assert.Len(t, circ, 0)
// assert.Len(t, resolver.GetResolvingErrors(), 0)
// assert.Len(t, resolver.GetCircularErrors(), 0)
//
// _, err := yaml.Marshal(resolver.resolvedRoot)
// assert.NoError(t, err)
//}
func TestResolver_CircularReferencesRequiredValid(t *testing.T) { func TestResolver_CircularReferencesRequiredValid(t *testing.T) {
circular, _ := ioutil.ReadFile("../test_specs/swagger-valid-recursive-model.yaml") circular, _ := ioutil.ReadFile("../test_specs/swagger-valid-recursive-model.yaml")
var rootNode yaml.Node var rootNode yaml.Node
@@ -129,7 +376,7 @@ func TestResolver_DeepJourney(t *testing.T) {
} }
index := index.NewSpecIndex(nil) index := index.NewSpecIndex(nil)
resolver := NewResolver(index) resolver := NewResolver(index)
assert.Nil(t, resolver.extractRelatives(nil, nil, journey, false)) assert.Nil(t, resolver.extractRelatives(nil, nil, nil, journey, false))
} }
func TestResolver_ResolveComponents_Stripe(t *testing.T) { func TestResolver_ResolveComponents_Stripe(t *testing.T) {

View File

@@ -236,6 +236,9 @@ func FindKeyNodeTop(key string, nodes []*yaml.Node) (keyNode *yaml.Node, valueNo
} }
if strings.EqualFold(key, v.Value) { if strings.EqualFold(key, v.Value) {
if i+1 >= len(nodes) {
return v, nodes[i]
}
return v, nodes[i+1] // next node is what we need. return v, nodes[i+1] // next node is what we need.
} }
} }