Skip to content
This repository has been archived by the owner on Nov 19, 2023. It is now read-only.

Incorrect validation for additionalProperties? #245

Closed
idesoto-rover opened this issue Dec 16, 2021 · 3 comments
Closed

Incorrect validation for additionalProperties? #245

idesoto-rover opened this issue Dec 16, 2021 · 3 comments

Comments

@idesoto-rover
Copy link
Contributor

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!

@Goldziher
Copy link
Member

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

idesoto-rover added a commit to idesoto-rover/drf-openapi-tester that referenced this issue Dec 17, 2021
sondrelg added a commit that referenced this issue Dec 18, 2021
…validation

#245 fix validation for additionalProperties
@sondrelg
Copy link
Member

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.

@idesoto-rover
Copy link
Contributor Author

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 😃

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants