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

Bugfix/issue638 #700

Merged
merged 4 commits into from Dec 14, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions openapi3/issue542_test.go
Expand Up @@ -10,6 +10,5 @@ func TestIssue542(t *testing.T) {
sl := NewLoader()

_, err := sl.LoadFromFile("testdata/issue542.yml")
require.Error(t, err)
require.Contains(t, err.Error(), CircularReferenceError)
require.NoError(t, err)
}
10 changes: 2 additions & 8 deletions openapi3/issue615_test.go
Expand Up @@ -9,17 +9,11 @@ import (
)

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
require.NoError(t, err)
}

var old int
Expand Down
21 changes: 21 additions & 0 deletions openapi3/issue638_test.go
@@ -0,0 +1,21 @@
package openapi3

import (
"testing"

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

func TestIssue638(t *testing.T) {
for i := 0; i < 50; i++ {
loader := NewLoader()
loader.IsExternalRefsAllowed = true
// This path affects the occurrence of the issue #638.
// ../openapi3/testdata/issue638/test1.yaml : reproduce
// ./testdata/issue638/test1.yaml : not reproduce
// testdata/issue638/test1.yaml : reproduce
doc, err := loader.LoadFromFile("testdata/issue638/test1.yaml")
require.NoError(t, err)
require.Equal(t, "int", doc.Components.Schemas["test1d"].Value.Type)
}
}
103 changes: 32 additions & 71 deletions openapi3/loader.go
Expand Up @@ -287,20 +287,21 @@ func (loader *Loader) resolveComponent(
path *url.URL,
resolved interface{},
) (
componentDoc *T,
componentPath *url.URL,
err error,
) {
if doc, ref, componentPath, err = loader.resolveRef(doc, ref, path); err != nil {
return nil, err
if componentDoc, ref, componentPath, err = loader.resolveRef(doc, ref, path); err != nil {
return nil, nil, err
}

parsedURL, err := url.Parse(ref)
if err != nil {
return nil, fmt.Errorf("cannot parse reference: %q: %v", ref, parsedURL)
return nil, nil, fmt.Errorf("cannot parse reference: %q: %v", ref, parsedURL)
}
fragment := parsedURL.Fragment
if !strings.HasPrefix(fragment, "/") {
return nil, fmt.Errorf("expected fragment prefix '#/' in URI %q", ref)
return nil, nil, fmt.Errorf("expected fragment prefix '#/' in URI %q", ref)
}

drill := func(cursor interface{}) (interface{}, error) {
Expand All @@ -318,28 +319,28 @@ func (loader *Loader) resolveComponent(
return cursor, nil
}
var cursor interface{}
if cursor, err = drill(doc); err != nil {
if cursor, err = drill(componentDoc); err != nil {
if path == nil {
return nil, err
return nil, nil, err
}
var err2 error
data, err2 := loader.readURL(path)
if err2 != nil {
return nil, err
return nil, nil, err
}
if err2 = unmarshal(data, &cursor); err2 != nil {
return nil, err
return nil, nil, err
}
if cursor, err2 = drill(cursor); err2 != nil || cursor == nil {
return nil, err
return nil, nil, err
}
err = nil
}

switch {
case reflect.TypeOf(cursor) == reflect.TypeOf(resolved):
reflect.ValueOf(resolved).Elem().Set(reflect.ValueOf(cursor).Elem())
return componentPath, nil
return componentDoc, componentPath, nil

case reflect.TypeOf(cursor) == reflect.TypeOf(map[string]interface{}{}):
codec := func(got, expect interface{}) error {
Expand All @@ -353,12 +354,12 @@ func (loader *Loader) resolveComponent(
return nil
}
if err := codec(cursor, resolved); err != nil {
return nil, fmt.Errorf("bad data in %q", ref)
return nil, nil, fmt.Errorf("bad data in %q", ref)
}
return componentPath, nil
return componentDoc, componentPath, nil

default:
return nil, fmt.Errorf("bad data in %q", ref)
return nil, nil, fmt.Errorf("bad data in %q", ref)
}
}

Expand Down Expand Up @@ -429,18 +430,6 @@ func drillIntoField(cursor interface{}, fieldName string) (interface{}, error) {
}
}

func (loader *Loader) documentPathForRecursiveRef(current *url.URL, resolvedRef string) *url.URL {
if loader.rootDir == "" {
return current
}

if resolvedRef == "" {
return &url.URL{Path: loader.rootLocation}
}

return &url.URL{Path: path.Join(loader.rootDir, resolvedRef)}
}

func (loader *Loader) resolveRef(doc *T, ref string, path *url.URL) (*T, string, *url.URL, error) {
if ref != "" && ref[0] == '#' {
return doc, ref, path, nil
Expand Down Expand Up @@ -492,15 +481,15 @@ func (loader *Loader) resolveHeaderRef(doc *T, component *HeaderRef, documentPat
component.Value = &header
} else {
var resolved HeaderRef
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
if err != nil {
return err
}
if err := loader.resolveHeaderRef(doc, &resolved, componentPath); err != nil {
return err
}
component.Value = resolved.Value
documentPath = loader.documentPathForRecursiveRef(documentPath, resolved.Ref)
return nil
}
}
value := component.Value
Expand Down Expand Up @@ -540,15 +529,15 @@ func (loader *Loader) resolveParameterRef(doc *T, component *ParameterRef, docum
component.Value = &param
} else {
var resolved ParameterRef
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
if err != nil {
return err
}
if err := loader.resolveParameterRef(doc, &resolved, componentPath); err != nil {
return err
}
component.Value = resolved.Value
documentPath = loader.documentPathForRecursiveRef(documentPath, resolved.Ref)
return nil
}
}
value := component.Value
Expand Down Expand Up @@ -597,15 +586,15 @@ func (loader *Loader) resolveRequestBodyRef(doc *T, component *RequestBodyRef, d
component.Value = &requestBody
} else {
var resolved RequestBodyRef
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
if err != nil {
return err
}
if err = loader.resolveRequestBodyRef(doc, &resolved, componentPath); err != nil {
return err
}
component.Value = resolved.Value
documentPath = loader.documentPathForRecursiveRef(documentPath, resolved.Ref)
return nil
}
}
value := component.Value
Expand Down Expand Up @@ -659,15 +648,15 @@ func (loader *Loader) resolveResponseRef(doc *T, component *ResponseRef, documen
component.Value = &resp
} else {
var resolved ResponseRef
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
if err != nil {
return err
}
if err := loader.resolveResponseRef(doc, &resolved, componentPath); err != nil {
return err
}
component.Value = resolved.Value
documentPath = loader.documentPathForRecursiveRef(documentPath, resolved.Ref)
return nil
}
}
value := component.Value
Expand Down Expand Up @@ -742,19 +731,15 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat
visited = append(visited, ref)

var resolved SchemaRef
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
if err != nil {
return err
}
if err := loader.resolveSchemaRef(doc, &resolved, componentPath, visited); err != nil {
return err
}
component.Value = resolved.Value
foundPath, rerr := loader.getResolvedRefPath(ref, &resolved, documentPath, componentPath)
if rerr != nil {
return fmt.Errorf("failed to resolve file from reference %q: %w", ref, rerr)
}
documentPath = loader.documentPathForRecursiveRef(documentPath, foundPath)
return nil
}
if loader.visitedSchema == nil {
loader.visitedSchema = make(map[*Schema]struct{})
Expand Down Expand Up @@ -805,30 +790,6 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat
return nil
}

func (loader *Loader) getResolvedRefPath(ref string, resolved *SchemaRef, cur, found *url.URL) (string, error) {
if referencedFilename := strings.Split(ref, "#")[0]; referencedFilename == "" {
if cur != nil {
if loader.rootDir != "" && strings.HasPrefix(cur.Path, loader.rootDir) {
return cur.Path[len(loader.rootDir)+1:], nil
}

return path.Base(cur.Path), nil
}
return "", nil
}
// ref. to external file
if resolved.Ref != "" {
return resolved.Ref, nil
}

if loader.rootDir == "" {
return found.Path, nil
}

// found dest spec. file
return filepath.Rel(loader.rootDir, found.Path)
}

func (loader *Loader) resolveSecuritySchemeRef(doc *T, component *SecuritySchemeRef, documentPath *url.URL) (err error) {
if component != nil && component.Value != nil {
if loader.visitedSecurityScheme == nil {
Expand All @@ -852,15 +813,15 @@ func (loader *Loader) resolveSecuritySchemeRef(doc *T, component *SecurityScheme
component.Value = &scheme
} else {
var resolved SecuritySchemeRef
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
if err != nil {
return err
}
if err := loader.resolveSecuritySchemeRef(doc, &resolved, componentPath); err != nil {
return err
}
component.Value = resolved.Value
_ = loader.documentPathForRecursiveRef(documentPath, resolved.Ref)
return nil
}
}
return nil
Expand Down Expand Up @@ -889,15 +850,15 @@ func (loader *Loader) resolveExampleRef(doc *T, component *ExampleRef, documentP
component.Value = &example
} else {
var resolved ExampleRef
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
if err != nil {
return err
}
if err := loader.resolveExampleRef(doc, &resolved, componentPath); err != nil {
return err
}
component.Value = resolved.Value
_ = loader.documentPathForRecursiveRef(documentPath, resolved.Ref)
return nil
}
}
return nil
Expand All @@ -916,15 +877,15 @@ func (loader *Loader) resolveCallbackRef(doc *T, component *CallbackRef, documen
component.Value = &resolved
} else {
var resolved CallbackRef
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
if err != nil {
return err
}
if err := loader.resolveCallbackRef(doc, &resolved, componentPath); err != nil {
return err
}
component.Value = resolved.Value
documentPath = loader.documentPathForRecursiveRef(documentPath, resolved.Ref)
return nil
}
}
value := component.Value
Expand Down Expand Up @@ -1014,15 +975,15 @@ func (loader *Loader) resolveLinkRef(doc *T, component *LinkRef, documentPath *u
component.Value = &link
} else {
var resolved LinkRef
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
if err != nil {
return err
}
if err := loader.resolveLinkRef(doc, &resolved, componentPath); err != nil {
return err
}
component.Value = resolved.Value
_ = loader.documentPathForRecursiveRef(documentPath, resolved.Ref)
return nil
}
}
return nil
Expand Down
15 changes: 15 additions & 0 deletions openapi3/testdata/issue638/test1.yaml
@@ -0,0 +1,15 @@
openapi: "3.0.0"
info:
version: 1.0.0
title: reference test part 1
description: reference test part 1
components:
schemas:
test1a:
$ref: "test2.yaml#/components/schemas/test2a"
test1b:
$ref: "#/components/schemas/test1c"
test1c:
type: int
test1d:
$ref: "test2.yaml#/components/schemas/test2b"
13 changes: 13 additions & 0 deletions openapi3/testdata/issue638/test2.yaml
@@ -0,0 +1,13 @@
openapi: "3.0.0"
info:
version: 1.0.0
title: reference test part 2
description: reference test part 2
components:
schemas:
test2a:
type: number
test2b:
$ref: "test1.yaml#/components/schemas/test1b"
test1c:
type: string