fix: second-pass review bugs in server and core
- ExportRunner: stop batch on first error (was continuing, overwriting error status with done) - Export route: validate input_path against MEDIA_DIRS - Export route: validate encoder, portrait_ratio, folder_suffix, name - Export route: fix format check for WebP sequence - Export route: add _ separator in folder_suffix (match GUI) - Export route: use realpath consistently in delete endpoint - Export route: drop runner ref on completion (prevent memory leak) - ProcessedDB: use cursor-level row_factory (thread-safe) - WebSocket: catch all exceptions in connect, cleanup in finally - Dockerfile: use uvicorn[standard] for websockets support Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
+1
-1
@@ -3,6 +3,6 @@ RUN apt-get update && apt-get install -y ffmpeg && rm -rf /var/lib/apt/lists/*
|
|||||||
WORKDIR /app
|
WORKDIR /app
|
||||||
COPY core/ core/
|
COPY core/ core/
|
||||||
COPY server/ server/
|
COPY server/ server/
|
||||||
RUN pip install --no-cache-dir fastapi uvicorn
|
RUN pip install --no-cache-dir fastapi uvicorn[standard]
|
||||||
EXPOSE 8000
|
EXPOSE 8000
|
||||||
CMD ["uvicorn", "server.app:app", "--host", "0.0.0.0", "--port", "8000"]
|
CMD ["uvicorn", "server.app:app", "--host", "0.0.0.0", "--port", "8000"]
|
||||||
|
|||||||
+3
-3
@@ -121,14 +121,14 @@ class ProcessedDB:
|
|||||||
"""Return config dict for an output_path, or None."""
|
"""Return config dict for an output_path, or None."""
|
||||||
if not self._enabled:
|
if not self._enabled:
|
||||||
return None
|
return None
|
||||||
self._con.row_factory = sqlite3.Row
|
cur = self._con.cursor()
|
||||||
row = self._con.execute(
|
cur.row_factory = sqlite3.Row
|
||||||
|
row = cur.execute(
|
||||||
"SELECT label, category, short_side, portrait_ratio, crop_center, format,"
|
"SELECT label, category, short_side, portrait_ratio, crop_center, format,"
|
||||||
" clip_count, spread"
|
" clip_count, spread"
|
||||||
" FROM processed WHERE output_path = ?",
|
" FROM processed WHERE output_path = ?",
|
||||||
(output_path,),
|
(output_path,),
|
||||||
).fetchone()
|
).fetchone()
|
||||||
self._con.row_factory = None
|
|
||||||
return dict(row) if row else None
|
return dict(row) if row else None
|
||||||
|
|
||||||
def delete_by_output_path(self, output_path: str) -> None:
|
def delete_by_output_path(self, output_path: str) -> None:
|
||||||
|
|||||||
@@ -113,6 +113,7 @@ class ExportRunner:
|
|||||||
except Exception as e:
|
except Exception as e:
|
||||||
if "cancelled" not in str(e) and self._on_error:
|
if "cancelled" not in str(e) and self._on_error:
|
||||||
self._on_error(str(e))
|
self._on_error(str(e))
|
||||||
|
return
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
if self._on_error:
|
if self._on_error:
|
||||||
self._on_error(str(e))
|
self._on_error(str(e))
|
||||||
|
|||||||
+41
-12
@@ -8,14 +8,16 @@ from pydantic import BaseModel
|
|||||||
|
|
||||||
from core.export import ExportRunner
|
from core.export import ExportRunner
|
||||||
from core.paths import build_export_path, build_sequence_dir
|
from core.paths import build_export_path, build_sequence_dir
|
||||||
from core.ffmpeg import apply_keyframes_to_jobs
|
from core.ffmpeg import _RATIOS, apply_keyframes_to_jobs
|
||||||
from .. import ws as ws_module
|
from .. import ws as ws_module
|
||||||
from ..config import EXPORT_DIR
|
from ..config import EXPORT_DIR, MEDIA_DIRS
|
||||||
|
|
||||||
router = APIRouter()
|
router = APIRouter()
|
||||||
|
|
||||||
_jobs: dict[str, dict] = {}
|
_jobs: dict[str, dict] = {}
|
||||||
|
|
||||||
|
_VALID_ENCODERS = {"libx264", "h264_nvenc", "h264_vaapi", "h264_qsv", "h264_amf", "h264_videotoolbox"}
|
||||||
|
|
||||||
|
|
||||||
class ExportRequest(BaseModel):
|
class ExportRequest(BaseModel):
|
||||||
input_path: str
|
input_path: str
|
||||||
@@ -49,16 +51,41 @@ def _next_counter(folder: str, basename: str) -> int:
|
|||||||
return highest + 1
|
return highest + 1
|
||||||
|
|
||||||
|
|
||||||
|
def _validate_input_path(path: str) -> str:
|
||||||
|
"""Verify input_path falls under a configured MEDIA_DIR."""
|
||||||
|
real = os.path.realpath(path)
|
||||||
|
for root in MEDIA_DIRS:
|
||||||
|
root_real = os.path.realpath(root)
|
||||||
|
if real == root_real or real.startswith(root_real + os.sep):
|
||||||
|
return real
|
||||||
|
raise HTTPException(status_code=403, detail="input_path outside media directories")
|
||||||
|
|
||||||
|
|
||||||
@router.post("/export")
|
@router.post("/export")
|
||||||
def start_export(req: ExportRequest):
|
def start_export(req: ExportRequest):
|
||||||
from ..app import db
|
from ..app import db
|
||||||
|
|
||||||
|
# Validate inputs
|
||||||
|
input_path = _validate_input_path(req.input_path)
|
||||||
|
|
||||||
|
if req.encoder not in _VALID_ENCODERS:
|
||||||
|
raise HTTPException(status_code=400, detail=f"invalid encoder: {req.encoder}")
|
||||||
|
|
||||||
|
if req.portrait_ratio is not None and req.portrait_ratio not in _RATIOS:
|
||||||
|
raise HTTPException(status_code=400, detail=f"invalid portrait_ratio: {req.portrait_ratio}")
|
||||||
|
|
||||||
|
if req.folder_suffix and ("/" in req.folder_suffix or "\\" in req.folder_suffix or ".." in req.folder_suffix):
|
||||||
|
raise HTTPException(status_code=400, detail="folder_suffix must not contain path separators")
|
||||||
|
|
||||||
|
if "/" in req.name or "\\" in req.name or ".." in req.name:
|
||||||
|
raise HTTPException(status_code=400, detail="name must not contain path separators")
|
||||||
|
|
||||||
job_id = str(uuid.uuid4())[:8]
|
job_id = str(uuid.uuid4())[:8]
|
||||||
folder = EXPORT_DIR
|
folder = EXPORT_DIR
|
||||||
if req.folder_suffix:
|
if req.folder_suffix:
|
||||||
folder = folder + req.folder_suffix
|
folder = folder.rstrip(os.sep) + "_" + req.folder_suffix
|
||||||
|
|
||||||
image_sequence = req.format == "WebP"
|
image_sequence = req.format in ("WebP", "WebP sequence")
|
||||||
counter = _next_counter(folder, req.name)
|
counter = _next_counter(folder, req.name)
|
||||||
|
|
||||||
# Build job list: (start, output_path, portrait_ratio, crop_center)
|
# Build job list: (start, output_path, portrait_ratio, crop_center)
|
||||||
@@ -87,7 +114,7 @@ def start_export(req: ExportRequest):
|
|||||||
completed.append(path)
|
completed.append(path)
|
||||||
# Record in DB so markers show up
|
# Record in DB so markers show up
|
||||||
db.add(
|
db.add(
|
||||||
filename=os.path.basename(req.input_path),
|
filename=os.path.basename(input_path),
|
||||||
start_time=req.cursor,
|
start_time=req.cursor,
|
||||||
output_path=path,
|
output_path=path,
|
||||||
label=req.label,
|
label=req.label,
|
||||||
@@ -104,15 +131,17 @@ def start_export(req: ExportRequest):
|
|||||||
|
|
||||||
def on_all_done():
|
def on_all_done():
|
||||||
_jobs[job_id]["status"] = "done"
|
_jobs[job_id]["status"] = "done"
|
||||||
|
_jobs[job_id].pop("runner", None)
|
||||||
ws_module.broadcast({"type": "all_done", "job_id": job_id})
|
ws_module.broadcast({"type": "all_done", "job_id": job_id})
|
||||||
|
|
||||||
def on_error(msg: str):
|
def on_error(msg: str):
|
||||||
_jobs[job_id]["status"] = "error"
|
_jobs[job_id]["status"] = "error"
|
||||||
_jobs[job_id]["error"] = msg
|
_jobs[job_id]["error"] = msg
|
||||||
|
_jobs[job_id].pop("runner", None)
|
||||||
ws_module.broadcast({"type": "error", "job_id": job_id, "msg": msg})
|
ws_module.broadcast({"type": "error", "job_id": job_id, "msg": msg})
|
||||||
|
|
||||||
runner = ExportRunner(
|
runner = ExportRunner(
|
||||||
input_path=req.input_path,
|
input_path=input_path,
|
||||||
jobs=jobs,
|
jobs=jobs,
|
||||||
short_side=req.short_side,
|
short_side=req.short_side,
|
||||||
image_sequence=image_sequence,
|
image_sequence=image_sequence,
|
||||||
@@ -154,9 +183,9 @@ def delete_export(output_path: str):
|
|||||||
real = os.path.realpath(output_path)
|
real = os.path.realpath(output_path)
|
||||||
if not real.startswith(os.path.realpath(EXPORT_DIR) + os.sep):
|
if not real.startswith(os.path.realpath(EXPORT_DIR) + os.sep):
|
||||||
raise HTTPException(status_code=403, detail="path outside export directory")
|
raise HTTPException(status_code=403, detail="path outside export directory")
|
||||||
db.delete_by_output_path(output_path)
|
db.delete_by_output_path(real)
|
||||||
if os.path.isfile(output_path):
|
if os.path.isfile(real):
|
||||||
os.unlink(output_path)
|
os.unlink(real)
|
||||||
elif os.path.isdir(output_path):
|
elif os.path.isdir(real):
|
||||||
shutil.rmtree(output_path)
|
shutil.rmtree(real)
|
||||||
return {"deleted": output_path}
|
return {"deleted": real}
|
||||||
|
|||||||
+3
-1
@@ -18,7 +18,9 @@ async def connect(ws: WebSocket):
|
|||||||
try:
|
try:
|
||||||
while True:
|
while True:
|
||||||
await ws.receive_text() # keep alive
|
await ws.receive_text() # keep alive
|
||||||
except WebSocketDisconnect:
|
except (WebSocketDisconnect, Exception):
|
||||||
|
pass
|
||||||
|
finally:
|
||||||
with _lock:
|
with _lock:
|
||||||
if ws in _connections:
|
if ws in _connections:
|
||||||
_connections.remove(ws)
|
_connections.remove(ws)
|
||||||
|
|||||||
Reference in New Issue
Block a user