fix(downloads): strip emoji from device filename + free disk when a download is deleted

Two UAT findings:
1. The device download filename (Content-Disposition) kept emoji/symbols from the video title.
   Add storage.display_filename (drops emoji/symbol/control unicode, keeps spaces + accents)
   and use it for the download name — readable and clean ("…alapján!.mp4", no emoji).
2. Deleting/canceling a download removed the job but the shared MediaAsset (and its file) lingered
   as cache, so 'Ready files' stayed inflated and disk wasn't freed. Rework: _release_asset drops
   the hold and, once no job holds the asset, deletes the file + row immediately (the cache only
   needs to span overlapping holders). Also fixes cancel never decrementing (it flipped status to
   'canceled' before releasing, tripping the holding-state guard).

Verified: filename emoji-stripped; enqueue→delete removes the asset row + file from disk.
This commit is contained in:
npeter83 2026-07-03 02:16:36 +02:00
parent 0045d41a74
commit f61ca96b6c
2 changed files with 31 additions and 11 deletions

View file

@ -63,6 +63,17 @@ def sanitize(name: str, limit: int = _MAX_TITLE) -> str:
return text or "untitled" return text or "untitled"
def display_filename(name: str) -> str:
"""User-facing download filename (Content-Disposition): strip emoji/symbols/control +
filesystem-illegal chars, but KEEP spaces, accents, and ordinary punctuation unlike the
underscore-joined on-disk name, this is the readable name the user saves to their device."""
text = unicodedata.normalize("NFKC", name or "")
text = "".join(ch for ch in text if unicodedata.category(ch) not in _DROP_CATEGORIES)
text = _ILLEGAL.sub("", text)
text = re.sub(r"\s+", " ", text).strip(" .")
return text or "download"
def rel_path(meta: MediaMeta, ext: str, layout: str = "plex") -> str: def rel_path(meta: MediaMeta, ext: str, layout: str = "plex") -> str:
"""Path of the media file relative to DOWNLOAD_ROOT (forward slashes).""" """Path of the media file relative to DOWNLOAD_ROOT (forward slashes)."""
title = sanitize(meta.title) title = sanitize(meta.title)

View file

@ -32,7 +32,6 @@ router = APIRouter(prefix="/api/downloads", tags=["downloads"])
admin_router = APIRouter(prefix="/api/admin/downloads", tags=["admin-downloads"]) admin_router = APIRouter(prefix="/api/admin/downloads", tags=["admin-downloads"])
_VIDEO_ID = re.compile(r"^[A-Za-z0-9_-]{11}$") _VIDEO_ID = re.compile(r"^[A-Za-z0-9_-]{11}$")
_BASENAME_ILLEGAL = re.compile(r'[<>:"/\\|?*\x00-\x1f]')
# --- source + serialization ---------------------------------------------------------------- # --- source + serialization ----------------------------------------------------------------
@ -376,8 +375,8 @@ def cancel_download(
) -> dict: ) -> dict:
job = _own_job(db, user, job_id) job = _own_job(db, user, job_id)
if job.status not in ("done", "canceled"): if job.status not in ("done", "canceled"):
_release_asset(db, job) # release while the job still counts as holding
job.status = "canceled" job.status = "canceled"
_decrement_ref(db, job)
db.commit() db.commit()
asset = db.get(MediaAsset, job.asset_id) if job.asset_id else None asset = db.get(MediaAsset, job.asset_id) if job.asset_id else None
return _serialize(job, asset) return _serialize(job, asset)
@ -388,25 +387,35 @@ def delete_download(
job_id: int, user: User = Depends(require_human), db: Session = Depends(get_db) job_id: int, user: User = Depends(require_human), db: Session = Depends(get_db)
) -> dict: ) -> dict:
job = _own_job(db, user, job_id) job = _own_job(db, user, job_id)
_decrement_ref(db, job) _release_asset(db, job)
db.delete(job) db.delete(job)
db.commit() db.commit()
return {"deleted": job_id} return {"deleted": job_id}
def _decrement_ref(db: Session, job: DownloadJob) -> None: def _release_asset(db: Session, job: DownloadJob) -> None:
"""Drop this job's hold on its asset. The asset itself lingers as cache until its TTL / """Drop this job's hold on its asset when it leaves a holding state. Once NO job holds the
eviction (so a re-add is still a cache hit); only the reference count changes here.""" asset anymore, delete the file + row immediately a deleted download should free its disk,
if job.asset_id and job.status not in ("canceled", "error"): and the shared cache only needs to span *overlapping* holders (a later re-add just downloads
asset = db.get(MediaAsset, job.asset_id) again). Idempotent: a job that's already left holding (canceled/error) is a no-op, so delete
if asset and asset.ref_count > 0: after cancel doesn't double-release."""
asset.ref_count -= 1 if not job.asset_id or job.status not in ("queued", "running", "paused", "done"):
return
asset = db.get(MediaAsset, job.asset_id)
if asset is None:
return
asset.ref_count = max(0, (asset.ref_count or 0) - 1)
if asset.ref_count == 0 and asset.status == "ready":
if asset.rel_path:
storage.delete_asset_files(settings.download_root, asset.rel_path)
db.delete(asset)
# --- file download (range-aware, custom display name) -------------------------------------- # --- file download (range-aware, custom display name) --------------------------------------
def _clean_basename(name: str) -> str: def _clean_basename(name: str) -> str:
return _BASENAME_ILLEGAL.sub("", name).strip() or "download" # Emoji/symbol-free but keeps spaces + accents (a readable device filename).
return storage.display_filename(name)
def _accessible_job(db: Session, user: User, job_id: int) -> DownloadJob: def _accessible_job(db: Session, user: User, job_id: int) -> DownloadJob: