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

Traversal of large spec broken after #3380 #3385

Closed
am-burban opened this issue Feb 26, 2024 · 5 comments
Closed

Traversal of large spec broken after #3380 #3385

am-burban opened this issue Feb 26, 2024 · 5 comments

Comments

@am-burban
Copy link

am-burban commented Feb 26, 2024

Q&A (please complete the following information)

  • OS: WSL Ubuntu 20.04
  • Environment: Node.js 18.19.1
  • Method of installation: yarn
  • Swagger-Client version: 3.25.3
  • Swagger/OpenAPI version: OpenAPI 3.0

Content & configuration

Feeding a large spec into swagger-ui is broken after they updated from swagger-client v3.25.1 to 3.25.3. The reason seems to be this commit: a80000d

It just stops traversing the spec when there is a huge number of schemas defined (even though they are not even referenced). There are also no circular references in the spec.

Swagger/OpenAPI definition: https://gist.github.com/am-burban/cc40c1813fa9cb9846747133ae82f689

https://petstore.swagger.io/index.html?url=https://gist.githubusercontent.com/am-burban/cc40c1813fa9cb9846747133ae82f689/raw/72498092c5ce3e04a6e4c24e75f02d2359d57cef/Traversal%2520Bug%2520Repro.yaml#/

Expected behavior

Spec is traversed correctly.

Screenshots

image

When you remove the s988 schema, it renders the totally unrelated query param fine again.

image

@Openyoueyes
Copy link

I have almost the same problem. I stopped displaying half of the example serializer request bodies and just returns "string" in the example

@glowcloud
Copy link
Contributor

Hi everyone,

With introducing TRAVERSE_LIMIT in a80000d we were trying to fix complex schemas with allOf keyword and references crashing Swagger UI.
Because of how the resolver is built, it gets confused with more complex specifications, causing multiple issues like not seeing circular references that should have been detected. We were trying to stop those circular references repeating by limiting the traversal depth.

We are working on finding a different solution to this without limiting traversal. Right now we are looking at traversing the specification twice - first without the allOf plugin and then only with the allOf plugin for the specification with resolved references.

@am-burban
Copy link
Author

Hey @glowcloud ,

thanks for the update!
Without having any deeper knowledge about swagger-ui/swagger-client, limiting traversal depth sounds like a good idea. I think the main problem with the change is that it doesn't really limit depth, but just stops after traversing 1000 schemas. No matter if they are nested or even referenced anywhere. Wouldn't it be possible to actually limit the depth?

@char0n
Copy link
Member

char0n commented Mar 1, 2024

Addressed in #3399.

swagger-bot pushed a commit that referenced this issue Mar 1, 2024
## [3.25.4](v3.25.3...v3.25.4) (2024-03-01)

### Bug Fixes

* **specmap:** avoid traversal limit for non-complex huge definitions  ([#3399](#3399)) ([042af17](042af17)), closes [#3385](#3385)
@glowcloud
Copy link
Contributor

We decided to leave the limit, but only for the number of patches that we go through in one plugin run. Before this, we were also limiting the traversal of paths, which is why larger specifications, even without nested references, were being incorrectly resolved. We also raised the traversal limit to 3000 patches per plugin run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants