diff --git a/datamodel/document_config.go b/datamodel/document_config.go index 5da94ef31..9c2df0c64 100644 --- a/datamodel/document_config.go +++ b/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. diff --git a/datamodel/low/v2/swagger.go b/datamodel/low/v2/swagger.go index 6d079cb9b..0d5f668e5 100644 --- a/datamodel/low/v2/swagger.go +++ b/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) } diff --git a/datamodel/low/v2/swagger_test.go b/datamodel/low/v2/swagger_test.go index 89ae03849..1ef93ed17 100644 --- a/datamodel/low/v2/swagger_test.go +++ b/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 diff --git a/datamodel/low/v3/create_document.go b/datamodel/low/v3/create_document.go index 6d037846a..7ae8f838e 100644 --- a/datamodel/low/v3/create_document.go +++ b/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) diff --git a/datamodel/low/v3/create_document_test.go b/datamodel/low/v3/create_document_test.go index 701a415f3..9eec996d4 100644 --- a/datamodel/low/v3/create_document_test.go +++ b/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 diff --git a/document.go b/document.go index 65b571a77..9c9bf3e40 100644 --- a/document.go +++ b/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) diff --git a/document_examples_test.go b/document_examples_test.go index 3a665e3a9..3ed624ee9 100644 --- a/document_examples_test.go +++ b/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, })), diff --git a/document_test.go b/document_test.go index ebfa533c2..7a2cc7511 100644 --- a/document_test.go +++ b/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) + }) +}