Skip to content
This repository was archived by the owner on Nov 8, 2024. It is now read-only.

Prevent treating overlapping paths as a circular reference #641

Merged
merged 2 commits into from
Jul 15, 2021

Conversation

kylef
Copy link
Member

@kylef kylef commented Jul 15, 2021

A check that a reference to #/definitions/User from within #/definitions/UserList would incorrectly been interpreted as a circular reference as a simplistic check that the current path starts with the current reference. We should ensure that the reference is either identical, or relative from another (such as with a trailing slash).

This was easiest to test at a unit level so I have added unit tests for this method. The top 4 added tests would already pass, just increasing test coverage to account for all the branches in the circular reference checker.

To aid review, the circular reference checker is passed all the references we've traversed (this is the first argument). If we would follow a series of references (such as if we are currently at #/items, and then we followed a reference to #/definitions/User which inside #/definitions/User/properties/comments/items referenced #/definitions/Comment. The current path would become #/definitions/Commentand the current traversed path would be something like['#/items', '#/definitions/User', '#/definitions/User/properties/comments/items']`. These parts of the current resolution are pushed and popped as we resolve a schema.

kylef added 2 commits July 15, 2021 16:59
A check that a reference to `#/definitions/User` from within
`#/definitions/UserList` would incorrectly been interpreted as a
circular reference as a simplistic check that the current path starts
with the current reference. We should ensure that the reference is
either identical, or relative from another (such as with a trailing
slash).
@kylef kylef added bug Something isn't working openapi2 labels Jul 15, 2021
@kylef kylef requested a review from opichals July 15, 2021 16:08
Copy link
Contributor

@opichals opichals left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM

@kylef kylef merged commit 1245a1d into master Jul 15, 2021
@kylef kylef deleted the kylef/oas2-circ branch July 15, 2021 16:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working openapi2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants