Fix: resolve remote relative refs correctly

bundler/bundler.go

@@ -10,7 +10,7 @@ import (
 
 	"github.com/pb33f/libopenapi"
 	"github.com/pb33f/libopenapi/datamodel"
-	"github.com/pb33f/libopenapi/datamodel/high/v3"
+	v3 "github.com/pb33f/libopenapi/datamodel/high/v3"
 	"github.com/pb33f/libopenapi/index"
 )
 
@@ -56,6 +56,7 @@ func BundleDocument(model *v3.Document) ([]byte, error) {
 
 func bundle(model *v3.Document, inline bool) ([]byte, error) {
 	rolodex := model.Rolodex
+
 	compact := func(idx *index.SpecIndex, root bool) {
 		mappedReferences := idx.GetMappedReferences()
 		sequencedReferences := idx.GetRawReferencesSequenced()
@@ -78,6 +79,7 @@ func bundle(model *v3.Document, inline bool) ([]byte, error) {
 				sequenced.Node.Content = mappedReference.Node.Content
 				continue
 			}
+
 			if mappedReference != nil && mappedReference.Circular {
 				if idx.GetLogger() != nil {
 					idx.GetLogger().Warn("[bundler] skipping circular reference",

bundler/bundler_test.go

@@ -6,8 +6,12 @@ package bundler
 import (
 	"bytes"
 	"errors"
+	"fmt"
 	"log"
 	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -101,6 +105,72 @@ func TestBundleDocument_Circular(t *testing.T) {
 	assert.Len(t, logEntries, 0)
 }
 
+func TestBundleDocument_MinimalRemoteRefsBundledLocally(t *testing.T) {
+	specBytes, err := os.ReadFile("../test_specs/minimal_remote_refs/openapi.yaml")
+	require.NoError(t, err)
+
+	require.NoError(t, err)
+
+	config := &datamodel.DocumentConfiguration{
+		AllowFileReferences:   true,
+		AllowRemoteReferences: false,
+		BundleInlineRefs:      false,
+		BasePath:              "../test_specs/minimal_remote_refs",
+		BaseURL:               nil,
+	}
+	require.NoError(t, err)
+
+	bytes, e := BundleBytes(specBytes, config)
+	assert.NoError(t, e)
+	assert.Contains(t, string(bytes), "Name of the account", "should contain all reference targets")
+}
+
+func TestBundleDocument_MinimalRemoteRefsBundledRemotely(t *testing.T) {
+	baseURL, err := url.Parse("https://raw.githubusercontent.com/felixjung/libopenapi/authed-remote/test_specs/minimal_remote_refs")
+
+	refBytes, err := os.ReadFile("../test_specs/minimal_remote_refs/schemas/components.openapi.yaml")
+	require.NoError(t, err)
+
+	wantURL := fmt.Sprintf("%s/%s", baseURL.String(), "schemas/components.openapi.yaml")
+
+	newRemoteHandlerFunc := func() utils.RemoteURLHandler {
+		handler := func(w http.ResponseWriter, r *http.Request) {
+			if r.URL.String() != wantURL {
+				w.WriteHeader(http.StatusNotFound)
+				return
+			}
+
+			w.Write(refBytes)
+		}
+
+		return func(url string) (*http.Response, error) {
+			req := httptest.NewRequest("GET", url, nil)
+			w := httptest.NewRecorder()
+			handler(w, req)
+
+			return w.Result(), nil
+		}
+	}
+
+	specBytes, err := os.ReadFile("../test_specs/minimal_remote_refs/openapi.yaml")
+	require.NoError(t, err)
+
+	require.NoError(t, err)
+
+	config := &datamodel.DocumentConfiguration{
+		BaseURL:               baseURL,
+		AllowFileReferences:   false,
+		AllowRemoteReferences: true,
+		BundleInlineRefs:      false,
+		RemoteURLHandler:      newRemoteHandlerFunc(),
+	}
+	require.NoError(t, err)
+
+	bytes, e := BundleBytes(specBytes, config)
+	assert.NoError(t, e)
+	assert.Contains(t, string(bytes), "Name of the account", "should contain all reference targets")
+}
+
 func TestBundleBytes(t *testing.T) {
 
 	digi, _ := os.ReadFile("../test_specs/circular-tests.yaml")

datamodel/document_config.go

@@ -4,11 +4,12 @@
 package datamodel
 
 import (
-	"github.com/pb33f/libopenapi/utils"
 	"io/fs"
 	"log/slog"
 	"net/url"
 	"os"
+
+	"github.com/pb33f/libopenapi/utils"
 )
 
 // DocumentConfiguration is used to configure the document creation process. It was added in v0.6.0 to allow
@@ -18,6 +19,8 @@ import (
 // any non-local (local being the specification, not the file system) references, will be ignored.
 type DocumentConfiguration struct {
 	// The BaseURL will be the root from which relative references will be resolved from if they can't be found locally.
+	// Make sure it does not point to a file as relative paths will be blindly added to the end of the
+	// BaseURL's path.
 	// Schema must be set to "http/https".
 	BaseURL *url.URL
 

datamodel/low/extraction_functions.go

@@ -87,7 +87,6 @@ func LocateRefNodeWithContext(ctx context.Context, root *yaml.Node, idx *index.S
 		for _, collection := range collections {
 			found = collection()
 			if found != nil && found[rv] != nil {
-
 				// if this is a ref node, we need to keep diving
 				// until we hit something that isn't a ref.
 				if jh, _, _ := utils.IsNodeRefValue(found[rv].Node); jh {
@@ -106,49 +105,75 @@ func LocateRefNodeWithContext(ctx context.Context, root *yaml.Node, idx *index.S
 			}
 		}
 
-		// perform a search for the reference in the index
-		// extract the correct root
+		// Obtain the absolute filepath/URL of the spec in which we are trying to
+		// resolve the reference value [rv] from. It's either available from the
+		// index or passed down through context.
 		specPath := idx.GetSpecAbsolutePath()
 		if ctx.Value(index.CurrentPathKey) != nil {
 			specPath = ctx.Value(index.CurrentPathKey).(string)
 		}
 
+		// explodedRefValue contains both the path to the file containing the
+		// reference value at index 0 and the path within that file to a specific
+		// sub-schema, should it exist, at index 1.
 		explodedRefValue := strings.Split(rv, "#")
 		if len(explodedRefValue) == 2 {
+			// The ref points to a component within either this file or another file.
 			if !strings.HasPrefix(explodedRefValue[0], "http") {
+				// The ref is not an absolute URL.
 				if !filepath.IsAbs(explodedRefValue[0]) {
+					// The ref is not an absolute local file path.
 					if strings.HasPrefix(specPath, "http") {
+						// The schema containing the ref is itself a remote file.
 						u, _ := url.Parse(specPath)
+						// p is the directory the referenced file is expected to be in.
 						p := ""
 						if u.Path != "" && explodedRefValue[0] != "" {
+							// We are using the path of the resolved URL from the rolodex to
+							// obtain the "folder" or base of the file URL.
 							p = filepath.Dir(u.Path)
 						}
 						if p != "" && explodedRefValue[0] != "" {
+							// We are resolving the relative URL against the absolute URL of
+							// the spec containing the reference.
 							u.Path = utils.ReplaceWindowsDriveWithLinuxPath(filepath.Join(p, explodedRefValue[0]))
 						}
 						u.Fragment = ""
+						// Turn the reference value [rv] into the absolute filepath/URL we
+						// resolved.
 						rv = fmt.Sprintf("%s#%s", u.String(), explodedRefValue[1])
-
 					} else {
+						// The schema containing the ref is a local file or doesn't have an
+						// absolute URL.
 						if specPath != "" {
+							// We have _some_ path for the schema containing the reference.
 							var abs string
 							if explodedRefValue[0] == "" {
+								// Reference is made within the schema file, so we are using the
+								// same absolute local filepath.
 								abs = specPath
 							} else {
 								// break off any fragments from the spec path
 								sp := strings.Split(specPath, "#")
+								// Create a clean (absolute?) path to the file containing the
+								// referenced value.
 								abs, _ = filepath.Abs(filepath.Join(filepath.Dir(sp[0]), explodedRefValue[0]))
 							}
 							rv = fmt.Sprintf("%s#%s", abs, explodedRefValue[1])
 						} else {
-							// check for a config baseURL and use that if it exists.
-							if idx.GetConfig().BaseURL != nil {
+							// We don't have a path for the schema we are trying to resolve
+							// relative references from. This likely happens when the schema
+							// is the root schema, i.e. the file given to libopenapi as entry.
+							//
 
+							// check for a config BaseURL and use that if it exists.
+							if idx.GetConfig().BaseURL != nil {
 								u := *idx.GetConfig().BaseURL
 								p := ""
 								if u.Path != "" {
-									p = filepath.Dir(u.Path)
+									p = u.Path
 								}
+
 								u.Path = filepath.Join(p, explodedRefValue[0])
 								rv = fmt.Sprintf("%s#%s", u.String(), explodedRefValue[1])
 							}

datamodel/low/extraction_functions_test.go

@@ -1867,6 +1867,35 @@ func TestLocateRefNode_NoExplode_NoSpecPath(t *testing.T) {
 	assert.NotNil(t, c)
 }
 
+func TestLocateRefNode_Explode_NoSpecPath(t *testing.T) {
+	no := yaml.Node{
+		Kind: yaml.MappingNode,
+		Content: []*yaml.Node{
+			{
+				Kind:  yaml.ScalarNode,
+				Value: "$ref",
+			},
+			{
+				Kind:  yaml.ScalarNode,
+				Value: "components/schemas/thing.yaml#/components/schemas/thing",
+			},
+		},
+	}
+
+	cf := index.CreateClosedAPIIndexConfig()
+	u, _ := url.Parse("http://smilfghfhfhfhfhes.com/bikes")
+	cf.BaseURL = u
+	idx := index.NewSpecIndexWithConfig(&no, cf)
+	ctx := context.Background()
+
+	n, i, e, c := LocateRefNodeWithContext(ctx, &no, idx)
+	assert.Nil(t, n)
+	assert.NotNil(t, i)
+	assert.NotNil(t, e)
+	assert.NotNil(t, c)
+	assert.ErrorContains(t, e, "http://smilfghfhfhfhfhes.com/bikes/components/schemas/thing.yaml#/components/schemas/thing")
+}
+
 func TestLocateRefNode_DoARealLookup(t *testing.T) {
 	lookup := "/root.yaml#/components/schemas/Burger"
 	if runtime.GOOS == "windows" {

datamodel/low/v3/create_document.go

@@ -3,7 +3,9 @@ package v3
 import (
 	"context"
 	"errors"
+	"net/url"
 	"path/filepath"
+	"strings"
 	"sync"
 	"time"
 
@@ -43,7 +45,7 @@ func createDocument(info *datamodel.SpecInfo, config *datamodel.DocumentConfigur
 	idxConfig.IgnoreArrayCircularReferences = config.IgnoreArrayCircularReferences
 	idxConfig.IgnorePolymorphicCircularReferences = config.IgnorePolymorphicCircularReferences
 	idxConfig.AvoidCircularReferenceCheck = true
-	idxConfig.BaseURL = config.BaseURL
+	idxConfig.BaseURL = urlWithoutTrailingSlash(config.BaseURL)
 	idxConfig.BasePath = config.BasePath
 	idxConfig.SpecFilePath = config.SpecFilePath
 	idxConfig.Logger = config.Logger
@@ -319,3 +321,13 @@ func extractWebhooks(ctx context.Context, info *datamodel.SpecInfo, doc *Documen
 	}
 	return nil
 }
+
+func urlWithoutTrailingSlash(u *url.URL) *url.URL {
+	if u == nil {
+		return nil
+	}
+
+	u.Path, _ = strings.CutSuffix(u.Path, "/")
+
+	return u
+}

datamodel/low/v3/create_document_test.go

@@ -881,3 +881,48 @@ func ExampleCreateDocument() {
 	fmt.Print(document.Info.Value.Contact.Value.Email.Value)
 	// Output: [email protected]
 }
+
+func TestURLWithoutTrailingSlash(t *testing.T) {
+	tc := []struct {
+		name string
+		url  string
+		want string
+	}{
+		{
+			name: "url with no path",
+			url:  "https://example.com",
+			want: "https://example.com",
+		},
+		{
+			name: "nil pointer",
+			url:  "",
+		},
+		{
+			name: "URL with path not ending in slash",
+			url:  "https://example.com/some/path",
+			want: "https://example.com/some/path",
+		},
+		{
+			name: "URL with path ending in slash",
+			url:  "https://example.com/some/path/",
+			want: "https://example.com/some/path",
+		},
+	}
+
+	for _, tt := range tc {
+		t.Run(tt.name, func(t *testing.T) {
+			u, _ := url.Parse(tt.url)
+			if tt.url == "" {
+				u = nil
+			}
+
+			got := urlWithoutTrailingSlash(u)
+
+			if u == nil {
+				assert.Nil(t, got)
+				return
+			}
+			assert.Equal(t, tt.want, got.String())
+		})
+	}
+}

document.go

@@ -305,7 +305,9 @@ func (d *document) BuildV3Model() (*DocumentModel[v3high.Document], []error) {
 	if d.config == nil {
 		d.config = &datamodel.DocumentConfiguration{
 			AllowFileReferences:   false,
+			BasePath:              "",
 			AllowRemoteReferences: false,
+			BaseURL:               nil,
 		}
 	}
 
@@ -335,6 +337,7 @@ func (d *document) BuildV3Model() (*DocumentModel[v3high.Document], []error) {
 		Model: *highDoc,
 		Index: lowDoc.Index,
 	}
+
 	return d.highOpenAPI3Model, errs
 }