Fix multiple file upload (#1813)

Fixes #1806
Fixes #1811

connexion/decorators/parameter.py

@@ -225,7 +225,8 @@ def get_arguments(
             content_type=content_type,
         )
     )
-    ret.update(_get_file_arguments(files, arguments, has_kwargs))
+    body_schema = operation.body_schema(content_type)
+    ret.update(_get_file_arguments(files, arguments, body_schema, has_kwargs))
     return ret
 
 
@@ -482,5 +483,13 @@ def _get_typed_body_values(body_arg, body_props, additional_props):
     return res
 
 
-def _get_file_arguments(files, arguments, has_kwargs=False):
-    return {k: v for k, v in files.items() if k in arguments or has_kwargs}
+def _get_file_arguments(files, arguments, body_schema: dict, has_kwargs=False):
+    results = {}
+    for k, v in files.items():
+        if not (k in arguments or has_kwargs):
+            continue
+        if body_schema.get("properties", {}).get(k, {}).get("type") != "array":
+            v = v[0]
+        results[k] = v
+
+    return results

connexion/validators/form_data.py

@@ -54,14 +54,37 @@ async def _parse(self, stream: t.AsyncGenerator[bytes, None], scope: Scope) -> d
         if self._uri_parser is not None:
             # Don't parse file_data
             form_data = {}
-            file_data = {}
-            for k, v in data.items():
-                if isinstance(v, str):
-                    form_data[k] = data.getlist(k)
-                elif isinstance(v, UploadFile):
-                    # Replace files with empty strings for validation
-                    file_data[k] = ""
-
+            file_data: t.Dict[str, t.Union[str, t.List[str]]] = {}
+            for key in data.keys():
+                # Extract files
+                param_schema = self._schema.get("properties", {}).get(key, {})
+                value = data.getlist(key)
+
+                def is_file(schema):
+                    return (
+                        schema.get("type") == "string"
+                        and schema.get("format") == "binary"
+                    )
+
+                # Single file upload
+                if is_file(param_schema):
+                    # Unpack if single file received
+                    if len(value) == 1:
+                        file_data[key] = ""
+                    # If multiple files received, replace with array so validation will fail
+                    else:
+                        file_data[key] = [""] * len(value)
+                # Multiple file upload, replace files with array of strings
+                elif is_file(param_schema.get("items", {})):
+                    file_data[key] = [""] * len(value)
+                # UploadFile received for non-file upload. Replace and let validation handle.
+                elif isinstance(value[0], UploadFile):
+                    file_data[key] = [""] * len(value)
+                # No files, add multi-value to form data and let uri parser handle multi-value
+                else:
+                    form_data[key] = value
+
+            # Resolve form data, not file data
             data = self._uri_parser.resolve_form(form_data)
             # Add the files again
             data.update(file_data)

docs/request.rst

@@ -224,6 +224,119 @@ Connexion will not automatically pass in the default values defined in your ``re
 definition, but you can activate this by configuring a different
 :ref:`RequestBodyValidator<validation:Custom validators>`.
 
+Files
+-----
+
+Connexion extracts the files from the body and passes them into your view function separately:
+
+.. tab-set::
+
+    .. tab-item:: OpenAPI 3
+        :sync: OpenAPI 3
+
+        .. code-block:: yaml
+            :caption: **openapi.yaml**
+
+            paths:
+              /path
+                post:
+                  operationId: api.foo_get
+                  requestBody:
+                    content:
+                      multipart/form-data:
+                        schema:
+                          type: object
+                          properties:
+                            file:
+                              type: string
+                              format: binary
+
+    .. tab-item:: Swagger 2
+        :sync: Swagger 2
+
+        In the Swagger 2 specification, you can define the name of your body. Connexion will pass
+        the body to your function using this name.
+
+        .. code-block:: yaml
+            :caption: **swagger.yaml**
+
+            paths:
+              /path
+                post:
+                  consumes:
+                    - application/json
+                  parameters:
+                    - name: file
+                      type: file
+                      in: formData
+
+
+.. tab-set::
+
+    .. tab-item:: AsyncApp
+        :sync: AsyncApp
+
+        If you're using the `AsyncApp`, the files are provided as `Starlette.UploadFile`_ instances.
+
+        .. code-block:: python
+            :caption: **api.py**
+
+                    def foo_get(file)
+                        assert isinstance(file, starlette.UploadFile)
+                        ...
+
+
+    .. tab-item:: FlaskApp
+        :sync: FlaskApp
+
+        If you're using the `FlaskApp`, the files are provided as `werkzeug.FileStorage`_ instances.
+
+        .. code-block:: python
+            :caption: **api.py**
+
+                    def foo_get(file)
+                        assert isinstance(file, werkzeug.FileStorage)
+                        ...
+
+When your specification defines an array of files:
+
+.. code-block:: yaml
+
+    type: array
+    items:
+        type: string
+        format: binary
+
+They will be provided to your view function as a list.
+
+.. tab-set::
+
+    .. tab-item:: AsyncApp
+        :sync: AsyncApp
+
+        .. code-block:: python
+            :caption: **api.py**
+
+                    def foo_get(file)
+                        assert isinstance(file, list)
+                        assert isinstance(file[0], starlette.UploadFile)
+                        ...
+
+
+    .. tab-item:: FlaskApp
+        :sync: FlaskApp
+
+        .. code-block:: python
+            :caption: **api.py**
+
+                    def foo_get(file)
+                        assert isinstance(file, list)
+                        assert isinstance(file[0], werkzeug.FileStorage)
+                        ...
+
+.. _Starlette.UploadFile: https://www.starlette.io/requests/#request-files
+.. _werkzeug.FileStorage: https://werkzeug.palletsprojects.com/en/3.0.x/datastructures/#werkzeug.datastructures.FileStorage
+
 Optional arguments & Defaults
 -----------------------------
 
@@ -488,6 +601,11 @@ request.
 .. dropdown:: View a detailed reference of the ``connexion.request`` class
     :icon: eye
 
+    .. warning::
+
+        The asynchronous body arguments (body, form, files) might already be consumed by connexion.
+        We recommend to let Connexion inject them into your view function as mentioned above.
+
     .. autoclass:: connexion.lifecycle.ConnexionRequest
        :members:
        :undoc-members:

pyproject.toml

@@ -103,3 +103,7 @@ exclude_lines = [
     "if t.TYPE_CHECKING:",
     "@t.overload",
 ]
+
+[[tool.mypy.overrides]]
+module = "referencing.jsonschema.*"
+follow_imports = "skip"

tests/api/test_parameters.py

@@ -270,22 +270,36 @@ def test_strict_formdata_extra_param(strict_app):
 
 
 def test_formdata_file_upload(simple_app):
+    """Test that a single file is accepted and provided to the user as a file object if the openapi
+    specification defines single file. Do not accept multiple files."""
     app_client = simple_app.test_client()
+
+    resp = app_client.post(
+        "/v1.0/test-formData-file-upload",
+        files=[
+            ("file", ("filename.txt", BytesIO(b"file contents"))),
+            ("file", ("filename2.txt", BytesIO(b"file2 contents"))),
+        ],
+    )
+    assert resp.status_code == 400
+
     resp = app_client.post(
         "/v1.0/test-formData-file-upload",
-        files={"fileData": ("filename.txt", BytesIO(b"file contents"))},
+        files={"file": ("filename.txt", BytesIO(b"file contents"))},
     )
     assert resp.status_code == 200
     assert resp.json() == {"filename.txt": "file contents"}
 
 
 def test_formdata_multiple_file_upload(simple_app):
+    """Test that multiple files are accepted and provided to the user as a list if the openapi
+    specification defines an array of files."""
     app_client = simple_app.test_client()
     resp = app_client.post(
-        "/v1.0/test-formData-file-upload",
+        "/v1.0/test-formData-multiple-file-upload",
         files=[
-            ("fileData", ("filename.txt", BytesIO(b"file contents"))),
-            ("fileData", ("filename2.txt", BytesIO(b"file2 contents"))),
+            ("file", ("filename.txt", BytesIO(b"file contents"))),
+            ("file", ("filename2.txt", BytesIO(b"file2 contents"))),
         ],
     )
     assert resp.status_code == 200
@@ -294,13 +308,20 @@ def test_formdata_multiple_file_upload(simple_app):
         "filename2.txt": "file2 contents",
     }
 
+    resp = app_client.post(
+        "/v1.0/test-formData-multiple-file-upload",
+        files={"file": ("filename.txt", BytesIO(b"file contents"))},
+    )
+    assert resp.status_code == 200
+    assert resp.json() == {"filename.txt": "file contents"}
+
 
 def test_mixed_formdata(simple_app):
     app_client = simple_app.test_client()
     resp = app_client.post(
         "/v1.0/test-mixed-formData",
         data={"formData": "test"},
-        files={"fileData": ("filename.txt", BytesIO(b"file contents"))},
+        files={"file": ("filename.txt", BytesIO(b"file contents"))},
     )
 
     assert resp.status_code == 200
@@ -320,8 +341,8 @@ def test_formdata_file_upload_bad_request(simple_app):
     )
     assert resp.status_code == 400
     assert resp.json()["detail"] in [
-        "Missing formdata parameter 'fileData'",
-        "'fileData' is a required property",  # OAS3
+        "Missing formdata parameter 'file'",
+        "'file' is a required property",  # OAS3
     ]
 
 

tests/fakeapi/hello/__init__.py

@@ -329,47 +329,53 @@ def test_formdata_missing_param():
     return ""
 
 
-async def test_formdata_file_upload(fileData, **kwargs):
-    """In Swagger, form paramaeters and files are passed separately"""
-    files = {}
-    for file_ in fileData:
-        filename = file_.filename
-        content = file_.read()
-        if asyncio.iscoroutine(content):
-            # AsyncApp
-            content = await content
+async def test_formdata_file_upload(file):
+    """In Swagger, form parameters and files are passed separately"""
+    filename = file.filename
+    content = file.read()
+    if asyncio.iscoroutine(content):
+        # AsyncApp
+        content = await content
 
-        files[filename] = content.decode()
+    return {filename: content.decode()}
 
-    return files
 
+async def test_formdata_multiple_file_upload(file):
+    """In Swagger, form parameters and files are passed separately"""
+    assert isinstance(file, list)
 
-async def test_mixed_formdata(fileData, formData):
-    files = {}
-    for file_ in fileData:
-        filename = file_.filename
-        content = file_.read()
+    results = {}
+
+    for f in file:
+        filename = f.filename
+        content = f.read()
         if asyncio.iscoroutine(content):
             # AsyncApp
             content = await content
 
-        files[filename] = content.decode()
+        results[filename] = content.decode()
 
-    return {"data": {"formData": formData}, "files": files}
+    return results
 
 
-async def test_mixed_formdata3(fileData, formData):
-    files = {}
-    for file_ in fileData:
-        filename = file_.filename
-        content = file_.read()
-        if asyncio.iscoroutine(content):
-            # AsyncApp
-            content = await content
+async def test_mixed_formdata(file, formData):
+    filename = file.filename
+    content = file.read()
+    if asyncio.iscoroutine(content):
+        # AsyncApp
+        content = await content
+
+    return {"data": {"formData": formData}, "files": {filename: content.decode()}}
+
 
-        files[filename] = content.decode()
+async def test_mixed_formdata3(file, formData):
+    filename = file.filename
+    content = file.read()
+    if asyncio.iscoroutine(content):
+        # AsyncApp
+        content = await content
 
-    return {"data": formData, "files": files}
+    return {"data": formData, "files": {filename: content.decode()}}
 
 
 def test_formdata_file_upload_missing_param():