Repository: getkin/kin-openapi
Pull request #248 (2020-09-02 15:51:53)

Fix openapi3.referencedDocumentPath
`openapi3.referencedDocument` tried to `path.Join` two URLs, and then
generate a new `url.URL` by parsing the obtained path. The URL
obtained at the end would be invalid, when the base path is HTTP URL.

Given base path as 'http://example.com/schemas' and ref as '/schema1',
the obtained end URL would be 'http:/example.com/schemas'. This happens
because `path.Join` cleans the path it generates and replaces repeated
'//' with a single '/'.

This is fixed by doing a `path.Join` on the base URL.Path and the ref,
and setting this result as the URL.Path of the new URL.

fenollp: Hi there, thanks for the patch!
Could you add a few tests on this function? Especially one that would not pass before your fix.
Thanks
kshlm: I've added the tests.

I'm not a 100% sure on the about the expected result of `referencedDocumentPath`. I would have expected it to be `path.Dir(basePath)+refPath`. But other tests seem to expect it to be `path.Dir(basePath)+path.Dir(refPath)+"/"`. Which is what I've implemented the changes and the tests for.

kshlm: Not sure why tests are failing. They're unrelated to my change in the `openapi3` package.

Patch:
diff --git a/openapi3/swagger_loader.go b/openapi3/swagger_loader.go
index 71216a2..aae0280 100644
--- a/openapi3/swagger_loader.go
+++ b/openapi3/swagger_loader.go
@@ -876,16 +876,19 @@ func unescapeRefString(ref string) string {
 }
 
 func referencedDocumentPath(documentPath *url.URL, ref string) (*url.URL, error) {
-	newDocumentPath := documentPath
-	if documentPath != nil {
-		refDirectory, err := url.Parse(path.Dir(ref))
-		if err != nil {
-			return nil, err
-		}
-		joinedDirectory := path.Join(path.Dir(documentPath.String()), refDirectory.String())
-		if newDocumentPath, err = url.Parse(joinedDirectory + "/"); err != nil {
-			return nil, err
-		}
+	if documentPath == nil {
+		return nil, nil
+	}
+
+	newDocumentPath, err := copyURL(documentPath)
+	if err != nil {
+		return nil, err
 	}
+	refPath, err := url.Parse(ref)
+	if err != nil {
+		return nil, err
+	}
+	newDocumentPath.Path = path.Join(path.Dir(newDocumentPath.Path), path.Dir(refPath.Path)) + "/"
+
 	return newDocumentPath, nil
 }


Test patch:
diff --git a/openapi3/swagger_loader_referenced_document_path_test.go b/openapi3/swagger_loader_referenced_document_path_test.go
new file mode 100644
index 0000000..de219c3
--- /dev/null
+++ b/openapi3/swagger_loader_referenced_document_path_test.go
@@ -0,0 +1,60 @@
+package openapi3
+
+import (
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestReferencedDocumentPath(t *testing.T) {
+	httpURL, err := url.Parse("http://example.com/path/to/schemas/test1.yaml")
+	require.NoError(t, err)
+
+	fileURL, err := url.Parse("path/to/schemas/test1.yaml")
+	require.NoError(t, err)
+
+	refEmpty := ""
+	refNoComponent := "moreschemas/test2.yaml"
+	refWithComponent := "moreschemas/test2.yaml#/components/schemas/someobject"
+
+	for _, test := range []struct {
+		path          *url.URL
+		ref, expected string
+	}{
+		{
+			path:     httpURL,
+			ref:      refEmpty,
+			expected: "http://example.com/path/to/schemas/",
+		},
+		{
+			path:     httpURL,
+			ref:      refNoComponent,
+			expected: "http://example.com/path/to/schemas/moreschemas/",
+		},
+		{
+			path:     httpURL,
+			ref:      refWithComponent,
+			expected: "http://example.com/path/to/schemas/moreschemas/",
+		},
+		{
+			path:     fileURL,
+			ref:      refEmpty,
+			expected: "path/to/schemas/",
+		},
+		{
+			path:     fileURL,
+			ref:      refNoComponent,
+			expected: "path/to/schemas/moreschemas/",
+		},
+		{
+			path:     fileURL,
+			ref:      refWithComponent,
+			expected: "path/to/schemas/moreschemas/",
+		},
+	} {
+		result, err := referencedDocumentPath(test.path, test.ref)
+		require.NoError(t, err)
+		require.Equal(t, test.expected, result.String())
+	}
+}