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

Fix fail to parse body for open api schema defining multiple content … #1435

Closed
wants to merge 1 commit into from

Conversation

MarcoRizk
Copy link

…types

Fixes
When using an open api schema with multiple content type definition like:

requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/DownTimeLogCreate'
          application/x-www-form-urlencoded:
            schema:
              $ref: '#/components/schemas/DownTimeLogCreate'
          multipart/form-data:
            schema:
              $ref: '#/components/schemas/DownTimeLogCreate'
        required: true

sending a request with a json body causes the following error:

  File "/home/marco/Projects/Synapse/azka_vision_mock_server/venv/lib/python3.6/site-packages/connexion/decorators/parameter.py", line 97, in wrapper
    request.files, arguments, has_kwargs, sanitize)
  File "/home/marco/Projects/Synapse/azka_vision_mock_server/venv/lib/python3.6/site-packages/connexion/operations/abstract.py", line 282, in get_arguments
    has_kwargs, sanitize))
  File "/home/marco/Projects/Synapse/azka_vision_mock_server/venv/lib/python3.6/site-packages/connexion/operations/openapi.py", line 296, in _get_body_argument
    body_arg.update(body or {})
TypeError: cannot convert dictionary update sequence element #0 to a sequence

Changes proposed in this pull request:

  • I am proposing that the request body parsing also considers the CONTENT-TYPE header sent with the request not just the open api schema definition

@RobbeSneyders
Copy link
Member

Hi @MarcoRizk, thanks for the PR.

There is currently indeed a limitation in Connexion when using multiple content types, but I don't think this is sufficient to solve it. I agree that we can use the headers, but they should be used to select the correct definition in the API contract, both for parameter parsing and validation.

@MarcoRizk
Copy link
Author

Hi @MarcoRizk, thanks for the PR.

There is currently indeed a limitation in Connexion when using multiple content types, but I don't think this is sufficient to solve it. I agree that we can use the headers, but they should be used to select the correct definition in the API contract, both for parameter parsing and validation.

I see your point, can i get some guidance on the changes need to be made ? I would love to help get this fixed.

@ioggstream
Copy link
Contributor

Hi @RobbeSneyders ! I'm willing to contribute on that too!

@gfleischer-meta
Copy link

hey any news on this PR
@MarcoRizk did u manage to fix this issue?

@RobbeSneyders
Copy link
Member

Fixed in #1588

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

Successfully merging this pull request may close these issues.

4 participants