Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking change no allowing saving output with no extension #32838

Closed
5 of 6 tasks
eyaler opened this issue Jul 6, 2024 · 3 comments · Fixed by #32841
Closed
5 of 6 tasks

Breaking change no allowing saving output with no extension #32838

eyaler opened this issue Jul 6, 2024 · 3 comments · Fixed by #32841

Comments

@eyaler
Copy link

eyaler commented Jul 6, 2024

  • I'm reporting a broken site support issue
  • I've verified that I'm running youtube-dl version 2021.12.17
  • I've checked that all provided URLs are alive and playable in a browser
  • I've checked that all URLs and arguments with special characters are properly quoted or escaped
  • I've searched the bugtracker for similar bug reports including closed ones
  • I've read bugs section in FAQ

I am running master (e.g commit 37cea84) due to #31530, in google colab

Code to recreate:

youtube-dl "https://www.youtube.com/watch?v=OziXYniB5x4" --merge-output-format mp4 -o /content/video

Due to #32830 this results in

[youtube] OziXYniB5x4: Downloading webpage
ERROR: {0} found; to avoid damaging your system, this value is disallowed. If you believe this is an error{1}

You may also notice that the error message is confusing and seems to be malformatted.

Verbose log:

[debug] System config: []
[debug] User config: []
[debug] Custom config: []
[debug] Command-line args: ['-v', 'https://www.youtube.com/watch?v=OziXYniB5x4', '--merge-output-format', 'mp4', '-o', '/content/video']
[debug] Encodings: locale UTF-8, fs utf-8, out utf-8, pref UTF-8
[debug] youtube-dl version 2021.12.17
[debug] Python 3.10.12 (CPython x86_64 64bit) - Linux-6.1.85+-x86_64-with-glibc2.35 - OpenSSL 3.0.2 15 Mar 2022 - glibc 2.35
[debug] exe versions: ffmpeg 4.4.2, ffprobe 4.4.2
[debug] Proxy map: {'colab_language_server': '/usr/colab/bin/language_service'}
[youtube] OziXYniB5x4: Downloading webpage
[debug] [youtube] Decrypted nsig 2cirocww1IfMfwA3P97 => j2TzYKo9mb2M7g
[debug] [youtube] Decrypted nsig I3fjIKhij7IArBUqrEm => 8dyQjnajYvPirQ
[debug] Default format spec: bestvideo+bestaudio/best
ERROR: {0} found; to avoid damaging your system, this value is disallowed. If you believe this is an error{1}
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/dist-packages/youtube_dl/YoutubeDL.py", line 138, in wrapper
    return func(self, *args, **kwargs)
  File "/usr/local/lib/python3.10/dist-packages/youtube_dl/YoutubeDL.py", line 2137, in process_info
    fname = prepend_extension(
  File "/usr/local/lib/python3.10/dist-packages/youtube_dl/utils.py", line 3958, in _change_extension
    return '.'.join((filename, sanitize_extension(ext)))
  File "/usr/local/lib/python3.10/dist-packages/youtube_dl/utils.py", line 6684, in sanitize_extension
    raise cls(extension)
youtube_dl.utils._UnsafeExtensionError: unsafe file extension: 'f136'

Description:

Saving a video file without an extension is a practice which may be used for having a format-agnostic filename. This change breaks multiple scripts with this behavior. Please consider reverting the behavior to allow saving without file extension.
In my case I had to fix ~15 colabs. While there are easy fixes as --exec or --no-check-extensions or just doing mv after download, this issue addresses the breaking change aspect. Please consider whether specifically the extensionless case constitutes a security risk which justifies the breaking change.

@dirkf
Copy link
Contributor

dirkf commented Jul 6, 2024

Thanks for finding this, which follows from a difference in the way the back-ported mitigation is applied because of slightly different core processing. Specifically, in yt-dlp the expected extension is maintained while downloading to-be-merged streams but discarded on merging, so the mitigation sees video.f136.mp4; but we see video.f136 instead. Apart from a misplaced ), f136 would have been the {0} in the reported error message.

A patch is being prepared to allow the problem command to run without work-arounds.

However, the previous and patched behaviour both result in a file with mp4 extension. Perhaps your scripts are now fetching formats that need to be merged instead of a single stream, perhaps because YT is no longer sending format 22? So the bug you reported, while a real bug, isn't the real problem.

If you want a file named just video you can either ask for -f best and get (now, for OziXYniB5x4) format 18, 360p avc1 @ 142k + aac 44kHz @ 128k, or use --exec ... to rename the output file. Otherwise accept that the file will be video.mp4 and specify that in the output template, as a fixed value, rather than video.%(ext)s.

If this had been more carefully designed many years ago, the program might have respected an output template with no extension even when merging but I don't think we can change the behaviour now.

@dirkf dirkf added the bug label Jul 6, 2024
@dirkf
Copy link
Contributor

dirkf commented Jul 6, 2024

Also, yt-dlp (2024.07.02) behaves the same wrt video vs merged video.mp4.

@dirkf
Copy link
Contributor

dirkf commented Jul 6, 2024

Patch as mentioned:

--- old/youtube-dl/youtube_dl/YoutubeDL.py
+++ new/youtube-dl/youtube_dl/YoutubeDL.py
@@ -139,8 +139,8 @@
         except _UnsafeExtensionError as error:
             self.report_error(
                 '{0} found; to avoid damaging your system, this value is disallowed.'
-                ' If you believe this is an error{1}').format(
-                    error.message, bug_reports_message(','))
+                ' If you believe this is an error{1}'.format(
+                    error_to_compat_str(error), bug_reports_message(',')))
 
     return wrapper
 
@@ -2114,18 +2114,26 @@
                         # TODO: Check acodec/vcodec
                         return False
 
-                    filename_real_ext = os.path.splitext(filename)[1][1:]
-                    filename_wo_ext = (
-                        os.path.splitext(filename)[0]
-                        if filename_real_ext == info_dict['ext']
-                        else filename)
+                    exts = [info_dict['ext']]
                     requested_formats = info_dict['requested_formats']
                     if self.params.get('merge_output_format') is None and not compatible_formats(requested_formats):
                         info_dict['ext'] = 'mkv'
                         self.report_warning(
                             'Requested formats are incompatible for merge and will be merged into mkv.')
+                    exts.append(info_dict['ext'])
+
                     # Ensure filename always has a correct extension for successful merge
-                    filename = '%s.%s' % (filename_wo_ext, info_dict['ext'])
+                    def correct_ext(filename, ext=exts[1]):
+                        if filename == '-':
+                            return filename
+                        f_name, f_real_ext = os.path.splitext(filename)
+                        f_real_ext = f_real_ext[1:]
+                        filename_wo_ext = f_name if f_real_ext in exts else filename
+                        if ext is None:
+                            ext = f_real_ext or None 
+                        return join_nonempty(filename_wo_ext, ext, delim='.')
+
+                    filename = correct_ext(filename)
                     if os.path.exists(encodeFilename(filename)):
                         self.to_screen(
                             '[download] %s has already been downloaded and '
@@ -2135,8 +2143,9 @@
                             new_info = dict(info_dict)
                             new_info.update(f)
                             fname = prepend_extension(
-                                self.prepare_filename(new_info),
-                                'f%s' % f['format_id'], new_info['ext'])
+                                correct_ext(
+                                    self.prepare_filename(new_info), new_info['ext']),
+                                'f%s' % (f['format_id'],), new_info['ext'])
                             if not ensure_dir_exists(fname):
                                 return
                             downloaded.append(fname)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants