Skip to content

Commit c1219e3

Browse files
ShouheiNishiShouheiNishi
authored
Bugfix/issue638 (#700)
1 parent f136047 commit c1219e3

6 files changed

Lines changed: 84 additions & 81 deletions

File tree

‎openapi3/issue542_test.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,5 @@ func TestIssue542(t *testing.T) {
1010
sl := NewLoader()
1111

1212
_, err := sl.LoadFromFile("testdata/issue542.yml")
13-
require.Error(t, err)
14-
require.Contains(t, err.Error(), CircularReferenceError)
13+
require.NoError(t, err)
1514
}

‎openapi3/issue615_test.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,11 @@ import (
99
)
1010

1111
func TestIssue615(t *testing.T) {
12-
for {
12+
{
1313
loader := openapi3.NewLoader()
1414
loader.IsExternalRefsAllowed = true
1515
_, err := loader.LoadFromFile("testdata/recursiveRef/issue615.yml")
16-
if err == nil {
17-
continue
18-
}
19-
// Test currently reproduces the issue 615: failure to load a valid spec
20-
// Upon issue resolution, this check should be changed to require.NoError
21-
require.Error(t, err, openapi3.CircularReferenceError)
22-
break
16+
require.NoError(t, err)
2317
}
2418

2519
var old int

‎openapi3/issue638_test.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package openapi3
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
)
8+
9+
func TestIssue638(t *testing.T) {
10+
for i := 0; i < 50; i++ {
11+
loader := NewLoader()
12+
loader.IsExternalRefsAllowed = true
13+
// This path affects the occurrence of the issue #638.
14+
// ../openapi3/testdata/issue638/test1.yaml : reproduce
15+
// ./testdata/issue638/test1.yaml : not reproduce
16+
// testdata/issue638/test1.yaml : reproduce
17+
doc, err := loader.LoadFromFile("testdata/issue638/test1.yaml")
18+
require.NoError(t, err)
19+
require.Equal(t, "int", doc.Components.Schemas["test1d"].Value.Type)
20+
}
21+
}

‎openapi3/loader.go‎

Lines changed: 32 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -287,20 +287,21 @@ func (loader *Loader) resolveComponent(
287287
path *url.URL,
288288
resolved interface{},
289289
) (
290+
componentDoc *T,
290291
componentPath *url.URL,
291292
err error,
292293
) {
293-
if doc, ref, componentPath, err = loader.resolveRef(doc, ref, path); err != nil {
294-
return nil, err
294+
if componentDoc, ref, componentPath, err = loader.resolveRef(doc, ref, path); err != nil {
295+
return nil, nil, err
295296
}
296297

297298
parsedURL, err := url.Parse(ref)
298299
if err != nil {
299-
return nil, fmt.Errorf("cannot parse reference: %q: %v", ref, parsedURL)
300+
return nil, nil, fmt.Errorf("cannot parse reference: %q: %v", ref, parsedURL)
300301
}
301302
fragment := parsedURL.Fragment
302303
if !strings.HasPrefix(fragment, "/") {
303-
return nil, fmt.Errorf("expected fragment prefix '#/' in URI %q", ref)
304+
return nil, nil, fmt.Errorf("expected fragment prefix '#/' in URI %q", ref)
304305
}
305306

306307
drill := func(cursor interface{}) (interface{}, error) {
@@ -318,28 +319,28 @@ func (loader *Loader) resolveComponent(
318319
return cursor, nil
319320
}
320321
var cursor interface{}
321-
if cursor, err = drill(doc); err != nil {
322+
if cursor, err = drill(componentDoc); err != nil {
322323
if path == nil {
323-
return nil, err
324+
return nil, nil, err
324325
}
325326
var err2 error
326327
data, err2 := loader.readURL(path)
327328
if err2 != nil {
328-
return nil, err
329+
return nil, nil, err
329330
}
330331
if err2 = unmarshal(data, &cursor); err2 != nil {
331-
return nil, err
332+
return nil, nil, err
332333
}
333334
if cursor, err2 = drill(cursor); err2 != nil || cursor == nil {
334-
return nil, err
335+
return nil, nil, err
335336
}
336337
err = nil
337338
}
338339

339340
switch {
340341
case reflect.TypeOf(cursor) == reflect.TypeOf(resolved):
341342
reflect.ValueOf(resolved).Elem().Set(reflect.ValueOf(cursor).Elem())
342-
return componentPath, nil
343+
return componentDoc, componentPath, nil
343344

344345
case reflect.TypeOf(cursor) == reflect.TypeOf(map[string]interface{}{}):
345346
codec := func(got, expect interface{}) error {
@@ -353,12 +354,12 @@ func (loader *Loader) resolveComponent(
353354
return nil
354355
}
355356
if err := codec(cursor, resolved); err != nil {
356-
return nil, fmt.Errorf("bad data in %q", ref)
357+
return nil, nil, fmt.Errorf("bad data in %q", ref)
357358
}
358-
return componentPath, nil
359+
return componentDoc, componentPath, nil
359360

360361
default:
361-
return nil, fmt.Errorf("bad data in %q", ref)
362+
return nil, nil, fmt.Errorf("bad data in %q", ref)
362363
}
363364
}
364365

@@ -429,18 +430,6 @@ func drillIntoField(cursor interface{}, fieldName string) (interface{}, error) {
429430
}
430431
}
431432

432-
func (loader *Loader) documentPathForRecursiveRef(current *url.URL, resolvedRef string) *url.URL {
433-
if loader.rootDir == "" {
434-
return current
435-
}
436-
437-
if resolvedRef == "" {
438-
return &url.URL{Path: loader.rootLocation}
439-
}
440-
441-
return &url.URL{Path: path.Join(loader.rootDir, resolvedRef)}
442-
}
443-
444433
func (loader *Loader) resolveRef(doc *T, ref string, path *url.URL) (*T, string, *url.URL, error) {
445434
if ref != "" && ref[0] == '#' {
446435
return doc, ref, path, nil
@@ -492,15 +481,15 @@ func (loader *Loader) resolveHeaderRef(doc *T, component *HeaderRef, documentPat
492481
component.Value = &header
493482
} else {
494483
var resolved HeaderRef
495-
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
484+
doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
496485
if err != nil {
497486
return err
498487
}
499488
if err := loader.resolveHeaderRef(doc, &resolved, componentPath); err != nil {
500489
return err
501490
}
502491
component.Value = resolved.Value
503-
documentPath = loader.documentPathForRecursiveRef(documentPath, resolved.Ref)
492+
return nil
504493
}
505494
}
506495
value := component.Value
@@ -540,15 +529,15 @@ func (loader *Loader) resolveParameterRef(doc *T, component *ParameterRef, docum
540529
component.Value = &param
541530
} else {
542531
var resolved ParameterRef
543-
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
532+
doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
544533
if err != nil {
545534
return err
546535
}
547536
if err := loader.resolveParameterRef(doc, &resolved, componentPath); err != nil {
548537
return err
549538
}
550539
component.Value = resolved.Value
551-
documentPath = loader.documentPathForRecursiveRef(documentPath, resolved.Ref)
540+
return nil
552541
}
553542
}
554543
value := component.Value
@@ -597,15 +586,15 @@ func (loader *Loader) resolveRequestBodyRef(doc *T, component *RequestBodyRef, d
597586
component.Value = &requestBody
598587
} else {
599588
var resolved RequestBodyRef
600-
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
589+
doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
601590
if err != nil {
602591
return err
603592
}
604593
if err = loader.resolveRequestBodyRef(doc, &resolved, componentPath); err != nil {
605594
return err
606595
}
607596
component.Value = resolved.Value
608-
documentPath = loader.documentPathForRecursiveRef(documentPath, resolved.Ref)
597+
return nil
609598
}
610599
}
611600
value := component.Value
@@ -659,15 +648,15 @@ func (loader *Loader) resolveResponseRef(doc *T, component *ResponseRef, documen
659648
component.Value = &resp
660649
} else {
661650
var resolved ResponseRef
662-
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
651+
doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
663652
if err != nil {
664653
return err
665654
}
666655
if err := loader.resolveResponseRef(doc, &resolved, componentPath); err != nil {
667656
return err
668657
}
669658
component.Value = resolved.Value
670-
documentPath = loader.documentPathForRecursiveRef(documentPath, resolved.Ref)
659+
return nil
671660
}
672661
}
673662
value := component.Value
@@ -742,19 +731,15 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat
742731
visited = append(visited, ref)
743732

744733
var resolved SchemaRef
745-
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
734+
doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
746735
if err != nil {
747736
return err
748737
}
749738
if err := loader.resolveSchemaRef(doc, &resolved, componentPath, visited); err != nil {
750739
return err
751740
}
752741
component.Value = resolved.Value
753-
foundPath, rerr := loader.getResolvedRefPath(ref, &resolved, documentPath, componentPath)
754-
if rerr != nil {
755-
return fmt.Errorf("failed to resolve file from reference %q: %w", ref, rerr)
756-
}
757-
documentPath = loader.documentPathForRecursiveRef(documentPath, foundPath)
742+
return nil
758743
}
759744
if loader.visitedSchema == nil {
760745
loader.visitedSchema = make(map[*Schema]struct{})
@@ -805,30 +790,6 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat
805790
return nil
806791
}
807792

808-
func (loader *Loader) getResolvedRefPath(ref string, resolved *SchemaRef, cur, found *url.URL) (string, error) {
809-
if referencedFilename := strings.Split(ref, "#")[0]; referencedFilename == "" {
810-
if cur != nil {
811-
if loader.rootDir != "" && strings.HasPrefix(cur.Path, loader.rootDir) {
812-
return cur.Path[len(loader.rootDir)+1:], nil
813-
}
814-
815-
return path.Base(cur.Path), nil
816-
}
817-
return "", nil
818-
}
819-
// ref. to external file
820-
if resolved.Ref != "" {
821-
return resolved.Ref, nil
822-
}
823-
824-
if loader.rootDir == "" {
825-
return found.Path, nil
826-
}
827-
828-
// found dest spec. file
829-
return filepath.Rel(loader.rootDir, found.Path)
830-
}
831-
832793
func (loader *Loader) resolveSecuritySchemeRef(doc *T, component *SecuritySchemeRef, documentPath *url.URL) (err error) {
833794
if component != nil && component.Value != nil {
834795
if loader.visitedSecurityScheme == nil {
@@ -852,15 +813,15 @@ func (loader *Loader) resolveSecuritySchemeRef(doc *T, component *SecurityScheme
852813
component.Value = &scheme
853814
} else {
854815
var resolved SecuritySchemeRef
855-
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
816+
doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
856817
if err != nil {
857818
return err
858819
}
859820
if err := loader.resolveSecuritySchemeRef(doc, &resolved, componentPath); err != nil {
860821
return err
861822
}
862823
component.Value = resolved.Value
863-
_ = loader.documentPathForRecursiveRef(documentPath, resolved.Ref)
824+
return nil
864825
}
865826
}
866827
return nil
@@ -889,15 +850,15 @@ func (loader *Loader) resolveExampleRef(doc *T, component *ExampleRef, documentP
889850
component.Value = &example
890851
} else {
891852
var resolved ExampleRef
892-
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
853+
doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
893854
if err != nil {
894855
return err
895856
}
896857
if err := loader.resolveExampleRef(doc, &resolved, componentPath); err != nil {
897858
return err
898859
}
899860
component.Value = resolved.Value
900-
_ = loader.documentPathForRecursiveRef(documentPath, resolved.Ref)
861+
return nil
901862
}
902863
}
903864
return nil
@@ -916,15 +877,15 @@ func (loader *Loader) resolveCallbackRef(doc *T, component *CallbackRef, documen
916877
component.Value = &resolved
917878
} else {
918879
var resolved CallbackRef
919-
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
880+
doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
920881
if err != nil {
921882
return err
922883
}
923884
if err := loader.resolveCallbackRef(doc, &resolved, componentPath); err != nil {
924885
return err
925886
}
926887
component.Value = resolved.Value
927-
documentPath = loader.documentPathForRecursiveRef(documentPath, resolved.Ref)
888+
return nil
928889
}
929890
}
930891
value := component.Value
@@ -1014,15 +975,15 @@ func (loader *Loader) resolveLinkRef(doc *T, component *LinkRef, documentPath *u
1014975
component.Value = &link
1015976
} else {
1016977
var resolved LinkRef
1017-
componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
978+
doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved)
1018979
if err != nil {
1019980
return err
1020981
}
1021982
if err := loader.resolveLinkRef(doc, &resolved, componentPath); err != nil {
1022983
return err
1023984
}
1024985
component.Value = resolved.Value
1025-
_ = loader.documentPathForRecursiveRef(documentPath, resolved.Ref)
986+
return nil
1026987
}
1027988
}
1028989
return nil

‎openapi3/testdata/issue638/test1.yaml‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
openapi: "3.0.0"
2+
info:
3+
version: 1.0.0
4+
title: reference test part 1
5+
description: reference test part 1
6+
components:
7+
schemas:
8+
test1a:
9+
$ref: "test2.yaml#/components/schemas/test2a"
10+
test1b:
11+
$ref: "#/components/schemas/test1c"
12+
test1c:
13+
type: int
14+
test1d:
15+
$ref: "test2.yaml#/components/schemas/test2b"

‎openapi3/testdata/issue638/test2.yaml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
openapi: "3.0.0"
2+
info:
3+
version: 1.0.0
4+
title: reference test part 2
5+
description: reference test part 2
6+
components:
7+
schemas:
8+
test2a:
9+
type: number
10+
test2b:
11+
$ref: "test1.yaml#/components/schemas/test1b"
12+
test1c:
13+
type: string

0 commit comments

Comments
 (0)