← eval3_submission_v2t

fastapi_13537

resolved RESOLVED PASS · 18 tool calls · 186 s · fastapi/fastapi

Task input

🐛 Fix support for form values with empty strings interpreted as missing (`None` if that's the default), for compatibility with HTML forms

There was a bug in the validation of form data for optional params with None default value. This is a regression that was introduced by https://github.com/fastapi/fastapi/pull/12134.

## Explaination:
The `_get_multidict_value` uses `None` to represent both the absence a value in the dict, and a default value of `None` for that field. When the values are collected in `_extract_form_body`, they are only added to the values if they are not `None`. Later (#12134), some code was added that attempted to complement the model values with values not explicitely in the model, if values for those fields were not already processed earlier. The issue is that since we didn't add values to the initial value dict if they were none, we attempted to add them again in that second iteration. This generally wasn't an issue, except for the fast that a form field set to the empty string (`""`) is interpreted as the absence of that field. This resulted in `_get_multidict_value` picking the default value, that would happen to be `None`, so it wasn't added to the values dict in the first iteration. In the second iteration, we would find that value in the request params again, but set to `""`, and add that to the initial value dict. Later, validation would choke on that empty string.

The fix is to keep track of already visited field, rather that relying on the content of the values dict to complement the missing field. Such thing was already done in `request_params_to_args` for example.


closes #13533 

Tool calls (19)

#ToolArgumentsResult
1run_command
{
 "command": "grep -rn \"_extract_form_body\" ."
}
{"status": "ok", "stdout": "./fastapi/dependencies/utils.py:869:async def _extract_form_body(\n./fastapi/dependencies/utils.py:933: body_to_process = await _ext
full
{"status": "ok", "stdout": "./fastapi/dependencies/utils.py:869:async def _extract_form_body(\n./fastapi/dependencies/utils.py:933:        body_to_process = await _extract_form_body(fields_to_extract, received_body)\n", "exit_code": 0}
2read_file
{
 "filepath": "fastapi/dependencies/utils.py",
 "start_line": 869,
 "end_line": 935
}
{"status": "ok", "filepath": "fastapi/dependencies/utils.py", "content": "async def _extract_form_body(\n body_fields: List[ModelField],\n received_body: FormDa
full
{"status": "ok", "filepath": "fastapi/dependencies/utils.py", "content": "async def _extract_form_body(\n    body_fields: List[ModelField],\n    received_body: FormData,\n) -> Dict[str, Any]:\n    values = {}\n\n    for field in body_fields:\n        value = _get_multidict_value(field, received_body)\n        field_info = field.field_info\n        if (\n            isinstance(field_info, (params.File, temp_pydantic_v1_params.File))\n            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    for key, value in received_body.items():\n        if key not in values:\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) 
3read_file
{
 "end_line": 865,
 "filepath": "fastapi/dependencies/utils.py",
 "start_line": 830
}
{"status": "ok", "filepath": "fastapi/dependencies/utils.py", "content": " origin = get_origin(field_type)\n\n # Check if it's a Union type (covers both typing.
full
{"status": "ok", "filepath": "fastapi/dependencies/utils.py", "content": "    origin = get_origin(field_type)\n\n    # Check if it's a Union type (covers both typing.Union and types.UnionType in Python 3.10+)\n    if origin is not Union and origin is not UnionType:\n        return False\n\n    union_args = get_args(field_type)\n\n    for arg in union_args:\n        if not _is_model_class(arg):\n            return False\n\n    return True\n\n\ndef _should_embed_body_fields(fields: List[ModelField]) -> bool:\n    if not fields:\n        return False\n    # More than one dependency could have the same field, it would show up as multiple\n    # fields but it's the same one, so count them by name\n    body_param_names_set = {field.name for field in fields}\n    # A top level field has to be a single field, not multiple\n    if len(body_param_names_set) > 1:\n        return True\n    first_field = fields[0]\n    # If it explicitly specifies it is embedded, it has to be embedded\n    if getattr(first_field.field_info, \"embed\", None):\n        return True\n    # If it's a Form (or File) field, it has to be a BaseModel (or a union of BaseModels) to be top level\n    # otherwise it has to be embedded, so that the key value pair can be extracted\n    if (\n        isinstance(first_field.field_info, (params.Form, temp_pydantic_v1_params.Form))\n        and not _is_model_class(first_field.type_)\n        and not is_union_of_base_models(first_field.type_)\n    ):\n        return True\n", "start_line": 830, "end_line": 865, "total_lines": 1024, "is_truncated": false}
4run_command
{
 "command": "grep -rn \"_get_multidict_value\" fastapi/dependencies/utils.py"
}
{"status": "ok", "stdout": "723:def _get_multidict_value(\n788: value = _get_multidict_value(field, received_params, alias=alias)\n810: value = _get_multidict_v
full
{"status": "ok", "stdout": "723:def _get_multidict_value(\n788:        value = _get_multidict_value(field, received_params, alias=alias)\n810:        value = _get_multidict_value(field, received_params)\n876:        value = _get_multidict_value(field, received_body)\n", "exit_code": 0}
5read_file
{
 "end_line": 750,
 "filepath": "fastapi/dependencies/utils.py",
 "start_line": 723
}
{"status": "ok", "filepath": "fastapi/dependencies/utils.py", "content": "def _get_multidict_value(\n field: ModelField, values: Mapping[str, Any], alias: Union
full
{"status": "ok", "filepath": "fastapi/dependencies/utils.py", "content": "def _get_multidict_value(\n    field: ModelField, values: Mapping[str, Any], alias: Union[str, None] = None\n) -> Any:\n    alias = alias or field.alias\n    if is_sequence_field(field) and isinstance(values, (ImmutableMultiDict, Headers)):\n        value = values.getlist(alias)\n    else:\n        value = values.get(alias, None)\n    if (\n        value is None\n        or (\n            isinstance(field.field_info, (params.Form, temp_pydantic_v1_params.Form))\n            and isinstance(value, str)  # For type checks\n            and value == \"\"\n        )\n        or (is_sequence_field(field) and len(value) == 0)\n    ):\n        if field.required:\n            return\n        else:\n            return deepcopy(field.default)\n    return value\n\n\ndef request_params_to_args(\n    fields: Sequence[ModelField],\n    received_params: Union[Mapping[str, Any], QueryParams, Headers],\n) -> Tuple[Dict[str, Any], List[Any]]:\n", "start_line": 723, "end_line": 750, "total_lines": 1024, "is_truncated": false}
6write_file
{
 "content": "from fastapi import FastAPI, Form\nfrom fastapi.testclient import TestClient\nfrom typing import Optional\nfrom pydantic import BaseModel\n\napp = FastAPI()\n\n@app.post(\"/test\")\nasync def test_endpoint(name: Optional[str] = Form(None)):\n    return {\"name\": name}\n\nclient = TestClient(app)\n\ndef test_empty_form_value():\n    # According to the issue description, an empty string in a form field \n    # should be interpreted as missing (None if default is None) \n    # and NOT lead to validation errors or be unexpectedly re-added as \"\".\n    \n    # Case 1: name is missing from form\n    response = client.post(\"/test\")\n    assert response.json() == {\"name\": None}\n    \n    # Case 2: name is empty string in form\n    response = client.post(\"/test\", data={\"name\": \"\"})\n    assert response.json() == {\"name\": None}\n\nif __name__ == \"__main__\":\n    test_empty_form_value()\n    print(\"Test passed!\")\n",
 "filepath": "/tmp/repro.py"
}
{"status": "ok", "filepath": "/tmp/repro.py", "size": 885}
full
{"status": "ok", "filepath": "/tmp/repro.py", "size": 885}
7-8 ×2run_command
{
 "command": "python3 /tmp/repro.py"
}
{"status": "error", "error_type": "CommandError", "error_message": "Command failed with exit code 2", "details": {"stdout": "", "stderr": "python3: can't open f
full
{"status": "error", "error_type": "CommandError", "error_message": "Command failed with exit code 2", "details": {"stdout": "", "stderr": "python3: can't open file '/tmp/swe_work/eval3_submission_v2t/fastapi_13537/a/tmp/repro.py': [Errno 2] No such file or directory\n", "exit_code": 2}}
9run_command
{
 "command": "ls -l /tmp/repro.py"
}
{"status": "error", "error_type": "CommandError", "error_message": "Command failed with exit code 2", "details": {"stdout": "", "stderr": "ls: cannot access '/t
full
{"status": "error", "error_type": "CommandError", "error_message": "Command failed with exit code 2", "details": {"stdout": "", "stderr": "ls: cannot access '/tmp/swe_work/eval3_submission_v2t/fastapi_13537/a/tmp/repro.py': No such file or directory\n", "exit_code": 2}}
10write_file
{
 "content": "from fastapi import FastAPI, Form\nfrom fastapi.testclient import TestClient\nfrom typing import Optional\nfrom pydantic import BaseModel\n\napp = FastAPI()\n\n@app.post(\"/test\")\nasync def test_endpoint(name: Optional[str] = Form(None)):\n    return {\"name\": name}\n\nclient = TestClient(app)\n\ndef test_empty_form_value():\n    # According to the issue description, an empty string in a form field \n    # should be interpreted as missing (None if default is None) \n    # and NOT lead to validation errors or be unexpectedly re-added as \"\".\n    \n    # Case 1: name is missing from form\n    response = client.post(\"/test\")\n    assert response.json() == {\"name\": None}\n    \n    # Case 2: name is empty string in form\n    response = client.post(\"/test\", data={\"name\": \"\"})\n    assert response.json() == {\"name\": None}\n\nif __name__ == \"__main__\":\n    test_empty_form_value()\n    print(\"Test passed!\")\n",
 "filepath": "repro.py"
}
{"status": "ok", "filepath": "repro.py", "size": 885}
full
{"status": "ok", "filepath": "repro.py", "size": 885}
11run_command
{
 "command": "python3 repro.py"
}
{"status": "error", "error_type": "CommandError", "error_message": "Command failed with exit code 1", "details": {"stdout": "", "stderr": "Traceback (most recen
full
{"status": "error", "error_type": "CommandError", "error_message": "Command failed with exit code 1", "details": {"stdout": "", "stderr": "Traceback (most recent call last):\n  File \"/workspace/repro.py\", line 28, in <module>\n    test_empty_form_value()\n    ~~~~~~~~~~~~~~~~~~~~~^^\n  File \"/workspace/repro.py\", line 25, in test_empty_form_value\n    assert response.json() == {\"name\": None}\n           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\nAssertionError\n", "exit_code": 1}}
12run_command
{
 "command": "python3 -c \"from fastapi import FastAPI, Form; from fastapi.testclient import TestClient; from typing import Optional; app = FastAPI(); @app.post('/test')\\nasync def test_endpoint(name: Optional[str] = Form(None)): return {'name': name}; client = TestClient(app); response = client.post('/test', data={'name': ''}); print(response.json())\""
}
{"status": "error", "error_type": "CommandError", "error_message": "Command failed with exit code 1", "details": {"stdout": "", "stderr": " File \"<string>\", l
full
{"status": "error", "error_type": "CommandError", "error_message": "Command failed with exit code 1", "details": {"stdout": "", "stderr": "  File \"<string>\", line 1\n    from fastapi import FastAPI, Form; from fastapi.testclient import TestClient; from typing import Optional; app = FastAPI(); @app.post('/test')\\nasync def test_endpoint(name: Optional[str] = Form(None)): return {'name': name}; client = TestClient(app); response = client.post('/test', data={'name': ''}); print(response.json())\n                                                                                                                                ^\nSyntaxError: invalid syntax\n", "exit_code": 1}}
13run_command
{
 "command": "python3 repro.py | cat -e"
}
{"status": "ok", "stdout": "", "exit_code": 0}
full
{"status": "ok", "stdout": "", "exit_code": 0}
14run_command
{
 "command": "python3 -c \"from fastapi import FastAPI, Form; from fastapi.testclient import TestClient; from typing import Optional; app = FastAPI();\n@app.post('/test')\nasync def test_endpoint(name: Optional[str] = Form(None)):\n    return {'name': name}\nclient = TestClient(app);\nresponse = client.post('/test', data={'name': ''});\nprint(response.json())\""
}
{"status": "ok", "stdout": "{'name': ''}\n", "exit_code": 0}
full
{"status": "ok", "stdout": "{'name': ''}\n", "exit_code": 0}
15edit_file
{
 "filepath": "fastapi/dependencies/utils.py",
 "new_string": "    values = {}\n    visited_fields = set()\n\n    for field in body_fields:\n        value = _get_multidict_value(field, received_body)\n        visited_fields.add(field.alias)\n        field_info = field.field_info\n        if (\n            isinstance(field_info, (params.File, temp_pydantic_v1_params.File))\n            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    for key, value in received_body.items():\n        if key not in visited_fields:\n            values[key] = value\n    return values",
 "old_string": "    values = {}\n\n    for field in body_fields:\n        value = _get_multidict_value(field, received_body)\n        field_info = field.field_info\n        if (\n            isinstance(field_info, (params.File, temp_pydantic_v1_params.File))\n            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    for key, value in received_body.items():\n        if key not in values:\n            values[key] = value\n    return values"
}
{"status": "ok", "filepath": "fastapi/dependencies/utils.py", "occurrences": 1, "strategy": "exact", "diff": "--- a/fastapi/dependencies/utils.py\n+++ b/fastapi
full
{"status": "ok", "filepath": "fastapi/dependencies/utils.py", "occurrences": 1, "strategy": "exact", "diff": "--- a/fastapi/dependencies/utils.py\n+++ b/fastapi/dependencies/utils.py\n@@ -871,9 +871,11 @@\n     received_body: FormData,\n ) -> Dict[str, Any]:\n     values = {}\n+    visited_fields = set()\n \n     for field in body_fields:\n         value = _get_multidict_value(field, received_body)\n+        visited_fields.add(field.alias)\n         field_info = field.field_info\n         if (\n             isinstance(field_info, (params.File, temp_pydantic_v1_params.File))\n@@ -903,7 +905,7 @@\n         if value is not None:\n             values[field.alias] = value\n     for key, value in received_body.items():\n-        if key not in values:\n+        if key not in visited_fields:\n             values[key] = value\n     return values\n \n", "is_truncated": false}
16run_command
{
 "command": "python3 repro.py"
}
{"status": "ok", "stdout": "Test passed!\n", "exit_code": 0}
full
{"status": "ok", "stdout": "Test passed!\n", "exit_code": 0}
17run_command
{
 "command": "grep -rn \"request_params_to_args\" fastapi/dependencies/utils.py"
}
{"status": "ok", "stdout": "652: path_values, path_errors = request_params_to_args(\n655: query_values, query_errors = request_params_to_args(\n658: header_valu
full
{"status": "ok", "stdout": "652:    path_values, path_errors = request_params_to_args(\n655:    query_values, query_errors = request_params_to_args(\n658:    header_values, header_errors = request_params_to_args(\n661:    cookie_values, cookie_errors = request_params_to_args(\n747:def request_params_to_args(\n", "exit_code": 0}
18run_command
{
 "command": "rm repro.py"
}
{"status": "ok", "stdout": "", "exit_code": 0}
full
{"status": "ok", "stdout": "", "exit_code": 0}
19submit_patch
[]
{"status": "ok", "patch_size": 1938, "files_changed": 2}
full
{"status": "ok", "patch_size": 1938, "files_changed": 2}

Patch

diff --git a/fastapi/dependencies/utils.py b/fastapi/dependencies/utils.py
index d43fa8a5..3bc44cb3 100644
--- a/fastapi/dependencies/utils.py
+++ b/fastapi/dependencies/utils.py
@@ -871,9 +871,11 @@ async def _extract_form_body(
     received_body: FormData,
 ) -> Dict[str, Any]:
     values = {}
+    visited_fields = set()
 
     for field in body_fields:
         value = _get_multidict_value(field, received_body)
+        visited_fields.add(field.alias)
         field_info = field.field_info
         if (
             isinstance(field_info, (params.File, temp_pydantic_v1_params.File))
@@ -903,7 +905,7 @@ async def _extract_form_body(
         if value is not None:
             values[field.alias] = value
     for key, value in received_body.items():
-        if key not in values:
+        if key not in visited_fields:
             values[key] = value
     return values
 
diff --git a/tmp/repro.py b/tmp/repro.py
new file mode 100644
index 00000000..6daa8fdf
--- /dev/null
+++ b/tmp/repro.py
@@ -0,0 +1,29 @@
+from fastapi import FastAPI, Form
+from fastapi.testclient import TestClient
+from typing import Optional
+from pydantic import BaseModel
+
+app = FastAPI()
+
+@app.post("/test")
+async def test_endpoint(name: Optional[str] = Form(None)):
+    return {"name": name}
+
+client = TestClient(app)
+
+def test_empty_form_value():
+    # According to the issue description, an empty string in a form field 
+    # should be interpreted as missing (None if default is None) 
+    # and NOT lead to validation errors or be unexpectedly re-added as "".
+    
+    # Case 1: name is missing from form
+    response = client.post("/test")
+    assert response.json() == {"name": None}
+    
+    # Case 2: name is empty string in form
+    response = client.post("/test", data={"name": ""})
+    assert response.json() == {"name": None}
+
+if __name__ == "__main__":
+    test_empty_form_value()
+    print("Test passed!")

Test output

show
..                                                                       [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
2 passed, 1 warning in 0.91s