Merge branch 'robustheit'
This commit is contained in:
+16
-3
@@ -146,7 +146,7 @@ verlässt sich das Image derzeit stillschweigend.
|
|||||||
Proxy-IP eintragen; dort ausserdem den Hinweis ergänzen, dass der
|
Proxy-IP eintragen; dort ausserdem den Hinweis ergänzen, dass der
|
||||||
veröffentlichte Port nur für den Proxy erreichbar sein darf.
|
veröffentlichte Port nur für den Proxy erreichbar sein darf.
|
||||||
|
|
||||||
### P3 — Öffentlicher 500er über `?limit=abc`
|
### P3 — Öffentlicher 500er über `?limit=abc` ✅ ERLEDIGT 14.09.2026
|
||||||
|
|
||||||
`pages.py` (`galerie`, `liste_ausschnitt`) rechnet
|
`pages.py` (`galerie`, `liste_ausschnitt`) rechnet
|
||||||
`int(request.query_params.get("limit") or SEITE)` ohne Prüfung — `/?limit=x`
|
`int(request.query_params.get("limit") or SEITE)` ohne Prüfung — `/?limit=x`
|
||||||
@@ -155,7 +155,7 @@ bedeutet in SQLite „alles". Die API-Seite macht es mit
|
|||||||
`Query(ge=1, le=200)` längst richtig; dieselbe Grenze hier nachziehen
|
`Query(ge=1, le=200)` längst richtig; dieselbe Grenze hier nachziehen
|
||||||
(ungültig → Standardwert).
|
(ungültig → Standardwert).
|
||||||
|
|
||||||
### P3 — Betreiber-Formulare: 500 statt Fehlermeldung
|
### P3 — Betreiber-Formulare: 500 statt Fehlermeldung ✅ ERLEDIGT 14.09.2026
|
||||||
|
|
||||||
In `erfassen` und `bearbeiten` (`pages.py`) wird `int(category_id)` ohne
|
In `erfassen` und `bearbeiten` (`pages.py`) wird `int(category_id)` ohne
|
||||||
Prüfung gerechnet, und `ItemAnlegen(...)` wirft bei ungültigen Werten eine
|
Prüfung gerechnet, und `ItemAnlegen(...)` wirft bei ungültigen Werten eine
|
||||||
@@ -164,7 +164,7 @@ ValidationError **im** Handler — beides ergibt einen 500er statt der sonst
|
|||||||
HTML-Formulare (Dropdown, `maxlength`) verhindern es auf dem normalen Weg —
|
HTML-Formulare (Dropdown, `maxlength`) verhindern es auf dem normalen Weg —
|
||||||
es bricht aber das Muster, das `reservieren` mit try/except vormacht.
|
es bricht aber das Muster, das `reservieren` mit try/except vormacht.
|
||||||
|
|
||||||
### P3 — Reservieren: Prüfen und Setzen sind nicht atomar
|
### P3 — Reservieren: Prüfen und Setzen sind nicht atomar ✅ ERLEDIGT 14.09.2026
|
||||||
|
|
||||||
`reservieren` prüft erst `status == available` und schreibt dann. Zwei
|
`reservieren` prüft erst `status == available` und schreibt dann. Zwei
|
||||||
gleichzeitige Anfragen können beide die Prüfung passieren; der zweite
|
gleichzeitige Anfragen können beide die Prüfung passieren; der zweite
|
||||||
@@ -205,3 +205,16 @@ funktionierende Rate-Limits, aber alle Besucher teilen sich die Zähler
|
|||||||
über die Proxy-IP. Das ist das kleinere Übel gegenüber frei erfindbaren
|
über die Proxy-IP. Das ist das kleinere Übel gegenüber frei erfindbaren
|
||||||
Absenderadressen. **Beim nächsten Deployment die Proxy-IP eintragen und
|
Absenderadressen. **Beim nächsten Deployment die Proxy-IP eintragen und
|
||||||
einmal prüfen, dass `request.client.host` die echte Besucher-IP zeigt.**
|
einmal prüfen, dass `request.client.host` die echte Besucher-IP zeigt.**
|
||||||
|
|
||||||
|
### Umsetzung der drei P3 am 14.09.2026
|
||||||
|
|
||||||
|
| Punkt | Ergebnis |
|
||||||
|
|---|---|
|
||||||
|
| `?limit=abc` | `_limit_lesen()` in `pages.py`: ungültig → Seitengrösse, Deckel 500 |
|
||||||
|
| Betreiber-Formulare | Angaben werden **vor** dem Bildspeichern geprüft; ValueError → Meldung statt 500 (und keine verwaisten Bilddateien mehr) |
|
||||||
|
| Reservieren | `crud.reservieren()` schreibt per `UPDATE … WHERE status='available'`; der Verlierer des Wettlaufs bekommt None → „schon weg" |
|
||||||
|
|
||||||
|
Vier neue Tests decken genau diese Fälle ab; alle vier wurden nach der
|
||||||
|
Hausregel einmal gegen den alten Code laufen gelassen und sind dabei
|
||||||
|
nachweislich rot geworden. Suite: **85 Tests** (84 lokal grün, der
|
||||||
|
HEIC-Test braucht `pillow-heif` und läuft im Docker-Testimage).
|
||||||
|
|||||||
+20
-8
@@ -8,7 +8,7 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import secrets
|
import secrets
|
||||||
|
|
||||||
from sqlalchemy import func, select
|
from sqlalchemy import func, select, update
|
||||||
from sqlalchemy.orm import Session, selectinload
|
from sqlalchemy.orm import Session, selectinload
|
||||||
|
|
||||||
from app.models import Groesse, Item, ItemBild, Kategorie, Status, jetzt
|
from app.models import Groesse, Item, ItemBild, Kategorie, Status, jetzt
|
||||||
@@ -135,17 +135,29 @@ def item_loeschen(db: Session, item: Item) -> list[str]:
|
|||||||
|
|
||||||
# --------------------------------------------------------- Reservierung ---
|
# --------------------------------------------------------- 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
|
"""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)
|
token = secrets.token_urlsafe(32)
|
||||||
item.status = Status.reserved.value
|
betroffen = db.execute(
|
||||||
item.reserved_by = name
|
update(Item)
|
||||||
item.reservation_token = token
|
.where(Item.id == item.id, Item.status == Status.available.value)
|
||||||
item.updated_at = jetzt()
|
.values(
|
||||||
|
status=Status.reserved.value,
|
||||||
|
reserved_by=name,
|
||||||
|
reservation_token=token,
|
||||||
|
updated_at=jetzt(),
|
||||||
|
)
|
||||||
|
).rowcount
|
||||||
db.commit()
|
db.commit()
|
||||||
db.refresh(item)
|
db.refresh(item)
|
||||||
return token
|
return token if betroffen else None
|
||||||
|
|
||||||
|
|
||||||
def freigeben(db: Session, item: Item) -> Item:
|
def freigeben(db: Session, item: Item) -> Item:
|
||||||
|
|||||||
@@ -154,6 +154,11 @@ def item_reservieren(
|
|||||||
status_code=409, detail="Dieses Kleidungsstück ist nicht mehr verfügbar."
|
status_code=409, detail="Dieses Kleidungsstück ist nicht mehr verfügbar."
|
||||||
)
|
)
|
||||||
token = crud.reservieren(db, item, daten.reserved_by)
|
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)
|
return ReservierenAus(item=_als_antwort(item), reservation_token=token)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
+53
-15
@@ -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)
|
@router.get("/", response_class=HTMLResponse)
|
||||||
def galerie(request: Request, db: Session = Depends(get_db)):
|
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(
|
return vorlagen.TemplateResponse(
|
||||||
request, "galerie.html", _umgebung(request, db, kategorien=crud.kategorien(db),
|
request, "galerie.html", _umgebung(request, db, kategorien=crud.kategorien(db),
|
||||||
groessen=crud.groessen(db), **_liste_daten(db, request, limit)),
|
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)
|
@router.get("/teil/liste", response_class=HTMLResponse)
|
||||||
def liste_ausschnitt(request: Request, db: Session = Depends(get_db)):
|
def liste_ausschnitt(request: Request, db: Session = Depends(get_db)):
|
||||||
"""Nur die Liste - von HTMX beim Filtern und Nachladen geholt."""
|
"""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(
|
return vorlagen.TemplateResponse(
|
||||||
request, "_liste.html", _umgebung(request, db, **_liste_daten(db, request, limit))
|
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")
|
return _weiter(f"/kleid/{item_id}", "Bitte nur einen Namen angeben.", "fehler")
|
||||||
|
|
||||||
token = crud.reservieren(db, item, geprueft.reserved_by)
|
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)
|
freigabe = request.url_for("freigeben_per_link", item_id=item.id)
|
||||||
return vorlagen.TemplateResponse(
|
return vorlagen.TemplateResponse(
|
||||||
request, "reserviert.html", _umgebung(request, db, item=item, freigabe_url=f"{freigabe}?token={token}"),
|
request, "reserviert.html", _umgebung(request, db, item=item, freigabe_url=f"{freigabe}?token={token}"),
|
||||||
@@ -319,16 +337,28 @@ def erfassen(
|
|||||||
security.betreiber_noetig(request)
|
security.betreiber_noetig(request)
|
||||||
_csrf_oder_fehler(request, csrf)
|
_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:
|
try:
|
||||||
namen = _bilder_speichern(dateien)
|
namen = _bilder_speichern(dateien)
|
||||||
except images.BildFehler as e:
|
except images.BildFehler as e:
|
||||||
return _weiter("/erfassen", str(e), "fehler")
|
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)
|
item = crud.item_anlegen(db, daten)
|
||||||
for name in namen:
|
for name in namen:
|
||||||
crud.bild_anhaengen(db, item, name)
|
crud.bild_anhaengen(db, item, name)
|
||||||
@@ -405,19 +435,27 @@ def bearbeiten(
|
|||||||
if item is None:
|
if item is None:
|
||||||
raise HTTPException(status_code=404, detail="Nicht gefunden.")
|
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:
|
try:
|
||||||
namen = _bilder_speichern(dateien)
|
namen = _bilder_speichern(dateien)
|
||||||
except images.BildFehler as e:
|
except images.BildFehler as e:
|
||||||
return _weiter(f"/kleid/{item_id}/bearbeiten", str(e), "fehler")
|
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:
|
try:
|
||||||
crud.item_aendern(db, item, daten)
|
crud.item_aendern(db, item, daten)
|
||||||
except IntegrityError:
|
except IntegrityError:
|
||||||
|
|||||||
@@ -89,3 +89,19 @@ def test_name_ohne_werbung(client, item_id):
|
|||||||
json={"reserved_by": "Jetzt billig kaufen http://spam.example"},
|
json={"reserved_by": "Jetzt billig kaufen http://spam.example"},
|
||||||
)
|
)
|
||||||
assert antwort.status_code == 422
|
assert antwort.status_code == 422
|
||||||
|
|
||||||
|
|
||||||
|
def test_wettlauf_beim_reservieren_gewinnt_nur_einer(client, item_id, db_sitzung):
|
||||||
|
"""Zwei Anfragen können beide die Statusprüfung in der Route passieren,
|
||||||
|
bevor eine schreibt. Das UPDATE mit Status-Bedingung lässt trotzdem nur
|
||||||
|
eine gewinnen - die zweite bekommt None und überschreibt nichts."""
|
||||||
|
from app import crud
|
||||||
|
|
||||||
|
item = crud.item_holen(db_sitzung, item_id)
|
||||||
|
erster = crud.reservieren(db_sitzung, item, "A")
|
||||||
|
zweiter = crud.reservieren(db_sitzung, item, "B")
|
||||||
|
|
||||||
|
assert erster is not None
|
||||||
|
assert zweiter is None
|
||||||
|
assert item.reserved_by == "A"
|
||||||
|
assert item.reservation_token is not None
|
||||||
|
|||||||
@@ -263,3 +263,26 @@ def test_farbschema_skript_laeuft_vor_dem_zeichnen(client):
|
|||||||
|
|
||||||
def test_umschalter_ist_vorhanden(client):
|
def test_umschalter_ist_vorhanden(client):
|
||||||
assert 'id="schema-knopf"' in client.get("/").text
|
assert 'id="schema-knopf"' in client.get("/").text
|
||||||
|
|
||||||
|
|
||||||
|
# ------------------------------------------------- Unsinnige Eingaben ---
|
||||||
|
|
||||||
|
def test_unsinniges_limit_ist_kein_serverfehler(gast):
|
||||||
|
"""?limit= kommt aus der URL - Buchstaben oder negative Werte dürfen
|
||||||
|
keinen 500er auslösen, sondern fallen auf die Seitengrösse zurück."""
|
||||||
|
assert gast.get("/?limit=abc").status_code == 200
|
||||||
|
assert gast.get("/teil/liste?limit=-5").status_code == 200
|
||||||
|
|
||||||
|
|
||||||
|
def test_unsinnige_kategorie_gibt_meldung_statt_500(client, kategorie_id):
|
||||||
|
antwort = erfasse(client, kategorie_id, category_id="abc")
|
||||||
|
assert antwort.status_code == 200
|
||||||
|
assert "Bitte die Angaben prüfen" in antwort.text
|
||||||
|
|
||||||
|
|
||||||
|
def test_zu_langer_titel_gibt_meldung_statt_500(client, kategorie_id):
|
||||||
|
"""maxlength im HTML schützt nur den Browserweg - die Anwendung muss
|
||||||
|
es selbst abfangen."""
|
||||||
|
antwort = erfasse(client, kategorie_id, title="x" * 101)
|
||||||
|
assert antwort.status_code == 200
|
||||||
|
assert "Bitte die Angaben prüfen" in antwort.text
|
||||||
|
|||||||
Reference in New Issue
Block a user