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

2.0: stricter validation of additionalProperties #477

Conversation

dolmen
Copy link

@dolmen dolmen commented Oct 9, 2015

Following clarification of additionalProperties for a schema by @webron:

  • we now accept only false or a schema for additionalProperties: true is now rejected.
  • false is the new default for additionalProperties (instead of {} which means "anything is accepted" in JSON Schema).

Following clarification of 'additionalProperties' for a schema by
@webron:
swagger-api/swagger-codegen#1318 (comment)

We now accept only `false` or a schema for `additionalProperties`.
`false` is the new default for `additionalProperties` (instead of `{}`
which means "anything is accepted" in JSON Schema).
@dolmen
Copy link
Author

dolmen commented Oct 9, 2015

After more thinking about @webron comment I think I got it wrong and my patch is incorrect (in particular because my remark about {} in JSON Schema is irrelevant).

I was confused because of default: {} in both the JSON Schema of JSON Schema and the one of Swagger 2.0. JSON schemas are open by default (default: {} for additionalProperties which means any additional property of any type is accepted)), while Swagger schemas are closed by default and can only be extended by adding properties matching a single model. So the semantic of additionalProperties is different in the two spec, which implies the default value {} (which is the same in both schemas) has a different meaning.

So we should only remove the boolean type of value for additionalProperties and not change the default.

@webron
Copy link
Member

webron commented Oct 9, 2015

@dolmen - yeah, your second assertion is probably the better one.

@dolmen
Copy link
Author

dolmen commented Oct 27, 2015

Closing as this must not be merged as is.

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