From 5dcdcda10b8d7f46963d9832bd069e1aa7dc6248 Mon Sep 17 00:00:00 2001 From: Calvin Lobo Date: Fri, 16 Aug 2024 12:44:30 -0400 Subject: [PATCH 1/6] Instead of using "root.yaml" as a theoretical root, use the real filename of the root spec (often openapi.yaml). This fixes issues with resolving references in rules such as unsused-components. --- datamodel/document_config.go | 3 +++ datamodel/low/v3/create_document.go | 1 + index/index_model.go | 3 +++ index/rolodex.go | 2 +- 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/datamodel/document_config.go b/datamodel/document_config.go index 2fb7a88a..7f374e4f 100644 --- a/datamodel/document_config.go +++ b/datamodel/document_config.go @@ -39,6 +39,9 @@ type DocumentConfiguration struct { // To avoid sucking in all the files, set the FileFilter to a list of specific files to be included. BasePath string // set the Base Path for resolving relative references if the spec is exploded. + // SpecFilePath is the name of the root specification file (usually named "openapi.yaml"). + SpecFilePath string + // FileFilter is a list of specific files to be included by the rolodex when looking up references. If this value // is set, then only these specific files will be included. If this value is not set, then all files will be included. FileFilter []string diff --git a/datamodel/low/v3/create_document.go b/datamodel/low/v3/create_document.go index ca7737b4..20ac4941 100644 --- a/datamodel/low/v3/create_document.go +++ b/datamodel/low/v3/create_document.go @@ -45,6 +45,7 @@ func createDocument(info *datamodel.SpecInfo, config *datamodel.DocumentConfigur idxConfig.AvoidCircularReferenceCheck = true idxConfig.BaseURL = config.BaseURL idxConfig.BasePath = config.BasePath + idxConfig.SpecFilePath = config.SpecFilePath idxConfig.Logger = config.Logger extract := config.ExtractRefsSequentially idxConfig.ExtractRefsSequentially = extract diff --git a/index/index_model.go b/index/index_model.go index 623262bc..53659e61 100644 --- a/index/index_model.go +++ b/index/index_model.go @@ -87,6 +87,9 @@ type SpecIndexConfig struct { // If resolving locally, the BasePath will be the root from which relative references will be resolved from BasePath string // set the Base Path for resolving relative references if the spec is exploded. + // SpecFilePath is the name of the root specification file (usually named "openapi.yaml"). + SpecFilePath string + // In an earlier version of libopenapi (pre 0.6.0) the index would automatically resolve all references // They could have been local, or they could have been remote. This was a problem because it meant // There was a potential for a remote exploit if a remote reference was malicious. There aren't any known diff --git a/index/rolodex.go b/index/rolodex.go index 8e1f8c54..3a3450b1 100644 --- a/index/rolodex.go +++ b/index/rolodex.go @@ -312,7 +312,7 @@ func (r *Rolodex) IndexTheRolodex() error { } if len(r.localFS) > 0 || len(r.remoteFS) > 0 { - r.indexConfig.SpecAbsolutePath = filepath.Join(basePath, "root.yaml") + r.indexConfig.SpecAbsolutePath = filepath.Join(basePath, filepath.Base(r.indexConfig.SpecFilePath)) } } From bc413a1749e66de73e79ba026def25136c145cec Mon Sep 17 00:00:00 2001 From: Calvin Lobo Date: Mon, 19 Aug 2024 11:46:00 -0400 Subject: [PATCH 2/6] Added GetAllReferences() and GetAllMappedReferences() methods to Rolodex --- index/rolodex.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/index/rolodex.go b/index/rolodex.go index 3a3450b1..83568f6a 100644 --- a/index/rolodex.go +++ b/index/rolodex.go @@ -10,6 +10,7 @@ import ( "io" "io/fs" "log/slog" + "maps" "math" "os" "path/filepath" @@ -425,6 +426,26 @@ func (r *Rolodex) BuildIndexes() { r.manualBuilt = true } +// GetAllReferences returns all references found in the root and all other indices +func (r *Rolodex) GetAllReferences() map[string]*Reference { + allRefs := make(map[string]*Reference) + for _, idx := range append(r.GetIndexes(), r.GetRootIndex()) { + refs := idx.GetAllReferences() + maps.Copy(allRefs, refs) + } + return allRefs +} + +// GetAllMappedReferences returns all mapped references found in the root and all other indices +func (r *Rolodex) GetAllMappedReferences() map[string]*Reference { + mappedRefs := make(map[string]*Reference) + for _, idx := range append(r.GetIndexes(), r.GetRootIndex()) { + refs := idx.GetMappedReferences() + maps.Copy(mappedRefs, refs) + } + return mappedRefs +} + // Open opens a file in the rolodex, and returns a RolodexFile. func (r *Rolodex) Open(location string) (RolodexFile, error) { From eb7e09af4deb113b7af22b5eacc599075cffda60 Mon Sep 17 00:00:00 2001 From: Calvin Lobo Date: Wed, 28 Aug 2024 09:15:47 -0400 Subject: [PATCH 3/6] Set indexConfig.SpecAbsolutePath to "root.yaml" (theoretical root) when the spec's root does not exist on the filesystem (either read from remote URL or []byte) --- bundler/bundler_test.go | 1 + index/rolodex.go | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/bundler/bundler_test.go b/bundler/bundler_test.go index c2b366ec..56ee4d0b 100644 --- a/bundler/bundler_test.go +++ b/bundler/bundler_test.go @@ -38,6 +38,7 @@ func TestBundleDocument_DigitalOcean(t *testing.T) { digi, _ := os.ReadFile(spec) doc, err := libopenapi.NewDocumentWithConfiguration([]byte(digi), &datamodel.DocumentConfiguration{ + SpecFilePath: spec, BasePath: tmp + "/specification", ExtractRefsSequentially: true, Logger: slog.New(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{ diff --git a/index/rolodex.go b/index/rolodex.go index 83568f6a..709a5887 100644 --- a/index/rolodex.go +++ b/index/rolodex.go @@ -313,7 +313,13 @@ func (r *Rolodex) IndexTheRolodex() error { } if len(r.localFS) > 0 || len(r.remoteFS) > 0 { - r.indexConfig.SpecAbsolutePath = filepath.Join(basePath, filepath.Base(r.indexConfig.SpecFilePath)) + // For specs that are not read from a filesystem (either from remote URL or []byte), we need to + // assign a theoretical root file. Having a root file is necessary when mapping references. + rootFile := "root.yaml" + if r.indexConfig.SpecFilePath != "" { + rootFile = filepath.Base(r.indexConfig.SpecFilePath) + } + r.indexConfig.SpecAbsolutePath = filepath.Join(basePath, rootFile) } } From e4a49c08812ca5b27edea1fdd3bc66f4675c2d37 Mon Sep 17 00:00:00 2001 From: Calvin Lobo Date: Wed, 28 Aug 2024 10:29:22 -0400 Subject: [PATCH 4/6] Improved code coverage and fixed nil error when copying references --- index/rolodex.go | 6 ++++++ index/rolodex_test.go | 13 ++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/index/rolodex.go b/index/rolodex.go index 709a5887..640cc279 100644 --- a/index/rolodex.go +++ b/index/rolodex.go @@ -436,6 +436,9 @@ func (r *Rolodex) BuildIndexes() { func (r *Rolodex) GetAllReferences() map[string]*Reference { allRefs := make(map[string]*Reference) for _, idx := range append(r.GetIndexes(), r.GetRootIndex()) { + if idx == nil { + continue + } refs := idx.GetAllReferences() maps.Copy(allRefs, refs) } @@ -446,6 +449,9 @@ func (r *Rolodex) GetAllReferences() map[string]*Reference { func (r *Rolodex) GetAllMappedReferences() map[string]*Reference { mappedRefs := make(map[string]*Reference) for _, idx := range append(r.GetIndexes(), r.GetRootIndex()) { + if idx == nil { + continue + } refs := idx.GetMappedReferences() maps.Copy(mappedRefs, refs) } diff --git a/index/rolodex_test.go b/index/rolodex_test.go index c24e0c7b..566fa02c 100644 --- a/index/rolodex_test.go +++ b/index/rolodex_test.go @@ -24,6 +24,8 @@ import ( func TestRolodex_NewRolodex(t *testing.T) { c := CreateOpenAPIIndexConfig() rolo := NewRolodex(c) + assert.Len(t, rolo.GetAllReferences(), 0) + assert.Len(t, rolo.GetAllMappedReferences(), 0) assert.NotNil(t, rolo) assert.NotNil(t, rolo.indexConfig) assert.Nil(t, rolo.GetIgnoredCircularReferences()) @@ -1516,6 +1518,7 @@ func TestRolodex_SimpleTest_OneDoc(t *testing.T) { } cf := CreateOpenAPIIndexConfig() + cf.SpecFilePath = filepath.Join(baseDir, "doc1.yaml") cf.BasePath = baseDir cf.IgnoreArrayCircularReferences = true cf.IgnorePolymorphicCircularReferences = true @@ -1523,11 +1526,19 @@ func TestRolodex_SimpleTest_OneDoc(t *testing.T) { rolo := NewRolodex(cf) rolo.AddLocalFS(baseDir, fileFS) + rootBytes, err := os.ReadFile(cf.SpecFilePath) + assert.NoError(t, err) + var rootNode yaml.Node + _ = yaml.Unmarshal(rootBytes, &rootNode) + rolo.SetRootNode(&rootNode) + err = rolo.IndexTheRolodex() //assert.NotZero(t, rolo.GetIndexingDuration()) comes back as 0 on windows. - assert.Nil(t, rolo.GetRootIndex()) + assert.NotNil(t, rolo.GetRootIndex()) assert.Len(t, rolo.GetIndexes(), 10) + assert.Len(t, rolo.GetAllReferences(), 7) + assert.Len(t, rolo.GetAllMappedReferences(), 7) assert.NoError(t, err) assert.Len(t, rolo.indexes, 10) From f55c15118d987b97d8fbedb0a62ad3e5ca553ff0 Mon Sep 17 00:00:00 2001 From: Calvin Lobo Date: Wed, 28 Aug 2024 14:32:37 -0400 Subject: [PATCH 5/6] Fixed other references to "root.yaml" --- index/index_model.go | 12 ++++++++++++ index/rolodex.go | 10 ++++------ index/search_index.go | 2 +- index/spec_index.go | 11 +++++++++++ 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/index/index_model.go b/index/index_model.go index 53659e61..5dec6bb2 100644 --- a/index/index_model.go +++ b/index/index_model.go @@ -8,6 +8,7 @@ import ( "log/slog" "net/http" "net/url" + "path/filepath" "sync" "github.com/pb33f/libopenapi/datamodel" @@ -154,6 +155,17 @@ type SpecIndexConfig struct { uri []string } +// SetTheoreticalRoot sets the spec file paths to a theoretical file path +func (s *SpecIndexConfig) SetTheoreticalRoot() { + s.SpecFilePath = filepath.Join(s.BasePath, theoreticalRoot) + + basePath := s.BasePath + if !filepath.IsAbs(basePath) { + basePath, _ = filepath.Abs(basePath) + } + s.SpecAbsolutePath = filepath.Join(basePath, theoreticalRoot) +} + // CreateOpenAPIIndexConfig is a helper function to create a new SpecIndexConfig with the AllowRemoteLookup and // AllowFileLookup set to true. This is the default behaviour of the index in previous versions of libopenapi. (pre 0.6.0) // diff --git a/index/rolodex.go b/index/rolodex.go index 640cc279..f5dbe90b 100644 --- a/index/rolodex.go +++ b/index/rolodex.go @@ -303,7 +303,7 @@ func (r *Rolodex) IndexTheRolodex() error { // indexed and built every supporting file, we can build the root index (our entry point) if r.rootNode != nil { - // if there is a base path, then we need to set the root spec config to point to a theoretical root.yaml + // if there is a base path but no SpecFilePath, then we need to set the root spec config to point to a theoretical root.yaml // which does not exist, but is used to formulate the absolute path to root references correctly. if r.indexConfig.BasePath != "" && r.indexConfig.BaseURL == nil { @@ -313,13 +313,11 @@ func (r *Rolodex) IndexTheRolodex() error { } if len(r.localFS) > 0 || len(r.remoteFS) > 0 { - // For specs that are not read from a filesystem (either from remote URL or []byte), we need to - // assign a theoretical root file. Having a root file is necessary when mapping references. - rootFile := "root.yaml" if r.indexConfig.SpecFilePath != "" { - rootFile = filepath.Base(r.indexConfig.SpecFilePath) + r.indexConfig.SpecAbsolutePath = filepath.Join(basePath, filepath.Base(r.indexConfig.SpecFilePath)) + } else { + r.indexConfig.SetTheoreticalRoot() } - r.indexConfig.SpecAbsolutePath = filepath.Join(basePath, rootFile) } } diff --git a/index/search_index.go b/index/search_index.go index 25806bd0..0f46a533 100644 --- a/index/search_index.go +++ b/index/search_index.go @@ -125,7 +125,7 @@ func (index *SpecIndex) SearchIndexForReferenceByReferenceWithContext(ctx contex if strings.Contains(roloLookup, "#") { roloLookup = strings.Split(roloLookup, "#")[0] } - if filepath.Base(roloLookup) == "root.yaml" { + if filepath.Base(roloLookup) == index.GetSpecFileName() { return nil, index, ctx } rFile, err := index.rolodex.Open(roloLookup) diff --git a/index/spec_index.go b/index/spec_index.go index 7a74222f..56861b37 100644 --- a/index/spec_index.go +++ b/index/spec_index.go @@ -25,6 +25,10 @@ import ( "gopkg.in/yaml.v3" ) +const ( + theoreticalRoot = "root.yaml" +) + // NewSpecIndexWithConfig will create a new index of an OpenAPI or Swagger spec. It uses the same logic as NewSpecIndex // except it sets a base URL for resolving relative references, except it also allows for granular control over // how the index is set up. @@ -152,6 +156,13 @@ func (index *SpecIndex) GetRolodex() *Rolodex { return index.rolodex } +func (index *SpecIndex) GetSpecFileName() string { + if index == nil || index.rolodex == nil || index.rolodex.indexConfig == nil { + return theoreticalRoot + } + return index.rolodex.indexConfig.SpecFilePath +} + // GetGlobalTagsNode returns document root tags node. func (index *SpecIndex) GetGlobalTagsNode() *yaml.Node { return index.tagsNode From 839af444e381c7f9b2f67fc04dd681ad82c20671 Mon Sep 17 00:00:00 2001 From: Calvin Lobo Date: Wed, 28 Aug 2024 15:36:30 -0400 Subject: [PATCH 6/6] Added more docstrings --- index/index_model.go | 3 ++- index/spec_index.go | 7 +++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/index/index_model.go b/index/index_model.go index 5dec6bb2..02c87071 100644 --- a/index/index_model.go +++ b/index/index_model.go @@ -155,7 +155,8 @@ type SpecIndexConfig struct { uri []string } -// SetTheoreticalRoot sets the spec file paths to a theoretical file path +// SetTheoreticalRoot sets the spec file paths to point to a theoretical spec file, which does not exist but is required +// in order to formulate the absolute path to root references correctly. func (s *SpecIndexConfig) SetTheoreticalRoot() { s.SpecFilePath = filepath.Join(s.BasePath, theoreticalRoot) diff --git a/index/spec_index.go b/index/spec_index.go index 56861b37..ad146f6b 100644 --- a/index/spec_index.go +++ b/index/spec_index.go @@ -16,6 +16,7 @@ import ( "fmt" "log/slog" "os" + "path/filepath" "sort" "strings" "sync" @@ -26,6 +27,7 @@ import ( ) const ( + // theoreticalRoot is the name of the theoretical spec file used when a root spec file does not exist theoreticalRoot = "root.yaml" ) @@ -156,11 +158,12 @@ func (index *SpecIndex) GetRolodex() *Rolodex { return index.rolodex } +// GetSpecFileName returns the root spec filename, if it exists, otherwise returns the theoretical root spec func (index *SpecIndex) GetSpecFileName() string { - if index == nil || index.rolodex == nil || index.rolodex.indexConfig == nil { + if index == nil || index.rolodex == nil || index.rolodex.indexConfig == nil || index.rolodex.indexConfig.SpecFilePath == "" { return theoreticalRoot } - return index.rolodex.indexConfig.SpecFilePath + return filepath.Base(index.rolodex.indexConfig.SpecFilePath) } // GetGlobalTagsNode returns document root tags node.