Repository: getkin/kin-openapi
Pull request #700 (2022-12-13 08:29:17)

Bugfix/issue638
Fix for https://github.com/getkin/kin-openapi/issues/638.



Patch:
diff --git a/openapi3/loader.go b/openapi3/loader.go
index 3979e5d..4cc83c0 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 = &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
 			}
@@ -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/openapi3filter/validate_request.go b/openapi3filter/validate_request.go
index 4acb9ff..8a74772 100644
--- a/openapi3filter/validate_request.go
+++ b/openapi3filter/validate_request.go
@@ -5,6 +5,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"sort"
@@ -216,7 +217,19 @@ func ValidateRequestBody(ctx context.Context, input *RequestValidationInput, req
 			}
 		}
 		// Put the data back into the input
-		req.Body = ioutil.NopCloser(bytes.NewReader(data))
+		req.Body = nil
+		if req.GetBody != nil {
+			if req.Body, err = req.GetBody(); err != nil {
+				req.Body = nil
+			}
+		}
+		if req.Body == nil {
+			req.ContentLength = int64(len(data))
+			req.GetBody = func() (io.ReadCloser, error) {
+				return io.NopCloser(bytes.NewReader(data)), nil
+			}
+			req.Body, _ = req.GetBody() // no error return
+		}
 	}
 
 	if len(data) == 0 {
@@ -292,8 +305,14 @@ func ValidateRequestBody(ctx context.Context, input *RequestValidationInput, req
 			}
 		}
 		// Put the data back into the input
-		req.Body = ioutil.NopCloser(bytes.NewReader(data))
+		if req.Body != nil {
+			req.Body.Close()
+		}
 		req.ContentLength = int64(len(data))
+		req.GetBody = func() (io.ReadCloser, error) {
+			return io.NopCloser(bytes.NewReader(data)), nil
+		}
+		req.Body, _ = req.GetBody() // no error return
 	}
 
 	return nil


Test patch:
diff --git a/openapi3/issue542_test.go b/openapi3/issue542_test.go
index 7e0cb88..4ba017a 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 ceb317a..e7bd01e 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 0000000..1db8a6f
--- /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/testdata/issue638/test1.yaml b/openapi3/testdata/issue638/test1.yaml
new file mode 100644
index 0000000..f2ab555
--- /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 0000000..d3ca464
--- /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
diff --git a/openapi3filter/validate_request_test.go b/openapi3filter/validate_request_test.go
index 450ee59..8da550c 100644
--- a/openapi3filter/validate_request_test.go
+++ b/openapi3filter/validate_request_test.go
@@ -212,6 +212,12 @@ components:
 			assert.Equal(t, contentLen, bodySize, "expect ContentLength %d to equal body size %d", contentLen, bodySize)
 			bodyModified := originalBodySize != bodySize
 			assert.Equal(t, bodyModified, tc.expectedModification, "expect request body modification happened: %t, expected %t", bodyModified, tc.expectedModification)
+
+			validationInput.Request.Body, err = validationInput.Request.GetBody()
+			assert.NoError(t, err, "unable to re-generate body by GetBody(): %v", err)
+			body2, err := io.ReadAll(validationInput.Request.Body)
+			assert.NoError(t, err, "unable to read request body: %v", err)
+			assert.Equal(t, body, body2, "body by GetBody() is not matched")
 		})
 	}
 }