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

Stringification of response status codes leads to invalid swagger #426

Closed
DStape opened this issue Apr 8, 2019 · 5 comments

Comments

@DStape
Copy link
Contributor

commented Apr 8, 2019

Recent merge of #424 results in our swagger documentation being generated with the following response definitions:

"responses": {
          "HTTPStatus.OK": {

Previously:

"responses": {
          "200": {

i.e. usage of https://docs.python.org/3/library/http.html#http-status-codes results in the generation of invalid swagger.

Potential fix (get_ref() in /src/apispec/core.py):

for code, response in iteritems(operation["responses"]):
    try:
        status_code = int(code)
    except TypeError, ValueError:
        # log error and skip
     responses[str(status_code)] = get_ref("response", response, openapi_major_version)

I'm happy to open a PR for this, but would appreciate any feedback on the approach laid out above (cast the code to an int before casting to str).

Thanks,
David

@sloria

This comment has been minimized.

Copy link
Member

commented Apr 8, 2019

I think the try-except approach makes sense. @lafrech ?

@lafrech

This comment has been minimized.

Copy link
Member

commented Apr 8, 2019

I'm not a big fan of the try-except approach where we try to catch all kinds of errors, potentially hiding input errors. And I wouldn't log an error since this code raises on '4XX', for instance, which is valid as per #422.

We only ever tested/supported status codes as string or integers. AFAIU, HTTPStatus worked because it's a subclass of IntEnum and it would serialize as integer when json dumped.

Since HTTPStatus is part of Python core and is totally relevant here, it would be nice to support it and I think we can do it explicitly:

            for code, response in iteritems(operation["responses"]):
                if not PY2:
                    from http import HTTPStatus
                    if isinstance(code, HTTPStatus):
                        code = code.value
                responses[str(code)] = get_ref(
                    "response", response, openapi_major_version
                )
            operation["responses"] = responses
@DStape

This comment has been minimized.

Copy link
Contributor Author

commented Apr 16, 2019

Will take your comments on board and will have a PR out soon.

DStape added a commit to DStape/apispec that referenced this issue Apr 23, 2019

Fix stringification of response codes
OpenAPI 2 states that the available status codes are described by RFC
7231. The RFC states that the, "status-code element is a three-digit integer
code", therefore we interpret that as OpenAPI 2 does not allow
non-integer characters to be present in status codes.

OpenAPI 3 on the other hand is a lot less ambiguous and states that the
status code field "MAY contain the uppercase wild character X",
therefore meaning non-integer characters are valid (e.g. '2XX').

Also add support for parsing Python 3's in-built http.HTTPStatus.

Addresses issue marshmallow-code#426.
@DStape

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2019

PR #436

DStape added a commit to DStape/apispec that referenced this issue Apr 23, 2019

Fix stringification of response codes
OpenAPI 2 states that the available status codes are described by RFC
7231. The RFC states that the, "status-code element is a three-digit integer
code", therefore we interpret that as OpenAPI 2 does not allow
non-integer characters to be present in status codes.

OpenAPI 3 on the other hand is a lot less ambiguous and states that the
status code field "MAY contain the uppercase wild character X",
therefore meaning non-integer characters are valid (e.g. '2XX').

Also add support for parsing Python 3's in-built http.HTTPStatus.

Addresses issue marshmallow-code#426.

DStape added a commit to DStape/apispec that referenced this issue Apr 23, 2019

Fix stringification of response codes
OpenAPI 2 states that the available status codes are described by RFC
7231. The RFC states that the, "status-code element is a three-digit integer
code", therefore we interpret that as OpenAPI 2 does not allow
non-integer characters to be present in status codes.

OpenAPI 3 on the other hand is a lot less ambiguous and states that the
status code field "MAY contain the uppercase wild character X",
therefore meaning non-integer characters are valid (e.g. '2XX').

Also add support for parsing Python 3's in-built http.HTTPStatus.

Addresses issue marshmallow-code#426.

DStape added a commit to DStape/apispec that referenced this issue Apr 23, 2019

Fix stringification of response codes
OpenAPI 2 states that the available status codes are described by RFC
7231. The RFC states that the, "status-code element is a three-digit integer
code", therefore we interpret that as OpenAPI 2 does not allow
non-integer characters to be present in status codes.

OpenAPI 3 on the other hand is a lot less ambiguous and states that the
status code field "MAY contain the uppercase wild character X",
therefore meaning non-integer characters are valid (e.g. '2XX').

Also add support for parsing Python 3's in-built http.HTTPStatus.

Addresses issue marshmallow-code#426.

DStape added a commit to DStape/apispec that referenced this issue Apr 24, 2019

Fix stringification of response codes
OpenAPI 2 states that the available status codes are described by RFC
7231. The RFC states that the, "status-code element is a three-digit integer
code", therefore we interpret that as OpenAPI 2 does not allow
non-integer characters to be present in status codes.

OpenAPI 3 on the other hand is a lot less ambiguous and states that the
status code field "MAY contain the uppercase wild character X",
therefore meaning non-integer characters are valid (e.g. '2XX').

Also add support for parsing Python 3's in-built http.HTTPStatus.

Addresses issue marshmallow-code#426.

DStape added a commit to DStape/apispec that referenced this issue Apr 24, 2019

Fix stringification of response codes
OpenAPI 2 states that the available status codes are described by RFC
7231. The RFC states that the, "status-code element is a three-digit integer
code", therefore we interpret that as OpenAPI 2 does not allow
non-integer characters to be present in status codes.

OpenAPI 3 on the other hand is a lot less ambiguous and states that the
status code field "MAY contain the uppercase wild character X",
therefore meaning non-integer characters are valid (e.g. '2XX').

Also add support for parsing Python 3's in-built http.HTTPStatus.

Addresses issue marshmallow-code#426.
@sloria

This comment has been minimized.

Copy link
Member

commented Apr 30, 2019

The fix is released in 1.3.1.

@sloria sloria closed this Apr 30, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.