peter
b4bf71ea64
- The cut file is measured against what its owner has left, for a YouTube range (whose fetch never reads yt-dlp's size ceiling) and for a Plex clip (which fetches nothing), before the fetch and again before it is placed; a refusal leaves the asset pending for a Retry. - A silent Plex file is cut silent; a file the probe cannot read says so. - A second job on a failed or renamed clip gets the same YouTube diagnosis and the fetched title. - The "too long" refusal names the server's cap; a contract test holds the client's copies of the clip limits to clip.py; the shipped player clients are pinned against the installed yt-dlp. - One HDR transfer set and one rating_key lookup; job status words from JobStatus; the display-name guard sees subscript writes and the clip route; stale comments and types follow the clip job.
1208 lines
55 KiB
Python
1208 lines
55 KiB
Python
"""Local file upload ingest (the non-yt-dlp path into the media library).
|
|
|
|
Covers the ingest spine: media vs generic-file classification, the download-only serving of a
|
|
generic file (force-download so uploaded HTML/SVG can't execute inline), quota accounting with the
|
|
size known up front, the per-file cap enforced while streaming, filename sanitization, and the two
|
|
guards that keep an upload out of paths that assume a re-fetchable source (edit / redownload).
|
|
|
|
DB-backed (real Postgres) — skipped when no database is reachable. `settings.download_root` is
|
|
pointed at a per-test tmp dir so ingest's on-disk moves are isolated. The ingest/route tests
|
|
monkeypatch the shared `probe.probe_json` so they don't shell out to ffprobe; the classifier's OWN
|
|
parsing (image → file, cover-art skip, dims, duration) is covered directly by `TestProbeClassifier`
|
|
below, which stubs one level down (`run_ffprobe_json`) and exercises the real `probe_media`. The
|
|
phone-safe conversion of an upload runs the real ffmpeg (`TestPhoneSafeUpload`).
|
|
"""
|
|
|
|
import ast
|
|
import asyncio
|
|
import io
|
|
import pathlib
|
|
import shutil
|
|
import subprocess
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
from fastapi import HTTPException
|
|
|
|
from app.config import settings
|
|
from app.routes._common import capped_stream_to
|
|
from sqlalchemy import select
|
|
|
|
from app.downloads import gc as gcmod
|
|
from app.downloads import mediakind, pipeline, playable, quota, service, storage
|
|
from app.downloads import probe as probemod
|
|
from app.downloads import upload as uploadmod
|
|
from app.downloads.stated import StatedFailure
|
|
from app.models import DISPLAY_TEXT_MAX, DownloadJob, DownloadQuota, MediaAsset
|
|
|
|
|
|
# --- pure: the kind invariant ---------------------------------------------------------------
|
|
|
|
|
|
def test_derive_media_kind():
|
|
assert uploadmod.mediakind.derive("h264", "aac") == mediakind.VIDEO
|
|
assert uploadmod.mediakind.derive("vp9", None) == mediakind.VIDEO
|
|
assert uploadmod.mediakind.derive(None, "aac") == mediakind.AUDIO
|
|
assert uploadmod.mediakind.derive(None, None) == mediakind.FILE
|
|
|
|
|
|
# --- ingest (direct) ------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.fixture
|
|
def _root(tmp_path, monkeypatch):
|
|
monkeypatch.setattr(settings, "download_root", str(tmp_path))
|
|
return tmp_path
|
|
|
|
|
|
def _probe(monkeypatch, result):
|
|
"""Stub the shared ffprobe call (`probe.probe_json`, the same ground truth the download worker
|
|
reads) with the JSON a file of these (vcodec, acodec, width, height, duration) would give — its
|
|
video 8-bit 4:2:0, so an H.264 + AAC .mp4 is already phone-safe and ingests straight to `done`.
|
|
All-None is a file ffprobe could not read."""
|
|
v, a, w, h, d = result
|
|
streams = []
|
|
if v:
|
|
streams.append(
|
|
{"codec_type": "video", "codec_name": v, "pix_fmt": "yuv420p", "width": w, "height": h}
|
|
)
|
|
if a:
|
|
streams.append({"codec_type": "audio", "codec_name": a})
|
|
data = {"streams": streams, "format": {"format_name": "mov,mp4", "duration": d}}
|
|
monkeypatch.setattr(probemod, "probe_json", lambda _p: data if streams else None)
|
|
|
|
|
|
def test_ingest_generic_file_is_download_only(db, make_user, _root, monkeypatch):
|
|
_probe(monkeypatch, (None, None, None, None, None))
|
|
u = make_user()
|
|
src = _root / "src.bin"
|
|
src.write_bytes(b"%PDF-1.4 hello")
|
|
job = uploadmod.ingest_upload(
|
|
db, u.id, filename="My Notes.pdf", src_path=src, size=src.stat().st_size
|
|
)
|
|
asset = db.get(MediaAsset, job.asset_id)
|
|
assert asset.media_kind == mediakind.FILE
|
|
assert asset.status == "ready" and job.status == "done" and job.progress == 100
|
|
assert asset.container == "pdf"
|
|
assert asset.poster_path is None # a generic file has no player and no poster
|
|
assert asset.title == "My Notes" # display title from the original name
|
|
assert job.job_kind == "upload" and job.source_kind == "upload"
|
|
# The file was MOVED into the per-user uploads tree, inside the root, and the temp is gone.
|
|
assert not src.exists()
|
|
assert asset.rel_path.startswith(".uploads/")
|
|
assert storage.safe_abs_path(str(_root), asset.rel_path) is not None
|
|
|
|
|
|
def test_ingest_audio_is_media_no_poster(db, make_user, _root, monkeypatch):
|
|
_probe(monkeypatch, (None, "aac", None, None, 200))
|
|
u = make_user()
|
|
src = _root / "song.m4a"
|
|
src.write_bytes(b"fake-audio")
|
|
job = uploadmod.ingest_upload(db, u.id, filename="song.m4a", src_path=src, size=10)
|
|
asset = db.get(MediaAsset, job.asset_id)
|
|
assert asset.media_kind == mediakind.AUDIO
|
|
assert asset.duration_s == 200
|
|
assert asset.poster_path is None # audio has no frame to poster
|
|
|
|
|
|
def test_ingest_video_records_dims_and_poster(db, make_user, _root, monkeypatch):
|
|
_probe(monkeypatch, ("h264", "aac", 640, 480, 60))
|
|
# Stub the frame-cut so the test doesn't shell out to ffmpeg.
|
|
monkeypatch.setattr(uploadmod.editmod, "ensure_poster", lambda _a, _root: "p/x.jpg")
|
|
u = make_user()
|
|
src = _root / "clip.mp4"
|
|
src.write_bytes(b"fake-video")
|
|
job = uploadmod.ingest_upload(db, u.id, filename="clip.mp4", src_path=src, size=10)
|
|
asset = db.get(MediaAsset, job.asset_id)
|
|
assert asset.media_kind == mediakind.VIDEO
|
|
assert (asset.width, asset.height) == (640, 480)
|
|
assert asset.poster_path == "p/x.jpg"
|
|
|
|
|
|
def test_ingest_filename_is_sanitized_no_traversal(db, make_user, _root, monkeypatch):
|
|
_probe(monkeypatch, (None, None, None, None, None))
|
|
u = make_user()
|
|
src = _root / "src"
|
|
src.write_bytes(b"x")
|
|
job = uploadmod.ingest_upload(db, u.id, filename="../../etc/passwd", src_path=src, size=1)
|
|
asset = db.get(MediaAsset, job.asset_id)
|
|
assert ".." not in asset.rel_path
|
|
assert asset.rel_path.startswith(f".uploads/{u.id}/")
|
|
assert storage.safe_abs_path(str(_root), asset.rel_path) is not None
|
|
|
|
|
|
def test_ingest_rejects_over_footprint_cap_before_writing(db, make_user, _root, monkeypatch):
|
|
_probe(monkeypatch, (None, None, None, None, None))
|
|
u = make_user()
|
|
db.add(
|
|
DownloadQuota(user_id=u.id, max_bytes=1000, max_concurrent=2, max_jobs=50, unlimited=False)
|
|
)
|
|
db.commit()
|
|
src = _root / "big.bin"
|
|
src.write_bytes(b"x" * 10)
|
|
with pytest.raises(quota.QuotaExceeded) as e:
|
|
uploadmod.ingest_upload(db, u.id, filename="big.bin", src_path=src, size=2000)
|
|
assert e.value.reason == "max_bytes"
|
|
assert src.exists() # rejected before any disk move — nothing landed
|
|
assert db.query(MediaAsset).count() == 0 # and no orphan asset row
|
|
|
|
|
|
# --- route: the auth-first body ------------------------------------------------------------
|
|
|
|
|
|
def _counting_body(chunks: int = 4, size: int = 1024):
|
|
"""A request body that records how much of itself the server actually pulled.
|
|
|
|
httpx's ASGI transport feeds the app lazily, one `http.request` message per yielded chunk, so a
|
|
chunk that was never yielded is a chunk the app never asked for. That is the whole assertion
|
|
below: it is not "the upload was refused", it is "the refusal happened before the bytes moved".
|
|
"""
|
|
pulled: list[int] = []
|
|
|
|
def body():
|
|
for _ in range(chunks):
|
|
pulled.append(size)
|
|
yield b"x" * size
|
|
|
|
return body(), pulled
|
|
|
|
|
|
_BOUNDARY = "p1boundary"
|
|
|
|
|
|
def _counting_multipart(chunks: int = 4, size: int = 1024):
|
|
"""The same counter, wrapped in a real multipart envelope.
|
|
|
|
The content-type is the load-bearing part. Starlette's `request.form()` returns an EMPTY
|
|
FormData without touching the stream when the content-type is not multipart/urlencoded — so a
|
|
raw-body probe cannot tell "the route stopped reading strangers" apart from "there was nothing
|
|
here to parse". Only a well-formed multipart body makes the old shape actually spool.
|
|
"""
|
|
pulled: list[int] = []
|
|
head = (
|
|
f"--{_BOUNDARY}\r\n"
|
|
'Content-Disposition: form-data; name="file"; filename="clip.mp4"\r\n'
|
|
"Content-Type: application/octet-stream\r\n\r\n"
|
|
).encode()
|
|
tail = f"\r\n--{_BOUNDARY}--\r\n".encode()
|
|
|
|
def body():
|
|
pulled.append(len(head))
|
|
yield head
|
|
for _ in range(chunks):
|
|
pulled.append(size)
|
|
yield b"x" * size
|
|
pulled.append(len(tail))
|
|
yield tail
|
|
|
|
return body(), pulled
|
|
|
|
|
|
def test_an_anonymous_upload_is_refused_before_a_byte_of_it_is_read(client):
|
|
"""The defect an earlier fix bounded but did not close.
|
|
|
|
While the route declared `file: UploadFile = File(...)`, FastAPI parsed and spooled the body
|
|
BEFORE the first dependency ran, so the auth dependency could not gate it: an unauthenticated
|
|
caller could make the server write the whole per-file allowance (500 MiB by default) to temp,
|
|
per request. SEC-20's ceiling capped the number; it could not stop the writing.
|
|
|
|
The route now reads its own body from a dependency declared AFTER the auth one, the shape
|
|
`POST /api/debug-sessions/{sid}/screenshots` already uses. The claim under test is the strong
|
|
one — a stranger's bytes are never read WHATEVER they claim to send, so the probe sends the
|
|
multipart the old route would have spooled, not the raw body the new one expects."""
|
|
body, pulled = _counting_multipart()
|
|
r = client.post(
|
|
"/api/downloads/upload",
|
|
content=body,
|
|
headers={"content-type": f"multipart/form-data; boundary={_BOUNDARY}"},
|
|
)
|
|
assert r.status_code == 401
|
|
assert pulled == [], f"the server read {sum(pulled)} bytes from an anonymous caller"
|
|
|
|
|
|
def test_an_authenticated_upload_still_has_its_body_read(as_user, make_user, _root, monkeypatch):
|
|
"""The mutation check for the test above: a route that read NOBODY's body would pass it just as
|
|
well as the fixed one. The same counting body, signed in, must be pulled in full."""
|
|
_probe(monkeypatch, (None, None, None, None, None))
|
|
body, pulled = _counting_body()
|
|
c = as_user(make_user())
|
|
r = c.post(
|
|
"/api/downloads/upload",
|
|
content=body,
|
|
headers={"content-type": "application/octet-stream", "x-upload-filename": "clip.bin"},
|
|
)
|
|
assert r.status_code == 200, r.text
|
|
assert sum(pulled) == 4096
|
|
|
|
|
|
def test_capped_stream_to_drains_the_rest_before_refusing():
|
|
"""Measured in a real browser, then pinned HERE rather than at the route.
|
|
|
|
The cap now fires while the client is still sending, and answering + closing mid-upload is what
|
|
a browser reports as a NETWORK failure: the dialog said "the server couldn't be reached"
|
|
instead of naming the limit, because XHR never got to read the 422. `capped_stream_to` reads
|
|
the remainder to nowhere before raising, so the client finishes sending and reads the real
|
|
answer.
|
|
|
|
It is asserted against the helper and not through TestClient on purpose — httpx's ASGI
|
|
transport pumps the request body whether or not the app reads it, so a route-level version of
|
|
this test passes just as happily with the drain deleted. It was written that way first and
|
|
caught by its own mutation check.
|
|
"""
|
|
pulled = []
|
|
|
|
async def stream():
|
|
for i in range(8):
|
|
pulled.append(i)
|
|
yield b"x" * 1024
|
|
|
|
class _Req:
|
|
def stream(self):
|
|
return stream()
|
|
|
|
out = io.BytesIO()
|
|
refusal = HTTPException(status_code=422, detail={"code": "upload_too_large", "limit": 2048})
|
|
# `asyncio.run` rather than an async test: the suite has no async plugin, and adding one to
|
|
# pin four lines of helper would mean a new dev dependency and a lock regeneration.
|
|
with pytest.raises(HTTPException) as exc:
|
|
asyncio.run(capped_stream_to(_Req(), out, 2048, refusal))
|
|
assert exc.value is refusal
|
|
assert len(pulled) == 8, "the body was abandoned mid-stream, not drained"
|
|
# Drained means DISCARDED: nothing past the limit is written.
|
|
assert len(out.getvalue()) <= 2048
|
|
|
|
|
|
# --- route ----------------------------------------------------------------------------------
|
|
|
|
|
|
def test_upload_route_generic_forces_download(
|
|
as_user, make_user, _root, monkeypatch, upload_headers
|
|
):
|
|
_probe(monkeypatch, (None, None, None, None, None))
|
|
c = as_user(make_user())
|
|
r = c.post(
|
|
"/api/downloads/upload",
|
|
content=b"<script>alert(1)</script>",
|
|
headers=upload_headers("report.html"),
|
|
)
|
|
assert r.status_code == 200, r.text
|
|
job = r.json()
|
|
assert job["asset"]["media_kind"] == "file"
|
|
assert job["can_download"] is True
|
|
# The served file must be a neutral force-download attachment — never inline text/html.
|
|
fr = c.get(f"/api/downloads/{job['id']}/file")
|
|
assert fr.status_code == 200
|
|
assert fr.headers["content-type"].startswith("application/octet-stream")
|
|
assert "attachment" in fr.headers["content-disposition"].lower()
|
|
|
|
|
|
def test_upload_route_media_html_extension_not_served_as_html(
|
|
db, as_user, make_user, _root, monkeypatch, upload_headers
|
|
):
|
|
"""The XSS guard: a MEDIA-classified upload (real codecs) named with an executable extension must
|
|
NOT be served with a guessed text/html type. The served Content-Type comes from the verified
|
|
media_kind + a container allow-list ('html' isn't one), so it force-downloads as octet-stream —
|
|
on both the authenticated file endpoint AND the login-free public watch endpoint. Without the fix
|
|
the public inline branch let a media/HTML polyglot named evil.html run as stored XSS."""
|
|
_probe(monkeypatch, ("h264", "aac", 320, 240, 5)) # ffprobe says it's video…
|
|
c = as_user(make_user())
|
|
r = c.post(
|
|
"/api/downloads/upload",
|
|
content=b"<script>alert(1)</script>",
|
|
headers=upload_headers("evil.html"),
|
|
)
|
|
assert r.status_code == 200, r.text
|
|
job = r.json()
|
|
assert job["asset"]["media_kind"] == "video" and job["asset"]["container"] == "html"
|
|
# Media in no phone-safe container is queued for conversion to .mp4, so today it is never served
|
|
# as .html at all. The serving guard below is the second line: a row from before that rule (or
|
|
# one whose conversion is skipped some day) must still not run as HTML.
|
|
assert job["status"] == "queued"
|
|
row = db.get(DownloadJob, job["id"])
|
|
db.get(MediaAsset, row.asset_id).status = "ready"
|
|
row.status = "done"
|
|
db.commit()
|
|
|
|
# Authenticated endpoint: never text/html.
|
|
fr = c.get(f"/api/downloads/{job['id']}/file")
|
|
assert fr.status_code == 200
|
|
assert fr.headers["content-type"].startswith("application/octet-stream")
|
|
assert "attachment" in fr.headers["content-disposition"].lower()
|
|
|
|
# Public login-free endpoint via a STREAM-ONLY link (the inline branch) — still not text/html.
|
|
link = c.post(f"/api/downloads/{job['id']}/links", json={"allow_download": False}).json()
|
|
pr = c.get(f"/api/public/watch/{link['token']}/file")
|
|
assert pr.status_code == 200
|
|
assert not pr.headers["content-type"].startswith("text/html")
|
|
assert pr.headers["content-type"].startswith("application/octet-stream")
|
|
assert "attachment" in pr.headers["content-disposition"].lower()
|
|
|
|
|
|
def test_public_watch_file_forces_download_for_generic_file(
|
|
as_user, make_user, _root, monkeypatch, upload_headers
|
|
):
|
|
"""The login-free public serve of a generic upload (media_kind='file') must be a neutral
|
|
force-download attachment — the branch that stops uploaded HTML/SVG executing on the public
|
|
origin. This is a SEPARATE code path from the authenticated /file route, so it needs its own
|
|
route-level assertion (previously only the authenticated branch was tested)."""
|
|
_probe(monkeypatch, (None, None, None, None, None))
|
|
c = as_user(make_user())
|
|
job = c.post(
|
|
"/api/downloads/upload",
|
|
content=b"<script>alert(1)</script>",
|
|
headers=upload_headers("notes.html"),
|
|
).json()
|
|
assert job["asset"]["media_kind"] == "file"
|
|
link = c.post(f"/api/downloads/{job['id']}/links", json={"allow_download": True}).json()
|
|
pr = c.get(f"/api/public/watch/{link['token']}/file")
|
|
assert pr.status_code == 200
|
|
assert pr.headers["content-type"].startswith("application/octet-stream")
|
|
assert "attachment" in pr.headers["content-disposition"].lower()
|
|
|
|
|
|
def test_public_watch_file_serves_media_inline_with_real_type(
|
|
as_user, make_user, _root, monkeypatch, upload_headers
|
|
):
|
|
"""A recognised media container IS still served inline with its real audio/video type (so it
|
|
plays in the watch page) — the fix must not break legitimate playback. A stream-only link →
|
|
inline; the type is video/mp4 (from the container allow-list), never text/* and never a guess."""
|
|
_probe(monkeypatch, ("h264", "aac", 640, 360, 12))
|
|
c = as_user(make_user())
|
|
job = c.post(
|
|
"/api/downloads/upload",
|
|
content=b"fake-mp4-bytes",
|
|
headers=upload_headers("clip.mp4"),
|
|
).json()
|
|
link = c.post(f"/api/downloads/{job['id']}/links", json={"allow_download": False}).json()
|
|
pr = c.get(f"/api/public/watch/{link['token']}/file")
|
|
assert pr.status_code == 200
|
|
assert pr.headers["content-type"].startswith("video/mp4")
|
|
assert "inline" in pr.headers["content-disposition"].lower()
|
|
|
|
|
|
def test_upload_route_enforces_per_file_cap(
|
|
as_user, make_user, db, _root, monkeypatch, upload_headers
|
|
):
|
|
from app import sysconfig
|
|
|
|
sysconfig.set_value(db, "download_upload_max_bytes", 5)
|
|
db.commit()
|
|
c = as_user(make_user())
|
|
r = c.post(
|
|
"/api/downloads/upload",
|
|
content=b"way over the cap",
|
|
headers=upload_headers("big.bin"),
|
|
)
|
|
assert r.status_code == 422
|
|
assert r.json()["detail"]["code"] == "upload_too_large"
|
|
|
|
|
|
def test_the_cap_admits_a_body_of_EXACTLY_the_cap_and_refuses_one_byte_more(
|
|
as_user, make_user, db, _root, monkeypatch, upload_headers
|
|
):
|
|
"""The boundary, which the test above cannot see: it sends 16 bytes against a cap of 5, so a
|
|
`>` silently loosened to `>=` refuses it just the same and the mutation survives the whole
|
|
suite. The cap is `size > limit`, so a file of exactly the cap must be ACCEPTED.
|
|
"""
|
|
from app import sysconfig
|
|
|
|
_probe(monkeypatch, (None, None, None, None, None))
|
|
payload = b"x" * 64
|
|
sysconfig.set_value(db, "download_upload_max_bytes", len(payload))
|
|
db.commit()
|
|
c = as_user(make_user())
|
|
|
|
at_the_cap = c.post(
|
|
"/api/downloads/upload", content=payload, headers=upload_headers("exact.bin")
|
|
)
|
|
assert at_the_cap.status_code == 200, at_the_cap.text
|
|
|
|
over = c.post(
|
|
"/api/downloads/upload", content=payload + b"x", headers=upload_headers("over.bin")
|
|
)
|
|
assert over.status_code == 422
|
|
assert over.json()["detail"] == {"code": "upload_too_large", "limit": len(payload)}
|
|
|
|
|
|
def test_a_non_ascii_filename_survives_the_header_round_trip(
|
|
as_user, make_user, db, _root, monkeypatch, upload_headers
|
|
):
|
|
"""The percent-decoding, which every other test in this file leaves vacuous.
|
|
|
|
Headers are latin-1 on the wire, so the client `encodeURIComponent`s the name and the route
|
|
`unquote`s it. Every filename the suite sends is pure ASCII — and `quote()` is the IDENTITY
|
|
function on ASCII — so deleting the `unquote` call changes nothing anywhere else. A Hungarian
|
|
name is the only input that can tell the two apart: without the decode the sanitizer would be
|
|
handed the literal `nyaral%C3%A1s.bin` and would keep the percent-escapes in the stored name.
|
|
"""
|
|
_probe(monkeypatch, (None, None, None, None, None))
|
|
c = as_user(make_user())
|
|
r = c.post(
|
|
"/api/downloads/upload", content=b"data", headers=upload_headers("nyaralás ősszel.bin")
|
|
)
|
|
assert r.status_code == 200, r.text
|
|
asset = db.execute(select(MediaAsset).order_by(MediaAsset.id.desc())).scalars().first()
|
|
assert asset is not None
|
|
stored = Path(asset.rel_path).name
|
|
assert "%" not in stored and "c3" not in stored.lower(), stored
|
|
assert stored.startswith("nyaralas_osszel"), stored
|
|
|
|
|
|
def test_a_long_display_name_is_truncated_on_EVERY_route_that_takes_one(
|
|
db, as_user, make_user, _root, monkeypatch, upload_headers
|
|
):
|
|
"""The sweep, held end to end rather than at one site (the clip route joined it later).
|
|
|
|
`display_text` was introduced for the upload path and applied to three of the five writers of
|
|
this varchar(255) column; `POST /api/downloads` and `POST /api/downloads/edit` kept the old
|
|
`(x or "").strip() or None` and would still have answered a bare 500 (a psycopg DataError,
|
|
which main.py re-raises for everything but the NUL case) to a name a user simply pasted. Found
|
|
by four independent readers of one review — a sweep is exactly the kind of thing that
|
|
gets done four-fifths of the way, so this holds all of it at once.
|
|
"""
|
|
_probe(monkeypatch, ("h264", "aac", 640, 480, 60))
|
|
monkeypatch.setattr(uploadmod.editmod, "ensure_poster", lambda _a, _root: None)
|
|
long_name = "n" * 300
|
|
bound = "n" * DISPLAY_TEXT_MAX
|
|
c = as_user(make_user())
|
|
|
|
# 1. the upload route — the name arrives in a request header.
|
|
up = c.post(
|
|
"/api/downloads/upload", content=b"video", headers=upload_headers("a.mp4", long_name)
|
|
)
|
|
assert up.status_code == 200, up.text
|
|
assert up.json()["display_name"] == bound
|
|
|
|
# 2. the enqueue route — a plain JSON field, from the Add-download dialog. An inline `spec`
|
|
# rather than a profile id, so the test does not have to seed the builtin profile table.
|
|
enq = c.post(
|
|
"/api/downloads",
|
|
json={"source": "dQw4w9WgXcQ", "spec": {"kind": "video"}, "display_name": long_name},
|
|
)
|
|
assert enq.status_code == 200, enq.text
|
|
assert enq.json()["display_name"] == bound
|
|
|
|
# 3. the edit route — the same field, from the editor.
|
|
edit = c.post(
|
|
"/api/downloads/edit",
|
|
json={
|
|
"source_job_id": up.json()["id"],
|
|
"edit_spec": {"trim": {"start_s": 0, "end_s": 1}},
|
|
"display_name": long_name,
|
|
},
|
|
)
|
|
assert edit.status_code == 200, edit.text
|
|
assert edit.json()["display_name"] == bound
|
|
|
|
# 4 + 5. the metadata PATCH's two fields.
|
|
patched = c.patch(
|
|
f"/api/downloads/{up.json()['id']}",
|
|
json={"display_name": long_name, "display_uploader": long_name},
|
|
)
|
|
assert patched.status_code == 200, patched.text
|
|
assert patched.json()["display_name"] == bound
|
|
assert patched.json()["display_uploader"] == bound
|
|
|
|
# 6. the clip route — the clip panel's name field.
|
|
clip = c.post(
|
|
"/api/downloads/clip",
|
|
json={
|
|
"source": "youtube",
|
|
"ref": "dQw4w9WgXcQ",
|
|
"start_s": 0,
|
|
"end_s": 5,
|
|
"display_name": long_name,
|
|
},
|
|
)
|
|
assert clip.status_code == 200, clip.text
|
|
assert clip.json()["display_name"] == bound
|
|
|
|
|
|
def _display_write_sites() -> dict[tuple[str, str], set[str]]:
|
|
"""Every place under `backend/app/` that puts a value into a display field, as
|
|
(module relative to `app/`, enclosing function) -> the set of value expressions written there.
|
|
|
|
THREE SHAPES, because the first version of this guard walked only `ast.Assign` and was therefore
|
|
blind to the shape the upload route itself uses — `SpooledUpload(..., display_name=...)` is an
|
|
`ast.keyword`, not an assignment, so the one site with an unbounded REQUEST HEADER behind it was
|
|
the site the guard could not see. The third, `fields["display_name"] = …`, is a dict a finalize
|
|
call is spread from (the worker's), which the first two missed as well. A value passed
|
|
POSITIONALLY is still unseen: a writer binds it to a name or a keyword first, as every route here
|
|
does.
|
|
|
|
ANNOTATED assignments are excluded on purpose: `display_name: Mapped[str | None] =
|
|
mapped_column(...)` in `models.py` and `display_name: str | None = None` in a Pydantic model are
|
|
declarations of the field, not writes into it.
|
|
|
|
The whole of `app/`, not just `app/routes/`: a job row is constructed in `downloads/service.py`
|
|
and `downloads/upload.py`, so a walk stopping at the HTTP boundary misses the writers that
|
|
actually reach the column.
|
|
"""
|
|
app_dir = pathlib.Path(__file__).resolve().parents[1] / "app"
|
|
found: dict[tuple[str, str], set[str]] = {}
|
|
for py in sorted(app_dir.rglob("*.py")):
|
|
module = py.relative_to(app_dir).as_posix()
|
|
tree = ast.parse(py.read_text("utf-8"), filename=str(py))
|
|
enclosing: dict[int, str] = {}
|
|
for fn in ast.walk(tree):
|
|
if isinstance(fn, (ast.AsyncFunctionDef, ast.FunctionDef)):
|
|
for node in ast.walk(fn):
|
|
enclosing.setdefault(id(node), fn.name)
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.Assign):
|
|
names = {
|
|
t.id if isinstance(t, ast.Name) else t.attr
|
|
for t in node.targets
|
|
if isinstance(t, (ast.Name, ast.Attribute))
|
|
} | {
|
|
# `fields["display_name"] = …` — a dict the write is then spread from.
|
|
t.slice.value
|
|
for t in node.targets
|
|
if isinstance(t, ast.Subscript)
|
|
and isinstance(t.slice, ast.Constant)
|
|
and isinstance(t.slice.value, str)
|
|
}
|
|
value = node.value
|
|
elif isinstance(node, ast.keyword):
|
|
names = {node.arg} if node.arg else set()
|
|
value = node.value
|
|
else:
|
|
continue
|
|
if not names & {"display_name", "display_uploader"}:
|
|
continue
|
|
key = (module, enclosing.get(id(node), "<module>"))
|
|
found.setdefault(key, set()).add(ast.unparse(value))
|
|
return found
|
|
|
|
|
|
# Every write into a display field, with the expression each one writes. A varchar(255) column with
|
|
# a request header behind it turns a too-long name into a psycopg DataError — a 500 AFTER the whole
|
|
# file has transferred — so each of these has to be bounded, and each one here has been read.
|
|
#
|
|
# Bounded by `display_text` unless noted:
|
|
# service.redownload CLEARS the field so the fresh metadata wins — `None` is the
|
|
# one value that needs no bound, and a clear is still a write.
|
|
# service.enqueue / enqueue_edit / `display_name` is already bounded by its caller; the fallback
|
|
# enqueue_clip slices the source's title to the same width.
|
|
# downloads.upload_download passes through `spool_upload`'s value, bounded there.
|
|
# routes enqueue_clip the `display_name=` keyword passes on the name bound above it.
|
|
# pipeline._download / _process_clip / worker._finish_from_cache
|
|
# slice a fetched or stored title to the column's width.
|
|
#
|
|
# Held as EXACT EQUALITY rather than as a list of offenders. The offender form could only fail
|
|
# upward: refactoring the writers into a shape the walker did not recognise left it asserting
|
|
# `not []` and enforcing nothing, which is how a drift guard dies quietly. This one fails
|
|
# when a site appears, when one disappears, and when one changes what it writes.
|
|
DISPLAY_WRITE_SITES = {
|
|
("downloads/service.py", "redownload"): {"None"},
|
|
("downloads/service.py", "enqueue"): {
|
|
"display_name or (asset.title[:DISPLAY_TEXT_MAX] "
|
|
"if asset.status == 'ready' and asset.title else None)"
|
|
},
|
|
("downloads/service.py", "enqueue_edit"): {"display_name or asset.title[:DISPLAY_TEXT_MAX]"},
|
|
("downloads/service.py", "enqueue_clip"): {"display_name or asset.title[:DISPLAY_TEXT_MAX]"},
|
|
("downloads/pipeline.py", "_download"): {"meta.title[:DISPLAY_TEXT_MAX]"},
|
|
("downloads/pipeline.py", "_process_clip"): {"named[:DISPLAY_TEXT_MAX]"},
|
|
("worker.py", "_finish_from_cache"): {"asset.title[:DISPLAY_TEXT_MAX]"},
|
|
("downloads/upload.py", "ingest_upload"): {"display_text(display_name)"},
|
|
("routes/auth.py", "callback"): {"display_text(userinfo.get('name'))"},
|
|
("routes/auth.py", "_complete_link"): {"display_text(userinfo.get('name'))"},
|
|
("routes/downloads.py", "enqueue_download"): {"display_text(body.display_name)"},
|
|
("routes/downloads.py", "enqueue_edit"): {"display_text(body.display_name)"},
|
|
("routes/downloads.py", "enqueue_clip"): {"display_text(body.display_name)", "display_name"},
|
|
("routes/downloads.py", "update_download_meta"): {
|
|
"display_text(body.display_name)",
|
|
"display_text(body.display_uploader)",
|
|
},
|
|
("routes/downloads.py", "upload_download"): {"upload.display_name"},
|
|
("routes/downloads.py", "spool_upload"): {
|
|
"display_text(_header_text(request, UPLOAD_DISPLAY_NAME_HEADER))"
|
|
},
|
|
}
|
|
|
|
|
|
def test_no_code_path_assigns_a_display_field_without_the_shared_helper():
|
|
"""The drift guard for the test above: it holds five routes, this holds the rule.
|
|
|
|
Parsed, not grepped. Every value that reaches a display field takes a string a user typed (or
|
|
YouTube supplied) and puts it on its way to a varchar(255) column, so every one of them must be
|
|
bounded. A writer added tomorrow fails here instead of 500ing in production.
|
|
"""
|
|
found = _display_write_sites()
|
|
moved = set(found) ^ set(DISPLAY_WRITE_SITES)
|
|
changed = {
|
|
key: (found[key], DISPLAY_WRITE_SITES[key])
|
|
for key in set(found) & set(DISPLAY_WRITE_SITES)
|
|
if found[key] != DISPLAY_WRITE_SITES[key]
|
|
}
|
|
assert found == DISPLAY_WRITE_SITES, (
|
|
"a display-field write appeared, moved or changed shape. Every value reaching "
|
|
"display_name/display_uploader must be bounded to models.DISPLAY_TEXT_MAX — normally by "
|
|
"models.display_text — or a name longer than the column is a 500 rather than a truncation. "
|
|
f"Sites that appeared or vanished: {moved}. Sites that changed: {changed}."
|
|
)
|
|
|
|
|
|
def test_the_display_text_bound_matches_the_columns_it_guards():
|
|
"""A number living in two places is how the `cancelled`/`canceled` bug happened. `display_text`
|
|
truncates to `DISPLAY_TEXT_MAX`; the columns it feeds are `String(255)`. Held against the model
|
|
itself, so widening one without the other fails here rather than as a DataError in production."""
|
|
for name in ("display_name", "display_uploader"):
|
|
assert DownloadJob.__table__.c[name].type.length == DISPLAY_TEXT_MAX
|
|
|
|
|
|
def test_the_display_name_header_reaches_the_job_and_is_bounded(
|
|
as_user, make_user, _root, monkeypatch, upload_headers
|
|
):
|
|
"""The second header, which had no backend test at all — and the length nothing bounded.
|
|
|
|
The display name arrives in a REQUEST HEADER rather than as a form field, so the only
|
|
limit on it was the header size. The column is varchar(255), so a longer one reached Postgres as
|
|
a DataError: a 500 AFTER the whole file had been transferred. It is truncated now, by the same
|
|
helper the metadata PATCH uses.
|
|
"""
|
|
_probe(monkeypatch, (None, None, None, None, None))
|
|
c = as_user(make_user())
|
|
|
|
ok = c.post(
|
|
"/api/downloads/upload",
|
|
content=b"data",
|
|
headers=upload_headers("clip.bin", "Nyári vakáció"),
|
|
)
|
|
assert ok.status_code == 200, ok.text
|
|
assert ok.json()["display_name"] == "Nyári vakáció"
|
|
|
|
long_name = "n" * 300
|
|
bounded = c.post(
|
|
"/api/downloads/upload", content=b"data2", headers=upload_headers("clip2.bin", long_name)
|
|
)
|
|
assert bounded.status_code == 200, bounded.text
|
|
assert bounded.json()["display_name"] == "n" * DISPLAY_TEXT_MAX
|
|
|
|
|
|
def test_the_upload_holds_no_db_transaction_while_the_body_streams(
|
|
as_user, make_user, db, _root, monkeypatch, upload_headers
|
|
):
|
|
"""Moving the body read behind the auth dependency put the whole transfer INSIDE the
|
|
request's database session: resolving the caller and reading the cap are SELECTs, SQLAlchemy 2.0
|
|
autobegins on the first one, and the session then holds one of the twenty pooled connections —
|
|
`idle in transaction` to Postgres, with no `idle_in_transaction_session_timeout` to end it — for
|
|
as long as the client keeps sending. `db.py` sizes the pool on that not happening and
|
|
`plex/playback.py` records the production incident behind the rule.
|
|
|
|
Asserted on the session rather than on `engine.pool.checkedout()`: the transaction is the cause,
|
|
the checkout is its consequence (measured), and the pool counter is shared with whatever else
|
|
the test process is holding.
|
|
"""
|
|
from app.routes import downloads as downloads_routes
|
|
|
|
_probe(monkeypatch, (None, None, None, None, None))
|
|
seen: dict[str, bool] = {}
|
|
real_stream = downloads_routes.capped_stream_to
|
|
real_release = downloads_routes.release_read_connection
|
|
|
|
def spy_release(session):
|
|
# The PRECONDITION, so this test cannot pass vacuously. Asserting only "no transaction
|
|
# while streaming" would stay green if the auth/cap reads stopped opening one at all —
|
|
# and under `as_user` the auth SELECT is already overridden away, so that is a live risk
|
|
# rather than a theoretical one.
|
|
seen["before"] = bool(session.in_transaction())
|
|
real_release(session)
|
|
seen["after_release"] = bool(session.in_transaction())
|
|
|
|
async def spy_stream(request, out, limit, refusal):
|
|
seen["while_streaming"] = bool(db.in_transaction())
|
|
return await real_stream(request, out, limit, refusal)
|
|
|
|
monkeypatch.setattr(downloads_routes, "release_read_connection", spy_release)
|
|
monkeypatch.setattr(downloads_routes, "capped_stream_to", spy_stream)
|
|
c = as_user(make_user())
|
|
r = c.post("/api/downloads/upload", content=b"data", headers=upload_headers("held.bin"))
|
|
assert r.status_code == 200, r.text
|
|
assert seen["before"] is True, (
|
|
"nothing had opened a transaction before the release, so this test proves nothing — the "
|
|
"reads it is meant to follow must have moved or been stubbed away"
|
|
)
|
|
assert seen["after_release"] is False
|
|
assert seen["while_streaming"] is False, (
|
|
"the request's session was inside a transaction when the transfer began, so a pooled "
|
|
"connection is pinned for the whole upload"
|
|
)
|
|
|
|
|
|
def test_the_usage_route_reports_the_per_file_upload_cap(as_user, make_user, db):
|
|
"""The dialog refuses an over-cap file off this number, and it is the only layer that produces
|
|
a refusal carrying the limit on a default install — so the field is load-bearing, not
|
|
decorative. Nothing held it: the route had no test at all, and the frontend's `DownloadUsage`
|
|
is an unvalidated cast, so dropping the key would have been green everywhere.
|
|
"""
|
|
from app import sysconfig
|
|
|
|
c = as_user(make_user())
|
|
r = c.get("/api/downloads/usage")
|
|
assert r.status_code == 200, r.text
|
|
assert r.json()["upload_max_bytes"] == sysconfig.get_int(db, "download_upload_max_bytes")
|
|
|
|
# And it is the KNOB, not a constant: a lowered override must show through.
|
|
sysconfig.set_value(db, "download_upload_max_bytes", 4096)
|
|
db.commit()
|
|
assert c.get("/api/downloads/usage").json()["upload_max_bytes"] == 4096
|
|
|
|
|
|
def test_a_refused_upload_leaves_nothing_in_the_staging_directory(
|
|
as_user, make_user, db, _root, monkeypatch, upload_headers
|
|
):
|
|
"""`spool_upload` writes the body to a staging temp file as it arrives, so a refusal happens
|
|
with a partial file already on disk — its `finally` is what removes it. Nothing asserted that:
|
|
deleting the unlink left the whole suite green while every refused upload leaked its partial
|
|
bytes onto the volume, one file per attempt, with no job to ever clean them up.
|
|
"""
|
|
from app import sysconfig
|
|
|
|
sysconfig.set_value(db, "download_upload_max_bytes", 8)
|
|
db.commit()
|
|
staging = Path(settings.download_root) / ".staging" / "uploads"
|
|
c = as_user(make_user())
|
|
|
|
r = c.post("/api/downloads/upload", content=b"x" * 4096, headers=upload_headers("leaky.bin"))
|
|
assert r.status_code == 422
|
|
assert staging.exists(), "the dependency did not get as far as creating the staging dir"
|
|
assert list(staging.iterdir()) == [], "a refused upload left its partial file behind"
|
|
|
|
|
|
def test_upload_cap_cannot_be_disabled_with_zero(db):
|
|
"""download_upload_max_bytes floors at 1, and set_value must enforce it.
|
|
|
|
The falsy-0 footgun this originally guarded is gone — the route's old `if cap and size > cap`
|
|
read a stored 0 as "no cap at all", while `capped_stream_to` compares unconditionally and a 0
|
|
would refuse everything instead. Refusing everything is the safe direction, but it is still
|
|
wrong, and the knob is also the number the admin page and the 422 `limit` advertise. Pins the
|
|
min=1 invariant so a revert to min=0 can't pass the gate."""
|
|
from app import sysconfig
|
|
|
|
with pytest.raises(ValueError):
|
|
sysconfig.set_value(db, "download_upload_max_bytes", 0)
|
|
|
|
|
|
def test_upload_route_rejects_empty_file(as_user, make_user, _root, monkeypatch, upload_headers):
|
|
c = as_user(make_user())
|
|
r = c.post("/api/downloads/upload", content=b"", headers=upload_headers("empty.bin"))
|
|
assert r.status_code == 400
|
|
assert r.json()["detail"]["code"] == "upload_empty"
|
|
|
|
|
|
def test_redownload_rejects_an_upload(as_user, make_user, _root, monkeypatch, upload_headers):
|
|
_probe(monkeypatch, (None, None, None, None, None))
|
|
c = as_user(make_user())
|
|
job = c.post("/api/downloads/upload", content=b"data", headers=upload_headers("a.bin")).json()
|
|
r = c.post(f"/api/downloads/{job['id']}/redownload")
|
|
assert r.status_code == 409
|
|
|
|
|
|
def test_edit_rejects_a_generic_upload_source(
|
|
as_user, make_user, _root, monkeypatch, upload_headers
|
|
):
|
|
_probe(monkeypatch, (None, None, None, None, None))
|
|
c = as_user(make_user())
|
|
job = c.post("/api/downloads/upload", content=b"data", headers=upload_headers("a.bin")).json()
|
|
r = c.post(
|
|
"/api/downloads/edit",
|
|
json={"source_job_id": job["id"], "edit_spec": {"trim": {"start_s": 0, "end_s": 1}}},
|
|
)
|
|
assert r.status_code == 400
|
|
assert r.json()["detail"]["reason"] == "not_media"
|
|
|
|
|
|
def test_ingest_rejects_over_max_jobs(db, make_user, _root, monkeypatch):
|
|
"""The other check_upload rejection branch (holdings count, not bytes): once the user is at
|
|
max_jobs, a further upload is refused before any disk write."""
|
|
_probe(monkeypatch, (None, None, None, None, None))
|
|
u = make_user()
|
|
db.add(
|
|
DownloadQuota(user_id=u.id, max_bytes=10**12, max_concurrent=2, max_jobs=1, unlimited=False)
|
|
)
|
|
db.commit()
|
|
src1 = _root / "a.bin"
|
|
src1.write_bytes(b"a")
|
|
uploadmod.ingest_upload(db, u.id, filename="a.bin", src_path=src1, size=1) # now 1 holding job
|
|
src2 = _root / "b.bin"
|
|
src2.write_bytes(b"b")
|
|
with pytest.raises(quota.QuotaExceeded) as e:
|
|
uploadmod.ingest_upload(db, u.id, filename="b.bin", src_path=src2, size=1)
|
|
assert e.value.reason == "max_jobs"
|
|
assert src2.exists() # rejected before the move — nothing landed
|
|
|
|
|
|
def test_ingest_cleans_up_placed_files_on_commit_failure(db, make_user, _root, monkeypatch):
|
|
"""A failure AFTER storage.place_file has moved the file onto disk must roll back AND unlink the
|
|
placed file + any (even partial) poster — otherwise nothing names it (the per-user delete keys
|
|
off the rolled-back asset.rel_path) and it sits on disk until the GC's `.uploads` sweep. Exercises
|
|
the rollback/cleanup branch, incl. the poster case where ensure_poster left a partial file but
|
|
returned None."""
|
|
_probe(monkeypatch, ("h264", "aac", 320, 240, 5)) # video → the poster path runs
|
|
u = make_user()
|
|
|
|
# Simulate ffmpeg leaving a partial poster on disk, then failing (ensure_poster returns None).
|
|
def _partial_poster(asset, root):
|
|
storage.abs_path(root, str(Path(asset.rel_path).with_suffix(".jpg"))).write_bytes(
|
|
b"partial"
|
|
)
|
|
return None
|
|
|
|
monkeypatch.setattr(uploadmod.editmod, "ensure_poster", _partial_poster)
|
|
|
|
# Force the commit to fail after the file (and partial poster) are on disk.
|
|
def _boom():
|
|
raise RuntimeError("db down")
|
|
|
|
monkeypatch.setattr(db, "commit", _boom)
|
|
|
|
src = _root / "clip.mp4"
|
|
src.write_bytes(b"fake-video-bytes")
|
|
with pytest.raises(RuntimeError):
|
|
uploadmod.ingest_upload(db, u.id, filename="clip.mp4", src_path=src, size=16)
|
|
|
|
db.rollback() # drop the monkeypatched-commit session state before querying
|
|
uploads = _root / ".uploads"
|
|
leftover = [p for p in uploads.rglob("*") if p.is_file()] if uploads.exists() else []
|
|
assert leftover == [], f"orphaned files left on disk: {leftover}"
|
|
assert db.query(MediaAsset).count() == 0 # the flushed asset rolled back — no dangling row
|
|
|
|
|
|
def test_ingest_keeps_the_file_when_its_commit_landed_but_raised(db, make_user, _root, monkeypatch):
|
|
# The commit reached the server and lost its answer: the row names the file now, so removing it
|
|
# as "rolled back" would leave a ready upload pointing at nothing.
|
|
_probe(monkeypatch, ("h264", "aac", 320, 240, 5))
|
|
monkeypatch.setattr(uploadmod.editmod, "ensure_poster", lambda *_a: None)
|
|
u = make_user()
|
|
real_commit = db.commit
|
|
|
|
def _landed_then_lost():
|
|
real_commit()
|
|
raise RuntimeError("connection lost after COMMIT")
|
|
|
|
monkeypatch.setattr(db, "commit", _landed_then_lost)
|
|
src = _root / "clip.mp4"
|
|
src.write_bytes(b"fake-video-bytes")
|
|
with pytest.raises(RuntimeError):
|
|
uploadmod.ingest_upload(db, u.id, filename="clip.mp4", src_path=src, size=16)
|
|
|
|
(asset,) = db.query(MediaAsset).all()
|
|
assert storage.abs_path(str(_root), asset.rel_path).exists()
|
|
|
|
|
|
# --- classifier: the REAL probe_media parsing (stubs ffprobe one level down) ------------------
|
|
# The ingest/route tests above stub probe_media wholesale; these exercise its OWN logic — image
|
|
# detection, the cover-art skip, dimension/duration parsing — by faking only run_ffprobe_json, so a
|
|
# regression in the classifier the force-download guarantee leans on is actually caught.
|
|
|
|
|
|
class TestProbeClassifier:
|
|
def _probe_with(self, monkeypatch, data):
|
|
monkeypatch.setattr(probemod.editmod, "run_ffprobe_json", lambda *a, **k: data)
|
|
return probemod.probe_media(Path("x"))
|
|
|
|
def test_probe_failure_is_a_generic_file(self, monkeypatch):
|
|
# run_ffprobe_json returns None (ffprobe failed / not media) → all None → derive() == FILE.
|
|
v, a, w, h, d = self._probe_with(monkeypatch, None)
|
|
assert (v, a, w, h, d) == (None, None, None, None, None)
|
|
assert mediakind.derive(v, a) == mediakind.FILE
|
|
|
|
@pytest.mark.parametrize("fmt", ["image2", "png_pipe", "jpeg_pipe", "webp_pipe", "gif"])
|
|
def test_still_image_classifies_as_file_not_video(self, monkeypatch, fmt):
|
|
# A standalone image exposes a real video stream, but its container format is an image demuxer
|
|
# — it must NOT become media_kind='video' (broken player + inline serving). Stays a file.
|
|
data = {
|
|
"format": {"format_name": fmt},
|
|
"streams": [{"codec_type": "video", "codec_name": "png", "width": 8, "height": 8}],
|
|
}
|
|
v, a, w, h, d = self._probe_with(monkeypatch, data)
|
|
assert v is None and mediakind.derive(v, a) == mediakind.FILE
|
|
|
|
def test_cover_art_video_stream_is_skipped_audio_stays_audio(self, monkeypatch):
|
|
# Album art embedded in an audio file is a video stream flagged attached_pic — skip it so the
|
|
# asset classifies as audio, not video.
|
|
data = {
|
|
"format": {"format_name": "mp3", "duration": "212.5"},
|
|
"streams": [
|
|
{
|
|
"codec_type": "video",
|
|
"codec_name": "mjpeg",
|
|
"disposition": {"attached_pic": 1},
|
|
"width": 600,
|
|
"height": 600,
|
|
},
|
|
{"codec_type": "audio", "codec_name": "mp3"},
|
|
],
|
|
}
|
|
v, a, w, h, d = self._probe_with(monkeypatch, data)
|
|
assert (v, a, d) == (None, "mp3", 212)
|
|
assert mediakind.derive(v, a) == mediakind.AUDIO
|
|
|
|
def test_real_video_records_codecs_dims_duration(self, monkeypatch):
|
|
data = {
|
|
"format": {"format_name": "mov,mp4,m4a", "duration": "60.0"},
|
|
"streams": [
|
|
{"codec_type": "video", "codec_name": "h264", "width": 1920, "height": 1080},
|
|
{"codec_type": "audio", "codec_name": "aac"},
|
|
],
|
|
}
|
|
v, a, w, h, d = self._probe_with(monkeypatch, data)
|
|
assert (v, a, w, h, d) == ("h264", "aac", 1920, 1080, 60)
|
|
assert mediakind.derive(v, a) == mediakind.VIDEO
|
|
|
|
def test_unparseable_duration_is_none(self, monkeypatch):
|
|
data = {
|
|
"format": {"format_name": "wav", "duration": "N/A"},
|
|
"streams": [{"codec_type": "audio", "codec_name": "pcm_s16le"}],
|
|
}
|
|
v, a, w, h, d = self._probe_with(monkeypatch, data)
|
|
assert d is None and mediakind.derive(v, a) == mediakind.AUDIO
|
|
|
|
|
|
# --- resolve_media_facts: probe-of-the-file wins over yt-dlp's info (the FB regression fix) ---------
|
|
# The download worker's finalize must classify from the ACTUAL file, not the extractor's `info` dict.
|
|
# yt-dlp omits vcodec/acodec/dims for some sources (Facebook), and deriving media_kind from those
|
|
# nulls turned a real H.264 video into a download-only 'file' (no in-app play). These pin that the
|
|
# probe is authoritative and `info` is only a gap-filler.
|
|
|
|
|
|
class TestResolveMediaFacts:
|
|
# These stub run_ffprobe_json (one level below probe_media) so the REAL probe parsing + the
|
|
# probe-vs-info precedence are exercised together — the precedence keys off whether ffprobe RAN.
|
|
@staticmethod
|
|
def _ffprobe(monkeypatch, data):
|
|
monkeypatch.setattr(probemod.editmod, "run_ffprobe_json", lambda *a, **k: data)
|
|
|
|
def test_probe_wins_when_info_lacks_codecs(self, monkeypatch):
|
|
# THE original regression: info has no vcodec/acodec/dims (Facebook), but the file probes as
|
|
# h264/aac.
|
|
self._ffprobe(
|
|
monkeypatch,
|
|
{
|
|
"format": {"format_name": "mov,mp4,m4a"},
|
|
"streams": [
|
|
{"codec_type": "video", "codec_name": "h264", "width": 720, "height": 1280},
|
|
{"codec_type": "audio", "codec_name": "aac"},
|
|
],
|
|
},
|
|
)
|
|
v, a, w, h, _d = probemod.resolve_media_facts(Path("x"), {"width": None, "height": None})
|
|
assert (v, a, w, h) == ("h264", "aac", 720, 1280)
|
|
assert mediakind.derive(v, a) == mediakind.VIDEO # NOT 'file'
|
|
|
|
def test_the_duration_comes_from_the_file_when_info_has_none(self, monkeypatch):
|
|
# Facebook reel 257: yt-dlp gave no duration, the file is 128.4 s. A NULL duration hid
|
|
# the Library row's trim action.
|
|
self._ffprobe(
|
|
monkeypatch,
|
|
{
|
|
"format": {"format_name": "mov,mp4,m4a", "duration": "128.400000"},
|
|
"streams": [{"codec_type": "video", "codec_name": "h264"}],
|
|
},
|
|
)
|
|
assert probemod.resolve_media_facts(Path("x"), {"duration": None})[4] == 128
|
|
# The file wins over a differing info duration, like its codecs do.
|
|
assert probemod.resolve_media_facts(Path("x"), {"duration": 300})[4] == 128
|
|
|
|
def test_info_fills_the_duration_the_probe_could_not_read(self, monkeypatch):
|
|
self._ffprobe(
|
|
monkeypatch,
|
|
{
|
|
"format": {"format_name": "mov,mp4,m4a"},
|
|
"streams": [{"codec_type": "video", "codec_name": "h264"}],
|
|
},
|
|
)
|
|
assert probemod.resolve_media_facts(Path("x"), {"duration": 61.7})[4] == 61
|
|
self._ffprobe(monkeypatch, None)
|
|
assert probemod.resolve_media_facts(Path("x"), {"duration": 61.7})[4] == 61
|
|
|
|
def test_probe_wins_over_a_DIFFERING_populated_info(self, monkeypatch):
|
|
# Both the probe AND info are populated but DISAGREE (info carries a pre-remux/pre-merge codec).
|
|
# The finished file on disk is ground truth, so the probe wins outright — pins the precedence a
|
|
# `info or probe` inversion would silently break.
|
|
self._ffprobe(
|
|
monkeypatch,
|
|
{
|
|
"format": {"format_name": "mov,mp4,m4a"},
|
|
"streams": [
|
|
{"codec_type": "video", "codec_name": "h264", "width": 1920, "height": 1080},
|
|
{"codec_type": "audio", "codec_name": "aac"},
|
|
],
|
|
},
|
|
)
|
|
v, a, w, h, _d = probemod.resolve_media_facts(
|
|
Path("x"), {"vcodec": "vp9", "acodec": "opus", "width": 640, "height": 480}
|
|
)
|
|
assert (v, a, w, h) == ("h264", "aac", 1920, 1080) # probe, not info
|
|
|
|
def test_cover_art_audio_download_is_not_resurrected_to_video_by_info(self, monkeypatch):
|
|
# THE bug this fix targets: an audio-mode download from a progressive source embeds cover art,
|
|
# and yt-dlp's info still carries the source's video codec. The probe correctly finds only audio
|
|
# (the attached_pic is skipped); info's vcodec must NOT resurrect a video track → media_kind
|
|
# stays audio, not a broken in-app 'video'.
|
|
self._ffprobe(
|
|
monkeypatch,
|
|
{
|
|
"format": {"format_name": "mp3", "duration": "180.0"},
|
|
"streams": [
|
|
{
|
|
"codec_type": "video",
|
|
"codec_name": "mjpeg",
|
|
"disposition": {"attached_pic": 1},
|
|
},
|
|
{"codec_type": "audio", "codec_name": "mp3"},
|
|
],
|
|
},
|
|
)
|
|
v, a, w, h, _d = probemod.resolve_media_facts(
|
|
Path("x"), {"vcodec": "avc1", "acodec": "mp4a", "width": 1280, "height": 720}
|
|
)
|
|
assert v is None and a == "mp3" # info's avc1 did NOT win
|
|
assert mediakind.derive(v, a) == mediakind.AUDIO # NOT VIDEO
|
|
|
|
def test_still_image_download_is_not_resurrected_to_video_by_info(self, monkeypatch):
|
|
# A downloaded still image: the probe returns no vcodec (image-format guard). info's image
|
|
# "vcodec" must not turn it into a broken 'video' → stays a generic file.
|
|
self._ffprobe(
|
|
monkeypatch,
|
|
{
|
|
"format": {"format_name": "png_pipe"},
|
|
"streams": [{"codec_type": "video", "codec_name": "png", "width": 8, "height": 8}],
|
|
},
|
|
)
|
|
v, a, w, h, _d = probemod.resolve_media_facts(
|
|
Path("x"), {"vcodec": "png", "width": 8, "height": 8}
|
|
)
|
|
assert v is None and mediakind.derive(v, a) == mediakind.FILE
|
|
|
|
def test_a_file_the_guarded_probe_cannot_read_is_a_download_only_file(self, monkeypatch):
|
|
# ffprobe (media demuxers only) failed entirely: the player and the editor cannot open it
|
|
# either, so info's codecs must not make it an editable 'video'. Dimensions still gap-fill.
|
|
self._ffprobe(monkeypatch, None)
|
|
v, a, w, h, _d = probemod.resolve_media_facts(
|
|
Path("x"),
|
|
{"vcodec": "avc1.64001f", "acodec": "mp4a.40.2", "width": 1920, "height": 1080},
|
|
)
|
|
assert (v, a, w, h) == (None, None, 1920, 1080)
|
|
assert mediakind.derive(v, a) == mediakind.FILE
|
|
|
|
|
|
# --- the phone-safe conversion (an iPhone's HEVC, a webm, an mkv) ------------------------------
|
|
|
|
|
|
def _queued_upload(db, make_user, root, monkeypatch, name="clip.mov"):
|
|
"""An upload no phone could play as it is: probed as HEVC, so ingest queues it."""
|
|
_probe(monkeypatch, ("hevc", "aac", 640, 360, 6))
|
|
src = root / "src.bin"
|
|
src.write_bytes(b"hevc bytes")
|
|
job = uploadmod.ingest_upload(db, make_user().id, filename=name, src_path=src, size=10)
|
|
return job, db.get(MediaAsset, job.asset_id)
|
|
|
|
|
|
class TestPhoneSafeUpload:
|
|
def test_an_unsafe_video_is_queued_for_the_worker_not_done(
|
|
self, db, make_user, _root, monkeypatch
|
|
):
|
|
job, asset = _queued_upload(db, make_user, _root, monkeypatch)
|
|
assert (job.status, job.progress, asset.status) == ("queued", 0, "pending")
|
|
assert asset.rel_path.endswith(".mov") # the original waits at rel_path for the worker
|
|
assert storage.abs_path(str(_root), asset.rel_path).exists()
|
|
|
|
def test_a_safe_video_still_lands_done(self, db, make_user, _root, monkeypatch):
|
|
_probe(monkeypatch, ("h264", "aac", 640, 360, 6))
|
|
src = _root / "src.bin"
|
|
src.write_bytes(b"h264 bytes")
|
|
job = uploadmod.ingest_upload(
|
|
db, make_user().id, filename="clip.mp4", src_path=src, size=10
|
|
)
|
|
assert job.status == "done" and db.get(MediaAsset, job.asset_id).status == "ready"
|
|
|
|
def test_deleting_a_queued_upload_removes_its_original(self, db, make_user, _root, monkeypatch):
|
|
job, asset = _queued_upload(db, make_user, _root, monkeypatch)
|
|
path, asset_id = storage.abs_path(str(_root), asset.rel_path), asset.id
|
|
service.delete(db, job)
|
|
assert not path.exists()
|
|
assert db.get(MediaAsset, asset_id) is None
|
|
|
|
def test_one_cancelled_mid_conversion_is_reaped_once_the_worker_let_go(
|
|
self, db, make_user, _root, monkeypatch
|
|
):
|
|
job, asset = _queued_upload(db, make_user, _root, monkeypatch)
|
|
path, asset_id = storage.abs_path(str(_root), asset.rel_path), asset.id
|
|
asset.status = "downloading" # the worker child is reading the original
|
|
job.status = "running"
|
|
db.commit()
|
|
service.cancel(db, job)
|
|
assert path.exists() # never pulled out from under a running ffmpeg
|
|
db.get(MediaAsset, asset_id).status = "pending" # the worker stopped the child and let go
|
|
db.commit()
|
|
gcmod.run_download_gc(db)
|
|
db.expire_all()
|
|
assert not path.exists()
|
|
assert db.get(MediaAsset, asset_id) is None
|
|
|
|
def test_a_failed_conversion_keeps_the_original_for_retry(
|
|
self, db, make_user, _root, monkeypatch
|
|
):
|
|
job, asset = _queued_upload(db, make_user, _root, monkeypatch)
|
|
path = storage.abs_path(str(_root), asset.rel_path)
|
|
job.status, asset.status = "running", "downloading"
|
|
db.commit()
|
|
|
|
def _ffmpeg_fails(*_a, **_k):
|
|
raise StatedFailure("ffmpeg failed (1): unsupported")
|
|
|
|
monkeypatch.setattr(pipeline.editmod, "run_ffmpeg", _ffmpeg_fails)
|
|
pipeline._convert_upload(job.id, asset.id)
|
|
db.expire_all()
|
|
assert (db.get(DownloadJob, job.id).status, db.get(MediaAsset, asset.id).status) == (
|
|
"error",
|
|
"error",
|
|
)
|
|
assert path.exists()
|
|
service.resume(db, db.get(DownloadJob, job.id))
|
|
assert db.get(MediaAsset, asset.id).status == "pending" # Retry converts from it again
|
|
|
|
@pytest.mark.skipif(shutil.which("ffmpeg") is None, reason="ffmpeg not available")
|
|
def test_the_worker_converts_it_and_keeps_only_the_safe_file(
|
|
self, db, make_user, _root, monkeypatch
|
|
):
|
|
# Real bytes, real probe, real ffmpeg: a 10-bit H.264 + Opus Matroska, the kind of file
|
|
# that reached the share page as-is before this rule.
|
|
src = _root / "src.mkv"
|
|
subprocess.run(
|
|
[
|
|
"ffmpeg", "-y", "-loglevel", "error",
|
|
"-f", "lavfi", "-i", "testsrc2=size=160x120:rate=10:duration=1",
|
|
"-f", "lavfi", "-i", "sine=duration=1",
|
|
"-c:v", "libx264", "-pix_fmt", "yuv420p10le", "-c:a", "libopus", str(src),
|
|
],
|
|
check=True,
|
|
) # fmt: skip
|
|
job = uploadmod.ingest_upload(
|
|
db, make_user().id, filename="holiday.mkv", src_path=src, size=src.stat().st_size
|
|
)
|
|
asset = db.get(MediaAsset, job.asset_id)
|
|
original = storage.abs_path(str(_root), asset.rel_path)
|
|
assert job.status == "queued"
|
|
job.status, asset.status = "running", "downloading" # what the worker's claim does
|
|
db.commit()
|
|
|
|
pipeline._convert_upload(job.id, asset.id)
|
|
|
|
db.expire_all()
|
|
asset = db.get(MediaAsset, job.asset_id)
|
|
assert db.get(DownloadJob, job.id).status == "done"
|
|
assert (asset.status, asset.container, asset.vcodec, asset.acodec) == (
|
|
"ready",
|
|
"mp4",
|
|
"h264",
|
|
"aac",
|
|
)
|
|
final = storage.abs_path(str(_root), asset.rel_path)
|
|
assert final.suffix == ".mp4" and final.exists()
|
|
assert asset.size_bytes == final.stat().st_size
|
|
assert not original.exists() # one file per upload
|
|
assert not playable.plan(probemod.probe_json(final), final.suffix).needs_change
|