Merge pull request #101 from MaikSteiger/develop

feat: save downloads in per-user folders (use_username_dirs). Thanks @MaikSteiger.

README.md

@@ -606,6 +606,7 @@ file, in that order of precedence. Flags work before or after the subcommand.
 | `--listener-port`              | `SOULSEEK_LISTENER_PORT`            | `listener_port`              | `2234`                    |
 | `--no-listener` / `--listener` | `SOULSEEK_NO_LISTENER`              | `disable_listener`           | listener on               |
 | —                              | —                                   | `accept_children`            | off                       |
+| —                              | —                                   | `use_username_dirs`          | off                       |
 | `--max-concurrent-downloads`   | `SOULSEEK_MAX_CONCURRENT_DOWNLOADS` | `max_concurrent_downloads`   | `20`                      |
 | `--search-timeout`             | `SOULSEEK_SEARCH_TIMEOUT`           | `search_timeout`             | `10`                      |
 | `--daemon ADDR`                | `SOULSEEK_DAEMON`                   | `daemon`                     | local socket if one is up |
@@ -627,10 +628,10 @@ to them. It is off by default — each child costs a socket and a copy of the
 network's whole search stream — and it needs the listener, since a child has to
 be able to dial in.
 
-`config get` and `config set` cover the twelve settings the file holds
+`config get` and `config set` cover the thirteen settings the file holds
 (`username`, `server`, `listener_port`, `disable_listener`, `accept_children`,
-`download_dir`, `shared_dirs`, `max_concurrent_downloads`, `search_timeout`,
+`download_dir`, `use_username_dirs`, `shared_dirs`, `max_concurrent_downloads`,
-`password_cmd`, `daemon`, `daemon_token`), and list them back at you when you
+`search_timeout`, `password_cmd`, `daemon`, `daemon_token`), and list them back at you when you
 name something
 else. An empty string clears a key, and `shared_dirs` takes a comma-separated
 list. Waits expressed in seconds (`--search-timeout`, `--timeout`,

soulseek-rs/src/commands/mod.rs

@@ -34,6 +34,8 @@ pub struct Ctx {
     pub out: Out,
     pub settings: ClientSettings,
     pub download_dir: String,
+    /// Save each peer's files under `download_dir/<username>`.
+    pub use_username_dirs: bool,
     pub max_concurrent_downloads: usize,
     pub search_timeout: Duration,
     /// Where to find a daemon, when this run should use one rather than open
@@ -47,6 +49,16 @@ pub struct Ctx {
 }
 
 impl Ctx {
+    /// Where a download from `username` lands.
+    #[must_use]
+    pub fn dir_for(&self, username: &str) -> String {
+        crate::directories::download_dir_for(
+            &self.download_dir,
+            username,
+            self.use_username_dirs,
+        )
+    }
+
     /// Re-read a "found nothing", "timed out" or "transfer failed" verdict
     /// against the session that produced it.
     ///

soulseek-rs/src/commands/transfer.rs

@@ -458,7 +458,7 @@ fn download_all(
                         client.as_ref(),
                         &ctx.out,
                         &request,
-                        &ctx.download_dir,
+                        &ctx.dir_for(&request.user),
                         timeout,
                     );
                     if sender.send((request, outcome)).is_err() {

soulseek-rs/src/daemon/mod.rs

@@ -202,6 +202,7 @@ pub fn run(
         browses: Arc::clone(&browses),
         downloads: Arc::clone(&downloads),
         download_dir: Mutex::new(ctx.download_dir.clone()),
+        use_username_dirs: ctx.use_username_dirs,
         server: ctx.settings.server_address.to_string(),
         token: token.clone(),
         started: Instant::now(),

soulseek-rs/src/daemon/server/mod.rs

@@ -38,6 +38,7 @@ pub struct Daemon {
     /// Where transfers land, on *this* machine. A caller may point it at a
     /// different folder with `download.set_dir`, but never per download.
     pub download_dir: Mutex<String>,
+    pub use_username_dirs: bool,
     pub server: String,
     pub token: String,
     pub started: Instant,
@@ -428,7 +429,11 @@ impl Daemon {
         // per download would scatter transfers in ways `daemon.status` cannot
         // report; changing the folder is a deliberate session-wide act,
         // `download.set_dir`.
-        let directory = self.download_dir();
+        let directory = crate::directories::download_dir_for(
+            &self.download_dir(),
+            &params.username,
+            self.use_username_dirs,
+        );
         let (download, updates) = self
             .session
             .download_with_metadata(
@@ -800,6 +805,7 @@ mod tests {
             browses: Arc::new(PendingBrowses::default()),
             downloads: Arc::new(Mutex::new(Vec::new())),
             download_dir: Mutex::new("/tmp".to_string()),
+            use_username_dirs: false,
             server: "server.invalid:2416".to_string(),
             token: token.to_string(),
             started: Instant::now(),

soulseek-rs/src/directories.rs

@@ -50,9 +50,54 @@ pub fn resolve_shared_directories(dirs: &[String]) -> Vec<String> {
     resolved
 }
 
+/// The folder a download from `username` lands in: `download_dir` itself, or
+/// a per-user subfolder of it when `per_user` is set, as SoulseekQt does.
+#[must_use]
+pub fn download_dir_for(
+    download_dir: &str,
+    username: &str,
+    per_user: bool,
+) -> String {
+    if !per_user {
+        return download_dir.to_string();
+    }
+    std::path::Path::new(download_dir)
+        .join(sanitize_path_component(username))
+        .display()
+        .to_string()
+}
+
+/// A peer-chosen name made safe as one path component: separators and
+/// characters Windows refuses become `_`, so `../..` cannot climb out.
+fn sanitize_path_component(name: &str) -> String {
+    let mut out: String = 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) {
+        out.insert(0, '_');
+    }
+    out
+}
+
+/// Windows device names (`CON`, `COM1`, …) that cannot be a folder name.
+fn is_reserved(name: &str) -> bool {
+    let stem = name.split('.').next().unwrap_or(name).to_ascii_uppercase();
+    matches!(stem.as_str(), "CON" | "PRN" | "AUX" | "NUL")
+        || (matches!(stem.get(..3), Some("COM" | "LPT"))
+            && matches!(stem.as_bytes().get(3..), Some([b'1'..=b'9'])))
+}
+
 #[cfg(test)]
 mod tests {
-    use super::resolve_shared_directory;
+    use super::{download_dir_for, resolve_shared_directory};
     use std::sync::atomic::{AtomicU32, Ordering};
 
     fn unique_temp_dir() -> std::path::PathBuf {
@@ -97,4 +142,23 @@ mod tests {
         assert!(result.is_err());
         let _ = std::fs::remove_dir_all(dir);
     }
+
+    fn dl(name: &str) -> String {
+        std::path::Path::new("/dl").join(name).display().to_string()
+    }
+
+    #[test]
+    fn per_user_folder_is_a_child_of_the_download_dir() {
+        assert_eq!(download_dir_for("/dl", "alice", false), "/dl");
+        assert_eq!(download_dir_for("/dl", "alice", true), dl("alice"));
+    }
+
+    #[test]
+    fn a_hostile_username_cannot_leave_the_download_dir() {
+        assert_eq!(download_dir_for("/dl", "../..", true), dl(".._.._"));
+        assert_eq!(download_dir_for("/dl", "/tmp/x", true), dl("_tmp_x"));
+        assert_eq!(download_dir_for("/dl", "..", true), dl(".._"));
+        assert_eq!(download_dir_for("/dl", "", true), dl("_"));
+        assert_eq!(download_dir_for("/dl", "com1", true), dl("_com1"));
+    }
 }

soulseek-rs/src/models/settings.rs

@@ -38,6 +38,7 @@ impl SettingsMode {
 pub enum SettingsRow {
     ChangePassword,
     Logout,
+    UseUsernameDirs,
     DownloadDir,
     /// Share path at this index.
     Share(usize),
@@ -79,6 +80,7 @@ pub struct SettingsState {
     pub account: AccountInfo,
     pub download_dir: String,
     pub share_dirs: Vec<String>,
+    pub use_username_dirs: bool,
     /// Index into [`Self::rows`].
     pub selected: usize,
     pub mode: SettingsMode,
@@ -94,11 +96,13 @@ impl SettingsState {
         account: AccountInfo,
         download_dir: String,
         share_dirs: Vec<String>,
+        use_username_dirs: bool,
     ) -> Self {
         Self {
             account,
             download_dir,
             share_dirs,
+            use_username_dirs,
             selected: 0,
             mode: SettingsMode::Navigate,
             input: String::new(),
@@ -119,6 +123,7 @@ impl SettingsState {
         if self.can_log_out() {
             rows.push(SettingsRow::Logout);
         }
+        rows.push(SettingsRow::UseUsernameDirs);
         rows.push(SettingsRow::DownloadDir);
         rows.extend((0..self.share_dirs.len()).map(SettingsRow::Share));
         rows
@@ -191,6 +196,10 @@ impl SettingsState {
                 self.input.clear();
             }
             SettingsRow::Logout => self.mode = SettingsMode::ConfirmingLogout,
+            SettingsRow::UseUsernameDirs => {
+                self.use_username_dirs = !self.use_username_dirs;
+                return SettingsAction::Apply;
+            }
             SettingsRow::DownloadDir => {
                 self.mode = SettingsMode::EditingDownloadDir;
                 self.input.clone_from(&self.download_dir);
@@ -300,6 +309,7 @@ mod tests {
             account(),
             "/dl".into(),
             vec!["/dl".into(), "/music".into()],
+            false,
         )
     }
 
@@ -493,11 +503,13 @@ mod tests {
             },
             "/dl".into(),
             vec!["/music".into()],
+            false,
         );
         assert_eq!(
             s.rows(),
             vec![
                 SettingsRow::ChangePassword,
+                SettingsRow::UseUsernameDirs,
                 SettingsRow::DownloadDir,
                 SettingsRow::Share(0),
             ]
@@ -505,6 +517,8 @@ mod tests {
         // The row below "change password" is the download folder, not a
         // logout the window cannot perform.
         s.handle_key(key(KeyCode::Down));
+        assert_eq!(s.selected_row(), SettingsRow::UseUsernameDirs);
+        s.handle_key(key(KeyCode::Down));
         assert_eq!(s.selected_row(), SettingsRow::DownloadDir);
     }
 }

soulseek-rs/src/persist/config.rs

@@ -40,6 +40,7 @@ pub struct FileConfig {
     /// A TOML array rather than a `config set` key, because a query may itself
     /// contain a comma.
     pub wishlist: Option<Vec<String>>,
+    pub use_username_dirs: Option<bool>,
 }
 
 impl FileConfig {
@@ -59,6 +60,7 @@ impl FileConfig {
         "password_cmd",
         "daemon",
         "daemon_token",
+        "use_username_dirs",
     ];
 
     /// The value of `key`, or `None` when it is unset or unknown. Lists are
@@ -83,6 +85,9 @@ impl FileConfig {
             "password_cmd" => self.password_cmd.clone(),
             "daemon" => self.daemon.clone(),
             "daemon_token" => self.daemon_token.clone(),
+            "use_username_dirs" => {
+                self.use_username_dirs.map(|v| v.to_string())
+            }
             _ => None,
         }
     }
@@ -162,6 +167,17 @@ impl FileConfig {
                 // The single-folder spelling would otherwise shadow the list.
                 self.shared_dir = None;
             }
+            "use_username_dirs" => {
+                self.use_username_dirs = Some(match value {
+                    "true" | "1" | "yes" => true,
+                    "false" | "0" | "no" => false,
+                    other => {
+                        return Err(format!(
+                            "use_username_dirs wants true or false, got '{other}'"
+                        ));
+                    }
+                });
+            }
             other => {
                 return Err(format!(
                     "unknown setting '{other}' — try one of: {}",
@@ -277,6 +293,7 @@ pub struct Resolved {
     pub accept_children: bool,
     pub download_dir: String,
     pub shared_dirs: Vec<String>,
+    pub use_username_dirs: bool,
     pub max_concurrent_downloads: usize,
     pub search_timeout: u64,
     pub password_cmd: Option<String>,
@@ -330,6 +347,7 @@ pub fn resolve(cli: &crate::cli::Cli, file: &FileConfig) -> Resolved {
             && !disable_listener,
         download_dir: download_dir.clone(),
         shared_dirs: resolve_shared_dirs(cli, file, &download_dir),
+        use_username_dirs: file.use_username_dirs.unwrap_or(false),
         max_concurrent_downloads: cli
             .max_concurrent_downloads
             .or(file.max_concurrent_downloads)
@@ -573,6 +591,7 @@ mod tests {
             daemon: Some("nas.local:5030".into()),
             daemon_token: Some("deadbeef".into()),
             wishlist: None,
+            use_username_dirs: Some(false),
         };
         let resolved = resolve(&bare_cli(), &file);
         assert_eq!(resolved.username.as_deref(), Some("alice"));

soulseek-rs/src/run.rs

@@ -276,6 +276,7 @@ fn context(
             version: ClientVersion::REFERENCE_CLIENT,
         },
         download_dir,
+        use_username_dirs: resolved.use_username_dirs,
         max_concurrent_downloads: resolved.max_concurrent_downloads,
         search_timeout: Duration::from_secs(resolved.search_timeout),
         daemon,
@@ -522,6 +523,7 @@ fn run_default_tui(
             terminal,
             Arc::new(outcome.client),
             resolved.download_dir.clone(),
+            resolved.use_username_dirs,
             Duration::from_secs(resolved.search_timeout),
             store,
             config_path.clone(),
@@ -598,6 +600,7 @@ fn run_attached_tui(
         terminal,
         Arc::new(session),
         download_dir,
+        resolved.use_username_dirs,
         Duration::from_secs(resolved.search_timeout),
         None,
         config_path,

soulseek-rs/src/ui/main_tui/downloads.rs

@@ -1,4 +1,5 @@
 use super::MainTui;
+use crate::directories::download_dir_for;
 use crate::models::{DownloadEntry, FileDisplayData};
 use crate::ui::panes::{InfoSubject, selected_transfer};
 use soulseek_rs::types::UploadStatus;
@@ -197,6 +198,7 @@ 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 file in selected_files {
@@ -210,7 +212,11 @@ impl MainTui {
                     file.filename.clone(),
                     file.username.clone(),
                     file.size,
-                    download_dir.clone(),
+                    download_dir_for(
+                        &download_dir,
+                        &file.username,
+                        use_username_dirs,
+                    ),
                     metadata,
                 ) {
                     Ok((download, rx)) => {

soulseek-rs/src/ui/main_tui/mod.rs

@@ -23,6 +23,7 @@ pub struct MainTui {
     client: Arc<dyn SessionApi>,
     state: AppState,
     download_dir: String,
+    use_username_dirs: bool,
     search_timeout: Duration,
     spinner_state: usize,
     store: Option<StateStore>,
@@ -40,6 +41,7 @@ impl MainTui {
     pub fn new(
         client: Arc<dyn SessionApi>,
         download_dir: String,
+        use_username_dirs: bool,
         search_timeout: Duration,
         store: Option<StateStore>,
         config_path: Option<std::path::PathBuf>,
@@ -49,6 +51,7 @@ impl MainTui {
             state: AppState::new(),
             download_dir,
             search_timeout,
+            use_username_dirs,
             spinner_state: 0,
             store,
             config_path,
@@ -252,17 +255,25 @@ impl MainTui {
     }
 }
 
+#[expect(clippy::too_many_arguments, reason = "one call site per mode")]
 pub fn launch_main_tui(
     terminal: DefaultTerminal,
     client: Arc<dyn SessionApi>,
     download_dir: String,
+    use_username_dirs: bool,
     search_timeout: Duration,
     store: Option<StateStore>,
     config_path: Option<std::path::PathBuf>,
     listener_fallback: Option<(u16, u16)>,
 ) -> Result<TuiExit> {
-    let mut tui =
+    let mut tui = MainTui::new(
-        MainTui::new(client, download_dir, search_timeout, store, config_path);
+        client,
+        download_dir,
+        use_username_dirs,
+        search_timeout,
+        store,
+        config_path,
+    );
     tui.listener_fallback = listener_fallback;
     tui.run(terminal)
 }

soulseek-rs/src/ui/main_tui/render/mod.rs

@@ -369,6 +369,16 @@ impl MainTui {
         lines.push(Line::from(""));
 
         lines.push(section("Folders"));
+        lines.push(entry(
+            selected == SettingsRow::UseUsernameDirs,
+            "Use username directory ",
+            if settings.use_username_dirs {
+                "[x]"
+            } else {
+                "[ ]"
+            }
+            .to_string(),
+        ));
         lines.push(if settings.mode == SettingsMode::EditingDownloadDir {
             typing("Download folder: ", settings.input.clone())
         } else {

soulseek-rs/src/ui/main_tui/render/shortcuts.rs

@@ -166,6 +166,7 @@ fn settings_shortcuts(
                 SettingsRow::ChangePassword => ("Enter", "change password"),
                 SettingsRow::Logout => ("Enter", "log out"),
                 SettingsRow::DownloadDir => ("Enter", "edit folder"),
+                SettingsRow::UseUsernameDirs => ("Enter", "toggle option"),
                 SettingsRow::Share(_) => ("d", "remove share"),
             },
             ("a", "add share"),

soulseek-rs/src/ui/main_tui/settings.rs

@@ -20,6 +20,7 @@ impl MainTui {
             account,
             self.download_dir.clone(),
             self.client.shared_directories(),
+            self.use_username_dirs,
         ));
     }
 
@@ -66,6 +67,7 @@ impl MainTui {
         };
         let download_dir = settings.download_dir.clone();
         let share_dirs = settings.share_dirs.clone();
+        let use_username_dirs = settings.use_username_dirs;
 
         let shares = if self.client.daemon_endpoint().is_some() {
             share_dirs.clone()
@@ -107,6 +109,7 @@ impl MainTui {
         if dir_applied {
             self.download_dir.clone_from(&download_dir);
         }
+        self.use_username_dirs = use_username_dirs;
 
         // Persist to config.toml so the change survives a restart. What was
         // refused live is not written either: the file must not promise a
@@ -121,6 +124,7 @@ impl MainTui {
                     // load, so leaving it would resurrect a removed folder.
                     config.shared_dir = None;
                     config.shared_dirs = Some(share_dirs);
+                    config.use_username_dirs = Some(use_username_dirs);
                     config.save(&path)
                 });
             if let Err(e) = result {

soulseek-rs/src/ui/main_tui/tests/mod.rs

@@ -349,6 +349,7 @@ fn attach(session: Arc<TalkativeSession>) -> MainTui {
     MainTui::new(
         session,
         "/tmp".to_string(),
+        false,
         Duration::from_secs(1),
         None,
         None,

soulseek-rs/src/ui/main_tui/tests/settings.rs

@@ -17,6 +17,7 @@ fn applying_settings_attached_pushes_both_folders_to_the_daemon() {
     let mut tui = MainTui::new(
         session.clone(),
         "/daemon/old".to_string(),
+        false,
         Duration::from_secs(1),
         None,
         Some(config_path.clone()),
@@ -73,6 +74,7 @@ fn applying_settings_locally_stays_on_this_machine() {
     let mut tui = MainTui::new(
         session.clone(),
         "/tmp".to_string(),
+        false,
         Duration::from_secs(1),
         None,
         Some(config_path),
@@ -106,6 +108,30 @@ fn applying_settings_locally_stays_on_this_machine() {
     assert_eq!(tui.download_dir, downloads_dir);
 }
 
+#[test]
+fn toggling_username_folders_applies_live_and_persists() {
+    let session = Arc::new(TalkativeSession::default());
+    let scratch = tempfile::tempdir().expect("a temp dir");
+    let config_path = scratch.path().join("config.toml");
+    let mut tui = MainTui::new(
+        session,
+        scratch.path().to_str().expect("utf-8").to_string(),
+        false,
+        Duration::from_secs(1),
+        None,
+        Some(config_path.clone()),
+    );
+
+    tui.open_settings();
+    tui.state.settings.as_mut().expect("open").use_username_dirs = true;
+    tui.apply_settings();
+
+    assert!(tui.use_username_dirs, "the next download uses it");
+    let saved = crate::persist::config::FileConfig::load(&config_path)
+        .expect("config saved");
+    assert_eq!(saved.use_username_dirs, Some(true));
+}
+
 /// The account block answers "who is this session, and what is it
 /// offering" without a trip to another screen.
 #[test]
@@ -190,6 +216,7 @@ fn an_attached_window_offers_no_logout() {
     let mut tui = attach(session);
     press(&mut tui, KeyCode::Char('o'));
     press(&mut tui, KeyCode::Down);
+    press(&mut tui, KeyCode::Down);
     press(&mut tui, KeyCode::Enter);
 
     let settings = tui.state.settings.as_ref().expect("still open");

soulseek-rs/tests/tui_e2e.rs

@@ -56,6 +56,7 @@ impl Window {
         let tui = MainTui::new(
             client,
             downloads.display().to_string(),
+            false,
             SEARCH_WINDOW,
             Some(StateStore::new(state.to_path_buf())),
             None,

website/src/pages/install.tsx

@@ -129,6 +129,11 @@ const SETTINGS: readonly Setting[] = [
       </>
     ),
   },
+  {
+    key: 'use_username_dirs',
+    fallback: 'false',
+    body: <>Save each peer's files in a subfolder named after them.</>,
+  },
   {
     key: 'max_concurrent_downloads',
     fallback: '20',