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

Updated to check that $ref is a string #314

Merged
merged 1 commit into from
Oct 3, 2016

Conversation

calcinai
Copy link
Contributor

This is an issue when processing the OpenAPI/Swagger spec, where $ref is an object.

Example on line 315 of the OAPI spec:

"$ref": {
      "type": "string"
},

From what I understand, the spec is a valid document, but beaks the current resolver logic.

@calcinai
Copy link
Contributor Author

calcinai commented Oct 3, 2016

@bighappyface - what do you think about this one?

@bighappyface
Copy link
Collaborator

@calcinai thanks for the contribution. To be clear, will this adjustment only allow refs that are strings in this library, or does the check pass through object refs to support the example you provided from OAPI?

@calcinai
Copy link
Contributor Author

calcinai commented Oct 3, 2016

From what I understood, $ref is always a string if it's actually a reference. The issue here is that the current logic eagerly treats any object key $ref as a reference and tries to follow it, which in this case will fail.

The OAPI issue is that in the document it's actually defining what $ref can be.

Apologies if I didn't answer your question, can you elaborate if not?

@calcinai
Copy link
Contributor Author

calcinai commented Oct 3, 2016

Just to add - I deliberately made the smallest change possible to correct this to try and have the smallest impact.

@bighappyface
Copy link
Collaborator

I think we are good here. +1

@bighappyface bighappyface merged commit fa407eb into jsonrainbow:master Oct 3, 2016
@calcinai calcinai deleted the patch-2 branch October 3, 2016 13:48
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

Successfully merging this pull request may close these issues.

2 participants