release: 0.62.2 (bug/download-worker-redos-hotfix)

2026-08-12 18:50:02 +02:00
parent 6883fedc1b 4af919ac4d
commit e395fb816d
10 changed files with 179 additions and 14 deletions
@@ -1 +1 @@
-0.62.1
+0.62.2
@@ -25,6 +25,7 @@ import json
 import logging
 import math
 import queue
+import shutil
 import subprocess
 import threading
 import time
@@ -489,6 +490,61 @@ def build_poster_cmd(src: Path, dest: Path, at_s: float) -> list[str]:
    ]


+def build_thumbnail_jpg_cmd(src: Path, dest: Path) -> list[str]:
+    """Decode a downloaded thumbnail (webp/png/…) to a single baseline JPEG frame."""
+    return [
+        "ffmpeg", "-y", "-hide_banner", "-loglevel", "error", "-nostdin",
+        "-i", str(src), "-frames:v", "1", "-update", "1", str(dest),
+    ]
+
+
+def _run_ffmpeg_best_effort(cmd: list[str], timeout_s: int) -> Exception | None:
+    """Run a one-shot ffmpeg command, bounded by `timeout_s`. Returns None on success, or the
+    exception on a timeout/non-zero-exit/OS error — never raises. Shared by the best-effort image
+    steps (poster / thumbnail / storyboard) so the run+timeout+swallow policy lives in ONE place;
+    each caller logs its own context from the returned exception. (The download/transcode path uses
+    the richer `run_ffmpeg` instead, which streams progress and honours cooperative cancel.)"""
+    try:
+        subprocess.run(cmd, capture_output=True, timeout=timeout_s, check=True)
+        return None
+    except (subprocess.TimeoutExpired, subprocess.CalledProcessError, OSError) as exc:
+        return exc
+
+
+def _is_jpeg(path: Path) -> bool:
+    """True if the file starts with the JPEG SOI marker (FF D8 FF)."""
+    try:
+        with path.open("rb") as f:
+            return f.read(3) == b"\xff\xd8\xff"
+    except OSError:
+        return False
+
+
+def thumbnail_to_jpg(src: Path | None, dest: Path, timeout_s: int = 30) -> Path | None:
+    """Turn a downloaded thumbnail (webp / png / jpeg — whatever the source shipped) into a real
+    baseline JPEG for the `<name>.jpg` poster + og:image share sidecar.
+
+    Deliberately NOT yt-dlp's `FFmpegThumbnailsConvertor` (whose `-bsf:v mjpeg2jpeg` path crashes on
+    some already-JPEG thumbnails, e.g. Reddit's, and used to fail the entire download). A thumbnail
+    that is already JPEG is copied as-is; anything else is decoded via ffmpeg. Best-effort: returns
+    None on any failure, so the caller falls back to a cut video frame (`ensure_poster`) and the
+    download still succeeds."""
+    if src is None or not src.exists():
+        return None
+    if _is_jpeg(src):  # already JPEG (e.g. Facebook) — no decode/re-encode needed
+        try:
+            shutil.copyfile(src, dest)
+            return dest
+        except OSError as exc:
+            log.info("thumbnail copy failed (%s); poster falls back to a cut frame", str(exc)[:120])
+            return None
+    err = _run_ffmpeg_best_effort(build_thumbnail_jpg_cmd(src, dest), timeout_s)
+    if err is not None:
+        log.info("thumbnail->jpg conversion failed (%s); poster falls back to a cut frame", str(err)[:120])
+        return None
+    return dest if dest.exists() else None
+
+
 def ensure_poster(asset, download_root: str, timeout_s: int = 30) -> str | None:
    """Generate a poster frame for a ready asset that has no thumbnail (e.g. a direct media URL
    like a reddit HLS manifest, which yt-dlp can't get a thumbnail for). Written as the media's
@@ -506,10 +562,9 @@ def ensure_poster(asset, download_root: str, timeout_s: int = 30) -> str | None:
    if dest.exists():
        return rel
    at = min(3.0, (asset.duration_s or 10) * 0.1)
-    try:
-        subprocess.run(build_poster_cmd(src, dest, at), capture_output=True, timeout=timeout_s, check=True)
-    except (subprocess.TimeoutExpired, subprocess.CalledProcessError, OSError) as exc:
-        log.info("poster gen skipped for asset %s: %s", asset.id, str(exc)[:120])
+    err = _run_ffmpeg_best_effort(build_poster_cmd(src, dest, at), timeout_s)
+    if err is not None:
+        log.info("poster gen skipped for asset %s: %s", asset.id, str(err)[:120])
        return None
    return rel if dest.exists() else None

@@ -538,10 +593,9 @@ def ensure_storyboard(asset, download_root: str, timeout_s: int = 90) -> dict |
        return None
    dest.parent.mkdir(parents=True, exist_ok=True)
    cmd = build_storyboard_cmd(src, dest, asset.duration_s, geom)
-    try:
-        subprocess.run(cmd, capture_output=True, timeout=timeout_s, check=True)
-    except (subprocess.TimeoutExpired, subprocess.CalledProcessError, OSError) as exc:
-        log.info("storyboard gen skipped for asset %s: %s", asset.id, str(exc)[:120])
+    err = _run_ffmpeg_best_effort(cmd, timeout_s)
+    if err is not None:
+        log.info("storyboard gen skipped for asset %s: %s", asset.id, str(err)[:120])
        return None
    if not dest.exists():
        return None
@@ -21,6 +21,9 @@ _RULES: tuple[tuple[str, str], ...] = (
    ("available to this channel's members", "login_required"),
    ("join this channel", "login_required"),
    ("this video is only available to", "login_required"),
+    # Some extractors (e.g. Reddit) gate content behind a logged-in account: "Account
+    # authentication is required." — an auth wall we deliberately don't cross, not a transient error.
+    ("authentication is required", "login_required"),
    # Geo-restricted.
    ("in your country", "geo_blocked"),
    ("available in your location", "geo_blocked"),
@@ -211,9 +211,16 @@ def build_ydl_opts(
        opts["postprocessors"].append(
            {"key": "FFmpegMetadata", "add_chapters": True, "add_metadata": True}
        )
-    # Normalize the written thumbnail to jpg so we always have a `<name>.jpg` poster sidecar.
-    # `already_have_thumbnail` keeps that file on disk after embedding (default deletes it).
-    opts["postprocessors"].append({"key": "FFmpegThumbnailsConvertor", "format": "jpg"})
+    # NOTE: we deliberately do NOT add yt-dlp's `FFmpegThumbnailsConvertor` here. Its jpg conversion
+    # runs `ffmpeg -bsf:v mjpeg2jpeg` when the written thumbnail is already JPEG bytes (Reddit serves
+    # a JPEG under a `.png` name), and that bitstream filter *fails* on some such images — and since a
+    # postprocessor error is fatal, it used to kill the WHOLE download even though the media was fine.
+    # The `<name>.jpg` poster sidecar is instead produced robustly by our own pipeline (worker
+    # `_download` → `editmod.thumbnail_to_jpg`, with `ensure_poster` cutting a video frame as a
+    # fallback), so the og:image share preview (Facebook Messenger / MS Teams) keeps working.
+    # The opt-in EmbedThumbnail below is unaffected: it does its OWN format handling — it embeds a
+    # jpg/png thumbnail directly and converts anything else to PNG (not the crashing mjpeg2jpeg-to-jpg
+    # path), so it never needed the convertor and does not reintroduce the crash.
    if s["embed_thumbnail"]:
        opts["postprocessors"].append(
            {"key": "EmbedThumbnail", "already_have_thumbnail": True}
@@ -34,7 +34,15 @@ _ENGAGEMENT_PREFIX = re.compile(
    re.IGNORECASE,
 )
 _LETTER_RUN = re.compile(r"[^\W\d_]+", re.UNICODE)
-_TRAILING_HASHTAGS = re.compile(r"(?:\s+#[^\s#]*[^\W\d\s_][^\s#]*)+\s*$", re.UNICODE)
+# Trailing SEO hashtag cluster: a run of whitespace-separated `#tags` at the very end, where each
+# tag contains at least one letter (a bare numeric "#3" episode marker is NOT a hashtag and
+# survives). The "has a letter" test is a zero-width lookahead and the tag body is matched
+# possessively (`*+`), so there is exactly ONE way to split each tag and nothing to backtrack over.
+# The previous form `[^\s#]*[letter][^\s#]*` had TWO overlapping unbounded quantifiers around the
+# required letter: on a long hashtag run that ends with trailing non-tag text (a Facebook
+# "… #a #b #c | Page" title), the `$` anchor fails and the engine backtracks catastrophically
+# (ReDoS) — it pinned a worker thread at 100% CPU forever inside the download progress hook.
+_TRAILING_HASHTAGS = re.compile(r"(?:\s+#(?=[^\s#]*[^\W\d\s_])[^\s#]*+)+\s*$", re.UNICODE)
 _REPEAT_PUNCT = re.compile(r"([!?.,\-])\1{1,}")

 # Short function words → lowercased when the title is de-shouted (proper title-case style).
@@ -617,8 +617,12 @@ def _download(job_id: int, asset_id: int, source_kind: str, source_ref: str, spe
        layout, retention_days = _download_settings()
        rel = storage.rel_path(meta, ext, layout, asset_id=asset_id)
        thumb = _find_thumbnail(staging, produced)
+        # Convert the raw thumbnail (webp/png/jpeg) to a real JPEG ourselves — robustly — for the
+        # poster + og:image sidecar, now that yt-dlp's fatal thumbnail convertor is gone. On failure
+        # this is None and write_sidecars writes no poster, so ensure_poster cuts a frame instead.
+        poster_jpg = editmod.thumbnail_to_jpg(thumb, staging / "poster.jpg")
        storage.place_file(produced, settings.download_root, rel)
-        nfo_ok = storage.write_sidecars(settings.download_root, rel, meta, thumb)
+        nfo_ok = storage.write_sidecars(settings.download_root, rel, meta, poster_jpg)

        final = storage.abs_path(settings.download_root, rel)
        size = final.stat().st_size if final.exists() else None
@@ -12,6 +12,7 @@ def test_real_ytdlp_messages_map_to_categories():
        "ERROR: [youtube] abc: Sign in to confirm your age. This video may be inappropriate": "login_required",
        "ERROR: [youtube] abc: Sign in to confirm you're not a bot": "login_required",
        "ERROR: [youtube] abc: Join this channel to get access to members-only content": "login_required",
+        "ERROR: [Reddit] 1vi1x56: Account authentication is required.": "login_required",
        "ERROR: [youtube] abc: The uploader has not made this video available in your country": "geo_blocked",
        "ERROR: [youtube] abc: Requested format is not available": "format_unavailable",
        "ERROR: [youtube] abc: Only images are available for download": "format_unavailable",
@@ -0,0 +1,46 @@
+"""`thumbnail_to_jpg`: robustly turn a downloaded thumbnail into a real JPEG poster/og:image.
+
+This replaced yt-dlp's `FFmpegThumbnailsConvertor`, whose `-bsf:v mjpeg2jpeg` path crashed on some
+already-JPEG thumbnails (Reddit serves a JPEG under a `.png` name) — and because a postprocessor
+error is fatal, it used to fail the WHOLE download. The key regression property here is that a bad
+thumbnail returns None (poster falls back to a cut frame) instead of raising.
+"""
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+from app.downloads import edit
+
+_ffmpeg_missing = shutil.which("ffmpeg") is None
+needs_ffmpeg = pytest.mark.skipif(_ffmpeg_missing, reason="ffmpeg not available")
+
+
+def _is_jpeg(p: Path) -> bool:
+    return p.exists() and p.read_bytes()[:3] == b"\xff\xd8\xff"
+
+
+@needs_ffmpeg
+def test_converts_a_png_to_a_real_jpeg(tmp_path):
+    src = tmp_path / "thumb.png"
+    subprocess.run(
+        ["ffmpeg", "-y", "-loglevel", "error", "-f", "lavfi",
+         "-i", "color=c=red:s=64x64", "-frames:v", "1", str(src)],
+        check=True,
+    )
+    out = edit.thumbnail_to_jpg(src, tmp_path / "poster.jpg")
+    assert out is not None and _is_jpeg(out)
+
+
+def test_returns_none_for_a_missing_source(tmp_path):
+    assert edit.thumbnail_to_jpg(None, tmp_path / "p.jpg") is None
+    assert edit.thumbnail_to_jpg(tmp_path / "nope.png", tmp_path / "p.jpg") is None
+
+
+@needs_ffmpeg
+def test_returns_none_on_garbage_input_instead_of_raising(tmp_path):
+    # The whole point: a thumbnail ffmpeg can't decode must NOT crash the download.
+    bad = tmp_path / "bad.png"
+    bad.write_bytes(b"this is definitely not an image")
+    assert edit.thumbnail_to_jpg(bad, tmp_path / "p.jpg") is None
@@ -1,4 +1,9 @@
 """normalize_title: de-shout / de-clutter video titles without mangling non-English text."""
+from concurrent.futures import ThreadPoolExecutor
+from concurrent.futures import TimeoutError as FuturesTimeout
+
+import pytest
+
 from app.titles import normalize_title


@@ -24,6 +29,35 @@ def test_trailing_hashtags_are_stripped():
    assert normalize_title("Great cooking video #food #yummy") == "Great cooking video"


+def test_numeric_episode_marker_is_not_treated_as_a_hashtag():
+    # A bare "#3" is an episode marker, not an SEO hashtag (no letters) → it must survive.
+    assert normalize_title("My show #3") == "My show #3"
+
+
+def test_midstring_hashtags_dont_hang_and_are_kept_redos_regression():
+    # Facebook prepends an engagement prefix and appends a page name, leaving a long hashtag cluster
+    # in the MIDDLE. That is not a trailing SEO cluster, so the tags stay — and, critically, this
+    # exact shape used to trigger catastrophic backtracking (ReDoS) in `_TRAILING_HASHTAGS` that
+    # pinned a download-worker thread at 100% CPU forever inside the yt-dlp progress hook.
+    raw = (
+        "398K views · 6.1K reactions | The money should follow 🙄 #reel #reels #meme #memes "
+        "#fun #funny #work #worklife #office #viral #trending #fyp #foryou #foryoupage | Memebonanza4U"
+    )
+    # Run in a worker thread with a generous ceiling: the fixed regex returns in microseconds, so
+    # only a genuine catastrophic-backtracking regression fails to complete — a hang is caught
+    # deterministically rather than by asserting on wall-clock slack in the happy path.
+    ex = ThreadPoolExecutor(max_workers=1)
+    fut = ex.submit(normalize_title, raw)
+    try:
+        out = fut.result(timeout=5) or ""
+    except FuturesTimeout:
+        pytest.fail("normalize_title did not return within 5s — catastrophic backtracking regressed")
+    finally:
+        ex.shutdown(wait=False)  # never join a (hypothetically) wedged thread
+    assert "#foryoupage" in out and "#reel" in out  # mid-string hashtags preserved (not trailing)
+    assert "views" not in out  # the engagement-count prefix was still stripped
+
+
 def test_emoji_and_symbols_are_dropped():
    assert "🔥" not in (normalize_title("Fire recipe 🔥🔥") or "")

@@ -14,6 +14,14 @@ export interface ReleaseEntry {
 }

 export const RELEASE_NOTES: ReleaseEntry[] = [
+  {
+    version: "0.62.2",
+    date: "2026-08-12",
+    fixes: [
+      "Fixed a download that could get stuck and freeze the whole download queue — a video whose title had a long run of hashtags (common on Facebook reels) would hang and stop everything else from downloading. The queue keeps moving now.",
+      "Reddit links that failed right at the end while preparing the thumbnail now download correctly, and link-share previews (the thumbnail shown in Messenger / Teams) are generated more reliably.",
+    ],
+  },
  {
    version: "0.62.1",
    date: "2026-08-12",
@@ -1 +1 @@
 .62.1
 .62.2