Skip to content

Commit

Permalink
Bugfix/issue638 (#700)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ShouheiNishi
ShouheiNishi committed Dec 14, 2022
1 parent f136047 commit c1219e3
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 81 deletions.
3 changes: 1 addition & 2 deletions openapi3/issue542_test.go
View file
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
View file
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
View file
@@ -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
View file
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
View file
@@ -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
View file
@@ -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

0 comments on commit c1219e3

Please sign in to comment.