Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail validation for extra data #45

Open
zupo opened this issue Oct 24, 2019 · 3 comments
Open

Fail validation for extra data #45

zupo opened this issue Oct 24, 2019 · 3 comments

Comments

@zupo
Copy link
Collaborator

zupo commented Oct 24, 2019

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
Copy link
Collaborator Author

zupo commented May 4, 2020

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

@MatthewWilkes
Copy link
Contributor

MatthewWilkes commented Jun 16, 2020

   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.

@zupo
Copy link
Collaborator Author

zupo commented Jun 16, 2020

  1. sounds best to me.

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

No branches or pull requests

2 participants