diff --git a/openapi3/issue542_test.go b/openapi3/issue542_test.go index 7e0cb88c9..4ba017aed 100644 --- a/openapi3/issue542_test.go +++ b/openapi3/issue542_test.go @@ -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) } diff --git a/openapi3/issue615_test.go b/openapi3/issue615_test.go index ceb317ab0..e7bd01e92 100644 --- a/openapi3/issue615_test.go +++ b/openapi3/issue615_test.go @@ -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 diff --git a/openapi3/issue638_test.go b/openapi3/issue638_test.go new file mode 100644 index 000000000..1db8a6f51 --- /dev/null +++ b/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) + } +} diff --git a/openapi3/loader.go b/openapi3/loader.go index 3979e5d54..4cc83c0b2 100644 --- a/openapi3/loader.go +++ b/openapi3/loader.go @@ -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) { @@ -318,20 +319,20 @@ 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 } @@ -339,7 +340,7 @@ func (loader *Loader) resolveComponent( 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 { @@ -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) } } @@ -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 @@ -492,7 +481,7 @@ 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 } @@ -500,7 +489,7 @@ func (loader *Loader) resolveHeaderRef(doc *T, component *HeaderRef, documentPat return err } component.Value = resolved.Value - documentPath = loader.documentPathForRecursiveRef(documentPath, resolved.Ref) + return nil } } value := component.Value @@ -540,7 +529,7 @@ func (loader *Loader) resolveParameterRef(doc *T, component *ParameterRef, docum component.Value = ¶m } 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 } @@ -548,7 +537,7 @@ func (loader *Loader) resolveParameterRef(doc *T, component *ParameterRef, docum return err } component.Value = resolved.Value - documentPath = loader.documentPathForRecursiveRef(documentPath, resolved.Ref) + return nil } } value := component.Value @@ -597,7 +586,7 @@ 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 } @@ -605,7 +594,7 @@ func (loader *Loader) resolveRequestBodyRef(doc *T, component *RequestBodyRef, d return err } component.Value = resolved.Value - documentPath = loader.documentPathForRecursiveRef(documentPath, resolved.Ref) + return nil } } value := component.Value @@ -659,7 +648,7 @@ 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 } @@ -667,7 +656,7 @@ func (loader *Loader) resolveResponseRef(doc *T, component *ResponseRef, documen return err } component.Value = resolved.Value - documentPath = loader.documentPathForRecursiveRef(documentPath, resolved.Ref) + return nil } } value := component.Value @@ -742,7 +731,7 @@ 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 } @@ -750,11 +739,7 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat 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{}) @@ -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 { @@ -852,7 +813,7 @@ 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 } @@ -860,7 +821,7 @@ func (loader *Loader) resolveSecuritySchemeRef(doc *T, component *SecurityScheme return err } component.Value = resolved.Value - _ = loader.documentPathForRecursiveRef(documentPath, resolved.Ref) + return nil } } return nil @@ -889,7 +850,7 @@ 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 } @@ -897,7 +858,7 @@ func (loader *Loader) resolveExampleRef(doc *T, component *ExampleRef, documentP return err } component.Value = resolved.Value - _ = loader.documentPathForRecursiveRef(documentPath, resolved.Ref) + return nil } } return nil @@ -916,7 +877,7 @@ 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 } @@ -924,7 +885,7 @@ func (loader *Loader) resolveCallbackRef(doc *T, component *CallbackRef, documen return err } component.Value = resolved.Value - documentPath = loader.documentPathForRecursiveRef(documentPath, resolved.Ref) + return nil } } value := component.Value @@ -1014,7 +975,7 @@ 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 } @@ -1022,7 +983,7 @@ func (loader *Loader) resolveLinkRef(doc *T, component *LinkRef, documentPath *u return err } component.Value = resolved.Value - _ = loader.documentPathForRecursiveRef(documentPath, resolved.Ref) + return nil } } return nil diff --git a/openapi3/testdata/issue638/test1.yaml b/openapi3/testdata/issue638/test1.yaml new file mode 100644 index 000000000..f2ab5555c --- /dev/null +++ b/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" diff --git a/openapi3/testdata/issue638/test2.yaml b/openapi3/testdata/issue638/test2.yaml new file mode 100644 index 000000000..d3ca4648b --- /dev/null +++ b/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