diff --git a/tubesync/common/utils.py b/tubesync/common/utils.py index b0236e7c7..141b65a93 100644 --- a/tubesync/common/utils.py +++ b/tubesync/common/utils.py @@ -209,7 +209,7 @@ def parse_database_connection_string(database_connection_string): f'as a database connection string: {e}') from e driver = parts.scheme user_pass_host_port = parts.netloc - database = parts.path + database = parts.path.removeprefix('/') if driver not in valid_drivers: raise DatabaseConnectionError(f'Database connection string ' f'"{database_connection_string}" specified an ' @@ -220,6 +220,8 @@ def parse_database_connection_string(database_connection_string): if len(host_parts) != 2 or len(user_pass_parts) != 2: raise DatabaseConnectionError('Database connection string netloc must be in ' 'the format of user:pass@host') + # user_pass never used + # ruff: ignore[RUF059] user_pass, host_port = host_parts username, password = user_pass_parts host_port_parts = host_port.split(':') @@ -244,8 +246,6 @@ def parse_database_connection_string(database_connection_string): # Malformed raise DatabaseConnectionError('Database connection host must be a hostname or ' 'a hostname:port combination') - if database.startswith('/'): - database = database[1:] if not database: raise DatabaseConnectionError('Database connection string path must be a ' 'string in the format of /databasename') @@ -284,9 +284,9 @@ def append_uri_params(uri, params): return urlunsplit(('', '', uri, qs, '')) -def clean_filename(filename): +def clean_filename(filename: str) -> str: if not isinstance(filename, str): - raise ValueError(f'filename must be a str, got {type(filename)}') + raise TypeError(f'filename must be a str, got {type(filename)}') to_scrub = r'<>\/:*?"|%' for char in list(to_scrub): filename = filename.replace(char, '') @@ -299,19 +299,58 @@ def clean_filename(filename): return clean_filename.strip() -def clean_emoji(s): +def clean_emoji(s: str) -> str: if not isinstance(s, str): - raise ValueError(f'parameter must be a str, got {type(s)}') + raise TypeError(f'parameter must be a str, got {type(s)}') return emoji.replace_emoji(s) +def truncate_filename(filename: str, *, max_bytes=216, encoding='utf-8') -> str: + ''' + Shortens a filename to fit within `max_bytes` bytes (not characters) + while keeping its extension intact. Filesystems limit name length in + bytes (commonly 255), so multi-byte titles can exceed the limit with + far fewer characters (see issue #522). The default budget leaves + headroom for prefixes/suffixes appended later (e.g. `.fNNN`, + `.part-FragNN` fragments written by yt-dlp during downloads). + + Bytes are removed from the middle of the stem, keeping its start + (usually the beginning of the title) and its end (usually unique + suffixes such as the media key and format details, e.g. + `{title_full}_{key}_{format}` from the default media format), joined + by `_..._`. Truncation never splits a multi-byte character: partial + trailing/leading sequences are dropped when decoding. + ''' + max_bytes = max(96, min(232, max_bytes)) + if not isinstance(filename, str): + raise TypeError(f'filename must be a str, got {type(filename)}') + if len(filename.encode(encoding)) <= max_bytes: + return filename + path = Path(filename) + name, ext = clean_filename(path.stem), clean_filename(path.suffix) + ext_bytes = ext.encode(encoding) + if len(ext_bytes) >= max_bytes: + # Pathological extension; fall back to a plain byte cut + name, ext_bytes = filename, b'' + marker = '_..._' + stem_budget = max_bytes - len(ext_bytes) - len(marker.encode(encoding)) + stem_bytes = name.encode(encoding) + # Keep the unique suffixes at the end of the stem intact (up to a third of + # the bytes limit), then fill the rest from the front. + tail_keep = min(stem_budget // 2, max_bytes // 3) + head_keep = stem_budget - tail_keep + head = stem_bytes[:head_keep].decode(encoding, errors='ignore').rstrip() + tail = stem_bytes[-tail_keep:].decode(encoding, errors='ignore').lstrip() + return head + marker + tail + ext_bytes.decode(encoding) + + def seconds_to_timestr(seconds): seconds = seconds % (24 * 3600) hour = seconds // 3600 seconds %= 3600 minutes = seconds // 60 seconds %= 60 - return '{:02d}:{:02d}:{:02d}'.format(hour, minutes, seconds) + return f'{hour:02d}:{minutes:02d}:{seconds:02d}' def time_func(func): diff --git a/tubesync/sync/models/media.py b/tubesync/sync/models/media.py index 1c0c0acee..26f78dce2 100644 --- a/tubesync/sync/models/media.py +++ b/tubesync/sync/models/media.py @@ -5,6 +5,7 @@ from copy import deepcopy from datetime import datetime, timedelta, timezone as tz from pathlib import Path +from typing import ClassVar from xml.etree import ElementTree from django.conf import settings from django.db import models @@ -19,6 +20,7 @@ from common.utils import ( clean_filename, clean_emoji, directory_and_stem, glob_quote, mkdir_p, seconds_to_timestr, + truncate_filename, ) from ..youtube import ( get_media_info as get_youtube_media_info, @@ -45,6 +47,8 @@ ) from .source import Source +# ruff: file-ignore[B010,RUF059,SIM118] + class Media(models.Model): ''' @@ -56,14 +60,14 @@ class Media(models.Model): posix_epoch = datetime(1970, 1, 1, tzinfo=tz.utc) # Format to use to display a URL for the media - URLS = _srctype_dict('https://www.youtube.com/watch?v={key}') + URLS: ClassVar[dict[str, str]] = _srctype_dict('https://www.youtube.com/watch?v={key}') # Callback functions to get a list of media from the source - INDEXERS = _srctype_dict(get_youtube_media_info) + INDEXERS: ClassVar[dict[str, type(get_youtube_media_info)]] = _srctype_dict(get_youtube_media_info) # Maps standardised names to names used in source metdata _same_name = lambda n, k=None: {k or n: _srctype_dict(n) } - METADATA_FIELDS = { + METADATA_FIELDS: ClassVar[dict[str, dict[str, str]]] = { **(_same_name('upload_date')), **(_same_name('timestamp')), **(_same_name('title')), @@ -80,7 +84,7 @@ class Media(models.Model): **(_same_name('playlist_title')), } - STATE_ICONS = dict(zip( + STATE_ICONS: ClassVar[dict[str, str]] = dict(zip( MediaState.values, ( '', @@ -596,14 +600,17 @@ def metadata_clear(self, /, *, save=False): self.save() - def metadata_dumps(self, arg_dict=dict()): + def metadata_dumps(self, arg_dict=None): fallback = dict() try: fallback.update(self.new_metadata.with_formats) except ObjectDoesNotExist: pass - data = arg_dict or fallback - return json.dumps(data, separators=(',', ':'), cls=JSONEncoder) + return json.dumps( + arg_dict or fallback, + separators=(',', ':'), + cls=JSONEncoder, + ) def metadata_loads(self, arg_str='{}'): @@ -646,7 +653,6 @@ def save_to_metadata(self, key, value, /): migrated['_using_table'] = True self.metadata = self.metadata_dumps(arg_dict=migrated) self.save() - from common.logger import log log.debug(f'Saved to metadata: {self.key} / {self.uuid}: {key=}: {value}') @@ -677,11 +683,10 @@ def reduce_data(self): filtered_data = filter_response(data, True) filtered_data['_reduce_data_ran_at'] = round((now - self.posix_epoch).total_seconds()) filtered_json = self.metadata_dumps(arg_dict=filtered_data) - except Exception as e: - from common.logger import log - log.exception('reduce_data: %s', e) + # ruff: ignore[BLE001] + except Exception: + log.exception(f'Media.reduce_data: {self.pk}') else: - from common.logger import log # log the results of filtering / compacting on metadata size new_mdl = len(compact_json) if old_mdl > new_mdl: @@ -720,6 +725,7 @@ def loaded_metadata(self): pass setattr(self, '_cached_metadata_dict', data) return data + # ruff: ignore[BLE001] except Exception: return {} @@ -742,7 +748,6 @@ def ts_to_dt(self, /, timestamp): timestamp_float = float(timestamp) except (TypeError, ValueError,) as e: log.warn(f'Could not compute published from timestamp for: {self.source} / {self} with "{e}"') - pass else: return self.posix_epoch + timedelta(seconds=timestamp_float) return None @@ -775,14 +780,18 @@ def name(self): @property def upload_date(self): + ts = self.get_metadata_first_value('timestamp') + dt = self.ts_to_dt(ts) if ts else None + if dt and dt > self.posix_epoch: + return dt + upload_date_str = self.get_metadata_first_value('upload_date') if not upload_date_str: return None try: - return datetime.strptime(upload_date_str, '%Y%m%d') + return datetime.strptime(upload_date_str, '%Y%m%d').replace(tzinfo=tz.utc) except (AttributeError, ValueError) as e: log.debug(f'Media.upload_date: {self.source} / {self}: strptime: {e}') - pass return None @property @@ -841,7 +850,22 @@ def filename(self): media_format = str(self.source.media_format) media_details = self.format_dict result = media_format.format(**media_details) - return '.' + result if '/' == result[0] else result + result = '.' + result if '/' == result[0] else result + # Filesystems limit each path component to 255 bytes (not + # characters), and multi-byte titles can blow past that with far + # fewer characters — downloads then fail with: + # [Errno 36] File name too long + # (issue #522). Only the final component (the name) is shortened; + # any directories in the format string are preserved. The budget + # leaves headroom for suffixes appended during download + # (`.fNNN.ext.part-FragNNN.part` and thumbnail/subtitle siblings). + path = Path(result) + truncated = truncate_filename(path.name) + if truncated != path.name: + log.warning(f'Media filename exceeded the filesystem byte limit ' + f'and was shortened: {self!r}') + return str(path.with_name(truncated)) + return result @property def directory_path(self): @@ -1022,10 +1046,11 @@ def get_download_state(self, task=None): if self.downloaded: return Val(MediaState.DOWNLOADED) if task: + # Avoid the circular import `ImportError` from using this at the top of the file. + from ..tasks import get_media_download_task def running(arg_task, /): if hasattr(arg_task, 'locked_by_pid_running'): return arg_task.locked_by_pid_running() - from ..tasks import get_media_download_task return get_media_download_task(str(self.pk)) if running(task): return Val(MediaState.DOWNLOADING) @@ -1063,6 +1088,7 @@ def index_metadata(self): ''' indexer = self.INDEXERS.get(self.source.source_type, None) if not callable(indexer): + # ruff: ignore[TRY002,TRY004] raise Exception(f'Media with source type f"{self.source.source_type}" ' f'has no indexer') response = indexer(self.url) diff --git a/tubesync/sync/tests/test_filepath.py b/tubesync/sync/tests/test_filepath.py index a843e31fd..ed841b9b7 100644 --- a/tubesync/sync/tests/test_filepath.py +++ b/tubesync/sync/tests/test_filepath.py @@ -1,8 +1,10 @@ +import json import logging from pathlib import Path from django.conf import settings from django.test import TestCase from django.utils import timezone +from common.utils import truncate_filename from sync.models import Source, Media from sync.choices import ( Val, Fallback, SourceResolution, @@ -158,6 +160,77 @@ def test_media_filename(self): ('no-fancy-stuff-title_test_720p-720x1280-opus' '-vp9-30fps-hdr.mkv')) + def test_media_filename_truncates_to_filesystem_byte_limit(self): + # Filesystems limit each name component to 255 bytes (issue #522). + # Multi-byte titles reach that with far fewer characters, and the + # download then fails with '[Errno 36] File name too long'. + long_metadata = json.loads(metadata) + long_metadata['title'] = '耳' * 120 # 3 bytes per char = 360 bytes + long_title_media = Media.objects.create( + key='longkey', + source=self.source, + metadata=json.dumps(long_metadata), + ) + self.source.media_format = '{yyyy}/{title_full}_{key}.{ext}' + filename = long_title_media.filename + directory, _, name = filename.rpartition('/') + # Directories from the format string survive untouched + self.assertEqual('2017', directory) + # The name component fits in the byte budget... + self.assertLessEqual(len(name.encode('utf-8')), 216) + # ... keeps its extension and key suffix material intact ... + self.assertTrue(name.endswith('_longkey.mkv')) + # ... and was not cut mid multi-byte character (encodes cleanly) + name.encode('utf-8').decode('utf-8') + + def test_media_filename_unchanged_when_within_limit(self): + self.source.media_format = '{yyyy}/{key}.{ext}' + self.assertEqual(self.media.filename, '2017/mediakey.mkv') + + def test_truncate_filename_bytes_encoding_edge_cases(self): + # Bytes known to cause encoding/decoding trouble must never produce + # an invalid or over-budget name: the cut points land inside + # multi-byte sequences on purpose here. + + cases = [ + # 4-byte astral plane (emoji): cut lands mid-sequence + 'šŸ£' * 100 + '_key.mkv', + # combining characters (Ć© as e + U+0301) + ('e\u0301' * 150) + '_key.mkv', + # zero-width joiner sequences (family emoji) + ('šŸ‘Ø\u200dšŸ‘©\u200dšŸ‘§\u200dšŸ‘¦' * 30) + '_key.mkv', + # mixed 1-byte/3-byte at every boundary parity + ('a耳' * 120) + '_key.mkv', + # right-to-left text + ('שלום' * 60) + '_key.mkv', + # no extension at all + '⽕' * 200, + # dotfile-style name (suffix is empty for PurePosixPath) + '.' + ('h' * 300), + # very long "extension" exceeding the whole budget + 'name.' + ('x' * 300), + ] + for original in cases: + with self.subTest(original=original[:24]): + result = truncate_filename(original, max_bytes=208) + # fits the byte budget + self.assertLessEqual(len(result.encode('utf-8')), 208) + # still valid UTF-8 round-trip (no partial sequences kept) + self.assertEqual( + result, + result.encode('utf-8').decode('utf-8'), + ) + # never empty + self.assertTrue(result) + + def test_truncate_filename_bytes_rejects_non_str(self): + + for bad in (None, 42, b'bytes.mkv', Path('p.mkv')): + # ruff: ignore[SIM117] + with self.subTest(bad=bad): + with self.assertRaises(TypeError): + truncate_filename(bad) + def test_directory_prefix(self): # Confirm the setting exists and is valid self.assertTrue(hasattr(settings, 'SOURCE_DOWNLOAD_DIRECTORY_PREFIX'))