-
Notifications
You must be signed in to change notification settings - Fork 104
Hang when expanding circular $ref #79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,3 +17,4 @@ linters: | |
| enable-all: true | ||
| disable: | ||
| - maligned | ||
| - unparam | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| // Copyright 2015 go-swagger maintainers | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package spec | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "log" | ||
| "os" | ||
| "path/filepath" | ||
| "runtime" | ||
| ) | ||
|
|
||
| var ( | ||
| // Debug is true when the SWAGGER_DEBUG env var is not empty. | ||
| // It enables a more verbose logging of validators. | ||
| Debug = os.Getenv("SWAGGER_DEBUG") != "" | ||
| // validateLogger is a debug logger for this package | ||
| specLogger *log.Logger | ||
| ) | ||
|
|
||
| func init() { | ||
| debugOptions() | ||
| } | ||
|
|
||
| func debugOptions() { | ||
| specLogger = log.New(os.Stdout, "spec:", log.LstdFlags) | ||
| } | ||
|
|
||
| func debugLog(msg string, args ...interface{}) { | ||
| // A private, trivial trace logger, based on go-openapi/spec/expander.go:debugLog() | ||
| if Debug { | ||
| _, file1, pos1, _ := runtime.Caller(1) | ||
| specLogger.Printf("%s:%d: %s", filepath.Base(file1), pos1, fmt.Sprintf(msg, args...)) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| // Copyright 2015 go-swagger maintainers | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package spec | ||
|
|
||
| import ( | ||
| "io/ioutil" | ||
| "os" | ||
| "sync" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| var ( | ||
| logMutex = &sync.Mutex{} | ||
| ) | ||
|
|
||
| func TestDebug(t *testing.T) { | ||
| tmpFile, _ := ioutil.TempFile("", "debug-test") | ||
| tmpName := tmpFile.Name() | ||
| defer func() { | ||
| Debug = false | ||
| // mutex for -race | ||
| logMutex.Unlock() | ||
| _ = os.Remove(tmpName) | ||
| }() | ||
|
|
||
| // mutex for -race | ||
| logMutex.Lock() | ||
| Debug = true | ||
| debugOptions() | ||
| defer func() { | ||
| specLogger.SetOutput(os.Stdout) | ||
| }() | ||
|
|
||
| specLogger.SetOutput(tmpFile) | ||
|
|
||
| debugLog("A debug") | ||
| Debug = false | ||
| _ = tmpFile.Close() | ||
|
|
||
| flushed, _ := os.Open(tmpName) | ||
| buf := make([]byte, 500) | ||
| _, _ = flushed.Read(buf) | ||
| specLogger.SetOutput(os.Stdout) | ||
| assert.Contains(t, string(buf), "A debug") | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,11 +30,6 @@ import ( | |
| "github.com/go-openapi/swag" | ||
| ) | ||
|
|
||
| var ( | ||
| // Debug enables logging when SWAGGER_DEBUG env var is not empty | ||
| Debug = os.Getenv("SWAGGER_DEBUG") != "" | ||
| ) | ||
|
|
||
| // ExpandOptions provides options for expand. | ||
| type ExpandOptions struct { | ||
| RelativeBase string | ||
|
|
@@ -67,6 +62,21 @@ func initResolutionCache() ResolutionCache { | |
| }} | ||
| } | ||
|
|
||
| // resolverContext allows to share a context during spec processing. | ||
| // At the moment, it just holds the index of circular references found. | ||
| type resolverContext struct { | ||
| // circulars holds all visited circular references, which allows shortcuts. | ||
| // NOTE: this is not just a performance improvement: it is required to figure out | ||
| // circular references which participate several cycles. | ||
| circulars map[string]bool | ||
| } | ||
|
|
||
| func newResolverContext() *resolverContext { | ||
| return &resolverContext{ | ||
| circulars: make(map[string]bool), | ||
| } | ||
| } | ||
|
|
||
| // Get retrieves a cached URI | ||
| func (s *simpleCache) Get(uri string) (interface{}, bool) { | ||
| debugLog("getting %q from resolution cache", uri) | ||
|
|
@@ -87,7 +97,7 @@ func (s *simpleCache) Set(uri string, data interface{}) { | |
|
|
||
| // ResolveRefWithBase resolves a reference against a context root with preservation of base path | ||
| func ResolveRefWithBase(root interface{}, ref *Ref, opts *ExpandOptions) (*Schema, error) { | ||
| resolver, err := defaultSchemaLoader(root, opts, nil) | ||
| resolver, err := defaultSchemaLoader(root, opts, nil, nil) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
@@ -133,7 +143,7 @@ func ResolveParameter(root interface{}, ref Ref) (*Parameter, error) { | |
|
|
||
| // ResolveParameterWithBase resolves a parameter reference against a context root and base path | ||
| func ResolveParameterWithBase(root interface{}, ref Ref, opts *ExpandOptions) (*Parameter, error) { | ||
| resolver, err := defaultSchemaLoader(root, opts, nil) | ||
| resolver, err := defaultSchemaLoader(root, opts, nil, nil) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
@@ -152,7 +162,7 @@ func ResolveResponse(root interface{}, ref Ref) (*Response, error) { | |
|
|
||
| // ResolveResponseWithBase resolves response a reference against a context root and base path | ||
| func ResolveResponseWithBase(root interface{}, ref Ref, opts *ExpandOptions) (*Response, error) { | ||
| resolver, err := defaultSchemaLoader(root, opts, nil) | ||
| resolver, err := defaultSchemaLoader(root, opts, nil, nil) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
@@ -166,7 +176,7 @@ func ResolveResponseWithBase(root interface{}, ref Ref, opts *ExpandOptions) (*R | |
|
|
||
| // ResolveItems resolves header and parameter items reference against a context root and base path | ||
| func ResolveItems(root interface{}, ref Ref, opts *ExpandOptions) (*Items, error) { | ||
| resolver, err := defaultSchemaLoader(root, opts, nil) | ||
| resolver, err := defaultSchemaLoader(root, opts, nil, nil) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
@@ -183,7 +193,7 @@ func ResolveItems(root interface{}, ref Ref, opts *ExpandOptions) (*Items, error | |
|
|
||
| // ResolvePathItem resolves response a path item against a context root and base path | ||
| func ResolvePathItem(root interface{}, ref Ref, opts *ExpandOptions) (*PathItem, error) { | ||
| resolver, err := defaultSchemaLoader(root, opts, nil) | ||
| resolver, err := defaultSchemaLoader(root, opts, nil, nil) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
@@ -202,6 +212,7 @@ type schemaLoader struct { | |
| root interface{} | ||
| options *ExpandOptions | ||
| cache ResolutionCache | ||
| context *resolverContext | ||
| loadDoc func(string) (json.RawMessage, error) | ||
| } | ||
|
|
||
|
|
@@ -224,19 +235,23 @@ func init() { | |
| func defaultSchemaLoader( | ||
| root interface{}, | ||
| expandOptions *ExpandOptions, | ||
| cache ResolutionCache) (*schemaLoader, error) { | ||
| cache ResolutionCache, | ||
| context *resolverContext) (*schemaLoader, error) { | ||
|
|
||
| if cache == nil { | ||
| cache = resCache | ||
| } | ||
| if expandOptions == nil { | ||
| expandOptions = &ExpandOptions{} | ||
| } | ||
|
|
||
| if context == nil { | ||
| context = newResolverContext() | ||
| } | ||
| return &schemaLoader{ | ||
| root: root, | ||
| options: expandOptions, | ||
| cache: cache, | ||
| context: context, | ||
| loadDoc: func(path string) (json.RawMessage, error) { | ||
| debugLog("fetching document at %q", path) | ||
| return PathLoader(path) | ||
|
|
@@ -315,12 +330,6 @@ func nextRef(startingNode interface{}, startingRef *Ref, ptr *jsonpointer.Pointe | |
| return ret | ||
| } | ||
|
|
||
| func debugLog(msg string, args ...interface{}) { | ||
| if Debug { | ||
| log.Printf(msg, args...) | ||
| } | ||
| } | ||
|
|
||
| // normalize absolute path for cache. | ||
| // on Windows, drive letters should be converted to lower as scheme in net/url.URL | ||
| func normalizeAbsPath(path string) string { | ||
|
|
@@ -369,6 +378,19 @@ func normalizePaths(refPath, base string) string { | |
| return baseURL.String() | ||
| } | ||
|
|
||
| // denormalizePaths returns to simplest notation on file $ref, | ||
| // i.e. strips the absolute path and sets a path relative to the base path. | ||
| // | ||
| // This is currently used when we rewrite ref after a circular ref has been detected | ||
| func denormalizeFileRef(ref *Ref, relativeBase string) *Ref { | ||
| if ref.String() == "" || ref.IsRoot() || ref.HasFragmentOnly { | ||
| return ref | ||
| } | ||
| // strip relativeBase from URI | ||
| r, _ := NewRef(strings.TrimPrefix(ref.String(), relativeBase)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is causing the issue in the validation test. It is creating an invalid JSON Ref, if I change it to this: Then all of the test in the validate package pass except one. It should be really easy to fix, I have to run, but will try to get to it later if someone else hasn't. |
||
| return &r | ||
| } | ||
|
|
||
| // relativeBase could be an ABSOLUTE file path or an ABSOLUTE URL | ||
| func normalizeFileRef(ref *Ref, relativeBase string) *Ref { | ||
| // This is important for when the reference is pointing to the root schema | ||
|
|
@@ -377,8 +399,7 @@ func normalizeFileRef(ref *Ref, relativeBase string) *Ref { | |
| return &r | ||
| } | ||
|
|
||
| refURL := ref.GetURL() | ||
| debugLog("normalizing %s against %s (%s)", ref.String(), relativeBase, refURL.String()) | ||
| debugLog("normalizing %s against %s (%s)", ref.String(), relativeBase, ref.GetURL().String()) | ||
|
|
||
| s := normalizePaths(ref.String(), relativeBase) | ||
| r, _ := NewRef(s) | ||
|
|
@@ -478,7 +499,7 @@ func absPath(fname string) (string, error) { | |
|
|
||
| // ExpandSpec expands the references in a swagger spec | ||
| func ExpandSpec(spec *Swagger, options *ExpandOptions) error { | ||
| resolver, err := defaultSchemaLoader(spec, options, nil) | ||
| resolver, err := defaultSchemaLoader(spec, options, nil, nil) | ||
| // Just in case this ever returns an error. | ||
| if shouldStopOnError(err, resolver.options) { | ||
| return err | ||
|
|
@@ -575,7 +596,7 @@ func ExpandSchemaWithBasePath(schema *Schema, cache ResolutionCache, opts *Expan | |
| basePath, _ = absPath(opts.RelativeBase) | ||
| } | ||
|
|
||
| resolver, err := defaultSchemaLoader(nil, opts, cache) | ||
| resolver, err := defaultSchemaLoader(nil, opts, cache, nil) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -627,8 +648,18 @@ func basePathFromSchemaID(oldBasePath, id string) string { | |
| return u.String() | ||
| } | ||
|
|
||
| func isCircular(ref *Ref, basePath string, parentRefs ...string) bool { | ||
| return basePath != "" && swag.ContainsStringsCI(parentRefs, ref.String()) | ||
| func (r *schemaLoader) isCircular(ref *Ref, basePath string, parentRefs ...string) (foundCycle bool) { | ||
| normalizedRef := normalizePaths(ref.String(), basePath) | ||
| if _, ok := r.context.circulars[normalizedRef]; ok { | ||
| // circular $ref has been already detected in another explored cycle | ||
| foundCycle = true | ||
| return | ||
| } | ||
| foundCycle = swag.ContainsStringsCI(parentRefs, normalizedRef) | ||
| if foundCycle { | ||
| r.context.circulars[normalizedRef] = true | ||
| } | ||
| return | ||
| } | ||
|
|
||
| func expandSchema(target Schema, parentRefs []string, resolver *schemaLoader, basePath string) (*Schema, error) { | ||
|
|
@@ -666,12 +697,14 @@ func expandSchema(target Schema, parentRefs []string, resolver *schemaLoader, ba | |
|
|
||
| /* this means there is a circle in the recursion tree */ | ||
| /* return the Ref */ | ||
| if isCircular(normalizedRef, basePath, parentRefs...) { | ||
| target.Ref = *normalizedRef | ||
| if resolver.isCircular(normalizedRef, basePath, parentRefs...) { | ||
| debugLog("shortcut circular ref") | ||
| // circular refs cannot be expanded. We leave them as ref | ||
| target.Ref = *denormalizeFileRef(normalizedRef, basePath) | ||
| return &target, nil | ||
| } | ||
|
|
||
| debugLog("\nbasePath: %s", basePath) | ||
| debugLog("basePath: %s", basePath) | ||
| if Debug { | ||
| b, _ := json.Marshal(target) | ||
| debugLog("calling Resolve with target: %s", string(b)) | ||
|
|
@@ -687,7 +720,6 @@ func expandSchema(target Schema, parentRefs []string, resolver *schemaLoader, ba | |
| if shouldStopOnError(err, resolver.options) { | ||
| return nil, err | ||
| } | ||
|
|
||
| return expandSchema(*t, parentRefs, resolver, normalizedBasePath) | ||
| } | ||
| } | ||
|
|
@@ -797,7 +829,7 @@ func derefPathItem(pathItem *PathItem, parentRefs []string, resolver *schemaLoad | |
| normalizedRef := normalizeFileRef(&pathItem.Ref, basePath) | ||
| normalizedBasePath := normalizedRef.RemoteURI() | ||
|
|
||
| if isCircular(normalizedRef, basePath, parentRefs...) { | ||
| if resolver.isCircular(normalizedRef, basePath, parentRefs...) { | ||
| return nil | ||
| } | ||
|
|
||
|
|
@@ -904,7 +936,7 @@ func transitiveResolver(basePath string, ref Ref, resolver *schemaLoader) (*sche | |
| rootURL.Fragment = "" | ||
| root, _ := resolver.cache.Get(rootURL.String()) | ||
| var err error | ||
| resolver, err = defaultSchemaLoader(root, resolver.options, resolver.cache) | ||
| resolver, err = defaultSchemaLoader(root, resolver.options, resolver.cache, resolver.context) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
@@ -920,7 +952,7 @@ func ExpandResponse(response *Response, basePath string) error { | |
| opts := &ExpandOptions{ | ||
| RelativeBase: basePath, | ||
| } | ||
| resolver, err := defaultSchemaLoader(nil, opts, nil) | ||
| resolver, err := defaultSchemaLoader(nil, opts, nil, nil) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -935,7 +967,7 @@ func derefResponse(response *Response, parentRefs []string, resolver *schemaLoad | |
| normalizedRef := normalizeFileRef(&response.Ref, basePath) | ||
| normalizedBasePath := normalizedRef.RemoteURI() | ||
|
|
||
| if isCircular(normalizedRef, basePath, parentRefs...) { | ||
| if resolver.isCircular(normalizedRef, basePath, parentRefs...) { | ||
| return nil | ||
| } | ||
|
|
||
|
|
@@ -990,7 +1022,7 @@ func ExpandParameter(parameter *Parameter, basePath string) error { | |
| opts := &ExpandOptions{ | ||
| RelativeBase: basePath, | ||
| } | ||
| resolver, err := defaultSchemaLoader(nil, opts, nil) | ||
| resolver, err := defaultSchemaLoader(nil, opts, nil, nil) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -1004,7 +1036,7 @@ func derefParameter(parameter *Parameter, parentRefs []string, resolver *schemaL | |
| normalizedRef := normalizeFileRef(¶meter.Ref, basePath) | ||
| normalizedBasePath := normalizedRef.RemoteURI() | ||
|
|
||
| if isCircular(normalizedRef, basePath, parentRefs...) { | ||
| if resolver.isCircular(normalizedRef, basePath, parentRefs...) { | ||
| return nil | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defaultSchemaLoader - result 1 (error) is always nil