Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: detects circular references that can't be handled at the moment to avoid infinite loops loading documents #607

Merged
merged 3 commits into from
Sep 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions openapi3/issue542_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package openapi3

import (
"testing"

"github.com/stretchr/testify/require"
)

func TestIssue542(t *testing.T) {
sl := NewLoader()

_, err := sl.LoadFromFile("testdata/issue542.yml")
require.Error(t, err)
require.Contains(t, err.Error(), CircularReferenceError)
}
15 changes: 15 additions & 0 deletions openapi3/issue570_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package openapi3

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestIssue570(t *testing.T) {
loader := NewLoader()
_, err := loader.LoadFromFile("testdata/issue570.json")
require.Error(t, err)
assert.Contains(t, err.Error(), CircularReferenceError)
}
60 changes: 41 additions & 19 deletions openapi3/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (
"github.com/invopop/yaml"
)

var CircularReferenceError = "kin-openapi bug found: circular schema reference not handled"

func foundUnresolvedRef(ref string) error {
return fmt.Errorf("found unresolved ref: %q", ref)
}
Expand Down Expand Up @@ -197,7 +199,7 @@ func (loader *Loader) ResolveRefsIn(doc *T, location *url.URL) (err error) {
}
}
for _, component := range components.Schemas {
if err = loader.resolveSchemaRef(doc, component, location); err != nil {
if err = loader.resolveSchemaRef(doc, component, location, []string{}); err != nil {
return
}
}
Expand Down Expand Up @@ -480,7 +482,7 @@ func (loader *Loader) resolveHeaderRef(doc *T, component *HeaderRef, documentPat
}

if schema := value.Schema; schema != nil {
if err := loader.resolveSchemaRef(doc, schema, documentPath); err != nil {
if err := loader.resolveSchemaRef(doc, schema, documentPath, []string{}); err != nil {
return err
}
}
Expand Down Expand Up @@ -532,13 +534,13 @@ func (loader *Loader) resolveParameterRef(doc *T, component *ParameterRef, docum
}
for _, contentType := range value.Content {
if schema := contentType.Schema; schema != nil {
if err := loader.resolveSchemaRef(doc, schema, documentPath); err != nil {
if err := loader.resolveSchemaRef(doc, schema, documentPath, []string{}); err != nil {
return err
}
}
}
if schema := value.Schema; schema != nil {
if err := loader.resolveSchemaRef(doc, schema, documentPath); err != nil {
if err := loader.resolveSchemaRef(doc, schema, documentPath, []string{}); err != nil {
return err
}
}
Expand Down Expand Up @@ -592,7 +594,7 @@ func (loader *Loader) resolveRequestBodyRef(doc *T, component *RequestBodyRef, d
contentType.Examples[name] = example
}
if schema := contentType.Schema; schema != nil {
if err := loader.resolveSchemaRef(doc, schema, documentPath); err != nil {
if err := loader.resolveSchemaRef(doc, schema, documentPath, []string{}); err != nil {
return err
}
}
Expand Down Expand Up @@ -656,7 +658,7 @@ func (loader *Loader) resolveResponseRef(doc *T, component *ResponseRef, documen
contentType.Examples[name] = example
}
if schema := contentType.Schema; schema != nil {
if err := loader.resolveSchemaRef(doc, schema, documentPath); err != nil {
if err := loader.resolveSchemaRef(doc, schema, documentPath, []string{}); err != nil {
return err
}
contentType.Schema = schema
Expand All @@ -670,8 +672,12 @@ func (loader *Loader) resolveResponseRef(doc *T, component *ResponseRef, documen
return nil
}

func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPath *url.URL) (err error) {
if component != nil && component.Value != nil {
func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPath *url.URL, visited []string) (err error) {
if component == nil {
return errors.New("invalid schema: value MUST be an object")
}

if component.Value != nil {
if loader.visitedSchema == nil {
loader.visitedSchema = make(map[*Schema]struct{})
}
Expand All @@ -681,9 +687,6 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat
loader.visitedSchema[component.Value] = struct{}{}
}

if component == nil {
return errors.New("invalid schema: value MUST be an object")
}
ref := component.Ref
if ref != "" {
if isSingleRefElement(ref) {
Expand All @@ -693,12 +696,18 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat
}
component.Value = &schema
} else {
if visitedLimit(visited, ref, 3) {
visited = append(visited, ref)
return fmt.Errorf("%s - %s", CircularReferenceError, strings.Join(visited, " -> "))
}
visited = append(visited, ref)

var resolved SchemaRef
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
if err != nil {
return err
}
if err := loader.resolveSchemaRef(doc, &resolved, componentPath); err != nil {
if err := loader.resolveSchemaRef(doc, &resolved, componentPath, visited); err != nil {
return err
}
component.Value = resolved.Value
Expand All @@ -713,37 +722,37 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat

// ResolveRefs referred schemas
if v := value.Items; v != nil {
if err := loader.resolveSchemaRef(doc, v, documentPath); err != nil {
if err := loader.resolveSchemaRef(doc, v, documentPath, visited); err != nil {
return err
}
}
for _, v := range value.Properties {
if err := loader.resolveSchemaRef(doc, v, documentPath); err != nil {
if err := loader.resolveSchemaRef(doc, v, documentPath, visited); err != nil {
return err
}
}
if v := value.AdditionalProperties; v != nil {
if err := loader.resolveSchemaRef(doc, v, documentPath); err != nil {
if err := loader.resolveSchemaRef(doc, v, documentPath, visited); err != nil {
return err
}
}
if v := value.Not; v != nil {
if err := loader.resolveSchemaRef(doc, v, documentPath); err != nil {
if err := loader.resolveSchemaRef(doc, v, documentPath, visited); err != nil {
return err
}
}
for _, v := range value.AllOf {
if err := loader.resolveSchemaRef(doc, v, documentPath); err != nil {
if err := loader.resolveSchemaRef(doc, v, documentPath, visited); err != nil {
return err
}
}
for _, v := range value.AnyOf {
if err := loader.resolveSchemaRef(doc, v, documentPath); err != nil {
if err := loader.resolveSchemaRef(doc, v, documentPath, visited); err != nil {
return err
}
}
for _, v := range value.OneOf {
if err := loader.resolveSchemaRef(doc, v, documentPath); err != nil {
if err := loader.resolveSchemaRef(doc, v, documentPath, visited); err != nil {
return err
}
}
Expand Down Expand Up @@ -1046,3 +1055,16 @@ func (loader *Loader) resolvePathItemRefContinued(doc *T, pathItem *PathItem, do
func unescapeRefString(ref string) string {
return strings.Replace(strings.Replace(ref, "~1", "/", -1), "~0", "~", -1)
}

func visitedLimit(visited []string, ref string, limit int) bool {
visitedCount := 0
for _, v := range visited {
if v == ref {
visitedCount++
if visitedCount >= limit {
return true
}
}
}
return false
}
43 changes: 43 additions & 0 deletions openapi3/testdata/issue542.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
openapi: "3.0.0"
info:
version: 1.0.0
title: Swagger Petstore
license:
name: MIT
servers:
- url: http://petstore.swagger.io/v1
paths: {}
#paths:
# /pets:
# patch:
# requestBody:
# content:
# application/json:
# schema:
# oneOf:
# - $ref: '#/components/schemas/Cat'
# - $ref: '#/components/schemas/Kitten'
# discriminator:
# propertyName: pet_type
# responses:
# '200':
# description: Updated
components:
schemas:
Cat:
anyOf:
- $ref: "#/components/schemas/Kitten"
- type: object
# properties:
# hunts:
# type: boolean
# age:
# type: integer
# offspring:
Kitten:
$ref: "#/components/schemas/Cat" #ko

# type: string #ok

# allOf: #ko
# - $ref: '#/components/schemas/Cat'
155 changes: 155 additions & 0 deletions openapi3/testdata/issue570.json

Large diffs are not rendered by default.