Wettlauf beim Reservieren, 500er bei unsinnigen Eingaben

Drei P3-Funde aus der Durchsicht vom 14.09.:

Reservieren: Statusprüfung und Schreiben waren getrennt - zwei
gleichzeitige Anfragen konnten beide passieren, die zweite überschrieb
Name und Token der ersten, ohne dass die es erfuhr. Jetzt entscheidet
ein UPDATE mit Status-Bedingung; der Verlierer bekommt None und die
Route meldet "schon weg" (Seite) bzw. 409 (API).

/?limit=abc lieferte jedem anonymen Besucher einen internen
Serverfehler, limit=-1 hiess in SQLite "alles". _limit_lesen() fällt
bei Unsinn auf die Seitengrösse zurück und deckelt bei 500.

Erfassen/Bearbeiten: int(category_id) und die Pydantic-Prüfung warfen
im Handler - 500 statt Fehlermeldung. Jetzt Meldung; ausserdem werden
die Angaben VOR den Bildern geprüft, damit bei abgelehnten Angaben
keine verwaisten Bilddateien liegen bleiben.

Vier neue Tests, jeder einmal gegen den alten Code gelaufen und dabei
rot geworden. 84 lokal grün (HEIC-Test braucht pillow-heif, Docker).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
2026-09-14 17:22:54 +02:00
co-authored by Claude Fable 5
parent feceaacabb
commit c0a18ac735
6 changed files with 133 additions and 26 deletions
+20 -8
View File
@@ -8,7 +8,7 @@ from __future__ import annotations
import secrets
from sqlalchemy import func, select
from sqlalchemy import func, select, update
from sqlalchemy.orm import Session, selectinload
from app.models import Groesse, Item, ItemBild, Kategorie, Status, jetzt
@@ -135,17 +135,29 @@ def item_loeschen(db: Session, item: Item) -> list[str]:
# --------------------------------------------------------- Reservierung ---
def reservieren(db: Session, item: Item, name: str) -> str:
def reservieren(db: Session, item: Item, name: str) -> str | None:
"""Reserviert und gibt das Token zurück, mit dem sich das rückgängig
machen lässt. Das Token verlässt die Anwendung nur dieses eine Mal."""
machen lässt. Das Token verlässt die Anwendung nur dieses eine Mal.
Prüfen und Setzen stehen in EINEM UPDATE mit Status-Bedingung: zwei
gleichzeitige Anfragen könnten sonst beide die Prüfung in der Route
passieren, und die zweite überschriebe Name und Token der ersten -
ohne dass die es je erfährt. So gewinnt genau eine; die andere
bekommt None und der Aufrufer meldet "schon weg"."""
token = secrets.token_urlsafe(32)
item.status = Status.reserved.value
item.reserved_by = name
item.reservation_token = token
item.updated_at = jetzt()
betroffen = db.execute(
update(Item)
.where(Item.id == item.id, Item.status == Status.available.value)
.values(
status=Status.reserved.value,
reserved_by=name,
reservation_token=token,
updated_at=jetzt(),
)
).rowcount
db.commit()
db.refresh(item)
return token
return token if betroffen else None
def freigeben(db: Session, item: Item) -> Item:
+5
View File
@@ -154,6 +154,11 @@ def item_reservieren(
status_code=409, detail="Dieses Kleidungsstück ist nicht mehr verfügbar."
)
token = crud.reservieren(db, item, daten.reserved_by)
if token is None:
# Zwischen Prüfung und Schreiben war jemand schneller.
raise HTTPException(
status_code=409, detail="Dieses Kleidungsstück ist nicht mehr verfügbar."
)
return ReservierenAus(item=_als_antwort(item), reservation_token=token)
+53 -15
View File
@@ -127,9 +127,24 @@ def _liste_daten(db: Session, request: Request, limit: int) -> dict:
}
def _limit_lesen(request: Request) -> int:
"""?limit= kommt aus der URL und ist damit Nutzereingabe.
Ohne die Prüfung liefert /?limit=abc jedem anonymen Besucher einen
internen Serverfehler, und limit=-1 heisst in SQLite "alles". Der
Deckel ist bewusst grosszügiger als die 200 der API: "Mehr anzeigen"
wächst in 24er-Schritten und soll den ganzen Bestand erreichen können.
"""
try:
wert = int(request.query_params.get("limit", ""))
except ValueError:
return SEITE
return max(1, min(wert, 500))
@router.get("/", response_class=HTMLResponse)
def galerie(request: Request, db: Session = Depends(get_db)):
limit = int(request.query_params.get("limit") or SEITE)
limit = _limit_lesen(request)
return vorlagen.TemplateResponse(
request, "galerie.html", _umgebung(request, db, kategorien=crud.kategorien(db),
groessen=crud.groessen(db), **_liste_daten(db, request, limit)),
@@ -139,7 +154,7 @@ def galerie(request: Request, db: Session = Depends(get_db)):
@router.get("/teil/liste", response_class=HTMLResponse)
def liste_ausschnitt(request: Request, db: Session = Depends(get_db)):
"""Nur die Liste - von HTMX beim Filtern und Nachladen geholt."""
limit = int(request.query_params.get("limit") or SEITE)
limit = _limit_lesen(request)
return vorlagen.TemplateResponse(
request, "_liste.html", _umgebung(request, db, **_liste_daten(db, request, limit))
)
@@ -181,6 +196,9 @@ def reservieren(
return _weiter(f"/kleid/{item_id}", "Bitte nur einen Namen angeben.", "fehler")
token = crud.reservieren(db, item, geprueft.reserved_by)
if token is None:
# Zwischen Prüfung und Schreiben war jemand schneller.
return _weiter(f"/kleid/{item_id}", "Das ist leider schon weg.", "fehler")
freigabe = request.url_for("freigeben_per_link", item_id=item.id)
return vorlagen.TemplateResponse(
request, "reserviert.html", _umgebung(request, db, item=item, freigabe_url=f"{freigabe}?token={token}"),
@@ -319,16 +337,28 @@ def erfassen(
security.betreiber_noetig(request)
_csrf_oder_fehler(request, csrf)
# Erst die Angaben prüfen, dann die Bilder speichern: scheitert die
# Prüfung, lägen sonst schon Dateien ohne Eintrag auf der Platte.
# int() und die Pydantic-Prüfung werfen beide ValueError - ohne das
# Abfangen würde daraus ein 500er statt einer Fehlermeldung. Die
# HTML-Formulare verhindern das zwar auf dem normalen Weg, aber ein
# Handgriff an der Anfrage darf keinen Serverfehler auslösen.
try:
daten = ItemAnlegen(
title=title, description=description, size=size,
category_id=int(category_id), gender=gender, season=season,
condition=condition,
)
except ValueError:
return _weiter("/erfassen",
"Bitte die Angaben prüfen - Kategorie wählen, "
"Titel höchstens 100 Zeichen.", "fehler")
try:
namen = _bilder_speichern(dateien)
except images.BildFehler as e:
return _weiter("/erfassen", str(e), "fehler")
daten = ItemAnlegen(
title=title, description=description, size=size,
category_id=int(category_id), gender=gender, season=season,
condition=condition,
)
item = crud.item_anlegen(db, daten)
for name in namen:
crud.bild_anhaengen(db, item, name)
@@ -405,19 +435,27 @@ def bearbeiten(
if item is None:
raise HTTPException(status_code=404, detail="Nicht gefunden.")
war_entwurf = item.status == Status.draft.value
# Wie beim Erfassen: erst prüfen, dann Bilder - und ValueError (int,
# Pydantic) als Meldung statt als 500er.
try:
daten = ItemAendern(
title=title, description=description, size=size,
category_id=int(category_id), gender=gender, season=season,
condition=condition,
# Ein Entwurf wird durchs Nachtragen sichtbar.
status=Status.available if war_entwurf else None,
)
except ValueError:
return _weiter(f"/kleid/{item_id}/bearbeiten",
"Bitte die Angaben prüfen - Kategorie wählen, "
"Titel höchstens 100 Zeichen.", "fehler")
try:
namen = _bilder_speichern(dateien)
except images.BildFehler as e:
return _weiter(f"/kleid/{item_id}/bearbeiten", str(e), "fehler")
war_entwurf = item.status == Status.draft.value
daten = ItemAendern(
title=title, description=description, size=size,
category_id=int(category_id), gender=gender, season=season,
condition=condition,
# Ein Entwurf wird durchs Nachtragen sichtbar.
status=Status.available if war_entwurf else None,
)
try:
crud.item_aendern(db, item, daten)
except IntegrityError: