Repository: getkin/kin-openapi
Pull request #660 (2022-11-08 14:54:29)

Fixed recurive reference resolving when property referencies local co…
Fixed recurive reference resolving when property referencies local component schema type, which referencies json schema in another dir, in which property referencies another json in some non-root (not in the same dir as the specification with local reference) dir json schema.

Please, look at added test cases for more details about the fixed issue.

derbylock: > LGTM! Just you can remove the two `definitions.json` files. They are unused if I'm not mistaken.

Thanks, done

Patch:
diff --git a/openapi3/loader.go b/openapi3/loader.go
index 1ab693b..138431f 100644
--- a/openapi3/loader.go
+++ b/openapi3/loader.go
@@ -742,9 +742,16 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat
 				return err
 			}
 			component.Value = resolved.Value
-			foundPath := loader.getResolvedRefPath(ref, &resolved, documentPath, componentPath)
+			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)
 		}
+		if loader.visitedSchema == nil {
+			loader.visitedSchema = make(map[*Schema]struct{})
+		}
+		loader.visitedSchema[component.Value] = struct{}{}
 	}
 	value := component.Value
 	if value == nil {
@@ -790,23 +797,28 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat
 	return nil
 }
 
-func (loader *Loader) getResolvedRefPath(ref string, resolved *SchemaRef, cur, found *url.URL) string {
+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:]
+				return cur.Path[len(loader.rootDir)+1:], nil
 			}
 
-			return path.Base(cur.Path)
+			return path.Base(cur.Path), nil
 		}
-		return ""
+		return "", nil
 	}
 	// ref. to external file
 	if resolved.Ref != "" {
-		return resolved.Ref
+		return resolved.Ref, nil
 	}
+
+	if loader.rootDir == "" {
+		return found.Path, nil
+	}
+
 	// found dest spec. file
-	return path.Dir(found.Path)[len(loader.rootDir):]
+	return filepath.Rel(loader.rootDir, found.Path)
 }
 
 func (loader *Loader) resolveSecuritySchemeRef(doc *T, component *SecuritySchemeRef, documentPath *url.URL) (err error) {
diff --git a/openapi3filter/req_resp_encoder.go b/openapi3filter/req_resp_encoder.go
index dd410f5..36b7db6 100644
--- a/openapi3filter/req_resp_encoder.go
+++ b/openapi3filter/req_resp_encoder.go
@@ -16,8 +16,34 @@ func encodeBody(body interface{}, mediaType string) ([]byte, error) {
 	return encoder(body)
 }
 
-type bodyEncoder func(body interface{}) ([]byte, error)
+type BodyEncoder func(body interface{}) ([]byte, error)
 
-var bodyEncoders = map[string]bodyEncoder{
+var bodyEncoders = map[string]BodyEncoder{
 	"application/json": json.Marshal,
 }
+
+func RegisterBodyEncoder(contentType string, encoder BodyEncoder) {
+	if contentType == "" {
+		panic("contentType is empty")
+	}
+	if encoder == nil {
+		panic("encoder is not defined")
+	}
+	bodyEncoders[contentType] = encoder
+}
+
+// This call is not thread-safe: body encoders should not be created/destroyed by multiple goroutines.
+func UnregisterBodyEncoder(contentType string) {
+	if contentType == "" {
+		panic("contentType is empty")
+	}
+	delete(bodyEncoders, contentType)
+}
+
+// RegisteredBodyEncoder returns the registered body encoder for the given content type.
+//
+// If no encoder was registered for the given content type, nil is returned.
+// This call is not thread-safe: body encoders should not be created/destroyed by multiple goroutines.
+func RegisteredBodyEncoder(contentType string) BodyEncoder {
+	return bodyEncoders[contentType]
+}


Test patch:
diff --git a/openapi3/issue652_test.go b/openapi3/issue652_test.go
new file mode 100644
index 0000000..f36e920
--- /dev/null
+++ b/openapi3/issue652_test.go
@@ -0,0 +1,29 @@
+package openapi3_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/getkin/kin-openapi/openapi3"
+)
+
+func TestIssue652(t *testing.T) {
+	loader := openapi3.NewLoader()
+	loader.IsExternalRefsAllowed = true
+
+	// Test checks that no slice bounds out of range error occurs while loading
+	// from file that contains reference to file in the parent directory.
+	require.NotPanics(t, func() {
+		const schemaName = "ReferenceToParentDirectory"
+
+		spec, err := loader.LoadFromFile("testdata/issue652/nested/schema.yml")
+		require.NoError(t, err)
+		require.Contains(t, spec.Components.Schemas, schemaName)
+
+		schema := spec.Components.Schemas[schemaName]
+		assert.Equal(t, schema.Ref, "../definitions.yml#/components/schemas/TestSchema")
+		assert.Equal(t, schema.Value.Type, "string")
+	})
+}
diff --git a/openapi3/loader_test.go b/openapi3/loader_test.go
index 684e1b4..e792767 100644
--- a/openapi3/loader_test.go
+++ b/openapi3/loader_test.go
@@ -263,6 +263,29 @@ func TestLoadWithReferenceInReference(t *testing.T) {
 	require.Equal(t, "string", doc.Paths["/api/test/ref/in/ref"].Post.RequestBody.Value.Content["application/json"].Schema.Value.Properties["definition_reference"].Value.Type)
 }
 
+func TestLoadWithRecursiveReferenceInLocalReferenceInParentSubdir(t *testing.T) {
+	loader := NewLoader()
+	loader.IsExternalRefsAllowed = true
+	doc, err := loader.LoadFromFile("testdata/refInLocalRefInParentsSubdir/spec/openapi.json")
+	require.NoError(t, err)
+	require.NotNil(t, doc)
+	err = doc.Validate(loader.Context)
+	require.NoError(t, err)
+	require.Equal(t, "object", doc.Paths["/api/test/ref/in/ref"].Post.RequestBody.Value.Content["application/json"].Schema.Value.Properties["definition_reference"].Value.Type)
+}
+
+func TestLoadWithRecursiveReferenceInRefrerenceInLocalReference(t *testing.T) {
+	loader := NewLoader()
+	loader.IsExternalRefsAllowed = true
+	doc, err := loader.LoadFromFile("testdata/refInLocalRef/openapi.json")
+	require.NoError(t, err)
+	require.NotNil(t, doc)
+	err = doc.Validate(loader.Context)
+	require.NoError(t, err)
+	require.Equal(t, "integer", doc.Paths["/api/test/ref/in/ref"].Post.RequestBody.Value.Content["application/json"].Schema.Value.Properties["data"].Value.Properties["definition_reference"].Value.Properties["ref_prop_part"].Value.Properties["idPart"].Value.Type)
+	require.Equal(t, "int64", doc.Paths["/api/test/ref/in/ref"].Post.RequestBody.Value.Content["application/json"].Schema.Value.Properties["data"].Value.Properties["definition_reference"].Value.Properties["ref_prop_part"].Value.Properties["idPart"].Value.Format)
+}
+
 func TestLoadWithReferenceInReferenceInProperty(t *testing.T) {
 	loader := NewLoader()
 	loader.IsExternalRefsAllowed = true
diff --git a/openapi3/testdata/issue652/definitions.yml b/openapi3/testdata/issue652/definitions.yml
new file mode 100644
index 0000000..98ef692
--- /dev/null
+++ b/openapi3/testdata/issue652/definitions.yml
@@ -0,0 +1,4 @@
+components:
+    schemas:
+        TestSchema:
+            type: string
diff --git a/openapi3/testdata/issue652/nested/schema.yml b/openapi3/testdata/issue652/nested/schema.yml
new file mode 100644
index 0000000..ef321a1
--- /dev/null
+++ b/openapi3/testdata/issue652/nested/schema.yml
@@ -0,0 +1,4 @@
+components:
+    schemas:
+        ReferenceToParentDirectory:
+            $ref: "../definitions.yml#/components/schemas/TestSchema"
diff --git a/openapi3/testdata/refInLocalRef/messages/data.json b/openapi3/testdata/refInLocalRef/messages/data.json
new file mode 100644
index 0000000..cfdc18e
--- /dev/null
+++ b/openapi3/testdata/refInLocalRef/messages/data.json
@@ -0,0 +1,12 @@
+{
+  "type": "object",
+  "properties": {
+    "id": {
+      "type": "integer",
+      "format": "int32"
+    },
+    "ref_prop_part": {
+      "$ref": "./dataPart.json"
+    }
+  }
+}
diff --git a/openapi3/testdata/refInLocalRef/messages/dataPart.json b/openapi3/testdata/refInLocalRef/messages/dataPart.json
new file mode 100644
index 0000000..9ecb585
--- /dev/null
+++ b/openapi3/testdata/refInLocalRef/messages/dataPart.json
@@ -0,0 +1,9 @@
+{
+  "type": "object",
+  "properties": {
+    "idPart": {
+      "type": "integer",
+      "format": "int64"
+    }
+  }
+}
diff --git a/openapi3/testdata/refInLocalRef/messages/request.json b/openapi3/testdata/refInLocalRef/messages/request.json
new file mode 100644
index 0000000..7225ff1
--- /dev/null
+++ b/openapi3/testdata/refInLocalRef/messages/request.json
@@ -0,0 +1,11 @@
+{
+  "type": "object",
+  "required": [
+    "definition_reference"
+  ],
+  "properties": {
+    "definition_reference": {
+      "$ref": "./data.json"
+    }
+  }
+}
diff --git a/openapi3/testdata/refInLocalRef/messages/response.json b/openapi3/testdata/refInLocalRef/messages/response.json
new file mode 100644
index 0000000..b636f52
--- /dev/null
+++ b/openapi3/testdata/refInLocalRef/messages/response.json
@@ -0,0 +1,9 @@
+{
+  "type": "object",
+  "properties": {
+    "id": {
+      "type": "integer",
+      "format": "int32"
+    }
+  }
+}
diff --git a/openapi3/testdata/refInLocalRef/openapi.json b/openapi3/testdata/refInLocalRef/openapi.json
new file mode 100644
index 0000000..f0c9915
--- /dev/null
+++ b/openapi3/testdata/refInLocalRef/openapi.json
@@ -0,0 +1,46 @@
+{
+  "openapi": "3.0.3",
+  "info": {
+    "title": "Reference in reference example",
+    "version": "1.0.0"
+  },
+  "paths": {
+    "/api/test/ref/in/ref": {
+      "post": {
+        "requestBody": {
+          "content": {
+            "application/json": {
+              "schema": {
+                "type": "object",
+                "properties" : {
+                  "data": {
+                    "$ref": "#/components/schemas/Request"
+                  }
+                }
+              }
+            }
+          }
+        },
+        "responses": {
+          "200": {
+            "description": "Successful response",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "messages/response.json"
+                }
+              }
+            }
+          }
+        }
+      }
+    }
+  },
+  "components": {
+    "schemas": {
+      "Request": {
+        "$ref": "messages/request.json"
+      }
+    }
+  }
+}
diff --git a/openapi3/testdata/refInLocalRefInParentsSubdir/messages/data.json b/openapi3/testdata/refInLocalRefInParentsSubdir/messages/data.json
new file mode 100644
index 0000000..cfdc18e
--- /dev/null
+++ b/openapi3/testdata/refInLocalRefInParentsSubdir/messages/data.json
@@ -0,0 +1,12 @@
+{
+  "type": "object",
+  "properties": {
+    "id": {
+      "type": "integer",
+      "format": "int32"
+    },
+    "ref_prop_part": {
+      "$ref": "./dataPart.json"
+    }
+  }
+}
diff --git a/openapi3/testdata/refInLocalRefInParentsSubdir/messages/dataPart.json b/openapi3/testdata/refInLocalRefInParentsSubdir/messages/dataPart.json
new file mode 100644
index 0000000..9ecb585
--- /dev/null
+++ b/openapi3/testdata/refInLocalRefInParentsSubdir/messages/dataPart.json
@@ -0,0 +1,9 @@
+{
+  "type": "object",
+  "properties": {
+    "idPart": {
+      "type": "integer",
+      "format": "int64"
+    }
+  }
+}
diff --git a/openapi3/testdata/refInLocalRefInParentsSubdir/messages/request.json b/openapi3/testdata/refInLocalRefInParentsSubdir/messages/request.json
new file mode 100644
index 0000000..7225ff1
--- /dev/null
+++ b/openapi3/testdata/refInLocalRefInParentsSubdir/messages/request.json
@@ -0,0 +1,11 @@
+{
+  "type": "object",
+  "required": [
+    "definition_reference"
+  ],
+  "properties": {
+    "definition_reference": {
+      "$ref": "./data.json"
+    }
+  }
+}
diff --git a/openapi3/testdata/refInLocalRefInParentsSubdir/messages/response.json b/openapi3/testdata/refInLocalRefInParentsSubdir/messages/response.json
new file mode 100644
index 0000000..b636f52
--- /dev/null
+++ b/openapi3/testdata/refInLocalRefInParentsSubdir/messages/response.json
@@ -0,0 +1,9 @@
+{
+  "type": "object",
+  "properties": {
+    "id": {
+      "type": "integer",
+      "format": "int32"
+    }
+  }
+}
diff --git a/openapi3/testdata/refInLocalRefInParentsSubdir/spec/openapi.json b/openapi3/testdata/refInLocalRefInParentsSubdir/spec/openapi.json
new file mode 100644
index 0000000..0bf9bd3
--- /dev/null
+++ b/openapi3/testdata/refInLocalRefInParentsSubdir/spec/openapi.json
@@ -0,0 +1,46 @@
+{
+  "openapi": "3.0.3",
+  "info": {
+    "title": "Reference in reference example",
+    "version": "1.0.0"
+  },
+  "paths": {
+    "/api/test/ref/in/ref": {
+      "post": {
+        "requestBody": {
+          "content": {
+            "application/json": {
+              "schema": {
+                "$ref": "../messages/request.json"
+              }
+            }
+          }
+        },
+        "responses": {
+          "200": {
+            "description": "Successful response",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "type": "object",
+                  "properties": {
+                    "ref_prop": {
+                      "$ref": "#/components/schemas/Data"
+                    }
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
+    }
+  },
+  "components": {
+    "schemas": {
+      "Data": {
+        "$ref": "../messages/data.json"
+      }
+    }
+  }
+}
diff --git a/openapi3filter/req_resp_encoder_test.go b/openapi3filter/req_resp_encoder_test.go
new file mode 100644
index 0000000..11fe2af
--- /dev/null
+++ b/openapi3filter/req_resp_encoder_test.go
@@ -0,0 +1,43 @@
+package openapi3filter
+
+import (
+	"fmt"
+	"net/http"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestRegisterAndUnregisterBodyEncoder(t *testing.T) {
+	var encoder BodyEncoder
+	encoder = func(body interface{}) (data []byte, err error) {
+		return []byte(strings.Join(body.([]string), ",")), nil
+	}
+	contentType := "text/csv"
+	h := make(http.Header)
+	h.Set(headerCT, contentType)
+
+	originalEncoder := RegisteredBodyEncoder(contentType)
+	require.Nil(t, originalEncoder)
+
+	RegisterBodyEncoder(contentType, encoder)
+	require.Equal(t, fmt.Sprintf("%v", encoder), fmt.Sprintf("%v", RegisteredBodyEncoder(contentType)))
+
+	body := []string{"foo", "bar"}
+	got, err := encodeBody(body, contentType)
+
+	require.NoError(t, err)
+	require.Equal(t, []byte("foo,bar"), got)
+
+	UnregisterBodyEncoder(contentType)
+
+	originalEncoder = RegisteredBodyEncoder(contentType)
+	require.Nil(t, originalEncoder)
+
+	_, err = encodeBody(body, contentType)
+	require.Equal(t, &ParseError{
+		Kind:   KindUnsupportedFormat,
+		Reason: prefixUnsupportedCT + ` "text/csv"`,
+	}, err)
+}