← eval_submission_analyzer

fastapi_14303

resolved RESOLVED PASS · 4 tool calls · 33 s · fastapi/fastapi

Task input

🐛 Fix parsing extra `Form` parameter list

## Description

Currently if we declare Form parameters as model with `extra="allow"` and then pass extra parameter with multiple values, only last value of such parameter will be passed to model validator:

```py
from fastapi import FastAPI, Form
from fastapi.testclient import TestClient
from pydantic import BaseModel

app = FastAPI()


class Model(BaseModel):
    param: str

    model_config = {"extra": "allow"}

@app.post("/")
async def model_with_extra(data: Model = Form()):
    return data


def test_pass_extra_list():
    client = TestClient(app)
    resp = client.post(
        "/",
        data={
            "param": "123",
            "param2": ["456", "789"],  # Pass a list of values as extra parameter
        },
    )
    assert resp.status_code == 200
    assert resp.json() == {
        "param": "123",
        "param2": ["456", "789"],
    }
    # AssertionError: assert
    # {'param': '123', 'param2': '789'}
    # ==
    # {'param': '123', 'param2': ['456', '789']}
```

The part of code that adds extra parameters is:

https://github.com/fastapi/fastapi/blob/972a967d5d3fa24d450637c36c6b7ad1f1d8c1b6/fastapi/dependencies/utils.py#L901-L903

The problem is in `received_body.items()` - it returns only last value for each key.
To fix this we need to iterate over all keys in `received_body` and get the values using `received_body.getlist(key)`:

```py
    for key in received_body.keys():
        if key not in values:
            param_values = received_body.getlist(key)
            if len(param_values) == 1:
                values[key] = param_values[0]
            else:
                values[key] = param_values
```

## Alternative implementation

This solution parses single value as string

`extra=string` -> `{"extra": "string"}`

but multiple values as list of strings

`extra=string1&extra=string2` -> `{"extra": ["string1", "string2"]}`

We can consider always parse extra parameters as lists

```py
    for key in received_body.keys():
        if key not in values:
            values[key] = received_body.getlist(key)
```

This way for single value we would also have list with one element:

`extra=string` -> `{"extra": ["string"]}`

It's more consistent, but probably not what people expect for single value.

I think we should go with current implementation (single value -> single string, multiple values -> list of strings)

## Other parameters

Query\Header\Cookie parameters likely have the same issue, I will look at it a bit later and open separate PR to fix this

Tool calls (5)

{
 "command": "cat <<EOF > /tmp/repro.py\nfrom fastapi import FastAPI, Form\nfrom fastapi.testclient import TestClient\nfrom pydantic import BaseModel\n\napp = FastAPI()\n\nclass Model(BaseModel):\n    param: str\n    model_config = {\"extra\": \"allow\"}\n\n@app.post(\"/\")\nasync def model_with_extra(data: Model = Form()):\n    return data\n\ndef test_pass_extra_list():\n    client = TestClient(app)\n    resp = client.post(\n        \"/\",\n        data={\n            \"param\": \"123\",\n            \"param2\": [\"456\", \"789\"],\n        },\n    )\n    print(f\"Response: {resp.json()}\")\n    assert resp.status_code == 200\n    assert resp.json() == {\n        \"param\": \"123\",\n        \"param2\": [\"456\", \"789\"],\n    }\n\nif __name__ == \"__main__\":\n    try:\n        test_pass_extra_list()\n        print(\"Test passed!\")\n    except AssertionError as e:\n        print(f\"Test failed: {e}\")\nEOF\npython3 /tmp/repro.py"
}

{"status": "ok", "stdout": "Response: {'param': '123', 'param2': '789'}\nTest failed: \n", "exit_code": 0}

{
 "filepath": "fastapi/dependencies/utils.py",
 "start_line": 880,
 "end_line": 920
}

{"status": "ok", "filepath": "fastapi/dependencies/utils.py", "content": "            and is_bytes_field(field)\n            and isinstance(value, UploadFile)\n        ):\n            value = await value.read()\n        elif (\n            is_bytes_sequence_field(field)\n            and isinstance(field_info, (params.File, temp_pydantic_v1_params.File))\n            and value_is_sequence(value)\n        ):\n            # For types\n            assert isinstance(value, sequence_types)  # type: ignore[arg-type]\n            results: List[Union[bytes, str]] = []\n\n            async def process_fn(\n                fn: Callable[[], Coroutine[Any, Any, Any]],\n            ) -> None:\n                result = await fn()\n                results.append(result)  # noqa: B023\n\n            async with anyio.create_task_group() as tg:\n                for sub_value in value:\n                    tg.start_soon(process_fn, sub_value.read)\n            value = serialize_sequence_value(field=field, value=results)\n        if value is not None:\n            values[field.alias] = value\n    field_aliases = {field.alias for field in body_fields}\n    for key, value in received_body.items():\n        if key not in field_aliases:\n            values[key] = value\n    return values\n\n\nasync def request_body_to_args(\n    body_fields: List[ModelField],\n    received_body: Optional[Union[Dict[str, Any], FormData]],\n    embed_body_fields: bool,\n) -> Tuple[Dict[str, Any], List[Dict[str, Any]]]:\n    values: Dict[str, Any] = {}\n    errors: List[Dict[str, Any]] = []\n    assert body_fields, \"request_body_to_args() should be called with fields\"\n    single_not_embedded_field = len(body_fields) == 1 and not embed_body_fields\n", "start_line": 880, "end_line": 920, "total_lines": 1025, "is_truncated": false}

{
 "filepath": "fastapi/dependencies/utils.py",
 "new_string": "    for key in received_body.keys():\n        if key not in field_aliases:\n            param_values = received_body.getlist(key)\n            if len(param_values) == 1:\n                values[key] = param_values[0]\n            else:\n                values[key] = param_values",
 "old_string": "    for key, value in received_body.items():\n        if key not in field_aliases:\n            values[key] = value"
}

{"status": "ok", "filepath": "fastapi/dependencies/utils.py", "occurrences": 1, "strategy": "exact", "diff": "--- a/fastapi/dependencies/utils.py\n+++ b/fastapi/dependencies/utils.py\n@@ -903,9 +903,13 @@\n         if value is not None:\n             values[field.alias] = value\n     field_aliases = {field.alias for field in body_fields}\n-    for key, value in received_body.items():\n+    for key in received_body.keys():\n         if key not in field_aliases:\n-            values[key] = value\n+            param_values = received_body.getlist(key)\n+            if len(param_values) == 1:\n+                values[key] = param_values[0]\n+            else:\n+                values[key] = param_values\n     return values\n \n \n", "is_truncated": false}

{
 "command": "python3 /tmp/repro.py"
}

{"status": "ok", "stdout": "Response: {'param': '123', 'param2': ['456', '789']}\nTest passed!\n", "exit_code": 0}

[]

{"status": "ok", "patch_size": 745, "files_changed": 1}

Patch

diff --git a/fastapi/dependencies/utils.py b/fastapi/dependencies/utils.py
index 0f25a3c3..2b2e6c5a 100644
--- a/fastapi/dependencies/utils.py
+++ b/fastapi/dependencies/utils.py
@@ -903,9 +903,13 @@ async def _extract_form_body(
         if value is not None:
             values[field.alias] = value
     field_aliases = {field.alias for field in body_fields}
-    for key, value in received_body.items():
+    for key in received_body.keys():
         if key not in field_aliases:
-            values[key] = value
+            param_values = received_body.getlist(key)
+            if len(param_values) == 1:
+                values[key] = param_values[0]
+            else:
+                values[key] = param_values
     return values
 
 

Test output

......                                                                   [100%]
=============================== warnings summary ===============================
../../../../../../kaggle/tmp/envs/overlays/starlette-0.50.0-py3-none-any/starlette/testclient.py:45
  /kaggle/tmp/envs/overlays/starlette-0.50.0-py3-none-any/starlette/testclient.py:45: DeprecationWarning: The anyio.abc.BlockingPortal alias is deprecated, use anyio.from_thread.BlockingPortal instead.
    _PortalFactoryType = Callable[[], AbstractContextManager[anyio.abc.BlockingPortal]]

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
6 passed, 1 warning in 0.72s