Skip to content

Commit

Permalink
Hang when expanding circular $ref
Browse files Browse the repository at this point in the history
* Fixes #76 (absolute path left behind in resolved circular $ref)
* Fixes #74 (isCircular no more checks on basePath!="")
* Contributes #75 (hang is solved, but the (always correct) expanded result may differ from one run to another)
* Contributes go-swagger/go-swagger#957 (hang on circular spec)
  • Loading branch information
fredbi committed Jul 7, 2018
1 parent 16284f9 commit 5e6ee98
Show file tree
Hide file tree
Showing 7 changed files with 4,364 additions and 54 deletions.
47 changes: 47 additions & 0 deletions debug.go
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...))
}
}
59 changes: 59 additions & 0 deletions debug_test.go
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")
}
100 changes: 66 additions & 34 deletions expander.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -202,6 +212,7 @@ type schemaLoader struct {
root interface{}
options *ExpandOptions
cache ResolutionCache
context *resolverContext
loadDoc func(string) (json.RawMessage, error)
}

Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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))
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}

Expand Down Expand Up @@ -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
}
Expand All @@ -1004,7 +1036,7 @@ func derefParameter(parameter *Parameter, parentRefs []string, resolver *schemaL
normalizedRef := normalizeFileRef(&parameter.Ref, basePath)
normalizedBasePath := normalizedRef.RemoteURI()

if isCircular(normalizedRef, basePath, parentRefs...) {
if resolver.isCircular(normalizedRef, basePath, parentRefs...) {
return nil
}

Expand Down
Loading

0 comments on commit 5e6ee98

Please sign in to comment.