diff --git a/.gitignore b/.gitignore index 916e23e41..98e2f0821 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,4 @@ htmlcov/ .vscode/ venv/ src/ +venv/ diff --git a/connexion/__init__.py b/connexion/__init__.py index f88a7c10c..e77b1c643 100755 --- a/connexion/__init__.py +++ b/connexion/__init__.py @@ -45,4 +45,4 @@ def _required_lib(exc, *args, **kwargs): AioHttpApp = _aiohttp_not_installed_error # This version is replaced during release process. -__version__ = '2020.0.dev1' +__version__ = '2020.0.dev1-STAR-Informatics-Patched' diff --git a/connexion/apis/aiohttp_api.py b/connexion/apis/aiohttp_api.py index b7e94ad73..e774ee1ce 100644 --- a/connexion/apis/aiohttp_api.py +++ b/connexion/apis/aiohttp_api.py @@ -9,6 +9,7 @@ import aiohttp_jinja2 import jinja2 from aiohttp import web +from aiohttp.web_request import FileField from aiohttp.web_exceptions import HTTPNotFound, HTTPPermanentRedirect from aiohttp.web_middlewares import normalize_path_middleware from connexion.apis.abstract import AbstractAPI @@ -299,15 +300,55 @@ async def get_request(cls, req): :rtype: ConnexionRequest """ url = str(req.url) - logger.debug('Getting data and status code', - extra={'has_body': req.has_body, 'url': url}) + + logger.debug( + 'Getting data and status code', + extra={ + # has_body | can_read_body report if + # body has been read or not + # body_exists refers to underlying stream of data + 'body_exists': req.body_exists, + 'can_read_body': req.can_read_body, + 'content_type': req.content_type, + 'url': url, + }, + ) query = parse_qs(req.rel_url.query_string) headers = req.headers body = None - if req.body_exists: - body = await req.read() + # if request is not multipart, `post_data` will be empty dict + # and stream will not be consumed + post_data = await req.post() + + # set those up beforehand, they are needed anyway + files = {} + form = {} + + if post_data: + logger.debug('Reading multipart data from request') + for k, v in post_data.items(): + if isinstance(v, web.FileField): + if k in files: + # if multiple files arrive under the same name in the + # request, downstream requires that we put them all into + # a list under the same key in the files dict. + if isinstance(files[k], list): + files[k].append(v) + else: + files[k] = [files[k], v] + else: + files[k] = v + else: + # put normal fields as an array, that's how werkzeug does that for Flask + # and that's what Connexion expects in its processing functions + form[k] = [v] + body = b'' + else: + logger.debug('Reading data from request') + body = await req.read() + return ConnexionRequest(url=url, method=req.method.lower(), path_params=dict(req.match_info), @@ -315,7 +356,8 @@ async def get_request(cls, req): headers=headers, body=body, json_getter=lambda: cls.jsonifier.loads(body), - files={}, + form=form, + files=files, context=req) @classmethod diff --git a/connexion/decorators/validation.py b/connexion/decorators/validation.py index 307ffcf77..7452d47f1 100644 --- a/connexion/decorators/validation.py +++ b/connexion/decorators/validation.py @@ -2,6 +2,8 @@ import copy import functools import logging +from urllib.parse import parse_qs + import pkg_resources from jsonschema import Draft4Validator, ValidationError, draft4_format_checker @@ -23,7 +25,8 @@ 'integer': int, 'number': float, 'boolean': boolean, - 'object': dict + 'object': dict, + 'string': str } @@ -100,6 +103,20 @@ def validate_parameter_list(request_params, spec_params): return request_params.difference(spec_params) +def parse_body_parameters(body, encoding="utf-8"): + parsed_body = parse_qs(body.decode(encoding)) + # Flatten the parameters and take only the first value + params = dict() + for key, value in parsed_body.items(): + valen = len(value) + if valen: + if valen <= 1: + params[key] = value[0] # flatten single element lists + else: + params[key] = value # leave multi-valued lists intact + return params + + class RequestBodyValidator(object): def __init__(self, schema, consumes, api, is_null_value_valid=False, validator=None, @@ -159,7 +176,9 @@ def wrapper(request): if data is not None or not self.has_default: self.validate_schema(data, request.url) elif self.consumes[0] in FORM_CONTENT_TYPES: - data = dict(request.form.items()) or (request.body if len(request.body) > 0 else {}) + # TODO: parse_body_parameters() should probably be given an explicit encoding from the request + data = dict(request.form.items()) or \ + (parse_body_parameters(request.body) if len(request.body) > 0 else {}) data.update(dict.fromkeys(request.files, '')) # validator expects string.. logger.debug('%s validating schema...', request.url) diff --git a/connexion/operations/openapi.py b/connexion/operations/openapi.py index dae8aed58..8688f2bee 100644 --- a/connexion/operations/openapi.py +++ b/connexion/operations/openapi.py @@ -268,36 +268,79 @@ def body_definition(self): return {} def _get_body_argument(self, body, arguments, has_kwargs, sanitize): - x_body_name = sanitize(self.body_schema.get('x-body-name', 'body')) + if len(arguments) <= 0 and not has_kwargs: + return {} + + x_body_name_default = 'body' + x_body_name_explicit = True + x_body_name = sanitize(self.body_schema.get('x-body-name', '')) + if not x_body_name: + x_body_name_explicit = False + x_body_name = x_body_name_default + + # 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): - return {x_body_name: None} + if x_body_name in arguments or has_kwargs: + return {x_body_name: None} + return {} + # 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 + body_props = {sanitize(k): {"schema": v} for k, v in self.body_schema.get("properties", {}).items()} - # by OpenAPI specification `additionalProperties` defaults to `true` - # 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 {} - body_arg = deepcopy(default_body) - body_arg.update(body or {}) + # by OpenAPI specification `additionalProperties` defaults to `true` + # 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) - res = {} - if body_props or additional_props: - res = self._get_typed_body_values(body_arg, body_props, additional_props) + # supply the initial defaults and convert all values to the proper types by schema + x = deepcopy(default_body) + x.update(body or {}) + converted_body = self._get_typed_body_values(x, body_props, additional_props) + + # NOTE: + # Handlers could have a single argument to receive the whole body or they + # could have individual arguments to receive all the body's parameters, or + # they may have a **kwargs, arguments to receive anything. So, the question + # arises that if kwargs is given, do we pass to the handler a single body + # argument, or the broken out arguments, or both? + # + # #1 If 'x-body-arg' is explicitly given and it exists in [arguments], then the + # body, as a whole, will be passed to the handler with that name. STOP. + # + # #2 If kwargs is given, then we don't know what the handler cares about, so we + # pass the body as a whole as an argument named, 'body', along with the + # individual body properties. STOP. + # + # #3 Else, we pass the body's individual properties which exist in [arguments]. + # + # #4 Finally, if that resulting argument list is empty, then we include an argument + # named 'body' to the handler, but only if 'body' exists in [arguments] + + if x_body_name_explicit and x_body_name in arguments: #1 + return {x_body_name: converted_body} + + if has_kwargs: #2 + converted_body[x_body_name_default] = copy(converted_body) # copy just to avoid circular ref + return converted_body + + r = {k: converted_body[k] for k in converted_body if k in arguments} #3 + + if len(r) <= 0 and x_body_name_default in arguments: #4 + r[x_body_name_default] = converted_body + + return r - if x_body_name in arguments or has_kwargs: - return {x_body_name: res} - return {} def _get_typed_body_values(self, body_arg, body_props, additional_props): """ diff --git a/setup.py b/setup.py index d3295567f..42b13ebac 100755 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ def read_version(package): swagger_ui_require = 'swagger-ui-bundle>=0.0.2' flask_require = 'flask>=1.0.4' aiohttp_require = [ - 'aiohttp>=2.3.10', + 'aiohttp>=2.3.10,<4', 'aiohttp-jinja2>=0.14.0' ] diff --git a/tests/aiohttp/test_aiohttp_multipart.py b/tests/aiohttp/test_aiohttp_multipart.py new file mode 100644 index 000000000..07f68709b --- /dev/null +++ b/tests/aiohttp/test_aiohttp_multipart.py @@ -0,0 +1,118 @@ +import os + +import aiohttp +import pytest +from pathlib import Path + +from connexion import AioHttpApp + +try: + import ujson as json +except ImportError: + import json + + +@pytest.fixture +def aiohttp_app(aiohttp_api_spec_dir): + app = AioHttpApp(__name__, port=5001, + specification_dir=aiohttp_api_spec_dir, + debug=True) + app.add_api( + 'openapi_multipart.yaml', + validate_responses=True, + strict_validation=True, + pythonic_params=True, + pass_context_arg_name='request_ctx', + ) + return app + + +async def test_single_file_upload(aiohttp_app, aiohttp_client): + app_client = await aiohttp_client(aiohttp_app.app) + + resp = await app_client.post( + '/v1.0/upload_file', + data=aiohttp.FormData(fields=[('funky_funky', open(__file__, 'rb'))])(), + ) + + data = await resp.json() + assert resp.status == 200 + assert data['fileName'] == f'{__name__}.py' + assert data['content'] == Path(__file__).read_bytes().decode('utf8') + + +async def test_many_files_upload(aiohttp_app, aiohttp_client): + app_client = await aiohttp_client(aiohttp_app.app) + + dir_name = os.path.dirname(__file__) + files_field = [ + ('files', open(f'{dir_name}/{file_name}', 'rb')) \ + for file_name in sorted(os.listdir(dir_name)) if file_name.endswith('.py') + ] + + form_data = aiohttp.FormData(fields=files_field) + + resp = await app_client.post( + '/v1.0/upload_files', + data=form_data(), + ) + + data = await resp.json() + + assert resp.status == 200 + assert data['filesCount'] == len(files_field) + assert data['contents'] == [ + Path(f'{dir_name}/{file_name}').read_bytes().decode('utf8') \ + for file_name in sorted(os.listdir(dir_name)) if file_name.endswith('.py') + ] + + +async def test_mixed_multipart_single_file(aiohttp_app, aiohttp_client): + app_client = await aiohttp_client(aiohttp_app.app) + + form_data = aiohttp.FormData() + form_data.add_field('dirName', os.path.dirname(__file__)) + form_data.add_field('funky_funky', open(__file__, 'rb')) + + resp = await app_client.post( + '/v1.0/mixed_single_file', + data=form_data(), + ) + + data = await resp.json() + + assert resp.status == 200 + assert data['dirName'] == os.path.dirname(__file__) + assert data['fileName'] == f'{__name__}.py' + assert data['content'] == Path(__file__).read_bytes().decode('utf8') + + + +async def test_mixed_multipart_many_files(aiohttp_app, aiohttp_client): + app_client = await aiohttp_client(aiohttp_app.app) + + dir_name = os.path.dirname(__file__) + files_field = [ + ('files', open(f'{dir_name}/{file_name}', 'rb')) \ + for file_name in sorted(os.listdir(dir_name)) if file_name.endswith('.py') + ] + + form_data = aiohttp.FormData(fields=files_field) + form_data.add_field('dirName', os.path.dirname(__file__)) + form_data.add_field('testCount', str(len(files_field))) + + resp = await app_client.post( + '/v1.0/mixed_many_files', + data=form_data(), + ) + + data = await resp.json() + + assert resp.status == 200 + assert data['dirName'] == os.path.dirname(__file__) + assert data['testCount'] == len(files_field) + assert data['filesCount'] == len(files_field) + assert data['contents'] == [ + Path(f'{dir_name}/{file_name}').read_bytes().decode('utf8') \ + for file_name in sorted(os.listdir(dir_name)) if file_name.endswith('.py') + ] diff --git a/tests/api/test_parameters.py b/tests/api/test_parameters.py index 5c9900bac..6269ee99d 100644 --- a/tests/api/test_parameters.py +++ b/tests/api/test_parameters.py @@ -352,6 +352,9 @@ def test_nullable_parameter(simple_app): resp = app_client.put('/v1.0/nullable-parameters', data="None", headers=headers) assert json.loads(resp.data.decode('utf-8', 'replace')) == 'it was None' + resp = app_client.put('/v1.0/nullable-parameters-noargs', data="None", headers=headers) + assert json.loads(resp.data.decode('utf-8', 'replace')) == 'hello' + def test_args_kwargs(simple_app): app_client = simple_app.app.test_client() @@ -363,6 +366,16 @@ def test_args_kwargs(simple_app): assert resp.status_code == 200 assert json.loads(resp.data.decode('utf-8', 'replace')) == {'foo': 'a'} + if simple_app._spec_file == 'openapi.yaml': + body = { 'foo': 'a', 'bar': 'b' } + resp = app_client.post( + '/v1.0/body-params-as-kwargs', + data=json.dumps(body), + headers={'Content-Type': 'application/json'}) + assert resp.status_code == 200 + # having only kwargs and no explicit x-body-name, the handler would have been passed 'body' and the individual params from body + assert json.loads(resp.data.decode('utf-8', 'replace')) == {'body': {'foo': 'a', 'bar': 'b'}, 'foo': 'a', 'bar': 'b'} + def test_param_sanitization(simple_app): app_client = simple_app.app.test_client() diff --git a/tests/fakeapi/aiohttp_handlers.py b/tests/fakeapi/aiohttp_handlers.py index 503724753..5f6b09f33 100755 --- a/tests/fakeapi/aiohttp_handlers.py +++ b/tests/fakeapi/aiohttp_handlers.py @@ -88,3 +88,42 @@ async def get_date(): async def get_uuid(): return ConnexionResponse(body={'value': uuid.UUID(hex='e7ff66d0-3ec2-4c4e-bed0-6e4723c24c51')}) + + +async def aiohttp_multipart_single_file(funky_funky): + return aiohttp.web.json_response( + data={ + 'fileName': funky_funky.filename, + 'content': funky_funky.file.read().decode('utf8') + }, + ) + + +async def aiohttp_multipart_many_files(files): + return aiohttp.web.json_response( + data={ + 'filesCount': len(files), + 'contents': [ f.file.read().decode('utf8') for f in files ] + }, + ) + + +async def aiohttp_multipart_mixed_single_file(dir_name, funky_funky): + return aiohttp.web.json_response( + data={ + 'dirName': dir_name, + 'fileName': funky_funky.filename, + 'content': funky_funky.file.read().decode('utf8'), + }, + ) + + +async def aiohttp_multipart_mixed_many_files(dir_name, test_count, files): + return aiohttp.web.json_response( + data={ + 'filesCount': len(files), + 'dirName': dir_name, + 'testCount': test_count, + 'contents': [ f.file.read().decode('utf8') for f in files ] + }, + ) diff --git a/tests/fakeapi/hello/__init__.py b/tests/fakeapi/hello/__init__.py index 1abc0860f..8957ee23f 100644 --- a/tests/fakeapi/hello/__init__.py +++ b/tests/fakeapi/hello/__init__.py @@ -403,6 +403,9 @@ def test_nullable_param_put(contents): return 'it was None' return contents +def test_nullable_param_put_noargs(dummy=''): + return 'hello' + def test_custom_json_response(): return {'theResult': DummyClass()}, 200 @@ -460,6 +463,9 @@ def optional_auth(**kwargs): def test_args_kwargs(*args, **kwargs): return kwargs +def test_args_kwargs_post(*args, **kwargs): + return kwargs + def test_param_sanitization(query=None, form=None): result = {} diff --git a/tests/fixtures/aiohttp/openapi_multipart.yaml b/tests/fixtures/aiohttp/openapi_multipart.yaml new file mode 100644 index 000000000..ce23e24b5 --- /dev/null +++ b/tests/fixtures/aiohttp/openapi_multipart.yaml @@ -0,0 +1,132 @@ +--- +openapi: 3.0.0 +servers: + - url: /v1.0 +info: + title: "{{title}}" + version: "1.0" +paths: + "/mixed_single_file": + post: + summary: Reads multipart data + description: Handles multipart data reading + operationId: fakeapi.aiohttp_handlers.aiohttp_multipart_mixed_single_file + responses: + "200": + description: OK response + content: + 'application/json': + schema: + type: object + properties: + dirName: + type: string + fileName: + type: string + default: + description: unexpected error + requestBody: + required: true + content: + multipart/form-data: + schema: + type: object + properties: + dirName: + type: string + funky_funky: + type: string + format: binary + "/mixed_many_files": + post: + summary: Reads multipart data + description: Handles multipart data reading + operationId: fakeapi.aiohttp_handlers.aiohttp_multipart_mixed_many_files + responses: + "200": + description: OK response + content: + 'application/json': + schema: + type: object + properties: + dirName: + type: string + testCount: + type: number + filesCount: + type: number + default: + description: unexpected error + requestBody: + required: true + content: + multipart/form-data: + schema: + type: object + properties: + dirName: + type: string + testCount: + type: number + files: + type: array + items: + type: string + format: binary + "/upload_file": + post: + summary: Uploads single file + description: Handles multipart file upload. + operationId: fakeapi.aiohttp_handlers.aiohttp_multipart_single_file + responses: + "200": + description: OK response + content: + 'application/json': + schema: + type: object + properties: + fileName: + type: string + default: + description: unexpected error + requestBody: + required: true + content: + multipart/form-data: + schema: + type: object + properties: + funky_funky: + type: string + format: binary + "/upload_files": + post: + summary: Uploads many files + description: Handles multipart file upload. + operationId: fakeapi.aiohttp_handlers.aiohttp_multipart_many_files + responses: + "200": + description: OK response + content: + 'application/json': + schema: + type: object + properties: + filesCount: + type: number + default: + description: unexpected error + requestBody: + required: true + content: + multipart/form-data: + schema: + type: object + properties: + file: + type: array + items: + type: string + format: binary diff --git a/tests/fixtures/simple/openapi.yaml b/tests/fixtures/simple/openapi.yaml index 80a3bf4fc..d5bd65a3e 100644 --- a/tests/fixtures/simple/openapi.yaml +++ b/tests/fixtures/simple/openapi.yaml @@ -842,6 +842,24 @@ paths: responses: '200': description: OK + /nullable-parameters-noargs: + put: + operationId: fakeapi.hello.test_nullable_param_put_noargs + responses: + '200': + description: OK + requestBody: + content: + application/json: + schema: + nullable: true + x-body-name: contents + type: object + properties: + name: + type: string + description: Just a testing parameter. + required: true /custom-json-response: get: operationId: fakeapi.hello.test_custom_json_response @@ -893,6 +911,25 @@ paths: application/json: schema: type: object + /body-params-as-kwargs: + post: + operationId: fakeapi.hello.test_args_kwargs_post + requestBody: + content: + application/json: + schema: + type: object + properties: + foo: + type: string + additionalProperties: true + responses: + '200': + description: Return kwargs + content: + application/json: + schema: + type: object /text-request: post: operationId: fakeapi.hello.get_data_as_text diff --git a/tests/fixtures/simple/swagger.yaml b/tests/fixtures/simple/swagger.yaml index 32d680e60..83a37a5c3 100644 --- a/tests/fixtures/simple/swagger.yaml +++ b/tests/fixtures/simple/swagger.yaml @@ -692,6 +692,26 @@ paths: 200: description: OK + /nullable-parameters-noargs: + put: + operationId: fakeapi.hello.test_nullable_param_put_noargs + produces: + - application/json + parameters: + - name: contents + description: Just a testing parameter. + in: body + x-nullable: true + required: true + schema: + type: object + properties: + name: + type: string + responses: + 200: + description: OK + /custom-json-response: get: operationId: fakeapi.hello.test_custom_json_response @@ -746,6 +766,30 @@ paths: schema: type: object + /body-params-as-kwargs: + post: + operationId: fakeapi.hello.test_args_kwargs_post + produces: + - application/json + parameters: + - name: $body + description: Just a testing parameter in the body + in: body + required: true + schema: + type: object + properties: + foo: + type: string + bar: + type: string + additionalProperties: true + responses: + 200: + description: Return kwargs + schema: + type: object + /text-request: post: operationId: fakeapi.hello.get_data_as_text