Merge PR #1060: keep generated filenames within the filesystem limit

app/tests/test_ytdl_utils.py

@@ -377,6 +377,49 @@ class ConfinedYoutubeDLTests(unittest.TestCase):
         self.assertEqual(self._prepared_path(""), "")
         self.assertEqual(self._prepared_path("-"), "-")
 
+    def test_overlong_name_is_trimmed_to_fit_the_filesystem(self):
+        # A title long enough to blow the filename limit is what made these
+        # downloads fail outright with [Errno 36] File name too long.
+        long_path = os.path.join(self.base, "a" * 400 + ".mp4")
+
+        result = self._prepared_path(long_path)
+
+        name = os.path.basename(result)
+        self.assertTrue(name.endswith(".mp4"))
+        self.assertLessEqual(len(name.encode("utf-8")), 255 - 32)
+        self.assertEqual(os.path.dirname(result), self.base)
+        # The file must still be writable once yt-dlp adds its own suffixes.
+        self.assertLessEqual(len(f"{name}.f1229065279304024v.part".encode("utf-8")), 255)
+
+    def test_name_within_the_limit_is_left_alone(self):
+        ok = os.path.join(self.base, "Ordinary Title.mp4")
+        self.assertEqual(self._prepared_path(ok), ok)
+
+    def test_limit_counts_bytes_not_characters(self):
+        # 200 CJK characters are 600 bytes: a character count would pass this.
+        long_path = os.path.join(self.base, "音" * 200 + ".mp4")
+
+        name = os.path.basename(self._prepared_path(long_path))
+
+        self.assertLessEqual(len(name.encode("utf-8")), 255 - 32)
+        # A trim landing mid-character must not leave a broken byte sequence.
+        self.assertEqual(name, name.encode("utf-8").decode("utf-8"))
+        self.assertTrue(name.endswith(".mp4"))
+
+    def test_a_long_tail_is_not_mistaken_for_an_extension(self):
+        # os.path.splitext on a title containing a dot late in the string would
+        # otherwise "preserve" a 100-character extension and trim nothing.
+        long_path = os.path.join(self.base, "b" * 300 + "." + "c" * 100)
+
+        name = os.path.basename(self._prepared_path(long_path))
+
+        self.assertLessEqual(len(name.encode("utf-8")), 255 - 32)
+
+    def test_trimming_still_cannot_escape_the_download_directory(self):
+        escaping = os.path.join(self.base, "..", "..", "d" * 400 + ".mp4")
+        with self.assertRaises(ytdl.yt_dlp.utils.DownloadError):
+            self._prepared_path(escaping)
+
 
 class SanitizeEntryForPickleTests(unittest.TestCase):
     def test_nested(self):

app/ytdl.py

@@ -196,6 +196,56 @@ def _sanitize_path_component(value: Any) -> Any:
     return value.lstrip('.').strip() or '_'
 
 
+# Room left for the suffixes yt-dlp appends after prepare_filename has run:
+# '.part' and '.ytdl' while the download is in flight, '.f<format_id>' for a
+# stream fetched on its own before merging, '-Frag<n>' for fragmented
+# downloads. A name trimmed to exactly the limit would still fail the moment
+# one of those is added, which is what the '.part' in the reported errors is.
+_NAME_SUFFIX_RESERVE_BYTES = 32
+# POSIX guarantees at least this much, and it is what ext4/xfs/btrfs allow.
+_FALLBACK_NAME_MAX_BYTES = 255
+# Keep a recognisable stem even on a filesystem with a very short limit.
+_MIN_STEM_BYTES = 16
+# Longer than this is not really an extension (a title ending in '.something'),
+# so the whole name is treated as the stem rather than preserving it.
+_MAX_EXT_BYTES = 16
+
+
+def _name_max_bytes(directory: str) -> int:
+    """The filesystem's filename limit, in bytes, for *directory*."""
+    try:
+        return int(os.pathconf(directory or '.', 'PC_NAME_MAX'))
+    except (OSError, ValueError, AttributeError):
+        # The directory may not exist yet (CREATE_CUSTOM_DIRS makes it during
+        # the download), and pathconf is not available on every platform.
+        return _FALLBACK_NAME_MAX_BYTES
+
+
+def _trim_to_name_max(path: str) -> str:
+    """Shorten the final component of *path* to what the filesystem accepts.
+
+    The limit is a byte count, not a character count: a title of accented or
+    CJK characters hits it in half as many characters, or fewer. The extension
+    is preserved, since it is what decides how the file is handled afterwards.
+    """
+    directory, name = os.path.split(path)
+    if not name:
+        return path
+    encoded = name.encode('utf-8', 'surrogatepass')
+    limit = _name_max_bytes(directory) - _NAME_SUFFIX_RESERVE_BYTES
+    if len(encoded) <= limit:
+        return path
+
+    stem, ext = os.path.splitext(name)
+    ext_bytes = ext.encode('utf-8', 'surrogatepass')
+    if len(ext_bytes) > _MAX_EXT_BYTES:
+        stem, ext, ext_bytes = name, '', b''
+    stem_limit = max(limit - len(ext_bytes), _MIN_STEM_BYTES)
+    # 'ignore' drops a multi-byte character the cut landed inside of.
+    trimmed = stem.encode('utf-8', 'surrogatepass')[:stem_limit].decode('utf-8', 'ignore').rstrip()
+    return os.path.join(directory, (trimmed or '_') + ext)
+
+
 class _ConfinedYoutubeDL(yt_dlp.YoutubeDL):
     """A ``YoutubeDL`` that refuses to emit any output path outside the allowed roots.
 
@@ -219,6 +269,12 @@ class _ConfinedYoutubeDL(yt_dlp.YoutubeDL):
 
     def prepare_filename(self, *args, **kwargs):
         filename = super().prepare_filename(*args, **kwargs)
+        # Titles long enough to exceed the filesystem's filename limit are
+        # common on some sites, and the download fails outright when they do.
+        # Every output path comes through here, so trimming once keeps the
+        # main file, its chapter files, thumbnails and subtitles consistent.
+        if filename and filename != '-':
+            filename = _trim_to_name_max(filename)
         if filename and filename != '-' and self._allowed_roots:
             resolved = os.path.realpath(filename)
             if not any(_is_within_directory(root, resolved) for root in self._allowed_roots):