peter
3a688fa0f5
The nine Capture tables (files, jobs, folders, shares, links, quota, cookies, profiles, library resume points) leave app/models.py for the unit's own module; app.tables registers every unit's tables for the migrations, the test database and the ownership check. No schema change.
1201 lines
55 KiB
Python
1201 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.files_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
|
|
from app.downloads.models import 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, "files_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/files/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/files/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/files/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/files/{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/files/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, row.ready_at = "done", row.created_at
|
|
db.commit()
|
|
|
|
# Authenticated endpoint: never text/html.
|
|
fr = c.get(f"/api/files/{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/files/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/files/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/files/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/files/upload", content=payload, headers=upload_headers("exact.bin"))
|
|
assert at_the_cap.status_code == 200, at_the_cap.text
|
|
|
|
over = c.post("/api/files/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/files/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/files/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 library PATCH's two fields.
|
|
patched = c.patch(
|
|
f"/api/files/{up.json()['id']}",
|
|
json={"name": long_name, "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.
|
|
# files.ingest_spooled 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/files.py", "ingest_spooled"): {"upload.display_name"},
|
|
("routes/files.py", "update_file"): {
|
|
"display_text(body.name)",
|
|
"display_text(body.uploader)",
|
|
},
|
|
("routes/files.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/files/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/files/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 files as files_routes
|
|
|
|
_probe(monkeypatch, (None, None, None, None, None))
|
|
seen: dict[str, bool] = {}
|
|
real_stream = files_routes.capped_stream_to
|
|
real_release = files_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(files_routes, "release_read_connection", spy_release)
|
|
monkeypatch.setattr(files_routes, "capped_stream_to", spy_stream)
|
|
c = as_user(make_user())
|
|
r = c.post("/api/files/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/files/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/files/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.files_root) / ".staging" / "uploads"
|
|
c = as_user(make_user())
|
|
|
|
r = c.post("/api/files/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/files/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/files/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/files/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
|