Skip to content

oneOf + "additionalProperties: false" broken after 0.8 #124

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
diogobaeder opened this issue Mar 26, 2019 · 0 comments
Closed

oneOf + "additionalProperties: false" broken after 0.8 #124

diogobaeder opened this issue Mar 26, 2019 · 0 comments

Comments

@diogobaeder
Copy link

Hi,

I just updated openapi-core in a project I work on, from 0.8.0 to 0.9.0, and it broke a bunch of objects I had defined in my spec. The reason is, some of my objects are spec'd as having "superset" fields in a oneOf fashion - for example, one of the schemas defines properties foo and bar and the other only defines foo. Previously it worked because additionalProperties was set to None by default, hence not allowing additional properties by default, therefore those schemas were unambiguous. However, after a few recent changes (I think after 0.8.1) it seems that the library got updated to OpenAPI 3.0.2, which specifies that additionalProperties has to be true by default, and this precise change makes those kinds of schemas ambiguous if they don't define additionalProperties.

However, I tried defining additionalProperties: false in those schemas and it still didn't work, they're being deemed invalid. I checked the code that is on master now, and the reason why that doesn't work is because additionalProperties is being checked on the parent object that contains the oneOf items, and not on each item. So, for example, if oneOf the schemas allows only foo, but the value dict contains foo and bar, the validation passes because the container has additionalProperties: true by default (while this property should be checked in the granular schemas instead).

In summary: the recent changes make it impossible to use oneOf with schemas where one is a superset of another, even if they're unambiguous by being specified with additonalProperties: false. Something I haven't tried yet, though, is to set additionalProperties: false in the container with the oneOf, maybe this makes the validation pass, but it would make the specification wrong according to OpenAPI 3.

I'm trying to implement a fix for it now, but unfortunately they seem to have broken a lot of tests, so it will take me some time; if you have a fix in mind which is easy enough to implement, please let me know. The reason why I broke a bunch of tests (hence the production code) is because I noticed that the validation at _validate_object and _validate_properties is mixing properties from the oneOf items with the container itself, which doesn't seem correct to me, so I tried pushing the validation to each item instead but somehow this caused some mayhem.

diogobaeder pushed a commit to diogobaeder/openapi-core that referenced this issue Mar 26, 2019
@p1c2u p1c2u closed this as completed in b029066 Mar 26, 2019
p1c2u added a commit that referenced this issue Mar 26, 2019
Fix #124: Checking "additionalProperties" in "oneOf" items.
bjmc pushed a commit to bjmc/openapi-core that referenced this issue Jun 12, 2019
…ems.

This is important because it does the correct validation over items that
are restricted in "oneOf", so that it's possible to use schemas that are
superset of one another as items of "oneOf".
bjmc pushed a commit to bjmc/openapi-core that referenced this issue Jun 12, 2019
bjmc pushed a commit to bjmc/openapi-core that referenced this issue Jun 12, 2019
Fix python-openapi#124: Checking "additionalProperties" in "oneOf" items.
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

1 participant