Fix RuntimeError exception from **application/x-www-form-urlencoded** (see also PR# 1222)

.gitignore

@@ -14,3 +14,4 @@ htmlcov/
 .vscode/
 venv/
 src/
+venv/

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'

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,23 +300,64 @@ 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),
                                 query=query,
                                 headers=headers,
                                 body=body,
                                 json_getter=lambda: cls.jsonifier.loads(body),
-                                files={},
+                                form=form,
+                                files=files,
                                 context=req)
 
     @classmethod

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)
 

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):
         """

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'
 ]
 

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')
+    ]

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()