From 9805b807e577bd53064060634a033e686608e980 Mon Sep 17 00:00:00 2001 From: Maik Steiger Date: Fri, 9 Oct 2026 13:33:22 +0200 Subject: [PATCH] fix(tui): removed a ton of use_username_dirs references feat(tui): username is now appended to download_dir before passed as parameter fix(tui): username is sanitized before appended to path --- soulseek-rs-lib/examples/stress.rs | 10 ++---- soulseek-rs-lib/src/client/downloads.rs | 4 --- soulseek-rs-lib/src/client/tests/mod.rs | 1 - soulseek-rs-lib/src/client/tests/transfers.rs | 1 - soulseek-rs-lib/src/download_store.rs | 1 - soulseek-rs-lib/src/peer/download_peer.rs | 5 --- soulseek-rs-lib/src/peer/listen.rs | 1 - soulseek-rs-lib/src/types.rs | 1 - soulseek-rs/docs/openrpc.json | 4 --- soulseek-rs/src/api.rs | 14 +------- soulseek-rs/src/commands/transfer.rs | 1 - soulseek-rs/src/daemon/mod.rs | 1 - soulseek-rs/src/daemon/proto/dto.rs | 2 -- soulseek-rs/src/daemon/server/mod.rs | 4 --- soulseek-rs/src/persist/config.rs | 2 +- soulseek-rs/src/persist/snapshot.rs | 1 - soulseek-rs/src/persist/state.rs | 3 -- soulseek-rs/src/remote/mod.rs | 3 -- soulseek-rs/src/ui/downloads.rs | 1 - soulseek-rs/src/ui/main_tui/browse.rs | 2 -- soulseek-rs/src/ui/main_tui/downloads.rs | 24 +++++++------ soulseek-rs/src/ui/main_tui/mod.rs | 17 +++++++-- soulseek-rs/src/ui/main_tui/settings.rs | 2 +- soulseek-rs/src/ui/main_tui/tests/mod.rs | 3 -- soulseek-rs/src/ui/panes/browse_pane.rs | 1 - .../src/ui/panes/download_info_pane.rs | 1 - soulseek-rs/src/ui/utils.rs | 35 +++++++++++++++++++ soulseek-rs/tests/persist_e2e.rs | 1 - 28 files changed, 69 insertions(+), 77 deletions(-) diff --git a/soulseek-rs-lib/examples/stress.rs b/soulseek-rs-lib/examples/stress.rs index c216ce1..31d086e 100644 --- a/soulseek-rs-lib/examples/stress.rs +++ b/soulseek-rs-lib/examples/stress.rs @@ -1148,13 +1148,9 @@ fn main() { let completed = completed.clone(); workers.push(std::thread::spawn(move || { Metrics::bump(&metrics.downloads_issued); - let Ok((_download, status_rx)) = client.download( - filename.clone(), - peer.clone(), - size, - dir, - false, - ) else { + let Ok((_download, status_rx)) = + client.download(filename.clone(), peer.clone(), size, dir) + else { return; }; let deadline = run_deadline; diff --git a/soulseek-rs-lib/src/client/downloads.rs b/soulseek-rs-lib/src/client/downloads.rs index b9074eb..e47db5d 100644 --- a/soulseek-rs-lib/src/client/downloads.rs +++ b/soulseek-rs-lib/src/client/downloads.rs @@ -99,14 +99,12 @@ impl Client { username: String, size: u64, download_directory: String, - use_username_dirs: bool, ) -> Result<(Download, Receiver)> { self.download_with_metadata( filename, username, size, download_directory, - use_username_dirs, DownloadMetadata::default(), ) } @@ -117,7 +115,6 @@ impl Client { username: String, size: u64, download_directory: String, - use_username_dirs: bool, metadata: DownloadMetadata, ) -> Result<(Download, Receiver)> { info!("[client] Downloading {} from {}", filename, username); @@ -135,7 +132,6 @@ impl Client { token, size, download_directory, - use_username_dirs, status: DownloadStatus::Queued, sender: download_sender, queue_position: None, diff --git a/soulseek-rs-lib/src/client/tests/mod.rs b/soulseek-rs-lib/src/client/tests/mod.rs index 6d236ad..0862c05 100644 --- a/soulseek-rs-lib/src/client/tests/mod.rs +++ b/soulseek-rs-lib/src/client/tests/mod.rs @@ -23,7 +23,6 @@ fn download( token, size: 100, download_directory: "test".to_string(), - use_username_dirs: false, status, sender, queue_position: None, diff --git a/soulseek-rs-lib/src/client/tests/transfers.rs b/soulseek-rs-lib/src/client/tests/transfers.rs index fa49269..dbabdd3 100644 --- a/soulseek-rs-lib/src/client/tests/transfers.rs +++ b/soulseek-rs-lib/src/client/tests/transfers.rs @@ -97,7 +97,6 @@ fn download_without_a_connection_resolves_failed() { "peer".to_string(), 100, "test".to_string(), - false, ) .expect("download() should return a handle"); assert!(matches!( diff --git a/soulseek-rs-lib/src/download_store.rs b/soulseek-rs-lib/src/download_store.rs index 6b8b041..02f274a 100644 --- a/soulseek-rs-lib/src/download_store.rs +++ b/soulseek-rs-lib/src/download_store.rs @@ -242,7 +242,6 @@ mod tests { token, size: 100, download_directory: "test".to_string(), - use_username_dirs: false, status, sender: mpsc::channel().0, queue_position: None, diff --git a/soulseek-rs-lib/src/peer/download_peer.rs b/soulseek-rs-lib/src/peer/download_peer.rs index 5d19499..fd2e231 100644 --- a/soulseek-rs-lib/src/peer/download_peer.rs +++ b/soulseek-rs-lib/src/peer/download_peer.rs @@ -110,10 +110,6 @@ fn resolve_download_path(download: &Download) -> Result { .to_path_buf(); } - if download.use_username_dirs { - expanded_path = expanded_path.join(download.username.as_str()); - } - let final_path = expanded_path.join(extract_filename_from_path(&download.filename)); @@ -639,7 +635,6 @@ mod tests { token: 1, size, download_directory: dir.display().to_string(), - use_username_dirs: false, status: DownloadStatus::Queued, sender: mpsc::channel().0, queue_position: None, diff --git a/soulseek-rs-lib/src/peer/listen.rs b/soulseek-rs-lib/src/peer/listen.rs index 3890649..beb1f64 100644 --- a/soulseek-rs-lib/src/peer/listen.rs +++ b/soulseek-rs-lib/src/peer/listen.rs @@ -479,7 +479,6 @@ mod tests { token, size: 100, download_directory: std::env::temp_dir().display().to_string(), - use_username_dirs: false, status: DownloadStatus::Queued, sender: mpsc::channel().0, queue_position: None, diff --git a/soulseek-rs-lib/src/types.rs b/soulseek-rs-lib/src/types.rs index a34ac4b..8fa98e1 100644 --- a/soulseek-rs-lib/src/types.rs +++ b/soulseek-rs-lib/src/types.rs @@ -185,7 +185,6 @@ pub struct Download { pub token: u32, pub size: u64, pub download_directory: String, - pub use_username_dirs: bool, pub status: DownloadStatus, pub sender: Sender, pub queue_position: Option, diff --git a/soulseek-rs/docs/openrpc.json b/soulseek-rs/docs/openrpc.json index aaab7dd..cbf59a6 100644 --- a/soulseek-rs/docs/openrpc.json +++ b/soulseek-rs/docs/openrpc.json @@ -240,9 +240,6 @@ "minimum": 0, "type": "integer" }, - "use_username_dirs": { - "type": "boolean" - }, "username": { "type": "string" } @@ -253,7 +250,6 @@ "token", "size", "download_directory", - "use_username_dirs", "status" ], "type": "object" diff --git a/soulseek-rs/src/api.rs b/soulseek-rs/src/api.rs index 328000b..e3a10ba 100644 --- a/soulseek-rs/src/api.rs +++ b/soulseek-rs/src/api.rs @@ -102,7 +102,6 @@ pub trait SessionApi: Send + Sync { username: String, size: u64, download_directory: String, - use_username_dirs: bool, ) -> Result<(Download, Receiver)>; fn download_with_metadata( &self, @@ -110,7 +109,6 @@ pub trait SessionApi: Send + Sync { username: String, size: u64, download_directory: String, - use_username_dirs: bool, metadata: DownloadMetadata, ) -> Result<(Download, Receiver)>; fn get_all_downloads(&self) -> Vec; @@ -273,16 +271,8 @@ impl SessionApi for Client { username: String, size: u64, download_directory: String, - use_username_dirs: bool, ) -> Result<(Download, Receiver)> { - Self::download( - self, - filename, - username, - size, - download_directory, - use_username_dirs, - ) + Self::download(self, filename, username, size, download_directory) } fn download_with_metadata( @@ -291,7 +281,6 @@ impl SessionApi for Client { username: String, size: u64, download_directory: String, - use_username_dirs: bool, metadata: DownloadMetadata, ) -> Result<(Download, Receiver)> { Self::download_with_metadata( @@ -300,7 +289,6 @@ impl SessionApi for Client { username, size, download_directory, - use_username_dirs, metadata, ) } diff --git a/soulseek-rs/src/commands/transfer.rs b/soulseek-rs/src/commands/transfer.rs index c96da0c..b97b054 100644 --- a/soulseek-rs/src/commands/transfer.rs +++ b/soulseek-rs/src/commands/transfer.rs @@ -518,7 +518,6 @@ fn download_one( request.user.clone(), request.size, download_dir.to_string(), - false, ) .map_err(|e| CliError::transfer(format!("cannot start: {e}")))?; diff --git a/soulseek-rs/src/daemon/mod.rs b/soulseek-rs/src/daemon/mod.rs index 6fb086b..43453d9 100644 --- a/soulseek-rs/src/daemon/mod.rs +++ b/soulseek-rs/src/daemon/mod.rs @@ -441,7 +441,6 @@ fn restore( download.username.clone(), download.size, download.download_directory.clone(), - download.use_username_dirs, ) { Ok((_, updates)) => { if let Ok(mut pending) = downloads.lock() { diff --git a/soulseek-rs/src/daemon/proto/dto.rs b/soulseek-rs/src/daemon/proto/dto.rs index 514429e..2598859 100644 --- a/soulseek-rs/src/daemon/proto/dto.rs +++ b/soulseek-rs/src/daemon/proto/dto.rs @@ -220,7 +220,6 @@ pub struct DownloadDto { pub token: u32, pub size: u64, pub download_directory: String, - pub use_username_dirs: bool, pub status: DownloadStatusDto, #[serde(default)] pub queue_position: Option, @@ -236,7 +235,6 @@ impl From<&Download> for DownloadDto { token: download.token, size: download.size, download_directory: download.download_directory.clone(), - use_username_dirs: download.use_username_dirs, status: DownloadStatusDto::from(&download.status), queue_position: download.queue_position, metadata: DownloadMetadataDto::from(&download.metadata), diff --git a/soulseek-rs/src/daemon/server/mod.rs b/soulseek-rs/src/daemon/server/mod.rs index 5f2a7a7..201a6d0 100644 --- a/soulseek-rs/src/daemon/server/mod.rs +++ b/soulseek-rs/src/daemon/server/mod.rs @@ -436,7 +436,6 @@ impl Daemon { params.username.clone(), params.size, directory, - false, params.metadata.into(), ) .map_err(|e| { @@ -877,7 +876,6 @@ mod tests { username: String, size: u64, directory: String, - use_username_dirs: bool, ) -> soulseek_rs::Result<( soulseek_rs::types::Download, Receiver, @@ -887,7 +885,6 @@ mod tests { username, size, directory, - use_username_dirs, soulseek_rs::types::DownloadMetadata::default(), ) } @@ -897,7 +894,6 @@ mod tests { _username: String, _size: u64, _directory: String, - _use_username_dirs: bool, _metadata: soulseek_rs::types::DownloadMetadata, ) -> soulseek_rs::Result<( soulseek_rs::types::Download, diff --git a/soulseek-rs/src/persist/config.rs b/soulseek-rs/src/persist/config.rs index bde67f9..6a4578e 100644 --- a/soulseek-rs/src/persist/config.rs +++ b/soulseek-rs/src/persist/config.rs @@ -86,7 +86,7 @@ impl FileConfig { "daemon" => self.daemon.clone(), "daemon_token" => self.daemon_token.clone(), "use_username_dirs" => { - self.use_username_dirs.clone().map(|v| v.to_string()) + self.use_username_dirs.map(|v| v.to_string()) } _ => None, } diff --git a/soulseek-rs/src/persist/snapshot.rs b/soulseek-rs/src/persist/snapshot.rs index 1c8429a..554f3c8 100644 --- a/soulseek-rs/src/persist/snapshot.rs +++ b/soulseek-rs/src/persist/snapshot.rs @@ -126,7 +126,6 @@ mod tests { token: 7, size: 42, download_directory: "/music".into(), - use_username_dirs: false, status, sender, queue_position: None, diff --git a/soulseek-rs/src/persist/state.rs b/soulseek-rs/src/persist/state.rs index dcb7658..ffb6c54 100644 --- a/soulseek-rs/src/persist/state.rs +++ b/soulseek-rs/src/persist/state.rs @@ -22,7 +22,6 @@ pub struct PersistedDownload { pub filename: String, pub size: u64, pub download_directory: String, - pub use_username_dirs: bool, pub completed: bool, } @@ -37,7 +36,6 @@ impl PersistedDownload { filename: download.filename.clone(), size: download.size, download_directory: download.download_directory.clone(), - use_username_dirs: download.use_username_dirs, completed: matches!(download.status, DownloadStatus::Completed), }) } @@ -184,7 +182,6 @@ mod tests { filename: "@@abc\\music\\song.mp3".into(), size: 123, download_directory: "/music".into(), - use_username_dirs: false, completed: false, } } diff --git a/soulseek-rs/src/remote/mod.rs b/soulseek-rs/src/remote/mod.rs index 923ba14..6339eb1 100644 --- a/soulseek-rs/src/remote/mod.rs +++ b/soulseek-rs/src/remote/mod.rs @@ -367,7 +367,6 @@ fn rebuild(dto: DownloadDto, sender: Sender) -> Download { token: dto.token, size: dto.size, download_directory: dto.download_directory, - use_username_dirs: dto.use_username_dirs, status: dto.status.into(), sender, queue_position: dto.queue_position, @@ -579,7 +578,6 @@ impl SessionApi for RemoteSession { username: String, size: u64, _download_directory: String, - _use_username_dirs: bool, // the daemon's own download directory wins — a remote caller // cannot see the filesystem the bytes land on. Plumb a per-transfer // directory through if a client ever needs to sort into subfolders. @@ -598,7 +596,6 @@ impl SessionApi for RemoteSession { username: String, size: u64, _download_directory: String, - _use_username_dirs: bool, metadata: DownloadMetadata, ) -> Result<(Download, Receiver)> { self.start_download(filename, username, size, metadata) diff --git a/soulseek-rs/src/ui/downloads.rs b/soulseek-rs/src/ui/downloads.rs index 3f1e1bf..24eec71 100644 --- a/soulseek-rs/src/ui/downloads.rs +++ b/soulseek-rs/src/ui/downloads.rs @@ -177,7 +177,6 @@ mod tests { token: 1, size, download_directory: "/tmp".to_string(), - use_username_dirs: false, status, sender, queue_position: None, diff --git a/soulseek-rs/src/ui/main_tui/browse.rs b/soulseek-rs/src/ui/main_tui/browse.rs index bb21aa0..2727014 100644 --- a/soulseek-rs/src/ui/main_tui/browse.rs +++ b/soulseek-rs/src/ui/main_tui/browse.rs @@ -221,7 +221,6 @@ impl MainTui { let sender = self.downloads_sender(); let client = self.client.clone(); let download_dir = self.download_dir.clone(); - let use_username_dirs = self.use_username_dirs; thread::spawn(move || { for (path, size) in files { match client.download( @@ -229,7 +228,6 @@ impl MainTui { username.clone(), size, download_dir.clone(), - use_username_dirs, ) { Ok((download, rx)) => { let _ = sender.send((download, rx)); diff --git a/soulseek-rs/src/ui/main_tui/downloads.rs b/soulseek-rs/src/ui/main_tui/downloads.rs index c4d1e1b..37cf177 100644 --- a/soulseek-rs/src/ui/main_tui/downloads.rs +++ b/soulseek-rs/src/ui/main_tui/downloads.rs @@ -1,8 +1,10 @@ use super::MainTui; use crate::models::{DownloadEntry, FileDisplayData}; use crate::ui::panes::{InfoSubject, selected_transfer}; +use crate::ui::sanitize_path_component; use soulseek_rs::types::UploadStatus; use soulseek_rs::{DownloadStatus, types::Download}; +use std::path::Path; use std::{sync::mpsc, thread}; impl MainTui { @@ -48,16 +50,9 @@ impl MainTui { self.state.downloads.remove(index); self.select_download_after_removal(index); let _ = self.client.remove_download(&username, &filename); - let use_username_dirs = self.use_username_dirs; thread::spawn(move || { - match client.download( - filename.clone(), - username, - size, - directory, - use_username_dirs, - ) { + match client.download(filename.clone(), username, size, directory) { Ok((download, rx)) => { let _ = sender.send((download, rx)); } @@ -214,12 +209,21 @@ impl MainTui { peer_upload_speed: Some(file.speed), peer_free_slots: Some(file.slots), }; + let download_dir = if use_username_dirs { + let sanitized_username = + sanitize_path_component(file.username.as_str()); + Path::new(download_dir.as_str()) + .join(sanitized_username.as_str()) + .display() + .to_string() + } else { + download_dir.clone() + }; match client.download_with_metadata( file.filename.clone(), file.username.clone(), file.size, - download_dir.clone(), - use_username_dirs, + download_dir, metadata, ) { Ok((download, rx)) => { diff --git a/soulseek-rs/src/ui/main_tui/mod.rs b/soulseek-rs/src/ui/main_tui/mod.rs index c38f140..2b0a396 100644 --- a/soulseek-rs/src/ui/main_tui/mod.rs +++ b/soulseek-rs/src/ui/main_tui/mod.rs @@ -12,11 +12,13 @@ use crate::persist::{ snapshot::{Snapshot, restore_messages, restore_searches}, state::{PersistedMessage, StateStore}, }; +use crate::ui::sanitize_path_component; use color_eyre::Result; use ratatui::{ DefaultTerminal, crossterm::event::{self, Event, KeyEventKind, poll}, }; +use std::path::Path; use std::{sync::Arc, time::Duration}; pub struct MainTui { @@ -121,7 +123,6 @@ impl MainTui { token: 0, size: entry.size, download_directory: entry.download_directory, - use_username_dirs: entry.use_username_dirs, status: soulseek_rs::DownloadStatus::Completed, sender: std::sync::mpsc::channel().0, queue_position: None, @@ -133,13 +134,23 @@ impl MainTui { } else { let client = self.client.clone(); let sender = sender.clone(); + let use_username_dirs = self.use_username_dirs; std::thread::spawn(move || { + let sanitized_username = + sanitize_path_component(entry.username.as_str()); + let download_dir = if use_username_dirs { + Path::new(entry.download_directory.as_str()) + .join(sanitized_username.as_str()) + .display() + .to_string() + } else { + entry.download_directory + }; match client.download( entry.filename.clone(), entry.username, entry.size, - entry.download_directory, - entry.use_username_dirs, + download_dir, ) { Ok((download, rx)) => { let _ = sender.send((download, rx)); diff --git a/soulseek-rs/src/ui/main_tui/settings.rs b/soulseek-rs/src/ui/main_tui/settings.rs index 650d086..8416dc2 100644 --- a/soulseek-rs/src/ui/main_tui/settings.rs +++ b/soulseek-rs/src/ui/main_tui/settings.rs @@ -20,7 +20,7 @@ impl MainTui { account, self.download_dir.clone(), self.client.shared_directories(), - self.use_username_dirs.clone(), + self.use_username_dirs, )); } diff --git a/soulseek-rs/src/ui/main_tui/tests/mod.rs b/soulseek-rs/src/ui/main_tui/tests/mod.rs index 96420a3..5e74025 100644 --- a/soulseek-rs/src/ui/main_tui/tests/mod.rs +++ b/soulseek-rs/src/ui/main_tui/tests/mod.rs @@ -84,7 +84,6 @@ fn at_status( token: 1, size: 4096, download_directory: "/tmp".to_string(), - use_username_dirs: false, status, sender, queue_position: None, @@ -161,7 +160,6 @@ impl SessionApi for TalkativeSession { _username: String, _size: u64, _directory: String, - _use_username_dirs: bool, ) -> soulseek_rs::Result<( soulseek_rs::types::Download, std::sync::mpsc::Receiver, @@ -174,7 +172,6 @@ impl SessionApi for TalkativeSession { _username: String, _size: u64, _directory: String, - _use_username_dirs: bool, _metadata: soulseek_rs::types::DownloadMetadata, ) -> soulseek_rs::Result<( soulseek_rs::types::Download, diff --git a/soulseek-rs/src/ui/panes/browse_pane.rs b/soulseek-rs/src/ui/panes/browse_pane.rs index ad3ca08..cc95259 100644 --- a/soulseek-rs/src/ui/panes/browse_pane.rs +++ b/soulseek-rs/src/ui/panes/browse_pane.rs @@ -339,7 +339,6 @@ mod tests { token: 1, size: 4096, download_directory: "/tmp".to_string(), - use_username_dirs: false, status, sender, queue_position: None, diff --git a/soulseek-rs/src/ui/panes/download_info_pane.rs b/soulseek-rs/src/ui/panes/download_info_pane.rs index a006f51..d90c480 100644 --- a/soulseek-rs/src/ui/panes/download_info_pane.rs +++ b/soulseek-rs/src/ui/panes/download_info_pane.rs @@ -436,7 +436,6 @@ mod tests { token: 1, size: 100, download_directory: "~/dl".to_string(), - use_username_dirs: false, status, sender, queue_position: None, diff --git a/soulseek-rs/src/ui/utils.rs b/soulseek-rs/src/ui/utils.rs index fd09f44..83c97c1 100644 --- a/soulseek-rs/src/ui/utils.rs +++ b/soulseek-rs/src/ui/utils.rs @@ -72,6 +72,41 @@ pub const fn get_spinner_char(state: usize) -> &'static str { SPINNER_CHARS[state % SPINNER_CHARS.len()] } +#[must_use] +pub fn sanitize_path_component(name: &str) -> String { + let mut out = name + .chars() + .map(|c| match c { + '/' | '\\' | ':' | '*' | '?' | '"' | '<' | '>' | '|' => '_', + c if c.is_control() => '_', + c => c, + }) + .collect::(); + if out.is_empty() || out.ends_with(['.', ' ']) { + out.push('_'); + } + if is_reserved(out.as_str()) { + out.insert(0, '_'); + } + out +} + +#[cfg(target_os = "windows")] +fn is_reserved(name: &str) -> bool { + let stem = name.split('.').next().unwrap_or(name).to_ascii_uppercase(); + if matches!(stem.as_str(), "CON" | "PRN" | "AUX" | "NUL") { + true + } else { + matches!(stem.get(..3), Some("COM" | "LPT")) + && matches!(stem.as_bytes().get(3..), Some([b'1'..=b'9'])) + } +} + +#[cfg(not(target_os = "windows"))] +fn is_reserved(name: &str) -> bool { + false +} + #[cfg(test)] mod tests { use super::*; diff --git a/soulseek-rs/tests/persist_e2e.rs b/soulseek-rs/tests/persist_e2e.rs index 559518b..c678b23 100644 --- a/soulseek-rs/tests/persist_e2e.rs +++ b/soulseek-rs/tests/persist_e2e.rs @@ -17,7 +17,6 @@ fn download_entry(filename: &str, status: DownloadStatus) -> DownloadEntry { token: 1, size: 1000, download_directory: "/music".into(), - use_username_dirs: false, status, sender: std::sync::mpsc::channel().0, queue_position: None,