From 34bf80c62db61581d8d6cc9c8849068e7de517c4 Mon Sep 17 00:00:00 2001 From: NiChrosia Date: Sat, 28 Oct 2023 16:49:40 -0500 Subject: [PATCH] fix: keep invalid Windows characters if --no-windows-filenames is specified --- test/test_utils.py | 98 +++++++++++++++++++------------------- yt_dlp/YoutubeDL.py | 3 +- yt_dlp/__init__.py | 1 + yt_dlp/extractor/common.py | 2 +- yt_dlp/options.py | 12 ++++- yt_dlp/utils/_utils.py | 24 +++++----- 6 files changed, 76 insertions(+), 64 deletions(-) diff --git a/test/test_utils.py b/test/test_utils.py index 090da47f7..ef6c4c49a 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -146,93 +146,93 @@ class TestUtil(unittest.TestCase): self.assertTrue(timeconvert('bougrg') is None) def test_sanitize_filename(self): - self.assertEqual(sanitize_filename('', use_win_filenames=True), '') - self.assertEqual(sanitize_filename('abc', use_win_filenames=True), 'abc') - self.assertEqual(sanitize_filename('abc_d-e', use_win_filenames=True), 'abc_d-e') + self.assertEqual(sanitize_filename(''), '') + self.assertEqual(sanitize_filename('abc'), 'abc') + self.assertEqual(sanitize_filename('abc_d-e'), 'abc_d-e') - self.assertEqual(sanitize_filename('123', use_win_filenames=True), '123') + self.assertEqual(sanitize_filename('123'), '123') - self.assertEqual('abc⧸de', sanitize_filename('abc/de', use_win_filenames=True)) - self.assertFalse('/' in sanitize_filename('abc/de///', use_win_filenames=True)) + self.assertEqual('abc⧸de', sanitize_filename('abc/de')) + self.assertFalse('/' in sanitize_filename('abc/de///')) - self.assertEqual('abc_de', sanitize_filename('abc/<>\\*|de', use_win_filenames=True, is_id=False)) - self.assertEqual('xxx', sanitize_filename('xxx/<>\\*|', use_win_filenames=True, is_id=False)) - self.assertEqual('yes no', sanitize_filename('yes? no', use_win_filenames=True, is_id=False)) - self.assertEqual('this - that', sanitize_filename('this: that', use_win_filenames=True, is_id=False)) + self.assertEqual('abc_de', sanitize_filename('abc/<>\\*|de', is_id=False)) + self.assertEqual('xxx', sanitize_filename('xxx/<>\\*|', is_id=False)) + self.assertEqual('yes no', sanitize_filename('yes? no', is_id=False)) + self.assertEqual('this - that', sanitize_filename('this: that', is_id=False)) - self.assertEqual('abc_<>\\*|de', sanitize_filename('abc/<>\\*|de', use_win_filenames=False, is_id=False)) - self.assertEqual('xxx_<>\\*|', sanitize_filename('xxx/<>\\*|', use_win_filenames=False, is_id=False)) - self.assertEqual('yes? no', sanitize_filename('yes? no', use_win_filenames=False, is_id=False)) - self.assertEqual('this: that', sanitize_filename('this: that', use_win_filenames=False, is_id=False)) + self.assertEqual('abc_<>\\*|de', sanitize_filename('abc/<>\\*|de', keep_bad_win_chars=True, is_id=False)) + self.assertEqual('xxx_<>\\*|', sanitize_filename('xxx/<>\\*|', keep_bad_win_chars=True, is_id=False)) + self.assertEqual('yes? no', sanitize_filename('yes? no', keep_bad_win_chars=True, is_id=False)) + self.assertEqual('this: that', sanitize_filename('this: that', keep_bad_win_chars=True, is_id=False)) - self.assertEqual(sanitize_filename('AT&T', use_win_filenames=True), 'AT&T') + self.assertEqual(sanitize_filename('AT&T'), 'AT&T') aumlaut = 'ä' - self.assertEqual(sanitize_filename(aumlaut, use_win_filenames=True), aumlaut) + self.assertEqual(sanitize_filename(aumlaut), aumlaut) tests = '\u043a\u0438\u0440\u0438\u043b\u043b\u0438\u0446\u0430' - self.assertEqual(sanitize_filename(tests, use_win_filenames=True), tests) + self.assertEqual(sanitize_filename(tests), tests) self.assertEqual( - sanitize_filename('New World record at 0:12:34', use_win_filenames=True), + sanitize_filename('New World record at 0:12:34'), 'New World record at 0_12_34') self.assertEqual( - sanitize_filename('New World record at 0:12:34', use_win_filenames=False), + sanitize_filename('New World record at 0:12:34', keep_bad_win_chars=True), 'New World record at 0:12:34') - self.assertEqual(sanitize_filename('--gasdgf', use_win_filenames=True), '--gasdgf') - self.assertEqual(sanitize_filename('--gasdgf', use_win_filenames=True, is_id=True), '--gasdgf') - self.assertEqual(sanitize_filename('--gasdgf', use_win_filenames=True, is_id=False), '_-gasdgf') - self.assertEqual(sanitize_filename('.gasdgf', use_win_filenames=True), '.gasdgf') - self.assertEqual(sanitize_filename('.gasdgf', use_win_filenames=True, is_id=True), '.gasdgf') - self.assertEqual(sanitize_filename('.gasdgf', use_win_filenames=True, is_id=False), 'gasdgf') + self.assertEqual(sanitize_filename('--gasdgf'), '--gasdgf') + self.assertEqual(sanitize_filename('--gasdgf', is_id=True), '--gasdgf') + self.assertEqual(sanitize_filename('--gasdgf', is_id=False), '_-gasdgf') + self.assertEqual(sanitize_filename('.gasdgf'), '.gasdgf') + self.assertEqual(sanitize_filename('.gasdgf', is_id=True), '.gasdgf') + self.assertEqual(sanitize_filename('.gasdgf', is_id=False), 'gasdgf') forbidden = '"\0\\/' for fc in forbidden: for fbc in forbidden: - self.assertTrue(fbc not in sanitize_filename(fc, use_win_filenames=True)) + self.assertTrue(fbc not in sanitize_filename(fc)) def test_sanitize_filename_restricted(self): - self.assertEqual(sanitize_filename('abc', use_win_filenames=True, restricted=True), 'abc') - self.assertEqual(sanitize_filename('abc_d-e', use_win_filenames=True, restricted=True), 'abc_d-e') + self.assertEqual(sanitize_filename('abc', restricted=True), 'abc') + self.assertEqual(sanitize_filename('abc_d-e', restricted=True), 'abc_d-e') - self.assertEqual(sanitize_filename('123', use_win_filenames=True, restricted=True), '123') + self.assertEqual(sanitize_filename('123', restricted=True), '123') - self.assertEqual('abc_de', sanitize_filename('abc/de', use_win_filenames=True, restricted=True)) - self.assertFalse('/' in sanitize_filename('abc/de///', use_win_filenames=True, restricted=True)) + self.assertEqual('abc_de', sanitize_filename('abc/de', restricted=True)) + self.assertFalse('/' in sanitize_filename('abc/de///', restricted=True)) - self.assertEqual('abc_de', sanitize_filename('abc/<>\\*|de', use_win_filenames=True, restricted=True)) - self.assertEqual('xxx', sanitize_filename('xxx/<>\\*|', use_win_filenames=True, restricted=True)) - self.assertEqual('yes_no', sanitize_filename('yes? no', use_win_filenames=True, restricted=True)) - self.assertEqual('this_-_that', sanitize_filename('this: that', use_win_filenames=True, restricted=True)) + self.assertEqual('abc_de', sanitize_filename('abc/<>\\*|de', restricted=True)) + self.assertEqual('xxx', sanitize_filename('xxx/<>\\*|', restricted=True)) + self.assertEqual('yes_no', sanitize_filename('yes? no', restricted=True)) + self.assertEqual('this_-_that', sanitize_filename('this: that', restricted=True)) tests = 'aäb\u4e2d\u56fd\u7684c' - self.assertEqual(sanitize_filename(tests, use_win_filenames=True, restricted=True), 'aab_c') - self.assertTrue(sanitize_filename('\xf6', use_win_filenames=True, restricted=True) != '') # No empty filename + self.assertEqual(sanitize_filename(tests, restricted=True), 'aab_c') + self.assertTrue(sanitize_filename('\xf6', restricted=True) != '') # No empty filename forbidden = '"\0\\/&!: \'\t\n()[]{}$;`^,#' for fc in forbidden: for fbc in forbidden: - self.assertTrue(fbc not in sanitize_filename(fc, use_win_filenames=True, restricted=True)) + self.assertTrue(fbc not in sanitize_filename(fc, restricted=True)) # Handle a common case more neatly - self.assertEqual(sanitize_filename('\u5927\u58f0\u5e26 - Song', use_win_filenames=True, restricted=True), 'Song') - self.assertEqual(sanitize_filename('\u603b\u7edf: Speech', use_win_filenames=True, restricted=True), 'Speech') + self.assertEqual(sanitize_filename('\u5927\u58f0\u5e26 - Song', restricted=True), 'Song') + self.assertEqual(sanitize_filename('\u603b\u7edf: Speech', restricted=True), 'Speech') # .. but make sure the file name is never empty - self.assertTrue(sanitize_filename('-', use_win_filenames=True, restricted=True) != '') - self.assertTrue(sanitize_filename(':', use_win_filenames=True, restricted=True) != '') + self.assertTrue(sanitize_filename('-', restricted=True) != '') + self.assertTrue(sanitize_filename(':', restricted=True) != '') self.assertEqual(sanitize_filename( - 'ÂÃÄÀÁÅÆÇÈÉÊËÌÍÎÏÐÑÒÓÔÕÖŐØŒÙÚÛÜŰÝÞßàáâãäåæçèéêëìíîïðñòóôõöőøœùúûüűýþÿ', use_win_filenames=True, restricted=True), + 'ÂÃÄÀÁÅÆÇÈÉÊËÌÍÎÏÐÑÒÓÔÕÖŐØŒÙÚÛÜŰÝÞßàáâãäåæçèéêëìíîïðñòóôõöőøœùúûüűýþÿ', restricted=True), 'AAAAAAAECEEEEIIIIDNOOOOOOOOEUUUUUYTHssaaaaaaaeceeeeiiiionooooooooeuuuuuythy') def test_sanitize_ids(self): - self.assertEqual(sanitize_filename('_n_cd26wFpw', use_win_filenames=True, is_id=True), '_n_cd26wFpw') - self.assertEqual(sanitize_filename('_BD_eEpuzXw', use_win_filenames=True, is_id=True), '_BD_eEpuzXw') - self.assertEqual(sanitize_filename('N0Y__7-UOdI', use_win_filenames=True, is_id=True), 'N0Y__7-UOdI') + self.assertEqual(sanitize_filename('_n_cd26wFpw', is_id=True), '_n_cd26wFpw') + self.assertEqual(sanitize_filename('_BD_eEpuzXw', is_id=True), '_BD_eEpuzXw') + self.assertEqual(sanitize_filename('N0Y__7-UOdI', is_id=True), 'N0Y__7-UOdI') - self.assertEqual(sanitize_filename('_n_cd26wFpw', use_win_filenames=False, is_id=True), '_n_cd26wFpw') - self.assertEqual(sanitize_filename('_BD_eEpuzXw', use_win_filenames=False, is_id=True), '_BD_eEpuzXw') - self.assertEqual(sanitize_filename('N0Y__7-UOdI', use_win_filenames=False, is_id=True), 'N0Y__7-UOdI') + self.assertEqual(sanitize_filename('_n_cd26wFpw', keep_bad_win_chars=True, is_id=True), '_n_cd26wFpw') + self.assertEqual(sanitize_filename('_BD_eEpuzXw', keep_bad_win_chars=True, is_id=True), '_BD_eEpuzXw') + self.assertEqual(sanitize_filename('N0Y__7-UOdI', keep_bad_win_chars=True, is_id=True), 'N0Y__7-UOdI') def test_sanitize_path(self): if sys.platform != 'win32': diff --git a/yt_dlp/YoutubeDL.py b/yt_dlp/YoutubeDL.py index 83bd627dc..6cdb14997 100644 --- a/yt_dlp/YoutubeDL.py +++ b/yt_dlp/YoutubeDL.py @@ -1255,7 +1255,8 @@ class YoutubeDL: def filename_sanitizer(key, value, restricted=self.params.get('restrictfilenames')): return sanitize_filename( - str(value), self.params.get("windowsfilenames", False), restricted=restricted, is_id=( + str(value), self.params.get('keep_bad_win_chars', False), restricted=restricted, + is_id=( bool(re.search(r'(^|[_.])id(\.|$)', key)) if 'filename-sanitization' in self.params['compat_opts'] else NO_DEFAULT)) diff --git a/yt_dlp/__init__.py b/yt_dlp/__init__.py index 991dbcda7..b1d993280 100644 --- a/yt_dlp/__init__.py +++ b/yt_dlp/__init__.py @@ -809,6 +809,7 @@ def parse_options(argv=None): 'autonumber_start': opts.autonumber_start, 'restrictfilenames': opts.restrictfilenames, 'windowsfilenames': opts.windowsfilenames, + 'keep_bad_win_chars': opts.keep_bad_win_chars, 'ignoreerrors': opts.ignoreerrors, 'force_generic_extractor': opts.force_generic_extractor, 'allowed_extractors': opts.allowed_extractors or ['default'], diff --git a/yt_dlp/extractor/common.py b/yt_dlp/extractor/common.py index 26e3c643f..7f2811958 100644 --- a/yt_dlp/extractor/common.py +++ b/yt_dlp/extractor/common.py @@ -958,7 +958,7 @@ class InfoExtractor: if len(basen) > trim_length: h = '___' + hashlib.md5(basen.encode('utf-8')).hexdigest() basen = basen[:trim_length - len(h)] + h - filename = sanitize_filename(f'{basen}.dump', self.get_param("windowsfilenames", False), restricted=True) + filename = sanitize_filename(f'{basen}.dump', self.get_param('keep_bad_win_chars', False), restricted=True) # Working around MAX_PATH limitation on Windows (see # http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx) if compat_os_name == 'nt': diff --git a/yt_dlp/options.py b/yt_dlp/options.py index 4254974fc..013142625 100644 --- a/yt_dlp/options.py +++ b/yt_dlp/options.py @@ -264,6 +264,10 @@ def create_parser(): out_dict[key] = out_dict.get(key, []) + [val] if append else val setattr(parser.values, option.dest, out_dict) + def _store_multiple_callback(option, opt_str, value, parser, values): + for key, value in values.items(): + setattr(parser.values, key, value) + def when_prefix(default): return { 'default': {}, @@ -1334,7 +1338,13 @@ def create_parser(): help='Force filenames to be Windows-compatible') filesystem.add_option( '--no-windows-filenames', - action='store_false', dest='windowsfilenames', + action='callback', dest='keep_bad_win_chars', default=False, callback=_store_multiple_callback, + callback_kwargs={ + 'values': { + 'windowsfilenames': False, + 'keep_bad_win_chars': True + } + }, help='Make filenames Windows-compatible only if using Windows (default)') filesystem.add_option( '--trim-filenames', '--trim-file-names', metavar='LENGTH', diff --git a/yt_dlp/utils/_utils.py b/yt_dlp/utils/_utils.py index 7027d3c12..d51ea14b1 100644 --- a/yt_dlp/utils/_utils.py +++ b/yt_dlp/utils/_utils.py @@ -612,12 +612,12 @@ def timeconvert(timestr): return timestamp -def sanitize_filename(s, use_win_filenames, restricted=False, is_id=NO_DEFAULT): +def sanitize_filename(s, keep_bad_win_chars=False, restricted=False, is_id=NO_DEFAULT): """Sanitizes a string so it could be used as part of a filename. - @param use_win_filenames Whether to use Windows filename restrictions - @param restricted Use a stricter subset of allowed characters - @param is_id Whether this is an ID that should be kept unchanged if possible. - If unset, yt-dlp's new sanitization rules are in effect + @param keep_bad_win_chars Whether to keep characters invalid on Windows + @param restricted Use a stricter subset of allowed characters + @param is_id Whether this is an ID that should be kept unchanged if possible. + If unset, yt-dlp's new sanitization rules are in effect """ if s == '': return '' @@ -627,16 +627,16 @@ def sanitize_filename(s, use_win_filenames, restricted=False, is_id=NO_DEFAULT): return ACCENT_CHARS[char] elif not restricted and char == '\n': return '\0 ' - elif is_id is NO_DEFAULT and not restricted and char in '"*:<>?|/\\' and use_win_filenames: + elif is_id is NO_DEFAULT and not restricted and char in '"*:<>?|/\\' and not keep_bad_win_chars: # Replace with their full-width unicode counterparts return {'/': '\u29F8', '\\': '\u29f9'}.get(char, chr(ord(char) + 0xfee0)) - elif (use_win_filenames and char == '?') or ord(char) < 32 or ord(char) == 127: + elif (not keep_bad_win_chars and char == '?') or ord(char) < 32 or ord(char) == 127: return '' - elif use_win_filenames and char == '"': + elif not keep_bad_win_chars and char == '"': return '' if restricted else '\'' - elif use_win_filenames and char == ':': + elif not keep_bad_win_chars and char == ':': return '\0_\0-' if restricted else '\0 \0-' - elif (use_win_filenames and char in '\\|*<>') or char == "/": + elif (not keep_bad_win_chars and char in '\\|*<>') or char == '/': return '\0_' if restricted and (char in '!&\'()[]{}$;`^,#' or char.isspace() or ord(char) > 127): return '\0_' @@ -645,8 +645,8 @@ def sanitize_filename(s, use_win_filenames, restricted=False, is_id=NO_DEFAULT): # Replace look-alike Unicode glyphs if restricted and (is_id is NO_DEFAULT or not is_id): s = unicodedata.normalize('NFKC', s) - s = re.sub(r'[0-9]+(?::[0-9]+)+', lambda m: m.group(0).replace(':', '_') - if use_win_filenames else m.group(0), s) # Handle timestamps + s = re.sub(r'[0-9]+(?::[0-9]+)+', lambda m: m.group(0) if keep_bad_win_chars + else m.group(0).replace(':', '_'), s) # Handle timestamps result = ''.join(map(replace_insane, s)) if is_id is NO_DEFAULT: result = re.sub(r'(\0.)(?:(?=\1)..)+', r'\1', result) # Remove repeated substitute chars