fix for resolving looping relative references

In vacuum, a usecase was reported where an infinite loop occurred due to re-parsing the same reference over and over in a loop. It was re-creatable and it was because the loop happened before the index was ready.

This should be resolved now, at least for this use case. To be sure, I have included the specs as a new test.

https://github.com/daveshanley/vacuum/issues/268
Signed-off-by: Dave Shanley <dave@quobix.com>

index/extract_refs.go

@@ -430,4 +430,3 @@ func (index *SpecIndex) ExtractComponentsFromRefs(refs []*Reference) []*Referenc
 	}
 	return found
 }
-

index/find_component.go

@@ -267,11 +267,16 @@ func (index *SpecIndex) FindComponentInRoot(componentId string) *Reference {
 			return nil // no component found
 		}
 		res, _ := path.Find(index.root)
+
 		if len(res) == 1 {
+			resNode := res[0]
+			if res[0].Kind == yaml.DocumentNode {
+				resNode = res[0].Content[0]
+			}
 			ref := &Reference{
 				Definition:            componentId,
 				Name:                  name,
-				Node:                  res[0],
+				Node:                  resNode,
 				Path:                  friendlySearch,
 				RequiredRefProperties: index.extractDefinitionRequiredRefProperties(res[0], map[string][]string{}),
 			}
@@ -283,8 +288,7 @@ func (index *SpecIndex) FindComponentInRoot(componentId string) *Reference {
 }
 
 func (index *SpecIndex) performExternalLookup(uri []string, componentId string,
-	lookupFunction ExternalLookupFunction, parent *yaml.Node,
+	lookupFunction ExternalLookupFunction, parent *yaml.Node) *Reference {
-) *Reference {
 	if len(uri) > 0 {
 		index.externalLock.RLock()
 		externalSpecIndex := index.externalSpecIndex[uri[0]]
@@ -349,11 +353,16 @@ func (index *SpecIndex) performExternalLookup(uri []string, componentId string,
 					BasePath:          newBasePath,
 					AllowRemoteLookup: index.config.AllowRemoteLookup,
 					AllowFileLookup:   index.config.AllowFileLookup,
+					ParentIndex:       index,
 					seenRemoteSources: index.config.seenRemoteSources,
 					remoteLock:        index.config.remoteLock,
+					uri:               uri,
 				}
 
 				var newIndex *SpecIndex
+				seen := index.SearchAncestryForSeenURI(uri[0])
+				if seen == nil {
+
 					newIndex = NewSpecIndexWithConfig(newRoot, newConfig)
 					index.refLock.Lock()
 					index.externalLock.Lock()
@@ -364,6 +373,9 @@ func (index *SpecIndex) performExternalLookup(uri []string, componentId string,
 					index.AddChild(newIndex)
 					index.refLock.Unlock()
 					externalSpecIndex = newIndex
+				} else {
+					externalSpecIndex = seen
+				}
 			}
 		}
 

index/find_component_test.go

@@ -6,6 +6,7 @@ package index
 import (
 	"github.com/stretchr/testify/assert"
 	"gopkg.in/yaml.v3"
+	"os"
 	"testing"
 )
 
@@ -26,6 +27,22 @@ func TestSpecIndex_performExternalLookup(t *testing.T) {
 	assert.Len(t, index.GetPathsNode().Content, 1)
 }
 
+func TestSpecIndex_CheckCircularIndex(t *testing.T) {
+	yml, _ := os.ReadFile("../test_specs/first.yaml")
+	var rootNode yaml.Node
+	_ = yaml.Unmarshal([]byte(yml), &rootNode)
+
+	c := CreateOpenAPIIndexConfig()
+	c.BasePath = "../test_specs"
+	index := NewSpecIndexWithConfig(&rootNode, c)
+	assert.Nil(t, index.uri)
+	assert.NotNil(t, index.children[0].uri)
+	assert.NotNil(t, index.children[0].children[0].uri)
+	assert.NotNil(t, index.SearchIndexForReference("second.yaml#/properties/property2"))
+	assert.NotNil(t, index.SearchIndexForReference("second.yaml"))
+	assert.Nil(t, index.SearchIndexForReference("fourth.yaml"))
+}
+
 func TestSpecIndex_performExternalLookup_invalidURL(t *testing.T) {
 	yml := `openapi: 3.1.0
 components:

index/index_model.go

@@ -72,9 +72,14 @@ type SpecIndexConfig struct {
 	AllowRemoteLookup bool // Allow remote lookups for references. Defaults to false
 	AllowFileLookup   bool // Allow file lookups for references. Defaults to false
 
+	// ParentIndex allows the index to be created with knowledge of a parent, before being parsed. This allows
+	// a breakglass to be used to prevent loops, checking the tree before recursing down.
+	ParentIndex *SpecIndex
+
 	// private fields
 	seenRemoteSources *syncmap.Map
 	remoteLock        *sync.Mutex
+	uri               []string
 }
 
 // CreateOpenAPIIndexConfig is a helper function to create a new SpecIndexConfig with the AllowRemoteLookup and
@@ -221,6 +226,7 @@ type SpecIndex struct {
 	// what we have seen across indexes, so we need to be able to travel back up to the root
 	// cto avoid re-downloading sources.
 	parentIndex *SpecIndex
+	uri         []string
 	children    []*SpecIndex
 }
 

index/search_index.go

@@ -3,28 +3,13 @@
 
 package index
 
-import "gopkg.in/yaml.v3"
-
 // SearchIndexForReference searches the index for a reference, first looking through the mapped references
 // and then externalSpecIndex for a match. If no match is found, it will recursively search the child indexes
 // extracted when parsing the OpenAPI Spec.
 func (index *SpecIndex) SearchIndexForReference(ref string) []*Reference {
 	if r, ok := index.allMappedRefs[ref]; ok {
-        if r.Node.Kind == yaml.DocumentNode {
-            // the reference is an entire document, so we need to dig down a level and rewire the reference.
-            r.Node = r.Node.Content[0]
-        }
 		return []*Reference{r}
 	}
-    if r, ok := index.externalSpecIndex[ref]; ok {
-        return []*Reference{
-            {
-                Node:       r.root.Content[0],
-                Name:       ref,
-                Definition: ref,
-            },
-        }
-    }
 	for c := range index.children {
 		found := goFindMeSomething(index.children[c], ref)
 		if found != nil {
@@ -34,6 +19,16 @@ func (index *SpecIndex) SearchIndexForReference(ref string) []*Reference {
 	return nil
 }
 
+func (index *SpecIndex) SearchAncestryForSeenURI(uri string) *SpecIndex {
+	if index.parentIndex == nil {
+		return nil
+	}
+	if index.uri[0] != uri {
+		return index.parentIndex.SearchAncestryForSeenURI(uri)
+	}
+	return index
+}
+
 func goFindMeSomething(i *SpecIndex, ref string) []*Reference {
 	return i.SearchIndexForReference(ref)
 }

index/search_index_test.go

@@ -6,13 +6,13 @@ package index
 import (
 	"github.com/stretchr/testify/assert"
 	"gopkg.in/yaml.v3"
-    "io/ioutil"
 	"net/url"
+	"os"
 	"testing"
 )
 
 func TestSpecIndex_SearchIndexForReference(t *testing.T) {
-    petstore, _ := ioutil.ReadFile("../test_specs/petstorev3.json")
+	petstore, _ := os.ReadFile("../test_specs/petstorev3.json")
 	var rootNode yaml.Node
 	_ = yaml.Unmarshal(petstore, &rootNode)
 
@@ -26,7 +26,7 @@ func TestSpecIndex_SearchIndexForReference(t *testing.T) {
 func TestSpecIndex_SearchIndexForReference_ExternalSpecs(t *testing.T) {
 
 	// load up an index with lots of references
-    petstore, _ := ioutil.ReadFile("../test_specs/digitalocean.yaml")
+	petstore, _ := os.ReadFile("../test_specs/digitalocean.yaml")
 	var rootNode yaml.Node
 	_ = yaml.Unmarshal(petstore, &rootNode)
 

index/spec_index.go

@@ -32,6 +32,8 @@ func NewSpecIndexWithConfig(rootNode *yaml.Node, config *SpecIndexConfig) *SpecI
 	}
 	config.remoteLock = &sync.Mutex{}
 	index.config = config
+	index.parentIndex = config.ParentIndex
+	index.uri = config.uri
 	if rootNode == nil || len(rootNode.Content) <= 0 {
 		return index
 	}

index/spec_index_test.go

@@ -86,7 +86,7 @@ func TestSpecIndex_Asana(t *testing.T) {
 }
 
 func TestSpecIndex_DigitalOcean(t *testing.T) {
-	do, _ := ioutil.ReadFile("../test_specs/digitalocean.yaml")
+	do, _ := os.ReadFile("../test_specs/digitalocean.yaml")
 	var rootNode yaml.Node
 	_ = yaml.Unmarshal(do, &rootNode)
 
@@ -603,7 +603,6 @@ func TestSpecIndex_FindComponenth(t *testing.T) {
 	assert.Nil(t, index.FindComponent("I-do-not-exist", nil))
 }
 
-
 func TestSpecIndex_TestPathsNodeAsArray(t *testing.T) {
 	yml := `components:
   schemas:

resolver/resolver_test.go

@@ -274,7 +274,9 @@ func TestResolver_ResolveComponents_MixedRef(t *testing.T) {
     circ := resolver.Resolve()
     assert.Len(t, circ, 0)
     assert.Equal(t, 5, resolver.GetIndexesVisited())
-	assert.Equal(t, 209, resolver.GetRelativesSeen())
+
+    // in v0.8.2 a new check was added when indexing, to prevent re-indexing the same file multiple times.
+    assert.Equal(t, 191, resolver.GetRelativesSeen())
     assert.Equal(t, 35, resolver.GetJourneysTaken())
     assert.Equal(t, 62, resolver.GetReferenceVisited())
 }

test_specs/first.yaml

@@ -0,0 +1,33 @@
+openapi: 3.0.0
+info:
+  title: title
+  description: description
+  version: 0.0.0
+
+paths:
+
+  /items:
+    get:
+      tags:
+        - items
+      summary: summary
+      description: description
+      parameters: []
+      responses:
+        '200':
+          description: OK
+          content:
+            application/json:
+              schema:
+                title: Schema
+                description: description
+                type: object
+
+                additionalProperties:
+                  type: object
+                  title: first title
+                  description: first description
+                  additionalProperties: false
+                  properties:
+                    second:
+                      $ref: "second.yaml"

test_specs/second.yaml

@@ -0,0 +1,31 @@
+title: second doc title
+description: second doc description
+type: object
+
+additionalProperties: false
+
+properties:
+
+  property1:
+    title: title
+    description: property 1 description
+    type: array
+    items:
+      title: item
+      description: third description
+      type: object
+      additionalProperties: false
+      properties:
+        details:
+          $ref: "third.yaml"
+
+  property2:
+    title: title
+    description: property 2 description
+    type: object
+    additionalProperties: false
+    properties:
+      property:
+        title: title
+        description: tasty description
+        type: integer

test_specs/third.yaml

@@ -0,0 +1,15 @@
+title: third doc title
+description: third doc description
+type: object
+
+additionalProperties: false
+maxProperties: 1
+
+properties:
+
+  property:
+    title: title of third prop in third doc
+    type: object
+    properties:
+      statistics:
+        $ref: 'second.yaml#/properties/property2'

utils/utils.go

@@ -417,6 +417,10 @@ func IsNodeBoolValue(node *yaml.Node) bool {
 }
 
 func IsNodeRefValue(node *yaml.Node) (bool, *yaml.Node, string) {
+	if node == nil {
+		return false, nil, ""
+	}
+
 	for i, r := range node.Content {
 		if i%2 == 0 {
 			if r.Value == "$ref" {