snok/drf-openapi-tester

Incorrect validation for additionalProperties?

Closed this issue ยท 3 comments

Hello!

According to https://swagger.io/docs/specification/data-models/dictionaries/, you can specify a dictionary that contains string values like this:

type: object
additionalProperties:
  type: string

If the dictionary can contain any type of value, you would specify it as either of these two options:

type: object
additionalProperties: true
type: object
additionalProperties: {}

Currently, drf-openapi-tester only considers valid the additionalProperties: true option because of this line of code that only considers extra keys valid if additionalProperties is true.

When additionalProperties is a dictionary, the code is checking whether the keys are defined inside additionalProperties, but I don't think this is correct. I think the correct validation would be: if additionalProperties is a dictionary, validate that the values for any keys that are not defined in the properties section match the schema defined in additionalProperties. So for example, in the first example above we would just be validating that values are of type string. If the schema for additionalProperties is of type object, then we validate that the value matches that object specification (the value itself contains the keys specified in the properties section of the additionalProperties schema).

Let me try to show an example using an additionalProperties of type object:
For this schema:

type: object
properties:
    key_1:
        type: string
additionalProperties:
    type: object
    properties:
        key_2:
            type: string
        key_3:
            type: string

This data should be valid:

{
    'key_1': 'value_1',
    'some_extra_key': {
        'key_2': 'value_2',
        'key_3': 'value_3'
    }
}

But not this one because key_2 and key_3 are supposed to belong to the values of any extra keys, but not to the object itself. The validation error here should be that we were expecting an object for keys that are not key_1 but we got a string:

{
    'key_1': 'value_1',
    'key_2': 'value_2',
    'key_3': 'value_3'
}

And the error here is that it's missing key_1 which is specified in the schema:

{
    'some_extra_key': {
        'key_2': 'value_2',
        'key_3': 'value_3'
    }
}

I could try to fix this myself, but I would like confirmation that I'm not misunderstanding something and that what I describe should be the expected behaviour.

Thanks!

Hi there. Thanks for the detailed analysis. I think you are correct. We would very much appreciate a PR ๐Ÿ˜‰

Finally released v1.3.11 now with the fix. Thanks a lot for the report and fix @idesoto-rover!

@Goldziher and me also wanted to also ask if you, by chance, would be interested in helping us maintain the project? Neither one of us work with a tech stack where we get to use the project at the moment, so if you're interested we would be happy to include you ๐Ÿ‘ It wouldn't necessarily mean more than reviewing the occasional PR.

Yes, I'm ok with helping reviewing or testing PRs ๐Ÿ‘ I can't guarantee reacting quickly to PRs though, but I'll do my best ๐Ÿ˜ƒ