Address #565

datamodel/document_config.go

@@ -39,8 +39,8 @@ type DocumentConfiguration struct {
 	// RemoteURLHandler is a function that will be used to retrieve remote documents. If not set, the default
 	// remote document getter will be used.
 	//
-	// The remote handler is only used if the BaseURL is set. If the BaseURL is not set, then the remote handler
-	// will not be used, as there will be nothing to use it against.
+	// The remote handler is only used if AllowRemoteReferences is true. If AllowRemoteReferences is false, then
+	// the remote handler will not be used even when BaseURL is set.
 	//
 	// Resolves [#132]: https://github.com/pb33f/libopenapi/issues/132
 	RemoteURLHandler utils.RemoteURLHandler
@@ -89,12 +89,8 @@ type DocumentConfiguration struct {
 
 	// AllowRemoteReferences will allow the index to lookup remote references. This is disabled by default.
 	//
-	// This behavior is now driven by the inclusion of a BaseURL. If a BaseURL is set, then the
-	// rolodex will look for remote references. If no BaseURL is set, then the rolodex will not look for
-	// remote references. This value has no effect as of version 0.13.0 and will be removed in a future release.
-	//
-	// This value when set, will force the creation of a remote file system even when the BaseURL has not been set.
-	// it will suck in every http link it finds, and recurse through all references located in each document.
+	// BaseURL is used to resolve relative references, but it does not enable remote fetching on its own. Remote
+	// lookup only occurs when this value is true.
 	AllowRemoteReferences bool
 
 	// AvoidIndexBuild will avoid building the index. This is disabled by default, only use if you are sure you don't need it.

datamodel/low/v2/swagger.go

@@ -195,8 +195,8 @@ func createDocument(info *datamodel.SpecInfo, config *datamodel.DocumentConfigur
 		}
 	}
 
-	// if base url is provided, add a remote filesystem to the rolodex.
-	if idxConfig.BaseURL != nil {
+	// Only create a remote filesystem when the caller explicitly allows remote references.
+	if config.AllowRemoteReferences {
 
 		// create a remote filesystem
 		remoteFS, _ := index.NewRemoteFSWithConfig(idxConfig)
@@ -206,7 +206,11 @@ func createDocument(info *datamodel.SpecInfo, config *datamodel.DocumentConfigur
 		idxConfig.AllowRemoteLookup = true
 
 		// add to the rolodex
-		rolodex.AddRemoteFS(config.BaseURL.String(), remoteFS)
+		u := "default"
+		if config.BaseURL != nil {
+			u = config.BaseURL.String()
+		}
+		rolodex.AddRemoteFS(u, remoteFS)
 
 	}
 

datamodel/low/v2/swagger_test.go

@@ -6,8 +6,10 @@ package v2
 import (
 	"fmt"
 	"net/http"
+	"net/http/httptest"
 	"net/url"
 	"os"
+	"sync/atomic"
 	"testing"
 
 	"github.com/pb33f/libopenapi/index"
@@ -366,11 +368,51 @@ func TestRolodexRemoteFileSystem(t *testing.T) {
 	baseUrl := "https://raw.githubusercontent.com/pb33f/libopenapi/main/test_specs"
 	u, _ := url.Parse(baseUrl)
 	cf.BaseURL = u
+	cf.AllowRemoteReferences = true
 	lDoc, err := CreateDocumentFromConfig(info, cf)
 	assert.NotNil(t, lDoc)
 	assert.NoError(t, err)
 }
 
+func TestRolodexRemoteFileSystem_BaseURLDoesNotAllowRemoteReferences(t *testing.T) {
+	var hits int32
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		atomic.AddInt32(&hits, 1)
+		w.Header().Set("Content-Type", "application/yaml")
+		fmt.Fprint(w, `swagger: "2.0"
+info:
+  title: remote
+  version: "1.0"
+paths: {}
+definitions:
+  RemoteThing:
+    type: object
+`)
+	}))
+	defer server.Close()
+
+	spec := fmt.Sprintf(`swagger: "2.0"
+info:
+  title: local
+  version: "1.0"
+paths: {}
+definitions:
+  Thing:
+    $ref: %s/remote.yaml#/definitions/RemoteThing
+`, server.URL)
+	info, err := datamodel.ExtractSpecInfo([]byte(spec))
+	require.NoError(t, err)
+
+	cf := datamodel.NewDocumentConfiguration()
+	cf.BaseURL, _ = url.Parse(server.URL)
+	lDoc, _ := CreateDocumentFromConfig(info, cf)
+
+	require.NotNil(t, lDoc)
+	require.NotNil(t, lDoc.Index)
+	assert.Equal(t, int32(0), atomic.LoadInt32(&hits))
+	assert.False(t, lDoc.Index.GetConfig().AllowRemoteLookup)
+}
+
 func TestRolodexRemoteFileSystem_BadBase(t *testing.T) {
 	data, _ := os.ReadFile("../../../test_specs/first.yaml")
 	info, _ := datamodel.ExtractSpecInfo(data)
@@ -380,6 +422,7 @@ func TestRolodexRemoteFileSystem_BadBase(t *testing.T) {
 	baseUrl := "https://no-no-this-will-not-work-it-just-will-not-get-the-job-done-mate.com"
 	u, _ := url.Parse(baseUrl)
 	cf.BaseURL = u
+	cf.AllowRemoteReferences = true
 	lDoc, err := CreateDocumentFromConfig(info, cf)
 	assert.NotNil(t, lDoc)
 	assert.Error(t, err)
@@ -405,6 +448,7 @@ func TestRolodexRemoteFileSystem_CustomHttpHandler(t *testing.T) {
 	baseUrl := "https://no-no-this-will-not-work-it-just-will-not-get-the-job-done-mate.com"
 	u, _ := url.Parse(baseUrl)
 	cf.BaseURL = u
+	cf.AllowRemoteReferences = true
 
 	pizza := func(url string) (resp *http.Response, err error) {
 		return nil, nil
@@ -424,6 +468,7 @@ func TestRolodexRemoteFileSystem_FailRemoteFS(t *testing.T) {
 	baseUrl := "https://no-no-this-will-not-work-it-just-will-not-get-the-job-done-mate.com"
 	u, _ := url.Parse(baseUrl)
 	cf.BaseURL = u
+	cf.AllowRemoteReferences = true
 
 	pizza := func(url string) (resp *http.Response, err error) {
 		return nil, nil

datamodel/low/v3/create_document.go

@@ -247,8 +247,8 @@ func createDocument(info *datamodel.SpecInfo, config *datamodel.DocumentConfigur
 			rolodex.AddLocalFS(cwd, fileFS)
 		}
 	}
-	// if base url is provided, add a remote filesystem to the rolodex.
-	if idxConfig.BaseURL != nil || config.AllowRemoteReferences {
+	// Only create a remote filesystem when the caller explicitly allows remote references.
+	if config.AllowRemoteReferences {
 
 		// create a remote filesystem
 		remoteFS, _ := index.NewRemoteFSWithConfig(idxConfig)

datamodel/low/v3/create_document_test.go

@@ -8,8 +8,10 @@ import (
 	"fmt"
 	"log/slog"
 	"net/http"
+	"net/http/httptest"
 	"net/url"
 	"os"
+	"sync/atomic"
 	"testing"
 
 	"github.com/pb33f/libopenapi/index"
@@ -234,11 +236,53 @@ func TestRolodexRemoteFileSystem(t *testing.T) {
 	baseUrl := "https://raw.githubusercontent.com/pb33f/libopenapi/main/test_specs"
 	u, _ := url.Parse(baseUrl)
 	cf.BaseURL = u
+	cf.AllowRemoteReferences = true
 	lDoc, err := CreateDocumentFromConfig(info, cf)
 	assert.NotNil(t, lDoc)
 	assert.NoError(t, err)
 }
 
+func TestRolodexRemoteFileSystem_BaseURLDoesNotAllowRemoteReferences(t *testing.T) {
+	var hits int32
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		atomic.AddInt32(&hits, 1)
+		w.Header().Set("Content-Type", "application/yaml")
+		fmt.Fprint(w, `openapi: 3.1.0
+info:
+  title: remote
+  version: 1.0.0
+paths: {}
+components:
+  schemas:
+    RemoteThing:
+      type: object
+`)
+	}))
+	defer server.Close()
+
+	spec := fmt.Sprintf(`openapi: 3.1.0
+info:
+  title: local
+  version: 1.0.0
+paths: {}
+components:
+  schemas:
+    Thing:
+      $ref: %s/remote.yaml#/components/schemas/RemoteThing
+`, server.URL)
+	info, err := datamodel.ExtractSpecInfo([]byte(spec))
+	require.NoError(t, err)
+
+	cf := datamodel.NewDocumentConfiguration()
+	cf.BaseURL, _ = url.Parse(server.URL)
+	lDoc, _ := CreateDocumentFromConfig(info, cf)
+
+	require.NotNil(t, lDoc)
+	require.NotNil(t, lDoc.Index)
+	assert.Equal(t, int32(0), atomic.LoadInt32(&hits))
+	assert.False(t, lDoc.Index.GetConfig().AllowRemoteLookup)
+}
+
 func TestRolodexRemoteFileSystem_BadBase(t *testing.T) {
 	data, _ := os.ReadFile("../../../test_specs/first.yaml")
 	info, _ := datamodel.ExtractSpecInfo(data)
@@ -248,6 +292,7 @@ func TestRolodexRemoteFileSystem_BadBase(t *testing.T) {
 	baseUrl := "https://no-no-this-will-not-work-it-just-will-not-get-the-job-done-mate.com"
 	u, _ := url.Parse(baseUrl)
 	cf.BaseURL = u
+	cf.AllowRemoteReferences = true
 	lDoc, err := CreateDocumentFromConfig(info, cf)
 	assert.NotNil(t, lDoc)
 	assert.Error(t, err)
@@ -273,6 +318,7 @@ func TestRolodexRemoteFileSystem_CustomHttpHandler(t *testing.T) {
 	baseUrl := "https://no-no-this-will-not-work-it-just-will-not-get-the-job-done-mate.com"
 	u, _ := url.Parse(baseUrl)
 	cf.BaseURL = u
+	cf.AllowRemoteReferences = true
 
 	pizza := func(url string) (resp *http.Response, err error) {
 		return nil, nil

document.go

@@ -137,7 +137,7 @@ type DocumentModel[T v2high.Swagger | v3high.Document] struct {
 // This function will NOT automatically follow (meaning load) any file or remote references that are found.
 //
 // If this isn't the behavior you want, then you can use the NewDocumentWithConfiguration() function instead, which allows you to set a configuration that
-// will allow you to control if file or remote references are allowed. In particular the `AllowFileReferences` and `FollowRemoteReferences`
+// will allow you to control if file or remote references are allowed. In particular the `AllowFileReferences` and `AllowRemoteReferences`
 // properties.
 func NewDocument(specByteArray []byte) (Document, error) {
 	return NewDocumentWithTypeCheck(specByteArray, false)

document_examples_test.go

@@ -112,10 +112,11 @@ func ExampleNewDocument_fromWithDocumentConfigurationSuccess() {
 	// locked this in to a release, because the spec is throwing 404's occasionally.
 	baseURL, _ := url.Parse("https://raw.githubusercontent.com/digitalocean/openapi/ed0958267922794ec8cf540e19131a2d9664bfc7/specification")
 
-	// create a DocumentConfiguration that allows loading file and remote references, and sets the baseURL
+	// create a DocumentConfiguration that allows loading remote references, and sets the baseURL
 	// to somewhere that can resolve the relative references.
 	config := datamodel.DocumentConfiguration{
-		BaseURL: baseURL,
+		BaseURL:               baseURL,
+		AllowRemoteReferences: true,
 		Logger: slog.New(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{
 			Level: slog.LevelError,
 		})),

document_test.go

@@ -8,11 +8,13 @@ import (
 	"fmt"
 	"log/slog"
 	"net/http"
+	"net/http/httptest"
 	"net/url"
 	"os"
 	"runtime"
 	"strconv"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -2215,3 +2217,76 @@ paths: {}`
 	// (they're needed for hashing during what-changed comparisons)
 	assert.NotNil(t, rootIdx.GetConfig())
 }
+
+func TestNewDocument_SelfDoesNotEnableRemoteReferences(t *testing.T) {
+	newCanary := func() (*httptest.Server, *int32) {
+		var hits int32
+		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			atomic.AddInt32(&hits, 1)
+			w.Header().Set("Content-Type", "application/yaml")
+			fmt.Fprint(w, `openapi: 3.1.0
+info:
+  title: remote
+  version: 1.0.0
+paths: {}
+components:
+  schemas:
+    RemoteThing:
+      type: object
+`)
+		}))
+		return server, &hits
+	}
+
+	spec := func(base string) []byte {
+		return []byte(fmt.Sprintf(`openapi: 3.2.0
+$self: %s/root.yaml
+info:
+  title: issue565
+  version: 1.0.0
+paths: {}
+components:
+  schemas:
+    Thing:
+      $ref: %s/remote.yaml#/components/schemas/RemoteThing
+`, base, base))
+	}
+
+	t.Run("default NewDocument denies remote lookup", func(t *testing.T) {
+		server, hits := newCanary()
+		defer server.Close()
+
+		doc, err := NewDocument(spec(server.URL))
+		require.NoError(t, err)
+
+		model, buildErr := doc.BuildV3Model()
+
+		require.Error(t, buildErr)
+		assert.Nil(t, model)
+		assert.Contains(t, buildErr.Error(), "cannot resolve reference")
+		assert.Contains(t, buildErr.Error(), server.URL+"/remote.yaml#/components/schemas/RemoteThing")
+		assert.Equal(t, int32(0), atomic.LoadInt32(hits))
+		require.NotNil(t, doc.GetRolodex())
+		require.NotNil(t, doc.GetRolodex().GetRootIndex())
+		assert.False(t, doc.GetRolodex().GetRootIndex().GetConfig().AllowRemoteLookup)
+	})
+
+	t.Run("explicit AllowRemoteReferences permits remote lookup", func(t *testing.T) {
+		server, hits := newCanary()
+		defer server.Close()
+
+		config := datamodel.NewDocumentConfiguration()
+		config.AllowRemoteReferences = true
+		doc, err := NewDocumentWithConfiguration(spec(server.URL), config)
+		require.NoError(t, err)
+
+		model, buildErr := doc.BuildV3Model()
+		require.NoError(t, buildErr)
+		require.NotNil(t, model)
+
+		assert.Greater(t, atomic.LoadInt32(hits), int32(0))
+		require.NotNil(t, doc.GetRolodex())
+		require.NotNil(t, doc.GetRolodex().GetRootIndex())
+		assert.True(t, doc.GetRolodex().GetRootIndex().GetConfig().AllowRemoteLookup)
+	})
+}