From b36392c62bfa44e70513bfd94727ac11a1db5466 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Wed, 10 Jun 2026 16:29:09 -0700 Subject: [PATCH] Fix: a '/' in a song title was treated as a path separator (#835) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit YouTube/Tidal/Qobuz results encode the name as ``id||title``. When the title itself contains a '/' (e.g. the Sawano AoT track "YouSeeBIGGIRL/T:T"), two places wrongly basename-split it on the slash and kept only the last segment ("T:T"): - core/downloads/file_finder.py — the completed-download finder truncated its search target to "T:T", so the real on-disk file (slash sanitised by the writer) never matched → "not found after processing" → the download got QUARANTINED. Now an encoded ``id||title`` keeps the whole title as the target and contributes no remote-directory components; real Soulseek PATHS still get basename + dir extraction unchanged. - webui/static/downloads.js — the manual-search FILE column showed only "T:T". Added a ``||``-aware short-label helper (mirrors the correct handling already used elsewhere in the file); real file paths still show their basename. Tests: the finder locates "YouSeeBIGGIRL∕T: T.mp3" from the encoded title "…||YouSeeBIGGIRL/T:T" (the screenshot case), doesn't match an unrelated file, and a genuine Soulseek path still resolves to its last segment. 21 finder tests + 64 script-split integrity tests pass. --- core/downloads/file_finder.py | 33 ++++++++++++------ tests/downloads/test_file_finder.py | 53 +++++++++++++++++++++++++++++ webui/static/downloads.js | 13 ++++++- 3 files changed, 87 insertions(+), 12 deletions(-) diff --git a/core/downloads/file_finder.py b/core/downloads/file_finder.py index 98aa4a66..0e6e03a7 100644 --- a/core/downloads/file_finder.py +++ b/core/downloads/file_finder.py @@ -83,15 +83,17 @@ def _normalize_for_finding(text: str) -> str: def _extract_basename(api_filename: str) -> str: - """Cross-platform rightmost-separator split, with YouTube / - Tidal ``id||title`` encoded filenames pre-normalised — the id - half is stripped so the title becomes the basename. Mirrors - the strip-then-split order ``web_server`` used.""" + """Cross-platform rightmost-separator split for a real remote PATH. + + A YouTube/Tidal/Qobuz ``id||title`` encoded filename is handled by + returning the title VERBATIM: the title is not a filesystem path, so a '/' + in it (e.g. the Sawano track ``YouSeeBIGGIRL/T:T``) is part of the name and + must NOT be split on (issue #835).""" if not api_filename: return "" if '||' in api_filename: _id, title = api_filename.split('||', 1) - api_filename = title + return title last_slash = max(api_filename.rfind('/'), api_filename.rfind('\\')) return api_filename[last_slash + 1:] if last_slash != -1 else api_filename @@ -236,14 +238,23 @@ def find_completed_audio_file( ``None`` when the file isn't found anywhere — callers should treat that as "not yet" (still mid-write) or "lost". """ - # YouTube / Tidal encoded filenames carry the id ahead of ``||``. - # Strip it up front so basename + dir-component extraction both - # operate on the title half. + # YouTube / Tidal / Qobuz encoded filenames carry the id ahead of ``||``. + # The title half is NOT a filesystem path: a '/' in it (e.g. the Sawano + # track ``YouSeeBIGGIRL/T:T``) is part of the title, so it must NOT be + # basename-split or read as a remote directory component — doing so + # truncated the search target to ``T:T`` and the real file was never found, + # quarantining valid downloads (issue #835). Real remote paths (Soulseek) + # still get basename + dir-component extraction. + encoded_title = None if api_filename and '||' in api_filename: - _id, api_filename = api_filename.split('||', 1) - target_basename = _extract_basename(api_filename) + _id, encoded_title = api_filename.split('||', 1) + if encoded_title is not None: + target_basename = encoded_title + api_dirs = [] + else: + target_basename = _extract_basename(api_filename) + api_dirs = _api_dir_parts(api_filename) normalized_target = _normalize_for_finding(target_basename) - api_dirs = _api_dir_parts(api_filename) best_dl_path, dl_sim = _search_in_directory( download_dir, 'downloads', target_basename, normalized_target, api_dirs, diff --git a/tests/downloads/test_file_finder.py b/tests/downloads/test_file_finder.py index a012faa3..5e7d6291 100644 --- a/tests/downloads/test_file_finder.py +++ b/tests/downloads/test_file_finder.py @@ -343,3 +343,56 @@ def test_fuzzy_rejects_low_similarity(tmp_path): if __name__ == '__main__': pytest.main([__file__, '-v']) + + +# --------------------------------------------------------------------------- +# Issue #835 — a '/' in a YouTube/Tidal title is part of the NAME, not a path +# separator. The encoded ``id||title`` finder previously basename-split the +# title ("YouSeeBIGGIRL/T:T" -> "T:T"), so the real on-disk file (with the +# slash sanitised) never matched and valid downloads got quarantined. +# --------------------------------------------------------------------------- + +from core.downloads.file_finder import _extract_basename + + +def test_encoded_title_with_slash_is_not_basename_split(): + # The Sawano AoT track. The id||title encoding must keep the whole title. + assert _extract_basename('vy63u2hKoPE||YouSeeBIGGIRL/T:T') == 'YouSeeBIGGIRL/T:T' + + +def test_finds_youtube_file_whose_title_contains_a_slash(tmp_path): + downloads = tmp_path / 'downloads' + # On disk the slash is sanitised to a look-alike and the colon spaced out, + # exactly as in the issue screenshot. + target = downloads / 'YouSeeBIGGIRL∕T: T.mp3' + _touch(target) + + found, location = find_completed_audio_file( + str(downloads), 'vy63u2hKoPE||YouSeeBIGGIRL/T:T', + ) + + assert found == str(target), 'pre-#835 this truncated the target to "T:T" and missed the file' + assert location == 'downloads' + + +def test_slash_title_does_not_match_an_unrelated_file(tmp_path): + # Guard against the fix being too loose: a different track must NOT match. + downloads = tmp_path / 'downloads' + _touch(downloads / 'Some Totally Different Song.mp3') + + found, _ = find_completed_audio_file( + str(downloads), 'vy63u2hKoPE||YouSeeBIGGIRL/T:T', + ) + assert found is None + + +def test_real_soulseek_path_still_basenamed(tmp_path): + # Regression: a genuine remote PATH must still resolve to its last segment. + downloads = tmp_path / 'downloads' + target = downloads / '01 - Suddenly.flac' + _touch(target) + assert _extract_basename(r'shared\Billy Ocean\Very Best Of\01 - Suddenly.flac') == '01 - Suddenly.flac' + found, _ = find_completed_audio_file( + str(downloads), r'shared\Billy Ocean\Very Best Of\01 - Suddenly.flac', + ) + assert found == str(target) diff --git a/webui/static/downloads.js b/webui/static/downloads.js index aeca1bd1..3faa2cc6 100644 --- a/webui/static/downloads.js +++ b/webui/static/downloads.js @@ -3060,8 +3060,19 @@ function _candidatesFmtDur(ms) { // rows (different click binding scope). ``showSourceBadge`` adds a small // per-row source pill — used in hybrid "All sources" mode where the user // otherwise can't tell which source a row came from. +// Display label for a candidate's filename. Encoded ``id||title`` sources +// (youtube/tidal/qobuz/hifi) carry the title after ``||`` — a '/' in that title +// is part of the name, NOT a path separator, so it must not be basename-split +// (issue #835: "YouSeeBIGGIRL/T:T" was showing as just "T:T"). Real file paths +// (Soulseek) keep the rightmost-segment basename. +function _ssShortFileLabel(filename) { + if (!filename) return '-'; + if (filename.includes('||')) return filename.split('||').slice(1).join('||'); + return filename.split(/[/\\]/).pop(); +} + function _renderCandidateRow(c, index, rowClass, showSourceBadge) { - const shortFile = c.filename ? c.filename.split(/[/\\]/).pop() : '-'; + const shortFile = _ssShortFileLabel(c.filename); const qBadge = c.quality ? `${c.quality.toUpperCase()}` : '';