Fix body validator different error codes for bad Content-Types.

[positron96]

Fixes #628 .

Changes proposed in this pull request:

• Added a test for this bug.
• Fixed it by checking for non-empty HTTP POST payload by considering request.body, request.form and request.files (only request.body was checked)

[coveralls]

Coverage remained the same at 100.0% when pulling 85f1f7d on positron96:fix_bodyvalidator_formdata_refed into ccd62f8 on zalando:master.

[dtkav]

nice, I noticed this too when working OpenAPI3 support.
Since this may break existing clients, it might make sense to stage it for the 2.0 release.
Just my 2c though.

[positron96]

@dtkav That's not a big bug and not a big fix actually, it only changes 400 error to 415.

[dtkav]

my bad @positron96 , I think i am confusing it with a different issue where connexion accepts form data even if the mimetype is wrong.

[positron96]

That sounds serious. Is there a description of the bug?

…

On Jul 20, 2018 09:24, "Daniel Grossmann-Kavanagh" ***@***.***> wrote: my bad @positron96 <https://github.com/positron96> , I think i am confusing it with a different issue where connexion accepts form data even if the mimetype is wrong. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#629 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB_4rlEM75YfLRbTMhSAtrKBQ1SNXdbnks5uIT96gaJpZM4VUmBk> .

[dtkav]

I dug into this a little bit to refresh my memory. I couldn't remember if it was a problem with my branch, or also on master.
It turns out that form parameters won't be rejected based on the "consumes" part of the operation.
For example, the spec below still accepts the form parameters, even though consumes = ["application/json"].

  /test-formData-param:
    post:
      summary: Test formData parameter
      operationId: fakeapi.hello.test_formdata_param
      consumes:
        - application/json  # connexion assumes application/json if this is blank
      # - application/x-www-form-urlencoded
      parameters:
        - name: formData
          type: string
          in: formData
          required: true
      responses:
        200:
          description: OK

The client still sends the correct content type alongside the form data, but the spec doesn't specify that the endpoint consumes form content types, and connexion isn't strict - it let's the form data through.

It's an oddity of the swagger 2.0 spec that you can specify in: formData separately from the consumes block. This is fixed in the 3.0 spec with the introduction of requestBody. The existing behavior may actually make sense for the 2.0 spec - I'm not sure. If you were to reject formData parameter if the consumes block didn't match, that should/would probably be in a spec validator. What do you think?

You can reproduce this by running:

cd tests
PYTHONPATH=$PWD connexion run fixtures/simple/swagger.yaml -p 9090
# in another terminal...
curl -X POST -F "formData=somedata" http://localhost:9090/v1.0/test-formData-param

Either way, your change looks good to me!

[positron96]

@dtkav Yes, I've reproduced this behavior. It seems that Swagger 2.0 vagueness in this point caused it since it's not clear what to do in this case. Probably keeping formData is OK.

Regarding MIME types, #593 is more serious for me, I'll look into it sometime.

[jmcs]

I think not(request.body or request.form or request.files) would be more readable.

[positron96]

yes, it definitely will

[jmcs]

@positron96 I agree with @dtkav this should target the dev-2.0 branch (and be included in the changes in the README).

[dtkav]

@jmcs It turns out I was conflating this change with some other behavior (elaborated on above).
FWIW I now think this change is relatively benign.

[positron96]

BTW, I've changed the code as requested.

[jmcs]

👍

[jmcs]

Thanks for the PR.