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.

Sign up for GitHub

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

Jump to bottom

[BUG][PYTHON] Content-Type: application/json on GET(also HEAD,DELETE) requests #9831

Closed
5 of 6 tasks
johnboyes opened this issue Jun 22, 2021 · 0 comments · Fixed by #9852
Closed
5 of 6 tasks

[BUG][PYTHON] Content-Type: application/json on GET(also HEAD,DELETE) requests #9831

johnboyes opened this issue Jun 22, 2021 · 0 comments · Fixed by #9852
Labels
Client: Python Issue: Bug

Comments

@johnboyes
Copy link
Contributor

johnboyes commented Jun 22, 2021 •
edited
Loading

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

The same problem as #476, #1061 is also occurring in Python.

The Python generator makes the default Content-Type application/json for all HTTP methods (including GET, HEAD and DELETE), whereas there should never be a Content-Type set for GET, HEAD and DELETE.

This causes a problem with other tools/servers that insist that GET, HEAD and DELETE requests do not have a Content-Type (as per the OpenAPI 3 specification).

An example of the problem is when using Prism as a validation proxy.

Prism rejects any GET request that has a Content-Type.

Here is an example of the problem manifesting itself.

openapi-generator version

master 969cea8 (21 June, 2021) and latest release at time of filing this issue v5.1.1

Not a regression, this is a long-standing issue.

OpenAPI declaration file content or url

Any GET, HEAD or DELETE request generated from any OpenAPI spec via the Python generator.

Generation Details

Standard Python config

Steps to reproduce
  1. Start with any OpenAPI3 spec e.g. the Petstore example at https://editor.swagger.io/
  2. Generate Python client code for the spec using the standard Python generator
  3. Look at the generated rest.py e.g. in the standard sample in this repo and see that the Content-Type defaults to application/json for all HTTP methods (including GET, HEAD and DELETE), rather than there being no Content-Type for GET, HEAD and DELETE.
Related issues/PRs

#476, #1061, #5941, #6167

Suggest a fix

The fix looks straightforward and I am happy to submit a PR.

Proposed fix is to amend the rest.mustache so that the Content-Type is left unset for GET, HEAD and DELETE requests, e.g.

if method in ['POST', 'PUT', 'PATCH', 'OPTIONS']:
  if 'Content-Type' not in headers:
    headers['Content-Type'] = 'application/json'

instead of the existing

if 'Content-Type' not in headers:
    headers['Content-Type'] = 'application/json'
johnboyes added a commit to johnboyes/openapi-generator that referenced this issue Jun 27, 2021
@johnboyes
[BUG][PYTHON] Do not set Content-Type for GET, HEAD or DELETE requests
9dfe1f6 
The Python generator no longer sets a default `Content-Type` of
`application/json` for `GET`, `HEAD` and `DELETE` requests.

Having the `Content-Type` set for these requests was causing issues with
other tools which insist that GET, HEAD and DELETE requests do not have
a Content-Type (as per the OpenAPI 3 specification).

An example of the problem that this commit fixes is when using
[Prism][1] as a [validation proxy][2].

[Prism rejects any GET request that has a Content-Type][3].

Here is [an example of the problem manifesting itself][4].

To validate the fix in this commit:

1. Start with any OpenAPI3 spec e.g. the Petstore example at
https://editor.swagger.io/
2. Generate Python client code for the spec
3. Look at the generated `rest.py` e.g. in the [standard sample in this
repo][5] and see that the `Content-Type` defaults to `application/json`
for all HTTP methods (including `GET`, `HEAD` and `DELETE`), rather than
there being no `Content-Type` for `GET`, `HEAD` and `DELETE`.

Fixes OpenAPITools#9831

[1]: https://github.com/stoplightio/prism
[2]: https://meta.stoplight.io/docs/prism/docs/guides/03-validation-proxy.md
[3]: stoplightio/prism#1408 (comment)
[4]: https://github.com/agilepathway/gauge-openapi-example/pull/28/checks?check_run_id=2888606052#step:13:18
[5]: https://github.com/OpenAPITools/openapi-generator/blob/969cea8ce10cb9d012c3936fb377d631c0d044c9/samples/openapi3/client/petstore/python/petstore_api/rest.py#L141
@johnboyes johnboyes mentioned this issue Jun 27, 2021
Merged
5 tasks
wing328 added a commit that referenced this issue Jul 7, 2021
@johnboyes @wing328
[BUG][PYTHON] Do not set Content-Type for GET, HEAD or DELETE requests (
7084a79 
#9852)

* [BUG][PYTHON] Do not set Content-Type for GET, HEAD or DELETE requests

The Python generator no longer sets a default `Content-Type` of
`application/json` for `GET`, `HEAD` and `DELETE` requests.

Having the `Content-Type` set for these requests was causing issues with
other tools which insist that GET, HEAD and DELETE requests do not have
a Content-Type (as per the OpenAPI 3 specification).

An example of the problem that this commit fixes is when using
[Prism][1] as a [validation proxy][2].

[Prism rejects any GET request that has a Content-Type][3].

Here is [an example of the problem manifesting itself][4].

To validate the fix in this commit:

1. Start with any OpenAPI3 spec e.g. the Petstore example at
https://editor.swagger.io/
2. Generate Python client code for the spec
3. Look at the generated `rest.py` e.g. in the [standard sample in this
repo][5] and see that the `Content-Type` defaults to `application/json`
for all HTTP methods (including `GET`, `HEAD` and `DELETE`), rather than
there being no `Content-Type` for `GET`, `HEAD` and `DELETE`.

Fixes #9831

[1]: https://github.com/stoplightio/prism
[2]: https://meta.stoplight.io/docs/prism/docs/guides/03-validation-proxy.md
[3]: stoplightio/prism#1408 (comment)
[4]: https://github.com/agilepathway/gauge-openapi-example/pull/28/checks?check_run_id=2888606052#step:13:18
[5]: https://github.com/OpenAPITools/openapi-generator/blob/969cea8ce10cb9d012c3936fb377d631c0d044c9/samples/openapi3/client/petstore/python/petstore_api/rest.py#L141

* update samples

* Fix Python DELETE bug introduced in earlier commit

The earlier commit 9dfe1f6 introduced a bug for `DELETE` requests on the
standard Python generator.  This commit fixes that bug and also includes
the updated samples.

Co-authored-by: William Cheng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Client: Python Issue: Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[BUG][PYTHON] Do not set Content-Type for GET, HEAD or DELETE requests johnboyes/openapi-generator
2 participants
@johnboyes @spacether