pyramid_openapi3 icon indicating copy to clipboard operation
pyramid_openapi3 copied to clipboard

Fail validation for extra data

Open zupo opened this issue 5 years ago • 3 comments

Suppose your openapi.yaml defines two string fields, title & `description. Both are required.

  • Scenario A:

You send the following data to the API:

{
    'title': 'foo',
    'descriptioin': 'bar'
}

And the API will tell you that description field is required and you notice the descriptioin typo in your request.

  • Scenario B:

You change the openapi.yaml so that descriptioon is no longer a required field.

You send the following data to the API:

{
    'title': 'foo',
    'descriptioin': 'bar'
}

And the API will silently ignore descriptioin field, and since description is no longer required, will return a 200 OK. You move on and then notice after two weeks that you haven't saved any data 😱 😱 😱

zupo avatar Oct 24 '19 13:10 zupo

This one needs to be re-reproduced because openapi-core switched to using jsonschema under the bonnet.

zupo avatar May 04 '20 11:05 zupo

   A JSON Schema MAY contain properties which are not schema keywords.
   Unknown keywords SHOULD be ignored.

This makes sense to me, as you might want to push data across and not care about sanitising to match the specific fields an endpoint is interested in. That said, I totally agree that it's shady to fail to warn users about fields being ignored. Equally, we don't want to break valid requests that happen to contain extraneous keys right now, whenever people upgrade.

I can think of four broad approaches:

  1. Add a HTTP header to enable "strict" parsing, which will reject extraneous fields if set. This is the most compatible, but requires users to know to use it.
  2. Add a HTTP response header that includes warnings of unknown keys being used. This is very low impact, but it's also quite likely to be missed by users.
  3. Add an configuration option to always use strict parsing. This requires server administrators to have it enabled, so all users have to provide only valid fields. This is more likely to prevent errors due to careless users, as they don't have to know to enable strict mode, but it requires the website owner to make that decision.
  4. Enable the TRACE verb (there will be some question here over which verb is being traced), to allow people to verify the request body they're sending. This is a pretty close fit with the intention of TRACE from the HTTP spec, but I think it's the least useful overall, sadly.

MatthewWilkes avatar Jun 16 '20 14:06 MatthewWilkes

  1. sounds best to me.

zupo avatar Jun 16 '20 17:06 zupo