manchenkoff / openapi3-parser

OpenAPI 3 parser to use a specification inside of the code in your projects
https://pypi.org/project/openapi3-parser/
MIT License
61 stars 33 forks source link

The parser crashes if no schema type is specified in the OpenAPI document #61

Closed sergei-maertens closed 1 year ago

sergei-maertens commented 1 year ago

Describe the bug

The OpenAPI 3 specification prescribes that when no type is specified for a schema, this implicitly is equivalent to anyOf with all available types (see https://swagger.io/docs/specification/data-models/data-types/#any).

To Reproduce Steps to reproduce the behavior:

  1. Create an OpenAPI document with a schema with the following definition:
    SomeSchema:
      description: Can be any value
  2. Try to parse this schema
  3. Observe a KeyError due to the missing type key

Expected behavior

The parser does not crash and correctly infers the AnyOf data type.

System details (please complete the following information):

Additional context

Mostly reporting the findings of https://github.com/maykinmedia/openapi3-parser/pull/1/ to upstream.

manchenkoff commented 1 year ago

Hey @sergei-maertens, thanks for the contribution, but tbh I'm not sure that it's correct behavior even if it's posted on Swagger portal.

Swagger is not actual specification which was considered during development, here (official specs) is an example says that type has to be a string in Schema object.

Maybe I missed that part somewhere there, so if you can find it, then it would make sense to consider your suggestion.

sergei-maertens commented 1 year ago

Looking at https://spec.openapis.org/oas/v3.0.3#schema-object (the same as your github link), it indeed states that type must be a string, but it doesn't specify that type is a required field/key/property (looking at allOf below that there is similar phrasing, but this property is a bit easier to reason about that there's no guarantee it's present).

The properties are taken from (and adjusted) the JSON Schema definition, so looking at that specification (https://datatracker.ietf.org/doc/html/draft-wright-json-schema-validation-00) I can't find any rule saying that the validation keyword type must be present. More links on https://json-schema.org/specification-links.html

We are running into this with a particular schema (https://app.swaggerhub.com/apis/DH-Sandbox/handelsregister/1.3.0). Validating this with Spectral reports some errors but they are only related to the examples provided:

➜  validation ./node_modules/.bin/spectral lint /tmp/spec.yaml                                                                           
OpenAPI 3.x detected

/tmp/spec.yaml
 389:33  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.MaatschappelijkeActiviteitRaadplegen.allOf[1].properties.manifesteertZichAls
 391:31  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.MaatschappelijkeActiviteitRaadplegen.allOf[1].properties.wordtGeleidVanuit
 395:21  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.MaatschappelijkeActiviteitRaadplegen.allOf[1].properties.wordtUitgeoefendIn.items
 471:17  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.Onderneming.properties.wordtUitgeoefendIn.items
 596:27  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.NietNatuurlijkPersoonLijst.allOf[1].properties.bezoekLocatie
 610:21  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.NietNatuurlijkPersoonCollection.properties._embedded.properties.nietnatuurlijkpersonen.items
 624:27  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.NietNatuurlijkPersoon.allOf[1].properties.bezoekLocatie
 632:11  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.NietNatuurlijkPersoonRaadplegen.allOf[0]
 718:20  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.AdresBasis.properties.huisnummer.example
 747:11  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.NNPAdresRaadplegen.allOf[0]
 759:11  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.PostadresRaadplegen.allOf[0]
 779:11  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.AdresMetRegistratie.allOf[0]
 789:11  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.PostadresMetRegistratie.allOf[0]
 807:21  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.VestigingCollection.properties._embedded.properties.vestigingen.items
 832:27  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.VestigingLijst.allOf[1].properties.bezoekLocatie
 834:25  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.VestigingLijst.allOf[1].properties.postLocatie
 841:27  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.VestigingBeperkt.allOf[1].properties.bezoekLocatie
 843:25  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.VestigingBeperkt.allOf[1].properties.postLocatie
 859:27  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.VestigingRaadplegen.allOf[1].properties.bezoekLocatie
 861:25  error  oas3-valid-schema-example  `example` property type should be integer  components.schemas.VestigingRaadplegen.allOf[1].properties.postLocatie

✖ 20 problems (20 errors, 0 warnings, 0 infos, 0 hints)

One crashing example seems to be https://app.swaggerhub.com/apis/DH-Sandbox/handelsregister/1.3.0#/Geboorte - however looking at the rest of the schema I do believe they should include a type: object here, otherwise specifying properties seems to make little sense to me?

sergei-maertens commented 1 year ago

Discussed this a little bit with a colleague - and we both believe that the type key is indeed optional according to the spec, and Swagger.io just provides a bit more help/examples on how to interpret this situation. Having an independent validator tool not trip over the missing type key seems to confirm this.

The particular example we are running into does not seem to deliberately using this "feature", so we will report that to the appropriate channels, but that does not make this bug report invalid in our opinion.

edit: I do appreciate how quickly you are getting back to us on this issue!

manchenkoff commented 1 year ago

I do partially agree with your statement, it seems that there are no strict rules about type key, although JSON specification contains instance description which allows several types:

Anyway, this change leads to less strict models as a result, but since several validators allow it, I do not see any reason to be against it 😊

I'll try to take a look at your PR ASAP

sergei-maertens commented 1 year ago

Awesome - I find it a bit of a shame that the specification leads to less strict models, hopefully this is something they can tighten in future versions.

My colleague who authored the fix is off for a couple of days, so there's no massive urgency here (we've just begun a new sprint), and any feedback can be processed on our side if your time is scarce or better used elsewhere :wink: