swagger-api / swagger-parser

Swagger Spec to Java POJOs
http://swagger.io
Apache License 2.0
777 stars 525 forks source link

Swagger Parser doesn't handle relative file $refs in paths according to RFC3986 #1459

Closed feliperuiz closed 1 year ago

feliperuiz commented 3 years ago

When traversing $refs to local files, Swagger Parser breaks at the first level of indirection due to not properly adjusting the base path of subsequent $refs. An example might better illustrate it, so let's take the following tree:

├─ openapi
│  ├─ entrypoint.yaml
│  └─ common-params.yaml
│
└─ module-1
   └─ openapi
      ├─ paths
      │  └─ path-1.yaml
      │
      └─ schemas
         └─ schema-1.yaml

We also have the following $refs:

Spec file Has relative $ref to
openapi/entrypoint.yaml ../module-1/openapi/paths/path-1.yaml#/paths/~1path-1
module-1/openapi/path-1.yaml ../../../openapi/common-params.yaml#/components/parameters/param-1
../schemas/schema-1.yaml#/components/schemas/schema-1

Trying to parse this with the following snippet

var options = new ParseOptions();
options.setResolve(true);

var result = new OpenAPIParser().readLocation("openapi/entrypoint.yaml", null, options);

results in the error message below

Unable to load RELATIVE ref: ../../../openapi/common-params.yaml

I've done some investigation, it seems that once the parser resolves the $ref to ../module-1/openapi/paths/path-1.yaml#/paths/~1path-1, all of its contents are "dumped" to the result object being built, as if they came from the original entrypoint.yaml file. Local $refs within it are correctly updated to account for this change, but relative ones are not, meaning the parser will try to look for ../../../openapi/common-params.yaml starting from openapi/entrypoint.yaml. It could either look for ../../../openapi/common-params.yaml starting from module-1/openapi/paths/ or adjust the $ref to account for using the wrong base path.

I have a local patch in place that fixes this, but it could be a breaking change for anyone that's relying on Swagger Parser's wrong behavior, so I decided to bring it up for discussion before submitting a pull request.

BabisK commented 3 years ago

Have you uploaded your local patch somewhere? I would like to test it. I can't find it in your fork.

johanra commented 3 years ago

I have submitted PR#1557 which fixes this issue, the PR includes an additional testcase showing the problem My fix doesn't seem to break any existing testcases.

SimeonGerginov commented 3 years ago

It seems that this fix introduced a bug in the OpenAPI Generator.

hauner commented 2 years ago

This also breaks openapi-processor.

I have a test-case with $ref-chains (https://github.com/openapi-processor/openapi-processor-core/blob/master/src/testInt/resources/tests/ref-chain-spring-124.1/inputs/openapi.yaml) (just a link because it is a bit complicated to replicate here) that fails when updating from 2.0.28 to 2.0.29.

What I observe is that the names of the $referenced schemas in the components.schema map changed.

components.schema names with 2.0.28

User
UserPage
user-page
UserSearch
user-search
Pageable
page
user-content

components.schema names with 2.0.29

User
user.model.v1
UserPage
user-page.model.v1
UserSearch
user-search.model.v1
Pageable
pageable.model.v1

With 2.0.28 when the code is traversing the $refs, at some point it will get the $ref #/components/schemas/page which is available in components.schema, i.e. page after stripping the path.

With 2.0.29 it gets the $ref models/page.model.v1.yml instead and is unable to find it in components.schema.

This is probably the same issue why the OpenAPI Generator fails.