From 4e27600329fe6eef5414db9f19048028b854bb3b Mon Sep 17 00:00:00 2001 From: James Tew Date: Mon, 20 Jul 2026 22:20:18 +0100 Subject: [PATCH] Added handling for unsupported URL --- app/tests/test_download_queue.py | 48 +++++++++++++++++++ app/tests/test_ytdl_utils.py | 19 ++++++++ app/ytdl.py | 79 +++++++++++++++++++++++++++++++- ui/src/app/app.html | 5 -- 4 files changed, 145 insertions(+), 6 deletions(-) diff --git a/app/tests/test_download_queue.py b/app/tests/test_download_queue.py index 4b375f6..c002cf2 100644 --- a/app/tests/test_download_queue.py +++ b/app/tests/test_download_queue.py @@ -115,6 +115,54 @@ async def test_add_single_video_goes_to_pending_when_auto_start_false(dq_env): assert dq.pending.exists("https://example.com/watch?v=1") +@pytest.mark.asyncio +async def test_add_unsupported_url_recorded_as_failed_entry(dq_env): + """An unsupported/unextractable URL must show up as a red-cross entry in the + done list, not just a transient toast and a server log line.""" + import ytdl + + notifier = AsyncMock() + url = "https://example.com/not-a-video" + + def boom(self, url, ytdl_options_presets=None, ytdl_options_overrides=None): + raise ytdl.yt_dlp.utils.YoutubeDLError(f'Unsupported URL: {url}') + + dq = DownloadQueue(dq_env, notifier) + with patch.object(DownloadQueue, "_DownloadQueue__extract_info", boom): + result = await dq.add( + url, "video", "auto", "any", "best", "", "", 0, auto_start=True, + ) + assert result["status"] == "error" + assert dq.done.exists(url) + failed = dq.done.get(url) + assert failed.info.status == "error" + assert failed.info.error == result["msg"] + assert failed.info.url == url + # The full URL stays in .url/.error for the detail panel; the display + # title is shortened to the hostname so the Completed row stays readable. + assert failed.info.title == "example.com" + notifier.completed.assert_awaited() + + +@pytest.mark.asyncio +async def test_add_ssrf_rejected_url_recorded_as_failed_entry(dq_env): + """A URL rejected by the SSRF guard (before yt-dlp ever runs) must also + surface as a failed entry, not just an error status returned to the caller.""" + notifier = AsyncMock() + url = "file:///etc/passwd" + + dq = DownloadQueue(dq_env, notifier) + result = await dq.add( + url, "video", "auto", "any", "best", "", "", 0, auto_start=True, + ) + assert result["status"] == "error" + assert dq.done.exists(url) + failed = dq.done.get(url) + assert failed.info.status == "error" + assert failed.info.error == result["msg"] + notifier.completed.assert_awaited() + + @pytest.mark.asyncio async def test_cancel_removes_from_pending(dq_env): notifier = AsyncMock() diff --git a/app/tests/test_ytdl_utils.py b/app/tests/test_ytdl_utils.py index e3315a8..c52ff47 100644 --- a/app/tests/test_ytdl_utils.py +++ b/app/tests/test_ytdl_utils.py @@ -60,6 +60,7 @@ from ytdl import ( _resolve_outtmpl_fields, _sanitize_entry_for_pickle, _sanitize_path_component, + _short_title_for_failed_url, ) # Detect whether the real yt-dlp is loaded (as opposed to the minimal fake @@ -747,5 +748,23 @@ class CompactPersistedEntryTests(unittest.TestCase): self.assertIsNone(_compact_persisted_entry({"id": "x", "title": "y"})) +class ShortTitleForFailedUrlTests(unittest.TestCase): + def test_uses_hostname_for_a_normal_url(self): + self.assertEqual( + _short_title_for_failed_url("https://example.com/watch?v=1"), + "example.com", + ) + + def test_falls_back_to_raw_value_when_there_is_no_hostname(self): + # file:// URIs and bare search terms/video IDs have no netloc to extract. + self.assertEqual(_short_title_for_failed_url("file:///etc/passwd"), "file:///etc/passwd") + self.assertEqual(_short_title_for_failed_url("ytsearch:some query"), "ytsearch:some query") + + def test_falls_back_to_raw_value_on_unparseable_input(self): + # A malformed IPv6-looking host raises ValueError in urlsplit().hostname. + malformed = "https://[::1/watch" + self.assertEqual(_short_title_for_failed_url(malformed), malformed) + + if __name__ == "__main__": unittest.main() diff --git a/app/ytdl.py b/app/ytdl.py index a44e02b..b07bb7d 100644 --- a/app/ytdl.py +++ b/app/ytdl.py @@ -27,6 +27,7 @@ from datetime import datetime from state_store import AtomicJsonStore, from_json_compatible, read_legacy_shelf, to_json_compatible from subscriptions import _entry_id from url_guard import validate_url +from urllib.parse import urlsplit log = logging.getLogger('ytdl') @@ -477,6 +478,17 @@ _PERSISTED_DOWNLOAD_FIELDS = ( ) +def _short_title_for_failed_url(url: str) -> str: + """A concise display title for a URL that failed before yt-dlp could extract a + real title (unsupported URL, SSRF-rejected, extraction error). The full URL + remains available in DownloadInfo.url and the error-detail panel.""" + try: + hostname = urlsplit(url).hostname + except ValueError: + hostname = None + return hostname or url + + _COMPACT_ENTRY_EXTRA_KEYS = frozenset(("n_entries", "__last_playlist_index")) @@ -1485,6 +1497,58 @@ class DownloadQueue: return {'status': 'ok'} return {'status': 'error', 'msg': f'Unsupported resource "{etype}"'} + async def __record_add_failure( + self, + url, + msg, + download_type, + codec, + format, + quality, + folder, + custom_name_prefix, + playlist_item_limit, + split_by_chapters, + chapter_template, + subtitle_language, + subtitle_mode, + ytdl_options_presets, + ytdl_options_overrides, + clip_start, + clip_end, + ): + """Surface a URL that failed before a DownloadInfo could be created (unsupported + URL, SSRF-rejected, extraction error) as a failed entry in the done list, so the + frontend shows it with the same red-cross/retry/error-detail treatment as a + download that failed mid-stream, instead of only a toast and a server log line.""" + info = DownloadInfo( + id=url, + title=_short_title_for_failed_url(url), + url=url, + quality=quality, + download_type=download_type, + codec=codec, + format=format, + folder=folder, + custom_name_prefix=custom_name_prefix, + error=msg, + entry=None, + playlist_item_limit=playlist_item_limit, + split_by_chapters=split_by_chapters, + chapter_template=chapter_template, + subtitle_language=subtitle_language, + subtitle_mode=subtitle_mode, + ytdl_options_presets=ytdl_options_presets, + ytdl_options_overrides=ytdl_options_overrides, + clip_start=clip_start, + clip_end=clip_end, + ) + info.status = 'error' + info.msg = msg + download = Download(None, None, None, None, quality, format, {}, info) + self.done.put(download) + await self.notifier.completed(info) + async def add( self, url, @@ -1529,6 +1593,12 @@ class DownloadQueue: url_error = await asyncio.get_running_loop().run_in_executor(None, validate_url, url) if url_error is not None: log.warning('Rejected URL "%s": %s', url, url_error) + await self.__record_add_failure( + url, url_error, download_type, codec, format, quality, folder, + custom_name_prefix, playlist_item_limit, split_by_chapters, chapter_template, + subtitle_language, subtitle_mode, ytdl_options_presets, ytdl_options_overrides, + clip_start, clip_end, + ) return {'status': 'error', 'msg': url_error} try: entry = await asyncio.get_running_loop().run_in_executor( @@ -1536,7 +1606,14 @@ class DownloadQueue: partial(self.__extract_info, url, ytdl_options_presets, ytdl_options_overrides), ) except yt_dlp.utils.YoutubeDLError as exc: - return {'status': 'error', 'msg': str(exc)} + msg = str(exc) + await self.__record_add_failure( + url, msg, download_type, codec, format, quality, folder, + custom_name_prefix, playlist_item_limit, split_by_chapters, chapter_template, + subtitle_language, subtitle_mode, ytdl_options_presets, ytdl_options_overrides, + clip_start, clip_end, + ) + return {'status': 'error', 'msg': msg} return await self.__add_entry( entry, download_type, diff --git a/ui/src/app/app.html b/ui/src/app/app.html index 5e875f3..593da32 100644 --- a/ui/src/app/app.html +++ b/ui/src/app/app.html @@ -794,11 +794,6 @@ @if (entry[1].status === 'error') { } @else { {{entry[1].title}}