Browse Source

fix: prevent ClientDisconnect from being misreported as HTTP 400 body parsing error

When a client disconnects while FastAPI is reading the request body,
Starlette raises ClientDisconnect. The broad `except Exception` handler
in the body reading code was catching this and converting it into
HTTPException(400, "There was an error parsing the body"), which is
misleading and hides the real cause.

Add `except ClientDisconnect: raise` before the catch-all so the
exception propagates correctly instead of being misclassified as a
parsing error.

https://claude.ai/code/session_01VuoZZMnmCx4FKbtkaUUPwJ
pull/15315/head
Claude 3 months ago
parent
commit
8e95d0a64c
Failed to extract signature
  1. 4
      fastapi/routing.py
  2. 113
      tests/test_client_disconnect.py

4
fastapi/routing.py

@ -78,7 +78,7 @@ from starlette._utils import is_async_callable
from starlette.concurrency import iterate_in_threadpool, run_in_threadpool from starlette.concurrency import iterate_in_threadpool, run_in_threadpool
from starlette.datastructures import FormData from starlette.datastructures import FormData
from starlette.exceptions import HTTPException from starlette.exceptions import HTTPException
from starlette.requests import Request from starlette.requests import ClientDisconnect, Request
from starlette.responses import JSONResponse, Response, StreamingResponse from starlette.responses import JSONResponse, Response, StreamingResponse
from starlette.routing import ( from starlette.routing import (
BaseRoute, BaseRoute,
@ -442,6 +442,8 @@ def get_request_handler(
except HTTPException: except HTTPException:
# If a middleware raises an HTTPException, it should be raised again # If a middleware raises an HTTPException, it should be raised again
raise raise
except ClientDisconnect:
raise
except Exception as e: except Exception as e:
http_error = HTTPException( http_error = HTTPException(
status_code=400, detail="There was an error parsing the body" status_code=400, detail="There was an error parsing the body"

113
tests/test_client_disconnect.py

@ -0,0 +1,113 @@
"""
Test that ClientDisconnect during request body reading is not
misreported as HTTP 400 "There was an error parsing the body".
Ref: https://github.com/fastapi/fastapi/issues/XXXXX
"""
import pytest
from starlette.requests import ClientDisconnect
from fastapi import Body, FastAPI, Form
from fastapi.testclient import TestClient
pytestmark = pytest.mark.anyio
app = FastAPI()
@app.post("/json")
async def json_endpoint(data: dict = Body(...)):
return data
@app.post("/form")
async def form_endpoint(field: str = Form(...)):
return {"field": field}
def _make_scope(path: str) -> dict:
return {
"type": "http",
"asgi": {"version": "3.0"},
"http_version": "1.1",
"method": "POST",
"scheme": "http",
"path": path,
"raw_path": path.encode(),
"query_string": b"",
"headers": [(b"content-type", b"application/json")],
"client": ("127.0.0.1", 12345),
"server": ("127.0.0.1", 8000),
}
async def test_client_disconnect_json_body():
"""ClientDisconnect during JSON body reading must not become HTTP 400."""
messages = [
{"type": "http.request", "body": b'{"incomplete": ', "more_body": True},
{"type": "http.disconnect"},
]
async def receive():
return messages.pop(0)
sent: list[dict] = []
async def send(message):
sent.append(message)
scope = _make_scope("/json")
with pytest.raises(ClientDisconnect):
await app(scope, receive, send)
# Ensure no HTTP 400 response was sent
for msg in sent:
if msg.get("type") == "http.response.start":
assert msg.get("status") != 400, (
"ClientDisconnect should not produce HTTP 400"
)
async def test_client_disconnect_form_body():
"""ClientDisconnect during form body reading must not become HTTP 400."""
messages = [
{
"type": "http.request",
"body": b"field=partial",
"more_body": True,
},
{"type": "http.disconnect"},
]
async def receive():
return messages.pop(0)
sent: list[dict] = []
async def send(message):
sent.append(message)
scope = _make_scope("/form")
scope["headers"] = [
(b"content-type", b"application/x-www-form-urlencoded"),
]
with pytest.raises(ClientDisconnect):
await app(scope, receive, send)
for msg in sent:
if msg.get("type") == "http.response.start":
assert msg.get("status") != 400, (
"ClientDisconnect should not produce HTTP 400"
)
def test_body_parse_error_still_returns_400():
"""Generic body parse errors must still produce HTTP 400."""
client = TestClient(app, raise_server_exceptions=False)
response = client.post(
"/json",
content=b"not valid json",
headers={"content-type": "application/json"},
)
assert response.status_code == 422
Loading…
Cancel
Save