From db88f7b8bcea561bd073e37942d779cc1c641d61 Mon Sep 17 00:00:00 2001 From: marcinromaszewicz Date: Tue, 23 Jul 2024 10:16:48 -0700 Subject: [PATCH 1/3] Fix #867 by intercepting spec loading This change adds a shim into the Kin loader which attempts to find an `openapi` or `swagger` version key in the spec, and returns an error if it's an unsupported version. in the case where the version can't be detected, we do nothing and behave as previously, failing in the parsing step. --- cmd/oapi-codegen/oapi-codegen.go | 2 +- cmd/oapi-codegen/oapi-codegen_test.go | 2 +- pkg/codegen/codegen_test.go | 6 +- pkg/util/loader.go | 100 +++++++++++++++++++++++++- 4 files changed, 102 insertions(+), 8 deletions(-) diff --git a/cmd/oapi-codegen/oapi-codegen.go b/cmd/oapi-codegen/oapi-codegen.go index 22e147c6de..b4b85226aa 100644 --- a/cmd/oapi-codegen/oapi-codegen.go +++ b/cmd/oapi-codegen/oapi-codegen.go @@ -281,7 +281,7 @@ func main() { return } - swagger, err := util.LoadSwagger(flag.Arg(0)) + swagger, err := util.LoadOpenAPI(flag.Arg(0)) if err != nil { errExit("error loading swagger spec in %s\n: %s\n", flag.Arg(0), err) } diff --git a/cmd/oapi-codegen/oapi-codegen_test.go b/cmd/oapi-codegen/oapi-codegen_test.go index 9d9f06c204..7270c885c3 100644 --- a/cmd/oapi-codegen/oapi-codegen_test.go +++ b/cmd/oapi-codegen/oapi-codegen_test.go @@ -15,7 +15,7 @@ func TestLoader(t *testing.T) { for _, v := range paths { - swagger, err := util.LoadSwagger(v) + swagger, err := util.LoadOpenAPI(v) if err != nil { t.Error(err) } diff --git a/pkg/codegen/codegen_test.go b/pkg/codegen/codegen_test.go index 1d752601b3..12015a3073 100644 --- a/pkg/codegen/codegen_test.go +++ b/pkg/codegen/codegen_test.go @@ -97,7 +97,7 @@ func TestExtPropGoTypeSkipOptionalPointer(t *testing.T) { }, } spec := "test_specs/x-go-type-skip-optional-pointer.yaml" - swagger, err := util.LoadSwagger(spec) + swagger, err := util.LoadOpenAPI(spec) require.NoError(t, err) // Run our code generation: @@ -134,7 +134,7 @@ func TestGoTypeImport(t *testing.T) { }, } spec := "test_specs/x-go-type-import-pet.yaml" - swagger, err := util.LoadSwagger(spec) + swagger, err := util.LoadOpenAPI(spec) require.NoError(t, err) // Run our code generation: @@ -182,7 +182,7 @@ func TestRemoteExternalReference(t *testing.T) { }, } spec := "test_specs/remote-external-reference.yaml" - swagger, err := util.LoadSwagger(spec) + swagger, err := util.LoadOpenAPI(spec) require.NoError(t, err) // Run our code generation: diff --git a/pkg/util/loader.go b/pkg/util/loader.go index f29a160d7f..86e19e08f2 100644 --- a/pkg/util/loader.go +++ b/pkg/util/loader.go @@ -1,27 +1,121 @@ package util import ( + "errors" + "fmt" "net/url" + "strings" "github.com/getkin/kin-openapi/openapi3" + "gopkg.in/yaml.v2" ) +// Deprecated: LoadSwagger loads an OpenAPI 3.0 definition from a file or a +// URL. Use LoadOpenAPI instead. func LoadSwagger(filePath string) (swagger *openapi3.T, err error) { + loader := openapi3.NewLoader() + loader.IsExternalRefsAllowed = true + + u, err := url.Parse(filePath) + if err == nil && u.Scheme != "" && u.Host != "" { + return loader.LoadFromURI(u) + } else { + return loader.LoadFromFile(filePath) + } +} +// LoadOpenAPI loads an OpenAPI spec, and hooks into the kin loader to parse +// version information from the spec. +func LoadOpenAPI(filePath string) (openapi *openapi3.T, err error) { loader := openapi3.NewLoader() loader.IsExternalRefsAllowed = true + // We're using a shim to intercept the loads being done by the kin-openapi + // loader. We're going to peek inside and try to find version information + // about the file. + var ls loaderShim + loader.ReadFromURIFunc = ls.InterceptLoad + u, err := url.Parse(filePath) if err == nil && u.Scheme != "" && u.Host != "" { + // The shim needs a URL to compare with, since it can be called multiple + // times durin a load when resolving multiple refs. + ls.srcURL = u return loader.LoadFromURI(u) } else { + // In the case where the URL failed to parse, we'll construct one explicitly + // in the same way that Kin does. LoadFromFile simply calls LoadFromURI + // internally. + ls.srcURL = &url.URL{Path: filePath} return loader.LoadFromFile(filePath) } } -// Deprecated: In kin-openapi v0.126.0 (https://github.com/getkin/kin-openapi/tree/v0.126.0?tab=readme-ov-file#v01260) the Circular Reference Counter functionality was removed, instead resolving all references with backtracking, to avoid needing to provide a limit to reference counts. +type loaderShim struct { + srcURL *url.URL + version string +} + +// openAPIorSwaggerVersion is used to parse either OpenAPI or Swagger version +// from a JSON or YAML file. +type openAPIorSwaggerVersion struct { + Swagger string `json:"swagger" yaml:"swagger"` + OpenAPI string `json:"openapi" yaml:"openapi"` +} + +var ErrSwagger2NotSupported = errors.New("swagger version 2.0 is not supported") +var ErrOpenAPI31NotSupported = errors.New("OpenAPI version 3.1 is not yet supported") + +func (l *loaderShim) InterceptLoad(loader *openapi3.Loader, url *url.URL) ([]byte, error) { + buf, err := openapi3.DefaultReadFromURI(loader, url) + if err != nil { + return buf, err + } + + if l.srcURL.Scheme == url.Scheme && l.srcURL.Host == url.Host && l.srcURL.Path == url.Path { + var versionInfo openAPIorSwaggerVersion + // We've found our file of interest. Parse it and figure out a version. We'll parse as + // YAML since this handles JSON too. + err = yaml.Unmarshal(buf, &versionInfo) + // If we failed to unmarshal we don't react, maintaining previous behavior of + // trying to process the file. + if err != nil { + return buf, nil + } + + version := versionInfo.OpenAPI + if version == "" { + version = versionInfo.Swagger + } + + // Try to extract the major, minor. Openapi will have patch level, swagger won't + versionParts := strings.Split(version, ".") + if len(versionParts) < 2 { + return buf, nil + } + major := versionParts[0] + minor := versionParts[1] + if major == "2" { + // TODO: we can actually use openapi2conv to convert swagger2 to OpenAPI 3 + return nil, ErrSwagger2NotSupported + } + if major != "3" { + return nil, fmt.Errorf("OpenAPI/Swagger %v is not supported", version) + } + // Now, we know we've got major 3. + if minor != "0" { + return nil, ErrOpenAPI31NotSupported + } + } + + return buf, nil +} + +// Deprecated: In kin-openapi v0.126.0 (https://github.com/getkin/kin-openapi/tree/v0.126.0?tab=readme-ov-file#v01260) the +// Circular Reference Counter functionality was removed, instead resolving all references with backtracking, to avoid +// needing to provide a limit to reference counts. // -// This is now identital in method as `LoadSwagger`. +// This is now identical in method as `LoadSwagger`. func LoadSwaggerWithCircularReferenceCount(filePath string, _ int) (swagger *openapi3.T, err error) { - return LoadSwagger(filePath) + return LoadOpenAPI(filePath) } From b0ea787df29d393a752837b954e48dad26f2206a Mon Sep 17 00:00:00 2001 From: marcinromaszewicz Date: Tue, 23 Jul 2024 10:20:32 -0700 Subject: [PATCH 2/3] Make linter happy --- pkg/util/loader.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/util/loader.go b/pkg/util/loader.go index 86e19e08f2..1aefb3dc19 100644 --- a/pkg/util/loader.go +++ b/pkg/util/loader.go @@ -52,8 +52,7 @@ func LoadOpenAPI(filePath string) (openapi *openapi3.T, err error) { } type loaderShim struct { - srcURL *url.URL - version string + srcURL *url.URL } // openAPIorSwaggerVersion is used to parse either OpenAPI or Swagger version From da0255b8d9e0b25f0044cf81a337de4c125ee9da Mon Sep 17 00:00:00 2001 From: marcinromaszewicz Date: Tue, 23 Jul 2024 12:36:40 -0700 Subject: [PATCH 3/3] Rename LoadOpenAPI back to LoadSwagger Key off the error for unsupported version instead of doing another version check. The loader shim should not return errors, because it'll change the behavior of the loader. Only return existing errors, don't introduce new ones. --- cmd/oapi-codegen/oapi-codegen.go | 13 ++-- cmd/oapi-codegen/oapi-codegen_test.go | 2 +- pkg/codegen/codegen_test.go | 6 +- pkg/util/loader.go | 92 +++++++++++++-------------- 4 files changed, 56 insertions(+), 57 deletions(-) diff --git a/cmd/oapi-codegen/oapi-codegen.go b/cmd/oapi-codegen/oapi-codegen.go index b4b85226aa..483aca76ca 100644 --- a/cmd/oapi-codegen/oapi-codegen.go +++ b/cmd/oapi-codegen/oapi-codegen.go @@ -14,6 +14,7 @@ package main import ( + "errors" "flag" "fmt" "os" @@ -281,13 +282,13 @@ func main() { return } - swagger, err := util.LoadOpenAPI(flag.Arg(0)) + swagger, err := util.LoadSwagger(flag.Arg(0)) if err != nil { - errExit("error loading swagger spec in %s\n: %s\n", flag.Arg(0), err) - } - - if strings.HasPrefix(swagger.OpenAPI, "3.1.") { - fmt.Println("WARNING: You are using an OpenAPI 3.1.x specification, which is not yet supported by oapi-codegen (https://github.com/deepmap/oapi-codegen/issues/373) and so some functionality may not be available. Until oapi-codegen supports OpenAPI 3.1, it is recommended to downgrade your spec to 3.0.x") + if errors.Is(err, util.ErrOpenAPI31NotSupported) { + fmt.Println("WARNING: You are using an OpenAPI 3.1.x specification, which is not yet supported by oapi-codegen (https://github.com/deepmap/oapi-codegen/issues/373) and so some functionality may not be available. Until oapi-codegen supports OpenAPI 3.1, it is recommended to downgrade your spec to 3.0.x") + } else { + errExit("error loading swagger spec in %s\n: %s\n", flag.Arg(0), err) + } } if len(noVCSVersionOverride) > 0 { diff --git a/cmd/oapi-codegen/oapi-codegen_test.go b/cmd/oapi-codegen/oapi-codegen_test.go index 7270c885c3..9d9f06c204 100644 --- a/cmd/oapi-codegen/oapi-codegen_test.go +++ b/cmd/oapi-codegen/oapi-codegen_test.go @@ -15,7 +15,7 @@ func TestLoader(t *testing.T) { for _, v := range paths { - swagger, err := util.LoadOpenAPI(v) + swagger, err := util.LoadSwagger(v) if err != nil { t.Error(err) } diff --git a/pkg/codegen/codegen_test.go b/pkg/codegen/codegen_test.go index 12015a3073..1d752601b3 100644 --- a/pkg/codegen/codegen_test.go +++ b/pkg/codegen/codegen_test.go @@ -97,7 +97,7 @@ func TestExtPropGoTypeSkipOptionalPointer(t *testing.T) { }, } spec := "test_specs/x-go-type-skip-optional-pointer.yaml" - swagger, err := util.LoadOpenAPI(spec) + swagger, err := util.LoadSwagger(spec) require.NoError(t, err) // Run our code generation: @@ -134,7 +134,7 @@ func TestGoTypeImport(t *testing.T) { }, } spec := "test_specs/x-go-type-import-pet.yaml" - swagger, err := util.LoadOpenAPI(spec) + swagger, err := util.LoadSwagger(spec) require.NoError(t, err) // Run our code generation: @@ -182,7 +182,7 @@ func TestRemoteExternalReference(t *testing.T) { }, } spec := "test_specs/remote-external-reference.yaml" - swagger, err := util.LoadOpenAPI(spec) + swagger, err := util.LoadSwagger(spec) require.NoError(t, err) // Run our code generation: diff --git a/pkg/util/loader.go b/pkg/util/loader.go index 1aefb3dc19..2a917dc29b 100644 --- a/pkg/util/loader.go +++ b/pkg/util/loader.go @@ -10,23 +10,9 @@ import ( "gopkg.in/yaml.v2" ) -// Deprecated: LoadSwagger loads an OpenAPI 3.0 definition from a file or a -// URL. Use LoadOpenAPI instead. -func LoadSwagger(filePath string) (swagger *openapi3.T, err error) { - loader := openapi3.NewLoader() - loader.IsExternalRefsAllowed = true - - u, err := url.Parse(filePath) - if err == nil && u.Scheme != "" && u.Host != "" { - return loader.LoadFromURI(u) - } else { - return loader.LoadFromFile(filePath) - } -} - -// LoadOpenAPI loads an OpenAPI spec, and hooks into the kin loader to parse +// LoadSwagger loads an OpenAPI spec, and hooks into the kin loader to parse // version information from the spec. -func LoadOpenAPI(filePath string) (openapi *openapi3.T, err error) { +func LoadSwagger(filePath string) (*openapi3.T, error) { loader := openapi3.NewLoader() loader.IsExternalRefsAllowed = true @@ -36,23 +22,61 @@ func LoadOpenAPI(filePath string) (openapi *openapi3.T, err error) { var ls loaderShim loader.ReadFromURIFunc = ls.InterceptLoad + var openapi *openapi3.T + u, err := url.Parse(filePath) if err == nil && u.Scheme != "" && u.Host != "" { // The shim needs a URL to compare with, since it can be called multiple // times durin a load when resolving multiple refs. ls.srcURL = u - return loader.LoadFromURI(u) + openapi, err = loader.LoadFromURI(u) } else { // In the case where the URL failed to parse, we'll construct one explicitly // in the same way that Kin does. LoadFromFile simply calls LoadFromURI // internally. ls.srcURL = &url.URL{Path: filePath} - return loader.LoadFromFile(filePath) + openapi, err = loader.LoadFromFile(filePath) + } + + if err != nil { + return openapi, err + } + + // Now, our shim will contain version information. We can return errors here + // which won't affect loading, but will tell the higher level some information + // about versions. + + version := ls.versions.OpenAPI + if version == "" { + version = ls.versions.Swagger } + + // Try to extract the major, minor. Openapi will have patch level, swagger won't. If + // it doesn't match x.y pattern, we bail out without further checking. + versionParts := strings.Split(version, ".") + if len(versionParts) < 2 { + return openapi, nil + } + major := versionParts[0] + minor := versionParts[1] + if major == "2" { + // TODO: we can actually use openapi2conv to convert swagger2 to OpenAPI 3 + return openapi, ErrSwagger2NotSupported + } + if major != "3" { + return openapi, fmt.Errorf("OpenAPI/Swagger %v is not supported", version) + } + // Now, we know we've got major 3. + if minor != "0" { + return openapi, ErrOpenAPI31NotSupported + } + + return openapi, nil } type loaderShim struct { - srcURL *url.URL + srcURL *url.URL + versions openAPIorSwaggerVersion } // openAPIorSwaggerVersion is used to parse either OpenAPI or Swagger version @@ -72,41 +96,15 @@ func (l *loaderShim) InterceptLoad(loader *openapi3.Loader, url *url.URL) ([]byt } if l.srcURL.Scheme == url.Scheme && l.srcURL.Host == url.Host && l.srcURL.Path == url.Path { - var versionInfo openAPIorSwaggerVersion // We've found our file of interest. Parse it and figure out a version. We'll parse as // YAML since this handles JSON too. - err = yaml.Unmarshal(buf, &versionInfo) + err = yaml.Unmarshal(buf, &l.versions) // If we failed to unmarshal we don't react, maintaining previous behavior of // trying to process the file. if err != nil { return buf, nil } - - version := versionInfo.OpenAPI - if version == "" { - version = versionInfo.Swagger - } - - // Try to extract the major, minor. Openapi will have patch level, swagger won't - versionParts := strings.Split(version, ".") - if len(versionParts) < 2 { - return buf, nil - } - major := versionParts[0] - minor := versionParts[1] - if major == "2" { - // TODO: we can actually use openapi2conv to convert swagger2 to OpenAPI 3 - return nil, ErrSwagger2NotSupported - } - if major != "3" { - return nil, fmt.Errorf("OpenAPI/Swagger %v is not supported", version) - } - // Now, we know we've got major 3. - if minor != "0" { - return nil, ErrOpenAPI31NotSupported - } } - return buf, nil } @@ -116,5 +114,5 @@ func (l *loaderShim) InterceptLoad(loader *openapi3.Loader, url *url.URL) ([]byt // // This is now identical in method as `LoadSwagger`. func LoadSwaggerWithCircularReferenceCount(filePath string, _ int) (swagger *openapi3.T, err error) { - return LoadOpenAPI(filePath) + return LoadSwagger(filePath) }