From 7360339039094ce673ea543b1110fa0f7632a22f Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 11 Sep 2026 03:18:17 +0200 Subject: [PATCH] fix(api): a non-finite number in a JSON body is refused with a 422, not a 500 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The JSON parser under FastAPI accepts `Infinity`/`NaN`, and both progress checkpoints truncate the position with `int()` — a hand-written checkpoint came back as a 500 with a traceback. Their bodies now refuse non-finite floats, and the app's validation handler — FastAPI's own shape — echoes a non-finite input by name: the default one failed to serialise its own error (JSON has no NaN), which was a 500 by another route. Sibling of the subtitle routes' non-finite query parameters. --- backend/app/main.py | 18 +++++++++++++- backend/app/routes/feed.py | 6 ++++- backend/tests/test_progress_bounds.py | 34 +++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 backend/tests/test_progress_bounds.py diff --git a/backend/app/main.py b/backend/app/main.py index 7c2e6038..3acb4bde 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -1,10 +1,13 @@ import asyncio import logging +import math import sys from contextlib import asynccontextmanager, suppress from pathlib import Path -from fastapi import FastAPI, HTTPException +from fastapi import FastAPI, HTTPException, Request +from fastapi.encoders import jsonable_encoder +from fastapi.exceptions import RequestValidationError # When started via uvicorn --log-config the "siftlode" logger is already configured # (see log_config.json). This block is a timestamped fallback for other entrypoints @@ -112,6 +115,19 @@ async def _sweep_hls_orphans() -> None: app = FastAPI(title=settings.app_name, lifespan=lifespan) + +@app.exception_handler(RequestValidationError) +async def _validation_error(request: Request, exc: RequestValidationError) -> JSONResponse: + """FastAPI's own 422, in its own shape — except that it can be ENCODED. The JSON parser accepts + `NaN`/`Infinity`, a float field refuses them, and the refusal echoes the value back: the default + handler then failed to serialise its own error (JSON has no NaN) and the request became a 500 + with a traceback. A non-finite value is echoed as its name instead.""" + finite = {float: lambda x: x if math.isfinite(x) else str(x)} + return JSONResponse( + status_code=422, content={"detail": jsonable_encoder(exc.errors(), custom_encoder=finite)} + ) + + app.add_middleware( SessionMiddleware, secret_key=settings.secret_key, diff --git a/backend/app/routes/feed.py b/backend/app/routes/feed.py index 2d6f3635..e969de25 100644 --- a/backend/app/routes/feed.py +++ b/backend/app/routes/feed.py @@ -1,7 +1,7 @@ from datetime import date, datetime, timezone from fastapi import APIRouter, Depends, HTTPException, Query -from pydantic import BaseModel +from pydantic import BaseModel, ConfigDict from sqlalchemy import and_, func, or_, select from sqlalchemy.dialects.postgresql import insert as pg_insert from sqlalchemy.orm import Session @@ -303,6 +303,10 @@ def clear_video_state( class VideoProgressIn(BaseModel): + # Finite only: JSON parsing here accepts `Infinity`/`NaN`, and the handler's `int()` turned + # one into a 500. + model_config = ConfigDict(allow_inf_nan=False) + # Accept a float and truncate (below) — the player may post a raw currentTime (a float), which # the old `int(payload.get(...))` silently truncated. A plain `int` field would 422 that on the # 10s progress hot path, losing the resume position; `float` keeps the old tolerance. diff --git a/backend/tests/test_progress_bounds.py b/backend/tests/test_progress_bounds.py new file mode 100644 index 00000000..aea5dee5 --- /dev/null +++ b/backend/tests/test_progress_bounds.py @@ -0,0 +1,34 @@ +"""A progress checkpoint takes a FINITE position. + +The JSON parser under FastAPI accepts `Infinity` and `NaN`, and both progress handlers truncate the +position with `int()` — so one hand-written checkpoint came back as a 500 with a traceback. Found in +the 2026-09 bug hunt's second review round, as the sibling of the subtitle routes' non-finite +`at`/`offset`. Pure: the body is refused before the handler touches the database. +""" + +from types import SimpleNamespace + +import pytest +from fastapi.testclient import TestClient + +import app.state as app_state +from app.auth import current_user +from app.main import app + + +@pytest.fixture +def client(monkeypatch, override_dep) -> TestClient: + override_dep(current_user, lambda: SimpleNamespace(id=1)) + monkeypatch.setattr(app_state, "_configured_cache", True) # pass the instance-setup gate, no DB + return TestClient(app) + + +@pytest.mark.parametrize("path", ["/api/videos/v1/progress", "/api/plex/item/98614/progress"]) +@pytest.mark.parametrize("raw", ["Infinity", "-Infinity", "NaN"]) +def test_a_non_finite_position_is_refused(client, path, raw): + r = client.post( + path, + content=f'{{"position_seconds": {raw}, "duration_seconds": 3000}}', + headers={"content-type": "application/json"}, + ) + assert r.status_code == 422, r.text