Skip to content

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

Closed
@diogobaeder

Description

@diogobaeder

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions