From 8010616a13c0e98281c33b159891515b149aa4a5 Mon Sep 17 00:00:00 2001 From: Tseluiko Aleksandr Date: Sat, 17 Oct 2026 03:14:25 +0300 Subject: [PATCH] Fix request handling issues and add CI --- .github/workflows/ci.yml | 60 +++++++++ README.md | 32 +++-- pyproject.toml | 2 +- src/msgspec_fastapi/router.py | 73 +++++++++-- tests/test_router.py | 226 +++++++++++++++++++++++++++++++++- 5 files changed, 368 insertions(+), 25 deletions(-) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..caeda7b --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,60 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + test: + name: Python ${{ matrix.python }}, ${{ matrix.bounds }} dependency bounds + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python: ["3.11", "3.12", "3.13", "3.14"] + bounds: [latest, lowest] + env: + PYTHON: ${{ matrix.python }} + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: astral-sh/setup-uv@1c37ad07a6a961277cf70c0d37d6f313000f5884 # v10.3.0 + with: + enable-cache: false + - name: Install + run: uv sync --python "$PYTHON" --group test + - name: Install the lowest supported FastAPI and msgspec + if: matrix.bounds == 'lowest' + run: uv pip install "fastapi==0.142.2" "msgspec==0.22.0" + - name: Show versions + run: uv run --no-sync python -c "import sys, fastapi, msgspec, pydantic, starlette; print(sys.version); print(fastapi.__version__, msgspec.__version__, pydantic.__version__, starlette.__version__)" + - name: Check the lowest versions + if: matrix.bounds == 'lowest' + # `uv run` without --no-sync would reinstall the locked versions. + run: uv run --no-sync python -c "import fastapi, msgspec; assert (fastapi.__version__, msgspec.__version__) == ('0.142.2', '0.22.0')" + - name: Test + run: uv run --no-sync pytest + + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: astral-sh/setup-uv@1c37ad07a6a961277cf70c0d37d6f313000f5884 # v10.3.0 + with: + enable-cache: false + - name: Build + run: uv build + - name: Check the distributions + run: uvx twine@7.0.0 check --strict dist/* diff --git a/README.md b/README.md index 0eac0ca..532de87 100644 --- a/README.md +++ b/README.md @@ -41,18 +41,27 @@ install_openapi(app, router) ## What it does - A msgspec body parameter (a Struct or another type msgspec can decode) is decoded from - the raw request bytes by a `msgspec.json.Decoder` built once at route registration. - A non-JSON content type returns 415, malformed JSON returns 400, and a missing or - invalid body returns 422. Bodies are decoded in msgspec strict mode: for example, the - JSON string `"1"` is not accepted for an `int` field, unlike FastAPI with Pydantic. + the raw request bytes by a `msgspec.json.Decoder` built once at route registration. A + body with a non-JSON content type returns 415, malformed JSON or JSON nested too deeply + returns 400, and a missing or invalid body returns 422. The 415 and 400 responses are + sent before dependencies run; a 422 is sent after them, as in FastAPI, so a failing + dependency such as authentication takes precedence. Bodies are decoded in msgspec + strict mode: for example, the JSON string `"1"` is not accepted for an `int` field, + unlike FastAPI with Pydantic. +- A body without a content type follows FastAPI's `strict_content_type` setting: it is + rejected with 415 by default and parsed as JSON when the setting is `False`. FastAPI + returns 422 in the first case. - A msgspec response type, from the return annotation or `response_model`, is encoded with msgspec. An instance of exactly the declared Struct class is encoded without revalidation, as FastAPI does for a Pydantic `response_model`; other values are validated against the declared type. Nested values and type parameters of a trusted - instance are not checked. -- Responses of routes without a return annotation or `response_model` are also encoded - with msgspec, so the output can differ from FastAPI's `jsonable_encoder` (for example, - `bytes` are base64-encoded and `Decimal` is encoded as a string). + instance are not checked. A NaN or infinite value in a `float` field is encoded as + `null` unless a constraint rejects it. In a `Decimal` field it is encoded as a string, + while FastAPI with Pydantic rejects non-finite `Decimal` values. +- Responses of routes without a return annotation or `response_model`, or with + `-> None`, are also encoded with msgspec, so the output can differ from FastAPI's + `jsonable_encoder` (for example, `bytes` are base64-encoded and `Decimal` is encoded as + a string). - Path, query, header and cookie parameters keep FastAPI extraction, aliases and defaults, but are converted with `msgspec.convert(strict=False)` instead of Pydantic, so coercion differs for some inputs (for example, `" 1.5"` is rejected for `float` @@ -68,9 +77,12 @@ install_openapi(app, router) - Only one msgspec body parameter per route, and it cannot be combined with other FastAPI body fields. Only JSON bodies are supported: `application/json`, - `application/*+json`, or a request without a content type. + `application/*+json`, or a request without a content type when `strict_content_type` + is `False`. `Form()` and `File()` parameters are left to FastAPI. +- When both the parameters and the body are invalid, only the parameter errors are + returned; FastAPI returns both. - Structs, alone or inside containers, are not supported as path, query, header or - cookie parameters. + cookie parameters, or as `Form()` and `File()` parameters. - `response_model_include`, `response_model_exclude`, `response_model_exclude_*` and `response_model_by_alias=False` are rejected for typed msgspec responses. diff --git a/pyproject.toml b/pyproject.toml index 19e5dc6..4aa4e91 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,7 +26,7 @@ Source = "https://github.com/Siyet/msgspec-fastapi" Issues = "https://github.com/Siyet/msgspec-fastapi/issues" [dependency-groups] -test = ["pytest", "httpx", "httpx2"] +test = ["pytest", "httpx", "httpx2", "python-multipart"] [tool.hatch.build.targets.wheel] packages = ["src/msgspec_fastapi"] diff --git a/src/msgspec_fastapi/router.py b/src/msgspec_fastapi/router.py index 844b658..8a22309 100644 --- a/src/msgspec_fastapi/router.py +++ b/src/msgspec_fastapi/router.py @@ -12,7 +12,7 @@ from fastapi.datastructures import DefaultPlaceholder from fastapi.exceptions import RequestValidationError, ResponseValidationError from fastapi.openapi.utils import validation_error_definition, validation_error_response_definition -from fastapi.params import Body, Depends, Param +from fastapi.params import Body, Depends, Form, Param from fastapi.routing import APIRoute, _effective_route_context_var, iter_route_contexts from starlette.concurrency import run_in_threadpool @@ -85,16 +85,23 @@ def _hints(endpoint): return get_type_hints(target, include_extras=True) -def _parse_body(request, raw, config): +def _parse_body(request, raw, config, strict_content_type): decoder, default, embedded = config if not raw: if default is inspect.Signature.empty: - raise RequestValidationError([{'loc': ('body',), 'msg': 'Field required', 'type': 'missing'}]) + # Raised after dependencies run, as in FastAPI. + return RequestValidationError([{'loc': ('body',), 'msg': 'Field required', 'type': 'missing'}]) return deepcopy(default) - media_type = request.headers.get('content-type', '').split(';')[0].strip().lower() - if media_type and media_type != 'application/json' and not ( - media_type.startswith('application/') and media_type.endswith('+json') - ): + content_type = request.headers.get('content-type') + if content_type: + media_type = content_type.split(';')[0].strip().lower() + if media_type != 'application/json' and not ( + media_type.startswith('application/') and media_type.endswith('+json') + ): + raise HTTPException(415, 'Expected a JSON request body') + elif strict_content_type: + # Like FastAPI's strict_content_type: a body without a content type can be sent + # cross-origin without a CORS preflight, so it is not parsed as JSON. raise HTTPException(415, 'Expected a JSON request body') try: value = decoder.decode(raw) @@ -104,12 +111,38 @@ def _parse_body(request, raw, config): msgspec.json.decode(raw) except (msgspec.DecodeError, UnicodeError) as syntax: raise HTTPException(400, f'Malformed JSON body: {syntax}') from syntax - raise RequestValidationError([{'loc': ('body',), 'msg': str(exc), 'type': 'value_error.msgspec'}]) from exc + except RecursionError as nested: + raise HTTPException(400, 'JSON body is nested too deeply') from nested + return RequestValidationError([{'loc': ('body',), 'msg': str(exc), 'type': 'value_error.msgspec'}]) except (msgspec.DecodeError, UnicodeError) as exc: raise HTTPException(400, f'Malformed JSON body: {exc}') from exc + except RecursionError as exc: + raise HTTPException(400, 'JSON body is nested too deeply') from exc return value.value if embedded else value +def _revalidate_without_json(result, output_type, error): + """Validate a value whose JSON round trip failed, as for NaN and infinity. + + JSON encodes non-finite floats as null, and null is not valid for a float field. + FastAPI with Pydantic encodes them as null in a response, so validate the Python + values instead, with object keys converted to strings. A validation error from this + check is reported, because the one from the JSON round trip can point at a + non-finite float instead of the actual problem. Values that cannot be encoded as + JSON are not revalidated. + """ + if isinstance(error, msgspec.ValidationError): + try: + return msgspec.convert(msgspec.to_builtins(result, str_keys=True), output_type, str_keys=True) + except msgspec.ValidationError as exc: + error = exc + except TypeError: + # to_builtins does not support every value JSON encodes, such as msgspec.Raw. + pass + raise ResponseValidationError( + [{'loc': ('response',), 'msg': str(error), 'type': 'value_error.msgspec'}]) from error + + class MsgspecRoute(APIRoute): def get_route_handler(self): context = _effective_route_context_var.get() @@ -122,9 +155,14 @@ def get_route_handler(self): if route.body_field is not None: raise TypeError('Other FastAPI body fields cannot be combined with a msgspec body') + strict_content_type = route.strict_content_type + if isinstance(strict_content_type, DefaultPlaceholder): + strict_content_type = strict_content_type.value + async def handle(request): - # Invalid JSON must be rejected before dependency side effects. - request.scope['_msgspec_body_value'] = _parse_body(request, await request.body(), config) + # A body that is not JSON is rejected before dependency side effects. + request.scope['_msgspec_body_value'] = _parse_body( + request, await request.body(), config, strict_content_type) return await handler(request) return handle @@ -194,13 +232,17 @@ def add_api_route(self, path, endpoint, **kwargs): override = kwargs.get('route_class_override') if override is not None and not issubclass(override, MsgspecRoute): raise TypeError('route_class_override must extend MsgspecRoute') - # Included routes already have a prepared signature and response metadata. + # Routes copied from another MsgspecRouter with add_api_route already have a + # prepared signature and response metadata. if getattr(endpoint, '_msgspec_prepared', False): return super().add_api_route(path, endpoint, **kwargs) signature = inspect.signature(endpoint) hints = _hints(endpoint) parameters = [p.replace(annotation=hints.get(p.name, p.annotation)) for p in signature.parameters.values()] return_type = hints.get('return', signature.return_annotation) + if return_type is type(None): + # As in FastAPI, `-> None` declares no response model. + return_type = None explicit = kwargs.get('response_model', inspect.Signature.empty) inferred = explicit is inspect.Signature.empty or isinstance(explicit, DefaultPlaceholder) output_type = return_type if inferred else explicit @@ -241,6 +283,11 @@ def add_api_route(self, path, endpoint, **kwargs): metadata = _metadata(parameter, parameter.annotation) if any(isinstance(x, Depends) for x in metadata): continue + # Form and File parameters are left to FastAPI. + if any(isinstance(x, Form) for x in metadata): + if _contains_struct(parameter.annotation): + raise TypeError('msgspec Structs are not supported as Form or File parameters') + continue is_body = _contains_struct(parameter.annotation) or any(isinstance(x, Body) for x in metadata) if _body_type(parameter.annotation) and not any(isinstance(x, Param) for x in metadata): is_body = True @@ -359,6 +406,8 @@ async def wrapper(**values): if remove_request: values.pop(request_name) value = request.scope['_msgspec_body_value'] + if isinstance(value, RequestValidationError): + raise value values[body.name] = value if is_async: result = await endpoint(**values) @@ -375,7 +424,7 @@ async def wrapper(**values): # Typed decoding validates fields and Meta, even for existing Structs. result = output_decoder.decode(output_encoder.encode(result)) except (TypeError, ValueError, OverflowError) as exc: - raise ResponseValidationError([{'loc': ('response',), 'msg': str(exc), 'type': 'value_error.msgspec'}]) from exc + result = _revalidate_without_json(result, output_type, exc) status_code = sub_response.status_code or kwargs.get('status_code') or 200 if untyped_encoder is not None: response = Response(untyped_encoder.encode(result), status_code=status_code, diff --git a/tests/test_router.py b/tests/test_router.py index 3f4f0a7..184c6a2 100644 --- a/tests/test_router.py +++ b/tests/test_router.py @@ -1,9 +1,10 @@ """Tests for MsgspecRouter request decoding, parameter validation and response encoding.""" -from typing import Annotated, Generic, TypeVar +from typing import Annotated, Any, Generic, TypeVar import msgspec import pytest -from fastapi import APIRouter, Depends, FastAPI, Query, Response +from fastapi import APIRouter, Depends, FastAPI, File, Form, Header, HTTPException, Query, Response +from fastapi.exceptions import ResponseValidationError from fastapi.responses import HTMLResponse, PlainTextResponse from fastapi.testclient import TestClient from pydantic import BaseModel, Field @@ -319,3 +320,224 @@ async def endpoint(q: annotation): assert actual.status_code == expected.status_code if expected.status_code == 200: assert actual.json() == expected.json() + + +def strict_content_type_app(level, strict): + calls = [] + + def dependency(): + calls.append(1) + + async def endpoint(city: City): + return city + + app = FastAPI(**({'strict_content_type': strict} if level == 'app' else {})) + router = MsgspecRouter(**({'strict_content_type': strict} if level == 'router' else {})) + router.add_api_route('/value', endpoint, methods=['POST'], dependencies=[Depends(dependency)], + **({'strict_content_type': strict} if level == 'route' else {})) + app.include_router(router) + return TestClient(app, raise_server_exceptions=False), calls + + +@pytest.mark.parametrize('level', ['app', 'router', 'route']) +@pytest.mark.parametrize('strict', [True, False]) +def test_body_without_content_type_follows_strict_content_type(level, strict): + client, calls = strict_content_type_app(level, strict) + result = client.post('/value', content=b'{"population": 1}') + assert 'content-type' not in result.request.headers + if strict: + assert result.status_code == 415 and calls == [] + else: + assert result.status_code == 200 and result.json() == {'population': 1} + + +def test_route_strict_content_type_overrides_app(): + app = FastAPI(strict_content_type=False) + router = MsgspecRouter() + + async def endpoint(city: City): + return city + + router.add_api_route('/value', endpoint, methods=['POST'], strict_content_type=True) + app.include_router(router) + result = TestClient(app, raise_server_exceptions=False).post('/value', content=b'{"population": 1}') + assert result.status_code == 415 + + +@pytest.mark.parametrize('content_type', ['', ';', 'text/plain', 'application/x-www-form-urlencoded']) +def test_body_with_non_json_content_type_is_rejected(content_type): + client, calls = strict_content_type_app('app', False) + result = client.post('/value', content=b'{"population": 1}', headers={'Content-Type': content_type}) + expected = 200 if content_type == '' else 415 + assert result.status_code == expected + assert calls == ([1] if expected == 200 else []) + + +@pytest.mark.parametrize('content_type', ['application/json; charset=utf-8', 'application/merge-patch+json']) +def test_body_with_json_content_type_is_accepted(content_type): + client, _ = strict_content_type_app('app', True) + result = client.post('/value', content=b'{"population": 1}', headers={'Content-Type': content_type}) + assert result.status_code == 200 and result.json() == {'population': 1} + + +@pytest.mark.parametrize('annotation', [City, list[int], list[Any]]) +def test_deeply_nested_json_body_is_rejected(annotation): + router = MsgspecRouter() + + async def endpoint(value: annotation): + return {} + + router.add_api_route('/value', endpoint, methods=['POST']) + app = FastAPI() + app.include_router(router) + depth = 200_000 + result = TestClient(app, raise_server_exceptions=False).post( + '/value', content=b'[' * depth + b']' * depth, headers={'Content-Type': 'application/json'}) + assert result.status_code == 400 + + +class Reading(msgspec.Struct): + value: float + + +class LabeledReading(Reading): + label: str = '' + + +@pytest.mark.parametrize('annotation,value,expected', [ + (float, float('nan'), None), + (float, float('inf'), None), + (dict[str, float], {'a': float('-inf')}, {'a': None}), + (Reading, LabeledReading(float('nan'), 'x'), {'value': None}), + (list[Reading], [Reading(float('nan'))], [{'value': None}]), + (dict[str, float], {1: float('nan')}, {'1': None}), + (dict[int, float], {'1': float('nan')}, {'1': None}), +]) +def test_non_finite_floats_in_typed_response_are_encoded_as_null(annotation, value, expected): + # FastAPI with Pydantic also encodes these as null instead of failing. + result = response(setup(value, model=annotation)) + assert result.status_code == 200 and result.json() == expected + + +class Measurement(msgspec.Struct): + value: float + count: int + + +@pytest.mark.parametrize('annotation,value,message', [ + (Annotated[float, msgspec.Meta(ge=0)], float('nan'), 'Expected `float` >= 0.0'), + (Annotated[float, msgspec.Meta(ge=0)], float('-inf'), 'Expected `float` >= 0.0'), + (Measurement, {'value': float('nan'), 'count': 'x'}, 'Expected `int`, got `str` - at `$.count`'), + (Reading, {'value': 'x'}, 'Expected `float`, got `str` - at `$.value`'), + (float, 'nan', 'Expected `float`, got `str`'), + (float, msgspec.Raw(b'"x"'), 'Expected `float`, got `str`'), +]) +def test_invalid_typed_response_is_still_rejected(annotation, value, message): + router = MsgspecRouter() + + async def endpoint(): + return value + + router.add_api_route('/value', endpoint, methods=['GET'], response_model=annotation) + app = FastAPI() + app.include_router(router) + with pytest.raises(ResponseValidationError) as info: + TestClient(app).get('/value') + assert info.value.errors()[0]['msg'] == message + + +def auth_app(): + calls = [] + + def authenticate(token: Annotated[str | None, Header()] = None): + calls.append(token) + if token != 'secret': + raise HTTPException(401, 'Not authenticated') + + router = MsgspecRouter() + + @router.post('/value', dependencies=[Depends(authenticate)]) + async def endpoint(city: City): + return city + + app = FastAPI() + app.include_router(router) + return TestClient(app, raise_server_exceptions=False), calls + + +@pytest.mark.parametrize('body', [{'population': -1}, {}, None]) +def test_body_validation_errors_are_reported_after_dependencies(body): + # As in FastAPI, a failing dependency such as authentication takes precedence over + # a body that does not match the schema. + client, calls = auth_app() + options = {} if body is None else {'json': body} + assert client.post('/value', **options).status_code == 401 + assert calls == [None] + result = client.post('/value', headers={'token': 'secret'}, **options) + assert result.status_code == 422 + assert result.json()['detail'][0]['loc'] == ['body'] + + +def test_malformed_body_is_still_rejected_before_dependencies(): + client, calls = auth_app() + assert client.post('/value', content=b'{', headers={'Content-Type': 'application/json'}).status_code == 400 + assert calls == [] + + +@pytest.mark.parametrize('router_class', [APIRouter, MsgspecRouter]) +def test_none_return_annotation_declares_no_response_model(router_class): + router = router_class() + + @router.delete('/value', status_code=204) + async def delete() -> None: + pass + + @router.get('/value') + async def get() -> None: + pass + + app = FastAPI() + app.include_router(router) + client = TestClient(app) + deleted = client.delete('/value') + assert deleted.status_code == 204 and deleted.content == b'' + assert client.get('/value').json() is None + responses = app.openapi()['paths']['/value']['get']['responses'] + assert responses['200']['content']['application/json']['schema'] == {} + + +def test_form_and_file_parameters_are_left_to_fastapi(): + router = MsgspecRouter() + + @router.post('/value') + async def endpoint(name: Annotated[str, Form()], data: Annotated[bytes, File()]): + return {'name': name, 'size': len(data)} + + app = FastAPI() + app.include_router(router) + result = TestClient(app).post('/value', data={'name': 'x'}, files={'data': b'abc'}) + assert result.status_code == 200 and result.json() == {'name': 'x', 'size': 3} + + +def test_struct_form_parameter_is_rejected(): + router = MsgspecRouter() + + async def endpoint(city: Annotated[City, Form()]): + return city + + with pytest.raises(TypeError, match='Form or File'): + router.add_api_route('/value', endpoint, methods=['POST']) + + +@pytest.mark.parametrize('annotation,value', [(str, '\ud800'), (Any, {None: 1})]) +def test_unencodable_typed_response_raises_response_validation_error(annotation, value): + router = MsgspecRouter() + + async def endpoint(): + return value + + router.add_api_route('/value', endpoint, methods=['GET'], response_model=annotation) + app = FastAPI() + app.include_router(router) + with pytest.raises(ResponseValidationError): + TestClient(app).get('/value')