Skip to content

Swagger.Next: Forbid properties other than $ref inside JSON Pointer #629

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

Closed
IvanGoncharov opened this issue Apr 7, 2016 · 12 comments
Closed

Comments

@IvanGoncharov
Copy link
Contributor

From JSON Reference:

Any members other than "$ref" in a JSON Reference object SHALL be ignored.

Implementations MAY choose to replace the reference with the referenced value.

So technically JSON Reference doesn't forbid this behaviour, but it only makes the situation worse.
Since people expect different behaviour and it split lib/tools into two camps.
Ones who replace the reference and one who iterates through $refs.

@whitlockjc I believe you have some background on this issue.

@IvanGoncharov
Copy link
Contributor Author

+ it's really easy to implement such check in JSON Schema just add additionalProperties: false

@whitlockjc
Copy link
Member

👍

@webron
Copy link
Member

webron commented Apr 7, 2016

This is pretty much the opposite of #556 and honestly I'd rather see #556 implemented than this, for the value it provides. I mean, both this and #556 don't follow the JSON Reference spec as it is (which is fine by me), but if we break away, why not make it more useful?

@webron
Copy link
Member

webron commented Apr 7, 2016

Parent: #579

@IvanGoncharov
Copy link
Contributor Author

@webron This issue is really different from #556
This issue is compatible with JSON Pointer, OpenAPI is still could be resolved by tools intended for JSON Pointer. It purpose is to make validation sticker to a user from unexpected behavior.
Because properties that would be overridden is never an intended behaviour.

In any case, I think JSON Pointer compatibility is more important than this two issues put together.

@webron
Copy link
Member

webron commented Apr 12, 2016

This issue is incompatible with JSON Pointer because it adds a restriction that JSON Pointer doesn't have. If the JSON Pointer's spec writer wanted to disallow extra properties, they would have used different wording in the spec. I believe the wording there was intentional and we should not ignore it. And considering this feature wants to restrict JSON Pointer more than it is, and #556 calls for the opposite...

@fehguy fehguy removed the Meta Issue label Apr 14, 2016
@fehguy
Copy link
Contributor

fehguy commented Apr 14, 2016

I'm removing the meta issue from this--it's really part of our larger discussion about JSON schema support.

@webron
Copy link
Member

webron commented Apr 14, 2016

Yeah, the Meta Issue tag was by mistake, meant to add the openapi.next.

@webron
Copy link
Member

webron commented Jul 21, 2016

Tackling PR: #741

@webron
Copy link
Member

webron commented Feb 22, 2017

We've decided to follow the JSON Reference spec for our own Reference Object, and as such, allow additional properties, but saying they will be ignore per the spec.

@webron webron closed this as completed Feb 22, 2017
@handrews
Copy link
Member

@webron Just an FYI because it's easy to overlook, draft-wright-json-schema-00 absorbs JSON Reference but preserves that behavior with respect to additional properties. It also restricts "$ref" to places where a schema is expected, but there's no reason you can't allow it elsewhere since your goal is not a strict implementation of application/schema+json

https://tools.ietf.org/html/draft-wright-json-schema-00#section-7

@webron
Copy link
Member

webron commented Feb 22, 2017

@handrews thanks for the pointer. We noticed that and explicitly mention in our spec that $ref anywhere in our spec (including Schema Objects) is our own Reference Object with follows the JSON Reference (draft) spec. So the absorption of JSON Reference and the added restrictions do not affect the OAS. This also takes away the discussion of Internal and External Reference support as we had the other day.

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

5 participants