Fix: a '/' in a song title was treated as a path separator (#835)
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.
This commit is contained in:
parent
cc18ec266e
commit
b36392c62b
3 changed files with 87 additions and 12 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
? `<span class="candidates-quality-badge candidates-quality-${c.quality.toLowerCase()}">${c.quality.toUpperCase()}</span>`
|
||||
: '';
|
||||
|
|
|
|||
Loading…
Reference in a new issue