From cb4d06bb3d3c88645e49f45ec66cac0523fadad5 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 22 Jul 2026 00:00:44 +0200 Subject: [PATCH] fix(downloads): cap the media filename by BYTES, not characters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /code-review (S1, finding 1): sanitize truncated the title at 120 CHARACTERS, but a path component's hard limit is 255 BYTES and rel_path packs channel + date + id + ext into that same component — so a long accented/multi-byte title (Hungarian titles are common here) could already overflow to ENAMETOOLONG, and the new _a{id} suffix shrank the margin further. sanitize now truncates on a UTF-8 boundary by byte length, so the title (≤120B) + channel (≤80B) + fixed parts stay comfortably under 255B. Two tests: byte-cap on accented input, and every rel_path component ≤255B for a pathological long-accented channel+title+asset_id. --- backend/app/downloads/storage.py | 8 ++++++-- backend/tests/test_storage.py | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/backend/app/downloads/storage.py b/backend/app/downloads/storage.py index d8e12790..665e107c 100644 --- a/backend/app/downloads/storage.py +++ b/backend/app/downloads/storage.py @@ -58,8 +58,12 @@ def sanitize(name: str, limit: int = _MAX_TITLE) -> str: text = re.sub(r"\s+", "_", text.strip()) # Tidy separator pile-ups and trim junk from the ends. text = re.sub(r"_{2,}", "_", text).strip("_.-·—–|,;:!?ّ ") - if len(text) > limit: - text = text[:limit].rstrip("_.-") + # Cap by BYTES, not characters: a path component's hard limit is 255 BYTES, and rel_path packs + # the (byte-capped) channel + date + id + ext into that same component — so an accented/multi-byte + # title capped only by char count could still overflow to ENAMETOOLONG. Truncate on a UTF-8 + # boundary (decode(..., "ignore") drops a trailing partial char) so we never split a character. + if len(text.encode("utf-8")) > limit: + text = text.encode("utf-8")[:limit].decode("utf-8", "ignore").rstrip("_.-") return text or "untitled" diff --git a/backend/tests/test_storage.py b/backend/tests/test_storage.py index f1ce6716..fdd357e1 100644 --- a/backend/tests/test_storage.py +++ b/backend/tests/test_storage.py @@ -29,6 +29,14 @@ class TestSanitize: def test_length_is_capped(self): assert len(sanitize("x" * 500, limit=10)) <= 10 + def test_cap_is_by_bytes_not_characters(self): + # Accented (2-byte UTF-8) letters must not blow past the byte limit — the FS component cap + # is 255 BYTES, so a char-only cap would overflow for multi-byte titles. + out = sanitize("á" * 500, limit=10) + assert len(out.encode("utf-8")) <= 10 + # And it never splits a character (would raise otherwise / leave a replacement char). + out.encode("utf-8").decode("utf-8") + def test_empty_or_all_junk_falls_back_to_untitled(self): assert sanitize("") == "untitled" assert sanitize("🔥🔥") == "untitled" @@ -64,6 +72,19 @@ class TestRelPath: assert rel_path(self.meta, "mp4", layout="flat") == "Cool_Clip_[abc123].mp4" assert rel_path(self.meta, "mp4", layout="flat", asset_id=7) == "Cool_Clip_[abc123]_a7.mp4" + def test_every_component_stays_within_the_255_byte_fs_limit(self): + # A long accented channel AND title, plus the asset-id suffix, must not overflow any path + # component (the byte cap in sanitize is what guarantees this). + meta = MediaMeta( + video_id="abcdefghijk", + title="Á" * 300, + uploader="Ö" * 300, + upload_date=date(2024, 5, 2), + ) + out = rel_path(meta, "webm", asset_id=999999) + for component in out.split("/"): + assert len(component.encode("utf-8")) <= 255, component + class TestDownloadFilename: def test_appends_the_container_extension(self):