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

openapi: remove JSON body second validation and type casting #1170

Merged
merged 3 commits into from
Apr 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 20 additions & 18 deletions connexion/operations/openapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from connexion.operations.abstract import AbstractOperation

from ..decorators.uri_parsing import OpenAPIURIParser
from ..http_facts import FORM_CONTENT_TYPES
from ..utils import deep_get, deep_merge, is_null, is_nullable, make_type

logger = logging.getLogger("connexion.operations.openapi3")
Expand Down Expand Up @@ -286,13 +287,28 @@ def _get_body_argument(self, body, arguments, has_kwargs, sanitize):
'the requestBody instead.', DeprecationWarning)
x_body_name = sanitize(self.body_schema.get('x-body-name', 'body'))

if self.consumes[0] in FORM_CONTENT_TYPES:
result = self._get_body_argument_form(body)
else:
result = self._get_body_argument_json(body)

if x_body_name in arguments or has_kwargs:
return {x_body_name: result}
return {}

def _get_body_argument_json(self, body):
# if the body came in null, and the schema says it can be null, we decide
# to include no value for the body argument, rather than the default body
if is_nullable(self.body_schema) and is_null(body):
if x_body_name in arguments or has_kwargs:
return {x_body_name: None}
return {}
return None

if body is None:
default_body = self.body_schema.get('default', {})
return deepcopy(default_body)

return body

def _get_body_argument_form(self, body):
# now determine the actual value for the body (whether it came in or is default)
default_body = self.body_schema.get('default', {})
body_props = {k: {"schema": v} for k, v
Expand All @@ -302,25 +318,11 @@ def _get_body_argument(self, body, arguments, has_kwargs, sanitize):
# see: https://github.com/OAI/OpenAPI-Specification/blame/3.0.2/versions/3.0.2.md#L2305
additional_props = self.body_schema.get("additionalProperties", True)

if body is None:
body = deepcopy(default_body)

# if the body isn't even an object, then none of the concerns below matter
if self.body_schema.get("type") != "object":
if x_body_name in arguments or has_kwargs:
return {x_body_name: body}
return {}

# supply the initial defaults and convert all values to the proper types by schema
body_arg = deepcopy(default_body)
body_arg.update(body or {})

res = {}
if body_props or additional_props:
res = self._get_typed_body_values(body_arg, body_props, additional_props)

if x_body_name in arguments or has_kwargs:
return {x_body_name: res}
return self._get_typed_body_values(body_arg, body_props, additional_props)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this for form types and not json?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because for json, strings are already converted to appropriate python types via json.loads. But there's no such conversion for forms, only string splitting of arrays. So I have left forms to be processed the old way and focused on fixing json, which is simpler to fix and more important (oneOf is rare in form data).

In my opinion, proper solution would perform type conversions somehow via jsonschema validators, at one place, close to the start of the request handling pipeline. Currently, types are converted inconsistently here and there and there and there. But please, this was intended to be a quick simple bugfix MR, let's not dive into big refactorings here.

return {}

def _get_typed_body_values(self, body_arg, body_props, additional_props):
Expand Down
31 changes: 31 additions & 0 deletions tests/api/test_responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -388,3 +388,34 @@ def test_streaming_response(simple_app):
app_client = simple_app.app.test_client()
resp = app_client.get('/v1.0/get_streaming_response')
assert resp.status_code == 200


def test_oneof(simple_openapi_app):
app_client = simple_openapi_app.app.test_client()

post_greeting = app_client.post( # type: flask.Response
'/v1.0/oneof_greeting',
data=json.dumps({"name": 3}),
content_type="application/json"
)
assert post_greeting.status_code == 200
assert post_greeting.content_type == 'application/json'
greeting_reponse = json.loads(post_greeting.data.decode('utf-8', 'replace'))
assert greeting_reponse['greeting'] == 'Hello 3'

post_greeting = app_client.post( # type: flask.Response
'/v1.0/oneof_greeting',
data=json.dumps({"name": True}),
content_type="application/json"
)
assert post_greeting.status_code == 200
assert post_greeting.content_type == 'application/json'
greeting_reponse = json.loads(post_greeting.data.decode('utf-8', 'replace'))
assert greeting_reponse['greeting'] == 'Hello True'

post_greeting = app_client.post( # type: flask.Response
'/v1.0/oneof_greeting',
data=json.dumps({"name": "jsantos"}),
content_type="application/json"
)
assert post_greeting.status_code == 400
17 changes: 17 additions & 0 deletions tests/fixtures/simple/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1222,6 +1222,23 @@ paths:
schema:
type: string
format: binary
/oneof_greeting:
post:
operationId: fakeapi.hello.post_greeting3
requestBody:
content:
application/json:
schema:
type: object
properties:
name:
oneOf:
- {type: boolean}
- {type: number}
additionalProperties: false
responses:
'200':
description: Echo the validated request.

servers:
- url: http://localhost:{port}/{basePath}
Expand Down