Skip to content
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

Tight loop due to circular reference between two yaml files #132

Closed
charlesx2000 opened this issue Oct 22, 2019 · 3 comments
Closed

Tight loop due to circular reference between two yaml files #132

charlesx2000 opened this issue Oct 22, 2019 · 3 comments

Comments

@charlesx2000
Copy link

Currently, if there are two yaml files, say petstore1.yaml and petstore2.yaml.
In petstore1.yaml, we defined schema Cat. In petstore2.yaml, we defined schema Dog.
But in petstore1.yaml, we have external reference to Dog which defined in petstore2.yaml, and in petstore2.yaml, we have external reference to Cat which defined in petstore1.yaml.

When we load the petstore1.yaml, the openapi3.loader(), it enters tight loop between:
https://github.com/getkin/kin-openapi/blob/master/openapi3/swagger_loader.go#L190
and https://github.com/getkin/kin-openapi/blob/master/openapi3/swagger_loader.go#L519

Due to that, we can not add circular detection in up layer because the initial loading of yaml does not return.

Any suggestion for fixing or workaround this?

@fenollp
Copy link
Collaborator

fenollp commented Oct 23, 2019

I'd look into

	swaggerLoader.visited = make(map[interface{}]struct{})
	swaggerLoader.visitedFiles = make(map[string]struct{})

Maybe interface{} keys hash in a surprising way?
My guess is that these two maps should really be only one in order to catch this kind of loops. I'd merge the both of them into a single map[string]struct{} where keys would be absolute JSON Paths to components.

@charlesx2000
Copy link
Author

I get the latest (not released) code, with the adding of visitedFiles, the tight loop is gone. I think make the swaggerLoader.visited with absolute JSON path is good so we can lower the granularity of circular detection to object level. But we still need to keep the the swaggerLoader.visitedFiles to prevent re-load the same yaml file.
With the latest code, there is a new issue, that is it does not resolve the object definition in referenced yaml file being brought in due to load the referenced file. The object referenced as #/components/schemas/Object, but that local reference is relative to the referenced file, not in the referencing file. Thus, it throws error "Failed to resolve .. in fragment". (This is separated issue).

@fenollp
Copy link
Collaborator

fenollp commented Apr 25, 2020

See if #207 (v0.6.0) solves your second issue.
Closing since initial issue is fixed.

@fenollp fenollp closed this as completed Apr 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants