fix(api): a non-finite number in a JSON body is refused with a 422, not a 500
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.
This commit is contained in:
+17
-1
@@ -1,10 +1,13 @@
|
|||||||
import asyncio
|
import asyncio
|
||||||
import logging
|
import logging
|
||||||
|
import math
|
||||||
import sys
|
import sys
|
||||||
from contextlib import asynccontextmanager, suppress
|
from contextlib import asynccontextmanager, suppress
|
||||||
from pathlib import Path
|
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
|
# 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
|
# (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 = 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(
|
app.add_middleware(
|
||||||
SessionMiddleware,
|
SessionMiddleware,
|
||||||
secret_key=settings.secret_key,
|
secret_key=settings.secret_key,
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
from datetime import date, datetime, timezone
|
from datetime import date, datetime, timezone
|
||||||
|
|
||||||
from fastapi import APIRouter, Depends, HTTPException, Query
|
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 import and_, func, or_, select
|
||||||
from sqlalchemy.dialects.postgresql import insert as pg_insert
|
from sqlalchemy.dialects.postgresql import insert as pg_insert
|
||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
@@ -303,6 +303,10 @@ def clear_video_state(
|
|||||||
|
|
||||||
|
|
||||||
class VideoProgressIn(BaseModel):
|
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
|
# 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
|
# 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.
|
# 10s progress hot path, losing the resume position; `float` keeps the old tolerance.
|
||||||
|
|||||||
@@ -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
|
||||||
Reference in New Issue
Block a user