diff --git a/src-tauri/src/aerocrypt_provider.rs b/src-tauri/src/aerocrypt_provider.rs index 4fa8aeaa8..d2c1dfcaa 100644 --- a/src-tauri/src/aerocrypt_provider.rs +++ b/src-tauri/src/aerocrypt_provider.rs @@ -432,14 +432,14 @@ pub async fn aerocrypt_provider_create_remote( )? }; let config = overlay::parse_config(&config_json)?; - let root = overlay_create_root(&base_path, target_subpath.as_deref()); + let root = overlay_create_root(&base_path, target_subpath.as_deref())?; let mut provider_lock = provider_state.provider.lock().await; let provider = provider_lock .as_mut() .ok_or_else(|| "Not connected to any provider".to_string())?; log::debug!("[aerocrypt][create_remote] root={:?}", root); - if root != base_path.trim() { + if root != base_path { let _ = provider.mkdir(&root).await; // idempotent } write_new_overlay_config(provider.as_mut(), &root, &config_json, force).await?; @@ -536,16 +536,29 @@ pub struct AeroCryptCreatedVault { /// absolute folder the dialog was opened on), or `subpath` under it. The /// subpath is always relative to `base`, its slashes trimmed, so `/x` typed in /// the field does not escape to the remote root. -pub(crate) fn overlay_create_root(base: &str, subpath: Option<&str>) -> String { - let base = base.trim(); +/// +/// The folder is taken as given (`/reports ` is not `/reports`), but the +/// overlay anchor trims the ends of its folder (`norm_anchor`, and +/// `normCryptScope` in the app): a vault at `/reports ` would sit outside its +/// own anchor `/reports`, and the raw write guard +/// (`path_is_outside_crypt_scope`) would take `/reports /a.txt` for a folder +/// outside the vault and let it be written in clear. Such a root is refused; a +/// folder below it (`/reports /New`) is anchored as created. +pub(crate) fn overlay_create_root(base: &str, subpath: Option<&str>) -> Result { let base = if base.is_empty() { "/" } else { base }; - match subpath + let root = match subpath .map(|s| s.trim().trim_matches('/')) .filter(|s| !s.is_empty()) { Some(sub) => join_remote_path(base, sub), None => base.to_string(), + }; + if root.trim() != root { + return Err(format!( + "An encrypted folder cannot begin or end with a space: '{root}'. Create the vault in a folder below it, or rename the folder." + )); } + Ok(root) } /// Build and return the mandatory Emergency Kit from a (persisted) config JSON. @@ -1689,14 +1702,42 @@ mod tests { /// it, never at the provider's pwd, and a typed `/x` stays under it. #[test] fn dialog_create_roots_the_vault_at_or_under_its_folder() { - assert_eq!(overlay_create_root("/home/u", None), "/home/u"); - assert_eq!(overlay_create_root("/home/u", Some(" ")), "/home/u"); - assert_eq!(overlay_create_root("/home/u", Some("New")), "/home/u/New"); - assert_eq!(overlay_create_root("/home/u", Some("/New/")), "/home/u/New"); - assert_eq!(overlay_create_root("/home/u/", Some("a/b")), "/home/u/a/b"); - assert_eq!(overlay_create_root("/", Some("New")), "/New"); - assert_eq!(overlay_create_root("", Some("New")), "/New"); - assert_eq!(overlay_create_root("", None), "/"); + let root = |base, subpath| overlay_create_root(base, subpath).expect("a root"); + assert_eq!(root("/home/u", None), "/home/u"); + assert_eq!(root("/home/u", Some(" ")), "/home/u"); + assert_eq!(root("/home/u", Some("New")), "/home/u/New"); + assert_eq!(root("/home/u", Some("/New/")), "/home/u/New"); + assert_eq!(root("/home/u/", Some("a/b")), "/home/u/a/b"); + assert_eq!(root("/", Some("New")), "/New"); + assert_eq!(root("", Some("New")), "/New"); + assert_eq!(root("/reports ", Some("New")), "/reports /New"); + assert_eq!(root("", None), "/"); + } + + /// The anchor of a vault is its root with the ends trimmed: a root that + /// begins or ends with a space is refused, since the vault would sit + /// outside its anchor. A root whose spaced name is not at an end is + /// anchored as created. + #[test] + fn a_vault_root_the_anchor_would_change_is_refused() { + for (base, subpath) in [ + ("/reports ", None), + (" /reports", None), + ("/home/u/ ", None), + ] { + let refused = overlay_create_root(base, subpath).expect_err(base); + assert!( + refused.contains("cannot begin or end with a space"), + "{refused}" + ); + } + for (base, subpath) in [("/reports ", Some("New")), ("/reports /", None)] { + let root = overlay_create_root(base, subpath).expect("a root"); + assert_eq!( + crate::crypt_overlay_provider::norm_anchor(&root), + root.trim_end_matches('/') + ); + } } /// A backend that cannot put one marker over another refuses the slot diff --git a/src-tauri/src/ai_core/runner/worker_remote.rs b/src-tauri/src/ai_core/runner/worker_remote.rs index 15a6a5e77..bf963f0b5 100644 --- a/src-tauri/src/ai_core/runner/worker_remote.rs +++ b/src-tauri/src/ai_core/runner/worker_remote.rs @@ -67,7 +67,8 @@ pub(crate) fn confined_s3_list_path(root: &str, relative: &str) -> Result) -> Result { - let path = initial_path.unwrap_or("").trim().trim_matches('/'); + // As given, as the app lists it: `team ` is its own prefix. + let path = initial_path.unwrap_or("").trim_matches('/'); let root = if path.is_empty() { "/".to_string() } else { @@ -239,6 +240,7 @@ mod tests { assert_eq!(confined_s3_list_path("/team/a", "").unwrap(), "/team/a"); assert_eq!(confined_s3_list_path("/", "").unwrap(), "/"); assert_eq!(saved_s3_root(Some("/team/a/")).unwrap(), "/team/a"); + assert_eq!(saved_s3_root(Some("/team/a ")).unwrap(), "/team/a "); assert_eq!(saved_s3_root(None).unwrap(), "/"); assert!(saved_s3_root(Some("../other")).is_err()); for path in [ diff --git a/src-tauri/src/bin/aeroftp_cli.rs b/src-tauri/src/bin/aeroftp_cli.rs index 9b3181ed9..1e7881d70 100644 --- a/src-tauri/src/bin/aeroftp_cli.rs +++ b/src-tauri/src/bin/aeroftp_cli.rs @@ -9639,8 +9639,12 @@ impl CliBase { } /// The base of a connected provider: Proton Drive's start folder is where - /// its session stands right after connecting. + /// its session stands right after connecting, and a relative initial + /// folder is the folder the session entered (`util::start_folder`): a + /// base left relative was applied twice and `a.txt` reached + /// `docs/docs/a.txt`. async fn for_provider(path: impl Into, provider: &mut dyn StorageProvider) -> Self { + let path = ftp_client_gui_lib::util::start_folder::as_entered(&path.into(), provider).await; let start_folder = if provider.provider_type() == ProviderType::Proton { Some(provider.pwd().await.unwrap_or_else(|_| "/".to_string())) } else { @@ -30815,7 +30819,7 @@ async fn connect_mtp_device_profile( // GUI parity: land on saved remote path when set. Absolute path resolution // still works if cd fails; warn and keep listing via initialPath join. - if !initial_path.trim().is_empty() && initial_path.trim() != "/" { + if !initial_path.is_empty() && initial_path != "/" { if let Err(e) = mtp.cd(&initial_path).await { if !cli.quiet { eprintln!( @@ -30826,7 +30830,7 @@ async fn connect_mtp_device_profile( } } - let path_out = if initial_path.trim().is_empty() { + let path_out = if initial_path.is_empty() { "/".to_string() } else { initial_path @@ -30872,7 +30876,7 @@ async fn connect_mtp_device_profile_agent( .await .map_err(|e| format!("Failed to open MTP device: {}", e))?; - if !initial_path.trim().is_empty() && initial_path.trim() != "/" { + if !initial_path.is_empty() && initial_path != "/" { if let Err(e) = mtp.cd(&initial_path).await { // Soft: agent list/get still resolve absolute paths from initial_path. eprintln!( @@ -30882,7 +30886,7 @@ async fn connect_mtp_device_profile_agent( } } - let path_out = if initial_path.trim().is_empty() { + let path_out = if initial_path.is_empty() { "/".to_string() } else { initial_path @@ -32388,7 +32392,8 @@ fn resolve_cli_remote_path_in( emit_resolution_note: bool, start_folder: Option<&str>, ) -> String { - let base = initial_path.trim(); + // As given: a folder name may begin or end with a space. + let base = initial_path; // No meaningful initial_path: pass user_path through with minimal // rewriting: // - empty user_path -> "" so the provider applies its canonical @@ -35538,7 +35543,7 @@ async fn cmd_get( format: OutputFormat, cancelled: Arc, ) -> i32 { - if remote.trim().is_empty() { + if remote.is_empty() { print_error(format, "Missing remote path for get", 5); return 5; } @@ -37688,7 +37693,7 @@ async fn cmd_put( format: OutputFormat, cancelled: Arc, ) -> i32 { - if local.trim().is_empty() { + if local.is_empty() { print_error(format, "Missing local path for put", 5); return 5; } @@ -40145,7 +40150,7 @@ fn print_versions(versions: &[FileVersion], path: &str, format: OutputFormat) { /// `versions list`: list previous versions of a remote file. async fn cmd_versions_list(url: &str, path: &str, cli: &Cli, format: OutputFormat) -> i32 { - if path.trim().is_empty() { + if path.is_empty() { print_error(format, "Missing remote file path for 'versions list'", 5); return 5; } @@ -40185,7 +40190,7 @@ async fn cmd_versions_get( cli: &Cli, format: OutputFormat, ) -> i32 { - if path.trim().is_empty() { + if path.is_empty() { print_error(format, "Missing remote file path for 'versions get'", 5); return 5; } @@ -40197,7 +40202,7 @@ async fn cmd_versions_get( ); return 5; } - if local.trim().is_empty() { + if local.is_empty() { print_error( format, "Missing local destination path for 'versions get'", @@ -40269,7 +40274,7 @@ async fn cmd_versions_restore( cli: &Cli, format: OutputFormat, ) -> i32 { - if path.trim().is_empty() { + if path.is_empty() { print_error(format, "Missing remote file path for 'versions restore'", 5); return 5; } @@ -40325,7 +40330,7 @@ async fn cmd_versions_purge( cli: &Cli, format: OutputFormat, ) -> i32 { - if path.trim().is_empty() { + if path.is_empty() { print_error(format, "Missing remote file path for 'versions purge'", 5); return 5; } @@ -40702,8 +40707,24 @@ fn rcat_staging_file() -> Result { tempfile::NamedTempFile::new_in(&dir).map_err(|e| e.to_string()) } +#[cfg(test)] +thread_local! { + /// What `rcat` reads instead of stdin, put there by a test: a test reading + /// the real stdin would wait on the terminal it runs in. + static TEST_RCAT_STDIN: std::cell::RefCell>> = const { std::cell::RefCell::new(None) }; +} + +/// What `rcat` uploads: stdin. +fn rcat_input() -> Box { + #[cfg(test)] + if let Some(bytes) = TEST_RCAT_STDIN.with(|slot| slot.borrow_mut().take()) { + return Box::new(io::Cursor::new(bytes)); + } + Box::new(io::stdin().lock()) +} + async fn cmd_rcat(url: &str, remote: &str, cli: &Cli, format: OutputFormat) -> i32 { - if remote.trim().is_empty() { + if remote.is_empty() { print_error(format, "Missing remote path for rcat", 5); return 5; } @@ -40728,7 +40749,7 @@ async fn cmd_rcat(url: &str, remote: &str, cli: &Cli, format: OutputFormat) -> i } }; - let bytes = match io::copy(&mut io::stdin().lock(), &mut temp) { + let bytes = match io::copy(&mut rcat_input(), &mut temp) { Ok(bytes) => bytes, Err(e) => { print_error(format, &format!("Cannot read stdin: {}", e), 2); @@ -86143,6 +86164,21 @@ mod tests { ); } + #[test] + fn test_resolve_cli_remote_path_keeps_the_spaces_of_the_base() { + // `/reports ` and `/reports` are two folders: the profile's base is + // joined as given. + assert_eq!( + resolve_cli_remote_path("/reports ", "a.txt"), + "/reports /a.txt" + ); + assert_eq!( + resolve_cli_remote_path("/reports ", "/a.txt"), + "/reports /a.txt" + ); + assert_eq!(resolve_cli_remote_path("/ reports", ""), "/ reports"); + } + #[test] fn test_resolve_cli_remote_path_user_already_includes_base() { assert_eq!( @@ -88940,6 +88976,10 @@ mod tests { /// Opt-in: `stat` answers for a listed directory, and for a file as a /// listing names it, instead of not found. stat_listed: bool, + /// Opt-in: the folder the session stands in, which `pwd` answers. A + /// relative path is listed under it, as FTP, SFTP and the cloud + /// providers that keep a current folder resolve one. + cwd: Option, } impl MemTreeProvider { @@ -89007,16 +89047,22 @@ mod tests { true } async fn list(&mut self, path: &str) -> Result, ProviderError> { + let path = match &self.cwd { + Some(cwd) if !path.starts_with('/') => { + format!("{}/{}", cwd.trim_end_matches('/'), path) + } + _ => path.to_string(), + }; let mut listing = self .dirs - .get(path) + .get(&path) .cloned() - .ok_or_else(|| ProviderError::NotFound(path.to_string()))?; + .ok_or_else(|| ProviderError::NotFound(path.clone()))?; listing.retain(|entry| !self.hidden.contains(&entry.path)); Ok(listing) } async fn pwd(&mut self) -> Result { - Ok("/".to_string()) + Ok(self.cwd.clone().unwrap_or_else(|| "/".to_string())) } async fn cd(&mut self, _path: &str) -> Result<(), ProviderError> { Ok(()) @@ -89223,6 +89269,14 @@ mod tests { finish_download: bool, /// Refuses ranged hashing before the ordinary download stalls. refuse_hash_ranges: bool, + /// Keeps file versions: each version call answers, none are listed. + versions: bool, + /// Completes an upload instead of stalling. + finish_upload: bool, + /// The remote paths `stat`, an upload and the version calls were + /// asked for, and the local path a version was downloaded to, across + /// the clones. + asked: Arc>>, } impl StallingProvider { @@ -89364,11 +89418,49 @@ mod tests { async fn upload( &mut self, _local_path: &str, - _remote_path: &str, + remote_path: &str, _progress: Option>, ) -> Result<(), ProviderError> { + self.asked.lock().unwrap().push(remote_path.to_string()); + if self.finish_upload { + return Ok(()); + } std::future::pending().await } + fn supports_versions(&self) -> bool { + self.versions + } + async fn list_versions(&mut self, path: &str) -> Result, ProviderError> { + self.asked.lock().unwrap().push(path.to_string()); + Ok(Vec::new()) + } + async fn download_version( + &mut self, + path: &str, + _version_id: &str, + local_path: &str, + ) -> Result<(), ProviderError> { + let mut asked = self.asked.lock().unwrap(); + asked.push(path.to_string()); + asked.push(local_path.to_string()); + Ok(()) + } + async fn restore_version( + &mut self, + path: &str, + _version_id: &str, + ) -> Result<(), ProviderError> { + self.asked.lock().unwrap().push(path.to_string()); + Ok(()) + } + async fn delete_version( + &mut self, + path: &str, + _version_id: &str, + ) -> Result<(), ProviderError> { + self.asked.lock().unwrap().push(path.to_string()); + Ok(()) + } async fn mkdir(&mut self, _path: &str) -> Result<(), ProviderError> { Ok(()) } @@ -89388,6 +89480,7 @@ mod tests { Err(ProviderError::NotSupported("rename".to_string())) } async fn stat(&mut self, path: &str) -> Result { + self.asked.lock().unwrap().push(path.to_string()); if self.stat_stalls { std::future::pending::<()>().await; } @@ -89619,6 +89712,142 @@ mod tests { assert_eq!(code, 130, "-1 means the put was still running"); } + /// A remote name of spaces only is a name: `versions` list, get, restore + /// and purge, `get` and `rcat` refused it as a missing argument (exit 5) + /// before connecting. + #[test] + fn a_remote_name_of_spaces_is_not_a_missing_path() { + // Each reaches the provider with the name as given: a trim after the + // argument check would keep these exit codes and lose the name. + let spaces = || " ".to_string(); + assert_eq!( + asked_of(|_| async { + cmd_versions_list("memory://", " ", &quiet_cli(), OutputFormat::Json).await + }), + (0, vec![spaces()]) + ); + let dir = tempfile::tempdir().unwrap(); + let local = dir.path().join("f.bin").to_string_lossy().into_owned(); + assert_eq!( + asked_of(|_| async { + cmd_versions_get( + "memory://", + " ", + "v1", + &local, + &quiet_cli(), + OutputFormat::Json, + ) + .await + }), + (0, vec![spaces(), local.clone()]) + ); + assert_eq!( + asked_of(|_| async { + cmd_versions_restore("memory://", " ", "v1", &quiet_cli(), OutputFormat::Json) + .await + }), + (0, vec![spaces()]) + ); + assert_eq!( + asked_of(|_| async { + cmd_versions_purge( + "memory://", + " ", + "v1", + true, + &quiet_cli(), + OutputFormat::Json, + ) + .await + }), + (0, vec![spaces()]) + ); + assert_eq!( + asked_of(|_| async { + TEST_RCAT_STDIN.with(|slot| *slot.borrow_mut() = Some(b"x".to_vec())); + cmd_rcat("memory://", " ", &quiet_cli(), OutputFormat::Json).await + }), + (0, vec![spaces()]) + ); + + // The get of ` ` asks the provider for ` `, starts, and is stopped. + let cli = Cli { + quiet: true, + ..test_cli() + }; + let (code, asked) = asked_of(|cancelled| async move { + cmd_get( + "memory://", + " ", + Some(local.as_str()), + false, + 1, + false, + &cli, + OutputFormat::Json, + cancelled, + ) + .await + }); + assert_eq!(code, 130, "the get of ` ` started and was stopped"); + assert!(asked.contains(&spaces()), "{asked:?}"); + } + + /// So is a local name: `put` and `versions get` refused it the same way. + #[test] + fn a_local_name_of_spaces_is_not_a_missing_path() { + // A version is downloaded to ` ` as given. + assert_eq!( + asked_of(|_| async { + cmd_versions_get( + "memory://", + "/f.bin", + "v1", + " ", + &quiet_cli(), + OutputFormat::Json, + ) + .await + }), + (0, vec!["/f.bin".to_string(), " ".to_string()]) + ); + // `put` looks ` ` up as a local file: none is there, so it stops at + // that (exit 2), past the check for a missing argument (exit 5). + let code = against_stalling(StallingProvider::default(), |cancelled| async move { + cmd_put( + "memory://", + " ", + Some("/f.bin"), + false, + false, + false, + None, + &quiet_cli(), + OutputFormat::Json, + cancelled, + ) + .await + }); + assert_eq!(code, 2); + } + + /// The exit code of `command` against a provider that keeps versions and + /// completes uploads, and the paths it was asked for. + fn asked_of>( + command: impl FnOnce(Arc) -> F + Send, + ) -> (i32, Vec) { + let provider = StallingProvider { + versions: true, + finish_upload: true, + ..StallingProvider::default() + }; + let asked = Arc::clone(&provider.asked); + let code = against_stalling(provider, command); + let asked = asked.lock().unwrap().clone(); + (code, asked) + } + /// `get` of `/f.bin` into a local file that already holds "keep me", with /// `--inplace`, against `provider`; the exit code and what the file holds /// afterwards. @@ -96468,6 +96697,89 @@ api_key = kNaQ0gIj57D0wb8CFzBMYQQMoWUZUopy0HOLAMHtu0uD assert_eq!(omitted.1, vec!["home.txt".to_string()]); } + /// A relative initial folder is where connect put the session, and the + /// provider resolves a relative path against it. The base was left + /// relative, so `ls` with the profile's `root` asked for `root`, which the + /// provider standing in `/root` listed as `/root/root`: the folder once, + /// not twice. + #[tokio::test] + async fn a_relative_initial_folder_is_applied_once() { + let cli = std::thread::Builder::new() + .stack_size(32 * 1024 * 1024) + .spawn(|| Cli::try_parse_from(["aeroftp-cli", "ls", "_"]).expect("ls parses")) + .expect("parse thread") + .join() + .expect("parse thread finished"); + let options = |path| CliListOptions { + path, + sort: "name", + reverse: false, + all: true, + limit: None, + files_only: false, + dirs_only: false, + emit_resolution_note: false, + }; + let names = |list: Result| match list { + Ok(list) => list.entries.into_iter().map(|e| e.name).collect::>(), + Err(_) => panic!("listing failed"), + }; + let mut provider = MemTreeProvider { + cwd: Some("/root".to_string()), + ..MemTreeProvider::tree(&[("a.txt", 1), ("sub/b.txt", 1)]) + }; + let base = CliBase::for_provider("root", &mut provider).await; + assert_eq!(base.path, "/root"); + assert_eq!( + names(collect_cli_list_entries(&mut provider, &base, &options(""), &cli).await), + ["sub", "a.txt"] + ); + assert_eq!( + names(collect_cli_list_entries(&mut provider, &base, &options("sub"), &cli).await), + ["b.txt"] + ); + // The login home's folder on FTP and SFTP, and a base written with a + // trailing slash or a trailing space. + for (cwd, initial) in [ + ("/home/u/root", "root/"), + ("/home/u/re ports ", "re ports "), + ] { + let mut home = MemTreeProvider { + cwd: Some(cwd.to_string()), + ..Default::default() + }; + assert_eq!(CliBase::for_provider(initial, &mut home).await.path, cwd); + } + } + + /// A provider that keeps the folder as a prefix of its own (S3's + /// `bucket/backups/`) stands at its root: the base stays as given. So does + /// a relative base the session does not stand in, and an absolute one. + #[tokio::test] + async fn a_base_the_session_does_not_stand_in_is_kept() { + let mut root = MemTreeProvider::default(); + assert_eq!( + CliBase::for_provider("bucket/backups/", &mut root) + .await + .path, + "bucket/backups/" + ); + let mut elsewhere = MemTreeProvider { + cwd: Some("/mydocs".to_string()), + ..Default::default() + }; + assert_eq!( + CliBase::for_provider("docs", &mut elsewhere).await.path, + "docs" + ); + assert_eq!( + CliBase::for_provider("/reports ", &mut elsewhere) + .await + .path, + "/reports " + ); + } + /// The TUI size of a folder counted what a looping listing sent it /// through again until its entry cap, and what a folder outside held. It /// counts each path under the folder once, and does not answer with a diff --git a/src-tauri/src/bridge_commands.rs b/src-tauri/src/bridge_commands.rs index e77f68a33..7be732f52 100644 --- a/src-tauri/src/bridge_commands.rs +++ b/src-tauri/src/bridge_commands.rs @@ -1075,15 +1075,15 @@ pub fn inject_rclone_onedrive_export_options( /// A Zoho folder/workspace id stored as `initialPath`. AeroFTP persists /// those with a leading slash (`/fmip…`); rclone's `root_folder_id` does -/// not. One leading slash is stripped, then any remaining `/` means this -/// is a real path, not an id. +/// not. One leading slash is stripped, then any remaining `/`, or a space, +/// means this is a real path, not an id: `/fmip… ` is a folder of that name. fn zoho_folder_id_from_initial_path(initial_path: Option<&str>) -> Option { - let s = initial_path?.trim(); + let s = initial_path?; if s.is_empty() || s == "/" { return None; } let s = s.strip_prefix('/').unwrap_or(s); - if s.is_empty() || s.contains('/') { + if s.is_empty() || s.contains('/') || s.contains(char::is_whitespace) { None } else { Some(s.to_string()) @@ -1117,10 +1117,7 @@ pub fn inject_rclone_path_export_options( if !matches!(protocol, "sftp" | "ftp" | "ftps" | "webdav") { return; } - let Some(path) = initial_path - .map(str::trim) - .filter(|p| !p.is_empty() && *p != "/") - else { + let Some(path) = initial_path.filter(|p| !p.is_empty() && *p != "/") else { return; }; if !matches!(options, Some(Value::Object(_))) { @@ -2227,6 +2224,11 @@ mod tests { zoho_folder_id_from_initial_path(Some("My Folders/docs")), None ); + // A folder whose name ends in a space, not an id. + assert_eq!( + zoho_folder_id_from_initial_path(Some("/fmip966f979e195e64ec78e6846976861eed5 ")), + None + ); } #[test] diff --git a/src-tauri/src/cli_tui/app.rs b/src-tauri/src/cli_tui/app.rs index aa6a09033..48c91bf07 100644 --- a/src-tauri/src/cli_tui/app.rs +++ b/src-tauri/src/cli_tui/app.rs @@ -2527,8 +2527,9 @@ impl AppState { host: host.clone(), port, username: form.username.trim().to_string(), - initial_path: form.initial_path.trim().to_string(), - local_initial_path: form.local_path.trim().to_string(), + // As typed, as the GUI saves them: `/reports ` is its own folder. + initial_path: form.initial_path.clone(), + local_initial_path: form.local_path.clone(), }; // The secret only crosses the channel when the field was actually @@ -5054,6 +5055,37 @@ mod tests { assert!(!app.overlay_active()); } + /// The folders are saved as typed, as the GUI saves them: `/reports ` + /// and `/reports` are two folders. + #[test] + fn profile_form_saves_the_folders_as_typed() { + let mut app = AppState::new_live(introhub_context()); + app.apply_action(TuiAction::AddProfile); + let type_text = |app: &mut AppState, text: &str| { + for c in text.chars() { + app.handle_overlay_key(OverlayKey::Char(c)); + } + }; + type_text(&mut app, "NAS"); + app.handle_overlay_key(OverlayKey::Tab); // -> Protocol + app.handle_overlay_key(OverlayKey::Tab); // -> Host + type_text(&mut app, "nas.local"); + app.handle_overlay_key(OverlayKey::Tab); // -> Port + app.handle_overlay_key(OverlayKey::Tab); // -> Username + app.handle_overlay_key(OverlayKey::Tab); // -> Initial path + type_text(&mut app, "/reports "); + app.handle_overlay_key(OverlayKey::Tab); // -> Local path + type_text(&mut app, "/home/u/ out"); + let commands = app.handle_overlay_key(OverlayKey::Submit); + match &commands[..] { + [WorkerCommand::SaveProfile { draft, .. }] => { + assert_eq!(draft.initial_path, "/reports "); + assert_eq!(draft.local_initial_path, "/home/u/ out"); + } + other => panic!("expected SaveProfile, got {:?}", other), + } + } + #[test] fn profile_form_validation_keeps_the_form_open() { let mut app = AppState::new_live(introhub_context()); diff --git a/src-tauri/src/cli_tui/session.rs b/src-tauri/src/cli_tui/session.rs index e01a58921..060a41b64 100644 --- a/src-tauri/src/cli_tui/session.rs +++ b/src-tauri/src/cli_tui/session.rs @@ -170,15 +170,15 @@ impl fmt::Debug for TuiSession { } fn normalize_remote_cwd(path: &str) -> String { - let trimmed = path.trim(); - if trimmed.is_empty() || trimmed == "." { + // As given: `/reports ` is its own folder, and a name of spaces a name. + if path.is_empty() || path == "." { return "/".to_string(); } - let with_root = if trimmed.starts_with('/') { - trimmed.to_string() + let with_root = if path.starts_with('/') { + path.to_string() } else { - format!("/{}", trimmed) + format!("/{}", path) }; let normalized = with_root.trim_end_matches('/'); @@ -254,4 +254,19 @@ mod tests { state.mark_cancelled(); assert_eq!(state.phase, TuiSessionPhase::Cancelled); } + + /// The initial folder and every folder the session moves to keep the + /// spaces of their names: `/reports ` and `/reports` are two folders. + #[test] + fn a_folder_keeps_the_spaces_of_its_name() { + let mut state = + TuiSessionState::planned_from_selection(&sample_user(), &sample_profile("/reports ")); + assert_eq!(state.cwd, "/reports "); + state.mark_connected(" a/b /"); + assert_eq!(state.cwd, "/ a/b "); + state.mark_connected(" "); + assert_eq!(state.cwd, "/ "); + state.mark_connected(""); + assert_eq!(state.cwd, "/"); + } } diff --git a/src-tauri/src/community_benchmark/engine.rs b/src-tauri/src/community_benchmark/engine.rs index c2ae79413..b085f5228 100644 --- a/src-tauri/src/community_benchmark/engine.rs +++ b/src-tauri/src/community_benchmark/engine.rs @@ -389,13 +389,18 @@ pub async fn run( let report_id = uuid::Uuid::new_v4().to_string(); // A relative prefix on Proton Drive lands where the session starts (see - // `benchmark_effective_prefix`); without a prefix the roots are relative - // and the provider resolves them there already. + // `benchmark_effective_prefix`); without a prefix the roots are under the + // start path. The folder the session stands in is asked under the stop + // signal too: a `pwd` that never answers ends at the cancel or the + // timeout, and the scratch folder below reports the stop. let prefix = match opts.test_root_prefix.as_deref() { Some(prefix) => { let kind = provider.provider_type(); let start = if kind == ProviderType::Proton { - provider.pwd().await.unwrap_or_else(|_| "/".to_string()) + until_stopped(&stop, provider.pwd()) + .await + .and_then(Result::ok) + .unwrap_or_else(|| "/".to_string()) } else { String::new() }; @@ -405,7 +410,17 @@ pub async fn run( }; let (bench_base, test_root) = match prefix.as_deref() { Some(prefix) => benchmark_remote_roots_from_prefix(prefix, &report_id), - None => benchmark_remote_roots(initial_path, &report_id), + // A relative start path is the folder the session entered: joined to + // it again, `team` put the scratch folder in `team/team`. + None => { + let entered = until_stopped( + &stop, + crate::util::start_folder::as_entered(initial_path, &mut *provider), + ) + .await + .unwrap_or_else(|| initial_path.to_string()); + benchmark_remote_roots(&entered, &report_id) + } }; // Create the scratch directory tree before any upload. `bench_base` may // already exist from a prior run (`AlreadyExists` is fine), but `test_root` diff --git a/src-tauri/src/community_benchmark/mod.rs b/src-tauri/src/community_benchmark/mod.rs index 3e23e14ff..13f43d5fe 100644 --- a/src-tauri/src/community_benchmark/mod.rs +++ b/src-tauri/src/community_benchmark/mod.rs @@ -767,7 +767,8 @@ pub fn benchmark_remote_roots(initial_path: &str, report_id: &str) -> (String, S // Drop the leading dot: dot-prefixed folders (`.aeroftp-bench`) are rejected by // some providers (Internxt, certain WebDAV servers) that treat them as hidden // or invalid names. Plain `aeroftp-bench` is portable across all 22 backends. - let base = initial_path.trim().trim_end_matches('/'); + // As given: `/srv/data ` is its own folder, and the run starts in it. + let base = initial_path.trim_end_matches('/'); let bench_base = if base.is_empty() { "aeroftp-bench".to_string() } else { diff --git a/src-tauri/src/community_benchmark/tests.rs b/src-tauri/src/community_benchmark/tests.rs index 62a6e8ac4..402f06b57 100644 --- a/src-tauri/src/community_benchmark/tests.rs +++ b/src-tauri/src/community_benchmark/tests.rs @@ -537,8 +537,9 @@ fn the_scratch_base_joins_the_start_path_on_a_folder_boundary() { // and dropped it. let (base, _) = benchmark_remote_roots("aero", "rid"); assert_eq!(base, "aero/aeroftp-bench"); - let (base, _) = benchmark_remote_roots(" /srv/data/ ", "rid"); - assert_eq!(base, "/srv/data/aeroftp-bench"); + // A start path whose name ends in a space is that folder, not another. + let (base, _) = benchmark_remote_roots("/srv/data ", "rid"); + assert_eq!(base, "/srv/data /aeroftp-bench"); } #[test] @@ -630,6 +631,14 @@ mod engine_runs { /// A folder three levels deep is refused, as FileLu refused the run /// folder under a prefix folder it had just created. refuse_deep_mkdir: bool, + /// The folder `pwd` reports, `/` when unset. + cwd: Option, + /// `pwd` never answers, as an FTP server that stopped. + hang_pwd: bool, + /// The provider it passes for, SFTP when unset. + kind: Option, + /// Every folder asked for, in order. + mkdirs: Vec, } struct MemRemote(Arc>); @@ -644,7 +653,7 @@ mod engine_runs { self } fn provider_type(&self) -> ProviderType { - ProviderType::Sftp + self.0.lock().unwrap().kind.unwrap_or(ProviderType::Sftp) } fn display_name(&self) -> String { "mem".into() @@ -676,7 +685,16 @@ mod engine_runs { Ok(out) } async fn pwd(&mut self) -> Result { - Ok("/".into()) + if self.0.lock().unwrap().hang_pwd { + std::future::pending::<()>().await; + } + Ok(self + .0 + .lock() + .unwrap() + .cwd + .clone() + .unwrap_or_else(|| "/".into())) } async fn cd(&mut self, _p: &str) -> Result<(), ProviderError> { Ok(()) @@ -728,6 +746,7 @@ mod engine_runs { Ok(()) } async fn mkdir(&mut self, p: &str) -> Result<(), ProviderError> { + self.0.lock().unwrap().mkdirs.push(p.into()); if self.0.lock().unwrap().refuse_deep_mkdir && p.matches('/').count() >= 3 { return Err(ProviderError::ServerError("Folder not found".into())); } @@ -893,6 +912,87 @@ mod engine_runs { (state.clone(), Box::new(MemRemote(state))) } + /// A relative start path is the folder connect entered, which the + /// session stands in: the scratch folder joined to it again was created + /// in `team /team /aeroftp-bench`, or not at all. A `..` in it is taken + /// with the folder before it: `docs/../team ` stood in `/home/u/team ` + /// unseen, and the scratch folder went to `docs/../team /aeroftp-bench` + /// under it, which a provider that keeps `..` as a name cannot create. + #[tokio::test] + async fn a_relative_start_path_is_applied_once() { + for start in ["team ", "docs/../team ", "./docs/../team /"] { + let (state, provider) = remote(); + let home = ["/data", "/home", "/home/u", "/home/u/team "].map(String::from); + { + let mut r = state.lock().unwrap(); + r.cwd = Some("/home/u/team ".into()); + r.dirs.extend(home.clone()); + } + let opts = options(BenchmarkLevel::Quick, "1K", None); + let outcome = run(provider, start, opts, &Silent).await.unwrap(); + assert!( + outcome.report.summary.errors.is_empty(), + "{start:?}: {:?}", + outcome.report.summary.errors + ); + let r = state.lock().unwrap(); + // Every folder asked for is where the session stands, none + // relative. + assert_eq!(r.mkdirs[0], "/home/u/team /aeroftp-bench", "{start:?}"); + assert!( + r.mkdirs + .iter() + .all(|dir| dir.starts_with('/') && !dir.contains("team /team ")), + "{start:?}: {:?}", + r.mkdirs + ); + assert!( + r.mkdirs + .iter() + .any(|dir| dir.starts_with("/home/u/team /aeroftp-bench/")), + "{start:?}: {:?}", + r.mkdirs + ); + assert!(r.files.is_empty(), "{start:?}: {:?}", r.files.keys()); + assert_eq!(r.dirs, BTreeSet::from(home), "{start:?}"); + } + } + + /// The run asks where the session stands before its scratch folder: for + /// a relative start path, and on Proton Drive for a relative prefix. A + /// `pwd` that never answers, as from an FTP server that stopped, held the + /// run past its profile timeout and its cancel; it ends at the timeout. + #[tokio::test] + async fn a_pwd_that_never_answers_ends_at_the_profile_timeout() { + for (kind, prefix) in [ + (ProviderType::Sftp, None), + (ProviderType::Proton, Some("live-tests")), + ] { + let (state, provider) = remote(); + { + let mut r = state.lock().unwrap(); + r.hang_pwd = true; + r.kind = Some(kind); + } + let mut opts = options(BenchmarkLevel::Quick, "1K", None); + opts.profile_timeout_secs = 1; + opts.test_root_prefix = prefix.map(str::to_string); + let outcome = tokio::time::timeout( + std::time::Duration::from_secs(10), + run(provider, "team", opts, &Silent), + ) + .await + .unwrap_or_else(|_| panic!("{kind:?}: the run waited on pwd past its timeout")); + let error = match outcome { + Err(error) => error, + Ok(_) => panic!("{kind:?}: the run stopped before its scratch folder"), + }; + assert!(error.contains("timed out"), "{kind:?}: {error}"); + let r = state.lock().unwrap(); + assert_eq!(r.dirs, BTreeSet::from(["/data".to_string()]), "{kind:?}"); + } + } + #[tokio::test] async fn a_run_measures_and_leaves_nothing_behind() { let (state, provider) = remote(); diff --git a/src-tauri/src/cross_profile_transfer.rs b/src-tauri/src/cross_profile_transfer.rs index 61a07b216..59dcd72f5 100644 --- a/src-tauri/src/cross_profile_transfer.rs +++ b/src-tauri/src/cross_profile_transfer.rs @@ -717,7 +717,7 @@ pub fn failures_with_tally_gap( /// plan. Copying that plan writes ciphertext and reports success. pub fn plan_includes_raw_crypt_marker(entries: &[CrossProfileTransferEntry]) -> bool { entries.iter().any(|entry| { - entry.display_name.trim() == ".aerocrypt.tsv" + entry.display_name == ".aerocrypt.tsv" || entry.source_path == ".aerocrypt.tsv" || entry.source_path.ends_with("/.aerocrypt.tsv") }) @@ -732,21 +732,23 @@ fn normalize_dir(path: &str) -> String { } } +/// The provider's root as a plan names it. A path of spaces is a folder of +/// that name, not the root. fn is_virtual_root_path(path: &str) -> bool { - let trimmed = path.trim(); - trimmed.is_empty() || trimmed == "/" || trimmed == "." + path.is_empty() || path == "/" || path == "." } +/// The name a plan shows for an entry: its own, as given, or the last +/// segment of its path when it has none. fn fallback_display_name(name: &str, path: &str) -> String { - let trimmed = name.trim(); - if !trimmed.is_empty() { - return trimmed.to_string(); + if !name.is_empty() { + return name.to_string(); } Path::new(path) .file_name() .map(|segment| segment.to_string_lossy().to_string()) - .filter(|segment| !segment.trim().is_empty()) + .filter(|segment| !segment.is_empty()) .unwrap_or_else(|| path.trim_matches('/').to_string()) } @@ -783,7 +785,7 @@ async fn effective_root_stat( } let stat = stat_for_plan(source, source_path).await?; - if stat.name.trim().is_empty() && is_virtual_root_path(&stat.path) { + if stat.name.is_empty() && is_virtual_root_path(&stat.path) { Ok(crate::providers::types::RemoteEntry { name: "/".to_string(), path: source_path.to_string(), @@ -860,6 +862,33 @@ fn source_root_candidates(request_path: &str, provider_root: &str) -> Vec String { - if path.trim().is_empty() { + // A name of spaces is a name, not the current folder. + if path.is_empty() { return self.current_path.clone(); } if path.starts_with('/') { @@ -673,10 +674,14 @@ impl CloudinaryProvider { if source == target { return Ok(()); } - let entry = self.stat(&source).await?; + // `stat` resolves its path: it takes the input, as given. The + // resolved path is relative to the root and would be resolved again + // against the current folder (`photos/a.jpg` from `photos` became + // `photos/photos/a.jpg`). + let entry = self.stat(from).await?; // The trait promises no overwrite: refuse an occupied destination // before either branch runs. - let displaced = match self.stat(&target).await { + let displaced = match self.stat(to).await { Ok(_) if !overwrite => return Err(ProviderError::AlreadyExists(to.to_string())), Ok(occupant) => Some(occupant), Err(ProviderError::NotFound(_)) => None, @@ -1358,9 +1363,11 @@ impl StorageProvider for CloudinaryProvider { return Err(ProviderError::NotConnected); } let resolved = self.resolve_path(path); - let entry = self.stat(&resolved).await?; + // `stat` and `rmdir` resolve the path themselves: they take the + // input, not `resolved` (see `move_asset`). + let entry = self.stat(path).await?; if entry.is_dir { - self.rmdir(&resolved).await + self.rmdir(path).await } else { let public_id = entry.metadata.get("public_id").cloned().ok_or_else(|| { ProviderError::NotFound("Missing Cloudinary public_id".to_string()) @@ -1902,7 +1909,8 @@ fn folder_to_entry(folder: &CloudinarySubFolder, parent: &str) -> RemoteEntry { /// its public id when it has none: the name on a dynamic-folder account. fn resource_display_name(item: &CloudinaryResource) -> String { if let Some(ref dn) = item.display_name { - if !dn.trim().is_empty() { + // As given: a name of spaces is a name too. + if !dn.is_empty() { let mut name = dn.clone(); if let Some(ref fmt) = item.format { if !name @@ -2133,6 +2141,41 @@ fn validate_download_url(url: &str) -> Result<(), ProviderError> { #[cfg(test)] mod tests { + /// A display name of spaces names the asset: read as absent, the asset + /// went by its public id, and a lookup of the name it was given failed. + #[test] + fn a_display_name_keeps_its_spaces() { + let asset = |display: &str| { + serde_json::from_value::(serde_json::json!({ + "public_id": "photos/x1", "display_name": display, "format": "jpg", + "bytes": 3, "resource_type": "image", + })) + .unwrap() + }; + assert_eq!(super::resource_display_name(&asset(" ")), " .jpg"); + assert_eq!(super::resource_display_name(&asset(" a ")), " a .jpg"); + assert_eq!(super::resource_display_name(&asset("")), "x1.jpg"); + } + + #[test] + fn a_name_keeps_its_leading_and_trailing_spaces() { + // A name with a leading or trailing space is a name of its own, and a + // name of spaces only is not the current folder. Paths here carry + // no leading slash. + assert_eq!(super::normalize_path("/ a"), " a"); + assert_eq!(super::normalize_path("/a /b "), "a /b "); + let mut p = super::CloudinaryProvider::new(super::CloudinaryConfig { + cloud_name: "test".to_string(), + api_key: "test".to_string(), + api_secret: secrecy::SecretString::from("test".to_string()), + initial_path: None, + }); + p.current_path = "Cloud".to_string(); + assert_eq!(p.resolve_path(" "), "Cloud/ "); + assert_eq!(p.resolve_path(" a"), "Cloud/ a"); + assert_eq!(p.resolve_path("/a "), "a "); + } + use super::*; /// The dedupe took an `upload` and a `private` asset with one public id @@ -2605,8 +2648,8 @@ mod tests { /// A fixed-folder Cloudinary double whose folder listings are given /// whole: each `(folder key, JSON array of subfolders)` in `subfolders` /// (any other folder holds none), and the image public ids in `images`, - /// listed by prefix. Returns a provider on it and every DELETE, as - /// `DELETE ?`. + /// listed by prefix. Returns a provider on it and every DELETE and + /// rename, as `DELETE ?` and `POST ?`. async fn provider_on_folder_listings( subfolders: &'static [(&'static str, &'static str)], images: &'static [&'static str], @@ -2621,6 +2664,11 @@ mod tests { async move { let path = req.uri().path().to_string(); let query = req.uri().query().unwrap_or("").to_string(); + if req.method() == axum::http::Method::POST && path.ends_with("/rename") { + let query = urlencoding::decode(&query).unwrap_or_default().into_owned(); + seen.lock().unwrap().push(format!("POST {path}?{query}")); + return axum::Json(serde_json::json!({})).into_response(); + } if req.method() == axum::http::Method::DELETE { let query = urlencoding::decode(&query).unwrap_or_default().into_owned(); let public_id = query.rsplit('=').next().unwrap_or("").to_string(); @@ -2691,6 +2739,58 @@ mod tests { (provider, deletes) } + /// After `cd` into a folder, an action on a path reaches what the path + /// names. `delete` and the move resolved the path, then handed the + /// result to `stat` and `rmdir`, which resolved it again against the + /// current folder: `/photos/a.jpg` from `photos` became + /// `photos/photos/a.jpg`, and the action failed as not found or went to + /// another folder. + #[tokio::test] + async fn an_action_after_cd_reaches_what_its_path_names() { + const SUBFOLDERS: &[(&str, &str)] = &[ + ("", r#"[{"name":"photos","path":"photos"}]"#), + ("photos", r#"[{"name":"sub","path":"photos/sub"}]"#), + ]; + let (mut provider, calls) = + provider_on_folder_listings(SUBFOLDERS, &["photos/a", "photos/b"]).await; + provider.cd("/photos").await.expect("cd"); + provider + .delete("/photos/a.jpg") + .await + .expect("delete a file"); + provider + .delete("/photos/sub") + .await + .expect("delete a folder"); + provider + .rename("/photos/b.jpg", "/photos/c.jpg") + .await + .expect("rename"); + let calls = calls.lock().unwrap().clone(); + assert!( + calls + .iter() + .any(|c| c.starts_with("DELETE /resources/") && c.ends_with("=photos/a")), + "{calls:?}" + ); + assert!( + calls + .iter() + .any(|c| c.starts_with("DELETE /folders/photos/sub?")), + "{calls:?}" + ); + assert!( + calls.iter().any(|c| c.starts_with("POST /image/rename?") + && c.contains("from_public_id=photos/b&") + && c.contains("to_public_id=photos/c")), + "{calls:?}" + ); + assert!( + calls.iter().all(|c| !c.contains("photos/photos")), + "{calls:?}" + ); + } + /// The images `keep` (at the root), `tree/a` and `other/o`, and the /// listing of `tree` naming one subfolder whose path points elsewhere: /// back at `tree` itself, at the root, or at `other`. diff --git a/src-tauri/src/providers/drime_cloud.rs b/src-tauri/src/providers/drime_cloud.rs index f5f81a9ec..518dfa388 100644 --- a/src-tauri/src/providers/drime_cloud.rs +++ b/src-tauri/src/providers/drime_cloud.rs @@ -393,31 +393,32 @@ impl DrimeCloudProvider { } fn normalize_path(path: &str) -> String { - let trimmed = path.trim().replace('\\', "/"); - if trimmed.is_empty() || trimmed == "/" { + // Spaces are part of a name: ` a` and `a ` are not `a`. + let path = path.replace('\\', "/"); + if path.is_empty() || path == "/" { return "/".to_string(); } - let p = if trimmed.starts_with('/') { - trimmed + let p = if path.starts_with('/') { + path } else { - format!("/{}", trimmed) + format!("/{}", path) }; p.trim_end_matches('/').to_string() } fn resolve_path(&self, path: &str) -> String { - let trimmed = path.trim(); - if trimmed.is_empty() || trimmed == "." { + if path.is_empty() || path == "." { return self.current_path.clone(); } // Check leading slash on the raw input before normalizing - // normalize_path unconditionally prepends "/", which would otherwise // make every relative input appear absolute and skip the current_path join. - if trimmed.starts_with('/') { - return Self::normalize_path(trimmed); + // `normalize_path` takes `\` for a separator: a leading one is the root too. + if path.starts_with(['/', '\\']) { + return Self::normalize_path(path); } let base = self.current_path.trim_end_matches('/'); - Self::normalize_path(&format!("{}/{}", base, trimmed)) + Self::normalize_path(&format!("{}/{}", base, path)) } fn split_path(path: &str) -> (&str, &str) { @@ -1226,7 +1227,8 @@ impl StorageProvider for DrimeCloudProvider { // Navigate to initial path if specified if let Some(ref initial) = self.config.initial_path { - let initial = initial.trim().to_string(); + // As given: a name may begin or end with a space. + let initial = initial.to_string(); if !initial.is_empty() && initial != "/" { let normalized = Self::normalize_path(&initial); drime_log(&format!("Navigating to initial path: {}", normalized)); @@ -2963,6 +2965,22 @@ mod tests { use super::*; + #[test] + fn a_name_keeps_its_leading_and_trailing_spaces() { + // A name with a leading or trailing space is a name of its own, and a + // name of spaces only is not the current folder. + assert_eq!(DrimeCloudProvider::normalize_path("/ a"), "/ a"); + assert_eq!(DrimeCloudProvider::normalize_path("/a /b "), "/a /b "); + let mut p = test_provider(); + p.current_path = "/Cloud".to_string(); + assert_eq!(p.resolve_path(" "), "/Cloud/ "); + assert_eq!(p.resolve_path(" a"), "/Cloud/ a"); + assert_eq!(p.resolve_path("/a "), "/a "); + // A leading backslash is the root, as normalize_path reads it. + assert_eq!(p.resolve_path("\\docs\\a "), "/docs/a "); + assert_eq!(p.resolve_path("sub\\a"), "/Cloud/sub/a"); + } + fn test_provider() -> DrimeCloudProvider { let config = DrimeCloudConfig { api_token: secrecy::SecretString::from("test-token".to_string()), @@ -3723,7 +3741,7 @@ mod tests { fn normalize_path_preserves_root_and_trims_trailing_slash() { assert_eq!(DrimeCloudProvider::normalize_path(""), "/"); assert_eq!(DrimeCloudProvider::normalize_path("/"), "/"); - assert_eq!(DrimeCloudProvider::normalize_path(" "), "/"); + assert_eq!(DrimeCloudProvider::normalize_path(" "), "/ "); assert_eq!(DrimeCloudProvider::normalize_path("foo"), "/foo"); assert_eq!(DrimeCloudProvider::normalize_path("/foo"), "/foo"); assert_eq!(DrimeCloudProvider::normalize_path("/foo/"), "/foo"); diff --git a/src-tauri/src/providers/filelu.rs b/src-tauri/src/providers/filelu.rs index 8898a8843..12b26a8fb 100644 --- a/src-tauri/src/providers/filelu.rs +++ b/src-tauri/src/providers/filelu.rs @@ -469,7 +469,8 @@ impl FileLuProvider { } fn normalize_path(path: &str) -> String { - let normalized = path.trim().replace('\\', "/"); + // Spaces are part of a name: ` a` and `a ` are not `a`. + let normalized = path.replace('\\', "/"); if normalized.is_empty() { return "/".to_string(); } @@ -493,7 +494,8 @@ impl FileLuProvider { } fn resolve_path(&self, path: &str) -> String { - if path.starts_with('/') { + // `normalize_path` takes `\` for a separator: a leading one is the root too. + if path.starts_with(['/', '\\']) { Self::normalize_path(path) } else { let base = self.current_path.trim_end_matches('/').to_string(); @@ -2569,6 +2571,24 @@ fn mime_from_ext(ext: &str) -> &'static str { #[cfg(test)] mod tests { + #[test] + fn a_name_keeps_its_leading_and_trailing_spaces() { + // A name with a leading or trailing space is a name of its own, and a + // name of spaces only is not the current folder. + assert_eq!(super::FileLuProvider::normalize_path("/ a"), "/ a"); + assert_eq!(super::FileLuProvider::normalize_path("/a /b "), "/a /b "); + let mut p = super::FileLuProvider::new(super::FileLuConfig { + api_key: secrecy::SecretString::from("k".to_string()), + initial_path: None, + }); + p.current_path = "/Cloud".to_string(); + assert_eq!(p.resolve_path(" "), "/Cloud/ "); + assert_eq!(p.resolve_path(" a"), "/Cloud/ a"); + assert_eq!(p.resolve_path("/a "), "/a "); + // A leading backslash is the root, as normalize_path reads it. + assert_eq!(p.resolve_path("\\docs\\a "), "/docs/a "); + assert_eq!(p.resolve_path("sub\\a"), "/Cloud/sub/a"); + } struct CapFixture { provider: FileLuProvider, diff --git a/src-tauri/src/providers/fourshared.rs b/src-tauri/src/providers/fourshared.rs index a1194d6c8..b09361273 100644 --- a/src-tauri/src/providers/fourshared.rs +++ b/src-tauri/src/providers/fourshared.rs @@ -427,11 +427,11 @@ impl FourSharedProvider { /// Normalize an absolute path (ensure starts with /, no trailing slash) fn normalize_path(path: &str) -> String { - let trimmed = path.trim(); - if trimmed.is_empty() || trimmed == "/" || trimmed == "." || trimmed == "./" { + // Spaces are part of a name: ` a` and `a ` are not `a`. + if path.is_empty() || path == "/" || path == "." || path == "./" { return "/".to_string(); } - let parts: Vec<&str> = trimmed + let parts: Vec<&str> = path .split('/') .filter(|s| !s.is_empty() && *s != "." && *s != "..") .collect(); @@ -447,12 +447,11 @@ impl FourSharedProvider { /// - "/Cloud" → "/Cloud" (absolute, as-is) /// - "Cloud" → current_path + "/Cloud" (relative) fn resolve_path(&self, path: &str) -> String { - let trimmed = path.trim(); - if trimmed.is_empty() || trimmed == "." || trimmed == "./" { + if path.is_empty() || path == "." || path == "./" { return self.current_path.clone(); } - if trimmed.starts_with('/') { - return Self::normalize_path(trimmed); + if path.starts_with('/') { + return Self::normalize_path(path); } // Relative path: join with current_path let base = if self.current_path == "/" { @@ -460,7 +459,7 @@ impl FourSharedProvider { } else { self.current_path.clone() }; - Self::normalize_path(&format!("{}/{}", base, trimmed)) + Self::normalize_path(&format!("{}/{}", base, path)) } /// Move the file or folder `id` (`kind` is `files` or `folders`) into @@ -1313,9 +1312,8 @@ impl StorageProvider for FourSharedProvider { offset += page_count; } - self.current_path = normalized; - self.current_folder_id = folder_id; - + // A listing does not move the current folder: `cd` does. Moved, a + // relative path listed twice reached `docs/docs`. Ok(entries) } @@ -1986,6 +1984,19 @@ fn refused_lookup(status: reqwest::StatusCode, body: &str, path: &str) -> Provid mod tests { use super::*; + #[test] + fn a_name_keeps_its_leading_and_trailing_spaces() { + // A name with a leading or trailing space is a name of its own, and a + // name of spaces only is not the current folder. + assert_eq!(FourSharedProvider::normalize_path("/ a"), "/ a"); + assert_eq!(FourSharedProvider::normalize_path("/a /b "), "/a /b "); + let mut p = test_provider(); + p.current_path = "/Cloud".to_string(); + assert_eq!(p.resolve_path(" "), "/Cloud/ "); + assert_eq!(p.resolve_path(" a"), "/Cloud/ a"); + assert_eq!(p.resolve_path("/a "), "/a "); + } + fn test_provider() -> FourSharedProvider { let config = FourSharedConfig { consumer_key: "ck".to_string(), diff --git a/src-tauri/src/providers/ftp.rs b/src-tauri/src/providers/ftp.rs index e1672d49b..631b7af5c 100644 --- a/src-tauri/src/providers/ftp.rs +++ b/src-tauri/src/providers/ftp.rs @@ -1360,6 +1360,7 @@ impl FtpProvider { .map(|p| p.to_string_lossy().to_string()) .unwrap_or_else(|| "/".to_string()); if let Some(mut entry) = self.parse_mlsd_entry(mlst_line.trim(), &parent) { + super::ftp_listing::name_as_asked(&mut entry, path); // Facts without a size, which RFC 3659 allows: ask SIZE, // as below for a LIST row. if !entry.is_dir && super::ftp_listing::size_is_unreadable(&entry) { diff --git a/src-tauri/src/providers/ftp_listing.rs b/src-tauri/src/providers/ftp_listing.rs index db72af77f..1edd13870 100644 --- a/src-tauri/src/providers/ftp_listing.rs +++ b/src-tauri/src/providers/ftp_listing.rs @@ -316,7 +316,7 @@ pub(crate) fn parse_unix_listing( now: NaiveDateTime, ) -> Option { let parts: Vec<&str> = line.split_whitespace().collect(); - if parts.len() < 9 { + if parts.len() < 8 { return None; } @@ -329,7 +329,16 @@ pub(crate) fn parse_unix_listing( // Sliced from the original line rather than rejoined from tokens, so a // name containing runs of spaces keeps them. - let name = field_tail(line, 8)?.to_string(); + let name = match field_tail(line, 8) { + Some(name) => name.to_string(), + // Only spaces after the date: a name of spaces, which begins one + // separator after the time or the year, as `ls -l` writes the row. + // The date has to read as one, or this is another row. + None if unix_list_date(parts[5], parts[6], parts[7], now).is_some() => { + name_of_spaces(line)? + } + None => return None, + }; // Handle symlinks (name -> target) let (actual_name, link_target) = if is_symlink && name.contains(" -> ") { @@ -371,6 +380,18 @@ pub(crate) fn parse_unix_listing( Some(entry) } +/// The name of spaces that ends a Unix row after its eighth field: what follows +/// the one separator after the time or the year. +fn name_of_spaces(line: &str) -> Option { + let start = field_start(line, 7)?; + let end = line[start..] + .find(char::is_whitespace) + .map_or(line.len(), |len| start + len); + let mut rest = line[end..].trim_end_matches(['\r', '\n']).chars(); + rest.next()?; + Some(rest.as_str().to_string()).filter(|name| !name.is_empty()) +} + /// Parse DOS-style listing (Windows FTP servers) pub(crate) fn parse_dos_listing(line: &str, base_path: &str) -> Option { // DOS format: 01-23-24 10:30AM folder_name @@ -542,12 +563,39 @@ fn dos_list_date(date: &str, time: &str) -> Option { )) } +/// suppaftp trims the MLST reply line, and with it the spaces a name ends +/// with: the reply about `/a ` names `/a`, another file. The entry is the +/// one asked for, so it takes back the name asked when that is all it lost. +/// A name of spaces is lost whole, and the reply about `/x/ ` names `/x/`, +/// its folder: the entry takes the name and the path asked. +pub(crate) fn name_as_asked(entry: &mut RemoteEntry, asked: &str) { + let asked = asked.trim_end_matches('/'); + let Some(asked_name) = asked.rsplit('/').next() else { + return; + }; + if asked_name == entry.name { + return; + } + if asked_name.trim_end() == entry.name { + if let Some(parent) = entry.path.strip_suffix(entry.name.as_str()) { + entry.path = format!("{parent}{asked_name}"); + entry.name = asked_name.to_string(); + } + } else if !asked_name.is_empty() && asked_name.trim().is_empty() { + let parent = &asked[..asked.len() - asked_name.len()]; + entry.path = join_remote_path(parent, asked_name); + entry.name = asked_name.to_string(); + } +} + /// Parse MLSD/MLST line (RFC 3659 machine-readable format) /// Format: "fact1=val1;fact2=val2; filename" pub(crate) fn parse_mlsd_entry(line: &str, base_path: &str) -> Option { // Split on first space after semicolons to get facts and filename let (facts_str, name) = line.split_once(' ')?; - let raw_name = name.trim(); + // The name is all that follows that one space (RFC 3659 7.2), its own + // spaces included; suppaftp hands the line without its CRLF. + let raw_name = name; let name = normalize_mlsd_name(raw_name); if name == "." || name == ".." { @@ -855,6 +903,77 @@ mod tests { /// its size, and the 0 it leaves is marked like an unreadable one: the /// CLI's `--immutable` trusted it as exact and refused every complete file /// on such a server as a partial. A directory needs no size. + /// The name is what follows the one space after the facts: trimmed, + /// ` a.txt ` was listed as `a.txt`, another file. + #[test] + fn an_mlsd_name_keeps_its_spaces() { + let entry = parse_mlsd_entry("type=file;size=1; a.txt ", "/d").expect("a file"); + assert_eq!( + (entry.name.as_str(), entry.path.as_str()), + (" a.txt ", "/d/ a.txt ") + ); + let entry = parse_mlsd_entry("type=dir; ", "/d").expect("a folder"); + assert_eq!((entry.name.as_str(), entry.path.as_str()), (" ", "/d/ ")); + } + + /// suppaftp trims the MLST line: the entry asked as `/x/a ` comes back + /// as `a`, and gets its name back. Any other difference is the server's. + #[test] + fn an_mlst_entry_takes_back_the_spaces_its_name_ended_with() { + let mut entry = parse_mlsd_entry("type=file;size=3; /x/a", "/x").expect("a file"); + name_as_asked(&mut entry, "/x/a "); + assert_eq!( + (entry.name.as_str(), entry.path.as_str()), + ("a ", "/x/a ") + ); + let mut entry = parse_mlsd_entry("type=file;size=3; /x/A.TXT", "/x").expect("a file"); + name_as_asked(&mut entry, "/x/a.txt"); + assert_eq!( + (entry.name.as_str(), entry.path.as_str()), + ("A.TXT", "/x/A.TXT") + ); + } + + /// A name of spaces is lost whole from the trimmed MLST line, which then + /// names the folder: `stat` of `/x/ ` answered with an entry `x`. + #[test] + fn an_mlst_entry_named_with_spaces_only_takes_back_its_name() { + for (line, base, asked, path) in [ + ("type=file;size=3; /x/", "/x", "/x/ ", "/x/ "), + ("type=dir; /", "/", "/ ", "/ "), + ("type=file;size=3; /x/", "/x", "/x/ /", "/x/ "), + ] { + let mut entry = parse_mlsd_entry(line, base).expect("an entry"); + name_as_asked(&mut entry, asked); + let name = path.rsplit('/').next().unwrap(); + assert_eq!((entry.name.as_str(), entry.path.as_str()), (name, path)); + } + } + + /// A LIST row whose name is only spaces has eight fields: it was dropped, + /// so `stat` of that file, which falls back to LIST without MLST, read it + /// as missing. The name begins one separator after the time or the year. + #[test] + fn a_unix_row_named_with_spaces_only_is_listed() { + let now = at("2026-10-31 12:00"); + for (row, name) in [ + ("-rw-r--r-- 1 user group 3 Oct 30 09:15 ", " "), + ("drwxr-xr-x 2 user group 4096 Jan 5 2024 \r\n", " "), + ] { + let entry = parse_unix_listing(row, "/x", now).expect("an entry"); + assert_eq!(entry.name, name); + assert_eq!(entry.path, format!("/x/{name}")); + } + // Eight fields and no name, or a row that is not one with a date. + for row in [ + "-rw-r--r-- 1 user group 3 Oct 30 09:15", + "-rw-r--r-- 1 user group 3 Oct 30 09:15 \r\n", + "-rw-r--r-- 1 user 3 Oct 30 09:15 f.txt ", + ] { + assert!(parse_unix_listing(row, "/x", now).is_none(), "{row:?}"); + } + } + #[test] fn an_mlsd_file_without_a_size_fact_is_marked() { let file = parse_mlsd_entry("type=file;modify=20240101000000; a.txt", "/").expect("a file"); diff --git a/src-tauri/src/providers/imagekit.rs b/src-tauri/src/providers/imagekit.rs index fa4b9fb6b..19c2557ac 100644 --- a/src-tauri/src/providers/imagekit.rs +++ b/src-tauri/src/providers/imagekit.rs @@ -288,7 +288,8 @@ impl ImageKitProvider { } fn resolve_path(&self, path: &str) -> String { - if path.trim().is_empty() { + // A name of spaces is a name, not the current folder. + if path.is_empty() { return self.current_path.clone(); } if path.starts_with('/') { @@ -404,6 +405,7 @@ impl ImageKitProvider { } else { basename(&target).to_string() }; + refuse_rewritten_name(&file_name, NameUse::Upload)?; let folder = folder_path(&parent_path(&target)); let meta = tokio::fs::metadata(local_path) .await @@ -550,6 +552,11 @@ impl ImageKitProvider { async fn copy_file(&self, from: &str, to: &str) -> Result<(), ProviderError> { let source = normalize_path(from); let dest_parent = folder_path(&parent_path(&normalize_path(to))); + // The copy keeps the name and a rename follows: refuse the new name + // before the copy, not after it. + if basename(&source) != basename(&normalize_path(to)) { + refuse_rewritten_name(basename(&normalize_path(to)), NameUse::RenameFile)?; + } let resp = self .auth(self.client.post(format!("{}/files/copy", self.api_base()))) .json(&CopyFileRequest { @@ -576,6 +583,7 @@ impl ImageKitProvider { } async fn rename_file(&self, from: &str, new_name: &str) -> Result<(), ProviderError> { + refuse_rewritten_name(new_name, NameUse::RenameFile)?; let source = normalize_path(from); let resp = self .auth(self.client.put(format!("{}/files/rename", self.api_base()))) @@ -638,6 +646,11 @@ impl ImageKitProvider { let src_name = basename(&source); let dest_name = basename(&target); let moves = folder_path(&src_parent) != folder_path(&dest_parent); + // A move and a rename are two calls: refuse the new name before the + // first one. + if src_name != dest_name { + refuse_rewritten_name(dest_name, NameUse::RenameFile)?; + } if !moves { return if src_name == dest_name { Ok(()) @@ -703,14 +716,27 @@ impl ImageKitProvider { } // The same folder move queued earlier and not seen to finish: wait for // that job. Its source may already be gone, and a second job would - // run after the first. - if let Some(outcome) = self - .resume_folder_job("moveFolder", &source, &parent_path(&target)) - .await - { - return outcome; + // run after the first. A moveFolder job keeps the folder's name, so a + // move to another name in that folder is another move, not this one. + if basename(&source) == basename(&target) { + if let Some(outcome) = self + .resume_folder_job("moveFolder", &source, &parent_path(&target)) + .await + { + return outcome; + } } let entry = self.stat(&source).await?; + // A new name ImageKit would store as another one is refused before + // anything is asked of the destination or changed. + if basename(&source) != basename(&target) { + let use_ = if entry.is_dir { + NameUse::RenameFolder + } else { + NameUse::RenameFile + }; + refuse_rewritten_name(basename(&target), use_)?; + } // The listing can omit stored entries (including newly created // folders). find_entry therefore refuses an unverified destination // rather than treating an unlisted name as free. Positive matches @@ -724,7 +750,6 @@ impl ImageKitProvider { // Only same-folder rename has a documented collision409. // Move APIs append versions/merge and have no such guard. if entry.is_dir { - refuse_rewritten_folder_name(basename(&target))?; return self.rename_folder_job(&source, basename(&target)).await; } return self.rename_file(&source, basename(&target)).await; @@ -751,7 +776,6 @@ impl ImageKitProvider { .start_folder_job("moveFolder", &source, &parent_path(&target), false) .await; } - refuse_rewritten_folder_name(dest_name)?; let src_parent = parent_path(&source); let dest_parent = parent_path(&target); if folder_path(&src_parent) == folder_path(&dest_parent) { @@ -1283,6 +1307,7 @@ impl StorageProvider for ImageKitProvider { if name.is_empty() { return Ok(()); } + refuse_rewritten_name(name, NameUse::CreateFolder)?; let parent = folder_path(&parent_path(&resolved)); let resp = self @@ -1613,28 +1638,72 @@ fn folder_copy_destination(source: &str, target: &str) -> Result Result<(), ProviderError> { - let rewritten: String = name - .chars() - .map(|c| { - if c.is_alphanumeric() || c == '-' { - c - } else { - '_' - } - }) - .collect(); - if rewritten == name { +/// What ImageKit does with a name it is given: it keeps the letters, marks +/// and digits of the Basic Multilingual Plane, `-`, `_` and, in a file name, +/// `.`, and stores every other character as `_`, one for each. Measured on the +/// API on 2026-10-31, 68 characters one by one: +/// +/// - kept: `é` (composed or with a combining accent), `ß`, `日`, `한`, `Ա`, +/// `कि`, `ｱ`, `Ａ`, `fi`, `Dž`, `ʰ`, `ª`, `Ω` (U+2126), `١`, `²`, `½`, `①`, +/// `Ⅷ`, `〇`, and the marks U+0345, U+20DD and U+0E31; +/// - `_`: space, no-break space, `(`, `&`, `'`, `’`, `·`, the circled letters +/// `Ⓐ` and `ⓐ` (symbols that Unicode counts as alphabetic), and every +/// character above U+FFFF, letters and digits included (`𝔸`, `𐐀`, `𠀀`, +/// `𝟎`, `𑁦`, an emoji): `a𝔸𝔸b` became `a__b`; +/// - a folder named `a.b` was created as `a_b`. +/// +/// The upload documentation lists only a-z, A-Z, 0-9, `.` and `-`, fewer than +/// it keeps. +fn imagekit_stored_name(name: &str, folder: bool) -> String { + name.chars() + .map(|c| if imagekit_keeps(c, folder) { c } else { '_' }) + .collect() +} + +/// Whether ImageKit keeps `c` in a name (see `imagekit_stored_name`). Of the +/// characters of the Basic Multilingual Plane that Rust's `is_alphanumeric` +/// accepts, the circled Latin letters U+24B6 to U+24E9 are the only ones that +/// are not a letter, a mark or a digit. +fn imagekit_keeps(c: char, folder: bool) -> bool { + let letter_mark_or_digit = u32::from(c) <= 0xFFFF + && !('\u{24B6}'..='\u{24E9}').contains(&c) + && (c.is_alphanumeric() || unicode_normalization::char::is_combining_mark(c)); + letter_mark_or_digit || c == '-' || c == '_' || (c == '.' && !folder) +} + +/// What a name is given to ImageKit for. +#[derive(Clone, Copy)] +enum NameUse { + Upload, + CreateFolder, + RenameFile, + RenameFolder, +} + +/// A name ImageKit would store as another one is refused before anything is +/// sent. Sent as it is, `a b.txt` was uploaded as `a_b.txt` and, the upload +/// overwriting, replaced the file of that name; a folder or a file renamed +/// to such a name was left under a name nobody asked for. +fn refuse_rewritten_name(name: &str, use_: NameUse) -> Result<(), ProviderError> { + let folder = matches!(use_, NameUse::CreateFolder | NameUse::RenameFolder); + let stored = imagekit_stored_name(name, folder); + if stored == name { return Ok(()); } + let (would, nothing) = match use_ { + NameUse::Upload => ("upload the file as", "Nothing was uploaded"), + NameUse::CreateFolder => ("create the folder as", "Nothing was created"), + NameUse::RenameFile => ("rename the file to", "Nothing was changed"), + NameUse::RenameFolder => ("rename the folder to", "Nothing was changed"), + }; + let keeps = if folder { + "a folder name there keeps only letters, marks and digits up to U+FFFF, '-' and '_'" + } else { + "a file name there keeps only letters, marks and digits up to U+FFFF, '.', '-' and '_'" + }; Err(ProviderError::NotSupported(format!( - "ImageKit would rename the folder to \"{rewritten}\", not \"{name}\": a folder name \ - there keeps only letters, digits and '-', and every other character becomes '_'. \ - Nothing was changed" + "ImageKit would {would} \"{stored}\", not \"{name}\": {keeps}, and every other \ + character becomes '_'. {nothing}" ))) } @@ -1712,6 +1781,23 @@ fn validate_download_url(url: &str) -> Result<(), ProviderError> { #[cfg(test)] mod tests { + #[test] + fn a_name_keeps_its_leading_and_trailing_spaces() { + // A name with a leading or trailing space is a name of its own, and a + // name of spaces only is not the current folder. + assert_eq!(super::normalize_path("/ a"), "/ a"); + assert_eq!(super::normalize_path("/a /b "), "/a /b "); + let mut p = super::ImageKitProvider::new(super::ImageKitConfig { + imagekit_id: "test".into(), + private_key: secrecy::SecretString::from("test".to_string()), + initial_path: None, + }); + p.current_path = "/Cloud".to_string(); + assert_eq!(p.resolve_path(" "), "/Cloud/ "); + assert_eq!(p.resolve_path(" a"), "/Cloud/ a"); + assert_eq!(p.resolve_path("/a "), "/a "); + } + #[tokio::test] async fn probe_contract_imagekit_create_only_upload_uses_the_native_guard() { use axum::response::IntoResponse; @@ -2356,12 +2442,186 @@ mod tests { "{outcome:?}" ); assert!(posts.lock().unwrap().is_empty()); - assert!(refuse_rewritten_folder_name("my albums") + assert!(refuse_rewritten_name("my albums", NameUse::RenameFolder) .unwrap_err() .to_string() .contains("my_albums")); - assert!(refuse_rewritten_folder_name("albums_2026-10").is_ok()); - assert!(refuse_rewritten_folder_name("fotografía").is_ok()); + assert!(refuse_rewritten_name("albums_2026-10", NameUse::RenameFolder).is_ok()); + assert!(refuse_rewritten_name("fotografía", NameUse::RenameFolder).is_ok()); + } + + /// What ImageKit keeps of a name, as measured on its API: letters, marks + /// and digits of the Basic Multilingual Plane, `-`, `_`, and `.` in a file + /// name only. Every character measured is here, so a rule that keeps one + /// ImageKit rewrote (every Unicode alphanumeric did: `Ⓐ`, `𝔸`) fails. + #[test] + fn imagekit_keeps_letters_marks_digits_and_three_signs() { + for kept in [ + "a_b-c.txt", + "aéb.txt", + "ae\u{301}b.txt", + "aßb.txt", + "a日b.txt", + "a\u{661}b.txt", + "a²b.txt", + // Each letter case, modifier letters, compatibility forms, other + // scripts, the three kinds of number and the three kinds of mark. + "a\u{2167}\u{3007}\u{bd}\u{2460}\u{663}b.txt", + "a\u{2b0}\u{1c5}\u{1fbc}\u{aa}\u{fb01}\u{640}b.txt", + "a\u{531}\u{915}\u{93f}\u{ff71}\u{ff21}\u{d55c}\u{130}b.txt", + "a\u{2126}\u{212b}b.txt", + "a\u{345}\u{20dd}\u{e31}b.txt", + ] { + assert_eq!(imagekit_stored_name(kept, false), kept); + } + for (sent, stored) in [ + ("a b.txt", "a_b.txt"), + (" lead.txt", "_lead.txt"), + (" ", "___"), + ("a(1).txt", "a_1_.txt"), + ("a&b'c.txt", "a_b_c.txt"), + ("a\u{a0}b\u{2019}c\u{b7}d.txt", "a_b_c_d.txt"), + ("a\u{1F600}.txt", "a_.txt"), + // Symbols Unicode counts as alphabetic: the circled letters. + ("a\u{24b6}\u{24d0}\u{24e9}b.txt", "a___b.txt"), + ("a\u{1F130}b.txt", "a_b.txt"), + // Above U+FFFF, letters, digits and marks too, one `_` each. + ("a\u{1d538}\u{1d538}b.txt", "a__b.txt"), + ("\u{915}\u{1d538}.txt", "\u{915}_.txt"), + ( + "a\u{20000}\u{1d7ce}\u{10400}\u{10330}\u{11066}\u{1d165}b.txt", + "a______b.txt", + ), + ] { + assert_eq!(imagekit_stored_name(sent, false), stored, "{sent:?}"); + } + // The circled digit after the circled letters is a digit, as `①` is. + assert_eq!(imagekit_stored_name("\u{24ea}", true), "\u{24ea}"); + assert_eq!(imagekit_stored_name("a.b", true), "a_b"); + assert_eq!(imagekit_stored_name("a-b_c\u{e9}", true), "a-b_c\u{e9}"); + } + + /// Every write, as `METHOD path name`: the name is the upload's + /// `fileName`, the folder's `folderName`, or the rename's `newFileName`. + async fn provider_recording_writes() -> ( + ImageKitProvider, + std::sync::Arc>>, + ) { + use std::sync::Arc; + let writes: Arc>> = Arc::default(); + let seen = Arc::clone(&writes); + let app = + axum::Router::new().fallback(axum::routing::any(move |req: axum::extract::Request| { + let seen = Arc::clone(&seen); + async move { + let method = req.method().as_str().to_string(); + let path = req.uri().path().to_string(); + let query = req.uri().query().unwrap_or("").to_string(); + let body = axum::body::to_bytes(req.into_body(), 1 << 20) + .await + .unwrap(); + if method == "GET" { + // The source file, and nothing else, in every listing. + let listed = if query.contains("path=%2Fsrc&") { + serde_json::json!([{ + "fileId": "A", "name": "a.txt", "filePath": "/src/a.txt", + "type": "file", "size": 3, + }]) + } else { + serde_json::json!([]) + }; + return axum::Json(listed); + } + let text = String::from_utf8_lossy(&body).to_string(); + let name = serde_json::from_str::(&text) + .ok() + .and_then(|v| { + ["newFileName", "folderName", "newFolderName"] + .iter() + .find_map(|k| v[*k].as_str().map(str::to_string)) + }) + .or_else(|| { + text.split("name=\"fileName\"\r\n\r\n") + .nth(1) + .and_then(|rest| rest.split("\r\n").next()) + .map(str::to_string) + }) + .unwrap_or_default(); + seen.lock().unwrap().push(format!("{method} {path} {name}")); + axum::Json(serde_json::json!({ "jobId": "J" })) + } + })); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { axum::serve(listener, app).await.ok() }); + let mut provider = empty_provider(); + provider.connected = true; + provider.api_base_override = Some(format!("http://{addr}")); + (provider, writes) + } + + /// Sent as it was, `a b.txt` was uploaded as `a_b.txt` and replaced the + /// file of that name (measured on the API). A name ImageKit would store + /// as another one is refused before any request; a name it keeps goes. + #[tokio::test] + async fn a_name_imagekit_would_rewrite_is_refused_before_any_request() { + let local = tempfile::NamedTempFile::new().unwrap(); + std::fs::write(local.path(), b"abc").unwrap(); + let local = local.path().to_str().unwrap(); + let (mut provider, writes) = provider_recording_writes().await; + + let refused = [ + ("upload", provider.upload(local, "/src/a b.txt", None).await), + ( + "upload", + provider.upload(local, "/src/ lead.txt", None).await, + ), + // A letter above U+FFFF: ImageKit stores it as `_`. + ( + "upload", + provider.upload(local, "/src/a\u{1d538}.txt", None).await, + ), + ("mkdir", provider.mkdir("/src/my albums").await), + ("mkdir", provider.mkdir("/src/a.b").await), + ( + "rename", + provider.rename("/src/a.txt", "/src/a b.txt").await, + ), + ("move", provider.rename("/src/a.txt", "/dst/a b.txt").await), + ( + "copy", + provider.server_copy("/src/a.txt", "/dst/a&b.txt").await, + ), + ]; + for (what, outcome) in &refused { + let message = match outcome { + Err(ProviderError::NotSupported(message)) => message, + other => panic!("{what}: {other:?}"), + }; + assert!( + message.contains("every other character becomes '_'"), + "{message}" + ); + } + assert!( + writes.lock().unwrap().is_empty(), + "{:?}", + writes.lock().unwrap() + ); + let upload = refused[0].1.as_ref().unwrap_err().to_string(); + assert!(upload.contains("\"a_b.txt\", not \"a b.txt\""), "{upload}"); + let astral = refused[2].1.as_ref().unwrap_err().to_string(); + assert!(astral.contains("\"a_.txt\""), "{astral}"); + + provider + .upload(local, "/src/aéb-1_2.txt", None) + .await + .unwrap(); + provider.mkdir("/src/fotografía-2026").await.unwrap(); + assert_eq!( + *writes.lock().unwrap(), + ["POST /upload aéb-1_2.txt", "POST /folder/ fotografía-2026"] + ); } #[tokio::test] @@ -2644,6 +2904,37 @@ mod tests { assert_eq!(posts, 1, "{:?}", calls.lock().unwrap()); } + /// A moveFolder job keeps the folder's name. A move to another name in + /// the same folder found the job queued for the first move and answered + /// with its outcome: `/dst/other` was reported moved while the folder + /// went to `/dst/photos`. + #[tokio::test] + async fn a_queued_folder_move_answers_only_a_move_that_keeps_its_name() { + let (mut provider, calls) = provider_on_tree(&["/src", "/src/photos"], &[500, 200]).await; + let first = provider + .start_folder_job("moveFolder", "/src/photos", "/dst", false) + .await; + assert!(first.is_err(), "{first:?}"); + let other = provider.rename("/src/photos", "/dst/other").await; + assert!(other.is_err(), "another name is another move: {other:?}"); + assert_eq!( + provider.unfinished_folder_jobs.lock().unwrap().len(), + 1, + "the first move is still waited for" + ); + provider + .rename("/src/photos", "/dst/photos") + .await + .expect("the same move waits for its job"); + let posts = calls + .lock() + .unwrap() + .iter() + .filter(|c| c.starts_with("POST /bulkJobs/")) + .count(); + assert_eq!(posts, 1, "{:?}", calls.lock().unwrap()); + } + /// A job ImageKit no longer knows (404) is over: remembering it made /// every later move of that folder poll the dead job again, so the folder /// could not be moved for the rest of the session. A later fresh attempt diff --git a/src-tauri/src/providers/internxt.rs b/src-tauri/src/providers/internxt.rs index 1778049c8..fbd3de941 100644 --- a/src-tauri/src/providers/internxt.rs +++ b/src-tauri/src/providers/internxt.rs @@ -1173,13 +1173,13 @@ impl InternxtProvider { /// Normalize path: ensure leading /, remove trailing /, collapse //, resolve . and .. fn normalize_path(path: &str) -> String { - let trimmed = path.trim(); - if trimmed.is_empty() || trimmed == "." || trimmed == "/" { + // Spaces are part of a name: ` a` and `a ` are not `a`. + if path.is_empty() || path == "." || path == "/" { return "/".to_string(); } let mut segments: Vec<&str> = Vec::new(); - for seg in trimmed.split('/') { + for seg in path.split('/') { match seg { "" | "." => continue, ".." => { @@ -1438,7 +1438,8 @@ impl InternxtProvider { // Navigate to initial path if specified if let Some(ref initial) = self.config.initial_path { - let initial = initial.trim().to_string(); + // As given: a name may begin or end with a space. + let initial = initial.to_string(); if !initial.is_empty() && initial != "/" { let normalized = Self::normalize_path(&initial); internxt_log(&format!( @@ -1677,7 +1678,8 @@ impl StorageProvider for InternxtProvider { // Navigate to initial path if specified if let Some(ref initial) = self.config.initial_path { - let initial = initial.trim().to_string(); + // As given: a name may begin or end with a space. + let initial = initial.to_string(); if !initial.is_empty() && initial != "/" { let normalized = Self::normalize_path(&initial); internxt_log(&format!( @@ -3830,6 +3832,19 @@ mod tests { assert_eq!((file.1.as_str(), file.2.as_str()), ("a.txt", "D")); } + #[test] + fn a_name_keeps_its_leading_and_trailing_spaces() { + // A name with a leading or trailing space is a name of its own, and a + // name of spaces only is not the current folder. + assert_eq!(InternxtProvider::normalize_path("/ a"), "/ a"); + assert_eq!(InternxtProvider::normalize_path("/a /b "), "/a /b "); + let mut p = test_provider(); + p.current_path = "/Cloud".to_string(); + assert_eq!(p.resolve_path(" "), "/Cloud/ "); + assert_eq!(p.resolve_path(" a"), "/Cloud/ a"); + assert_eq!(p.resolve_path("/a "), "/a "); + } + fn test_provider() -> InternxtProvider { let config = InternxtConfig { email: "alice@example.com".to_string(), diff --git a/src-tauri/src/providers/jottacloud.rs b/src-tauri/src/providers/jottacloud.rs index 43bcdea9a..942323ad8 100644 --- a/src-tauri/src/providers/jottacloud.rs +++ b/src-tauri/src/providers/jottacloud.rs @@ -881,24 +881,25 @@ impl JottacloudProvider { } fn normalize_path(path: &str) -> String { - let trimmed = path.trim().replace('\\', "/"); - if trimmed.is_empty() || trimmed == "/" { + // Spaces are part of a name: ` a` and `a ` are not `a`. + let path = path.replace('\\', "/"); + if path.is_empty() || path == "/" { return "/".to_string(); } - let p = if trimmed.starts_with('/') { - trimmed + let p = if path.starts_with('/') { + path } else { - format!("/{}", trimmed) + format!("/{}", path) }; p.trim_end_matches('/').to_string() } fn resolve_path(&self, path: &str) -> String { - let path = path.trim(); if path.is_empty() || path == "." { return self.current_path.clone(); } - if path.starts_with('/') { + // `normalize_path` takes `\` for a separator: a leading one is the root too. + if path.starts_with(['/', '\\']) { return Self::normalize_path(path); } let base = self.current_path.trim_end_matches('/'); @@ -1525,7 +1526,8 @@ impl StorageProvider for JottacloudProvider { // Step 6: Navigate to initial path self.current_path = "/".to_string(); if let Some(ref initial) = self.config.initial_path { - let initial = initial.trim().to_string(); + // As given: a name may begin or end with a space. + let initial = initial.to_string(); if !initial.is_empty() && initial != "/" { self.current_path = Self::normalize_path(&initial); } @@ -2341,6 +2343,9 @@ impl JottacloudProvider { /// Relative path under the live mountpoint: strip /// `/{user}/{device}/{mount}` from the trash `` (the original /// parent) and append the entry name. Root-level items stay `/{name}`. + /// The parent's names are the stored ones, `reports％` for `reports%`: + /// each is decoded like a listed name, since `jfs_url` encodes the path + /// again and an encoded name would reach another folder. fn trash_relative_path(abspath: &str, name: &str) -> String { let parts: Vec<&str> = abspath .trim_matches('/') @@ -2349,7 +2354,11 @@ impl JottacloudProvider { .collect(); // 0 user, 1 device, 2 mountpoint, 3… original parent under that mount. let parent = if parts.len() > 3 { - parts[3..].join("/") + parts[3..] + .iter() + .map(|s| crate::restricted_chars::decode_leaf(ProviderType::Jottacloud, s)) + .collect::>() + .join("/") } else { String::new() }; @@ -3120,13 +3129,13 @@ impl JottacloudProvider { let mut entries = Vec::new(); let mut reader = Reader::from_str(xml); // Names arrive as attributes. `` is Text + GeneralRef and - // is accumulated (then trimmed once) rather than assigned per chunk. + // is accumulated, as given, rather than assigned per chunk: a folder + // name may end with a space. // // Trimming is left OFF here on purpose: quick-xml trims every Text // EVENT, not the node, so an entity splits `Photos & Videos` into // three events whose own edges carry the spaces. Trimming per event - // welds the pieces into `Photos&Videos`, and the single trim applied - // when the path is derived cannot put back what was already dropped. + // welds the pieces into `Photos&Videos`. // The scalars that need it (`size`, `state`, `modified`) trim // themselves below, where a whitespace-only event is also harmless // because it never matches their tag. @@ -3274,7 +3283,7 @@ impl JottacloudProvider { entries.push(RemoteEntry { name: pending_folder_name.clone(), path: Self::trash_relative_path( - pending_folder_abspath.trim(), + &pending_folder_abspath, &pending_folder_name, ), is_dir: true, @@ -3313,7 +3322,7 @@ impl JottacloudProvider { entries.push(RemoteEntry { name: current_name.clone(), path: Self::trash_relative_path( - current_abspath.trim(), + ¤t_abspath, ¤t_name, ), is_dir: false, @@ -3743,6 +3752,22 @@ mod tests { assert_real_progress(&updates, 300 * 1024, false); } + #[test] + fn a_name_keeps_its_leading_and_trailing_spaces() { + // A name with a leading or trailing space is a name of its own, and a + // name of spaces only is not the current folder. + assert_eq!(JottacloudProvider::normalize_path("/ a"), "/ a"); + assert_eq!(JottacloudProvider::normalize_path("/a /b "), "/a /b "); + let mut p = test_provider(); + p.current_path = "/Cloud".to_string(); + assert_eq!(p.resolve_path(" "), "/Cloud/ "); + assert_eq!(p.resolve_path(" a"), "/Cloud/ a"); + assert_eq!(p.resolve_path("/a "), "/a "); + // A leading backslash is the root, as normalize_path reads it. + assert_eq!(p.resolve_path("\\docs\\a "), "/docs/a "); + assert_eq!(p.resolve_path("sub\\a"), "/Cloud/sub/a"); + } + fn test_provider() -> JottacloudProvider { let config = JottacloudConfig { login_token: SecretString::from("test-token".to_string()), @@ -3769,7 +3794,7 @@ mod tests { fn normalize_path_trims_and_normalizes_separators() { assert_eq!(JottacloudProvider::normalize_path(""), "/"); assert_eq!(JottacloudProvider::normalize_path("/"), "/"); - assert_eq!(JottacloudProvider::normalize_path(" "), "/"); + assert_eq!(JottacloudProvider::normalize_path(" "), "/ "); assert_eq!(JottacloudProvider::normalize_path("foo"), "/foo"); assert_eq!(JottacloudProvider::normalize_path("/foo/bar/"), "/foo/bar"); assert_eq!(JottacloudProvider::normalize_path(r"\foo\bar"), "/foo/bar"); @@ -4004,6 +4029,62 @@ mod tests { assert_eq!(file.path, "/Photos & Videos/x.txt"); } + #[test] + fn parse_trash_xml_keeps_a_parent_name_that_ends_with_a_space() { + // `reports ` and `reports` are two folders. The parent taken from + // `` was trimmed, so an entry deleted from `reports ` would + // be restored to `reports`. + let xml = r#" + + + /user123/Jotta/Archive/reports + + + + + /user123/Jotta/Archive/reports + 1 + + +"#; + let entries = JottacloudProvider::parse_trash_xml(xml); + let folder = entries.iter().find(|e| e.is_dir).expect("folder"); + assert_eq!(folder.path, "/reports /q3"); + let file = entries.iter().find(|e| !e.is_dir).expect("file"); + assert_eq!(file.path, "/reports /x.txt"); + } + + #[test] + fn parse_trash_xml_decodes_the_stored_names_of_the_parent() { + // `reports% ` is stored as `reports％ ` and named so in ``. + // Kept encoded, the path was encoded again by `jfs_url`, and restore + // or purge reached `reports‛％ `, another name. + let xml = r#" + + + /user123/Jotta/Archive/reports％ /a：b + + + + + /user123/Jotta/Archive/reports％ + 1 + + +"#; + let entries = JottacloudProvider::parse_trash_xml(xml); + let folder = entries.iter().find(|e| e.is_dir).expect("folder"); + assert_eq!(folder.path, "/reports% /a:b/q3"); + let file = entries.iter().find(|e| !e.is_dir).expect("file"); + assert_eq!(file.path, "/reports% /x%.txt"); + // Back to the stored names once, as the listing writes them. + let url = test_provider().jfs_url(&file.path); + assert!( + url.ends_with("/reports%EF%BC%85%20/x%EF%BC%85.txt"), + "{url}" + ); + } + #[test] fn rename_in_trash_builds_urls_that_stay_on_trash() { let p = test_provider(); diff --git a/src-tauri/src/providers/kdrive.rs b/src-tauri/src/providers/kdrive.rs index 4831d97a1..3a26dbd8e 100644 --- a/src-tauri/src/providers/kdrive.rs +++ b/src-tauri/src/providers/kdrive.rs @@ -555,32 +555,33 @@ impl KDriveProvider { } fn normalize_path(path: &str) -> String { - let trimmed = path.trim().replace('\\', "/"); - if trimmed.is_empty() || trimmed == "/" { + // Spaces are part of a name: ` a` and `a ` are not `a`. + let path = path.replace('\\', "/"); + if path.is_empty() || path == "/" { return "/".to_string(); } - let p = if trimmed.starts_with('/') { - trimmed + let p = if path.starts_with('/') { + path } else { - format!("/{}", trimmed) + format!("/{}", path) }; p.trim_end_matches('/').to_string() } fn resolve_path(&self, path: &str) -> String { - let trimmed = path.trim(); // "." or empty means current directory - if trimmed.is_empty() || trimmed == "." { + if path.is_empty() || path == "." { return self.current_path.clone(); } // Check leading slash on the raw input before normalizing - // normalize_path unconditionally prepends "/", which would otherwise // make every relative input appear absolute and skip the current_path join. - if trimmed.starts_with('/') { - return Self::normalize_path(trimmed); + // `normalize_path` takes `\` for a separator: a leading one is the root too. + if path.starts_with(['/', '\\']) { + return Self::normalize_path(path); } let base = self.current_path.trim_end_matches('/'); - Self::normalize_path(&format!("{}/{}", base, trimmed)) + Self::normalize_path(&format!("{}/{}", base, path)) } fn split_path(path: &str) -> (&str, &str) { @@ -900,7 +901,8 @@ impl StorageProvider for KDriveProvider { // Navigate to initial path if specified if let Some(ref initial) = self.config.initial_path { - let initial = initial.trim().to_string(); + // As given: a name may begin or end with a space. + let initial = initial.to_string(); if !initial.is_empty() && initial != "/" { let normalized = Self::normalize_path(&initial); kdrive_log(&format!("Navigating to initial path: {}", normalized)); @@ -1060,10 +1062,8 @@ impl StorageProvider for KDriveProvider { cursor = next_cursor; } - // Update current position - self.current_path = resolved; - self.current_file_id = folder_id; - + // A listing does not move the current folder: `cd` does. Moved, a + // relative path listed twice reached `docs/docs`. Ok(entries) } @@ -2579,6 +2579,22 @@ mod tests { ); } + #[test] + fn a_name_keeps_its_leading_and_trailing_spaces() { + // A name with a leading or trailing space is a name of its own, and a + // name of spaces only is not the current folder. + assert_eq!(KDriveProvider::normalize_path("/ a"), "/ a"); + assert_eq!(KDriveProvider::normalize_path("/a /b "), "/a /b "); + let mut p = test_provider(); + p.current_path = "/Cloud".to_string(); + assert_eq!(p.resolve_path(" "), "/Cloud/ "); + assert_eq!(p.resolve_path(" a"), "/Cloud/ a"); + assert_eq!(p.resolve_path("/a "), "/a "); + // A leading backslash is the root, as normalize_path reads it. + assert_eq!(p.resolve_path("\\docs\\a "), "/docs/a "); + assert_eq!(p.resolve_path("sub\\a"), "/Cloud/sub/a"); + } + fn test_provider() -> KDriveProvider { let config = KDriveConfig { api_token: secrecy::SecretString::from("test-token".to_string()), diff --git a/src-tauri/src/providers/koofr.rs b/src-tauri/src/providers/koofr.rs index 5d2c91f6f..87628e49c 100644 --- a/src-tauri/src/providers/koofr.rs +++ b/src-tauri/src/providers/koofr.rs @@ -524,30 +524,32 @@ impl KoofrProvider { } fn normalize_path(path: &str) -> String { - let trimmed = path.trim().replace('\\', "/"); - if trimmed.is_empty() || trimmed == "/" { + // Spaces are part of a name: ` a` and `a ` are not `a`. + let path = path.replace('\\', "/"); + if path.is_empty() || path == "/" { return "/".into(); } - let p = if trimmed.starts_with('/') { - trimmed + let p = if path.starts_with('/') { + path } else { - format!("/{}", trimmed) + format!("/{}", path) }; p.trim_end_matches('/').to_string() } fn resolve_path(&self, path: &str) -> String { - let trimmed = path.trim(); - if trimmed.is_empty() || trimmed == "." { + if path.is_empty() || path == "." { return self.current_path.clone(); } - let normalized = Self::normalize_path(trimmed); - if normalized.starts_with('/') { - normalized - } else { - let base = self.current_path.trim_end_matches('/'); - format!("{}/{}", base, normalized) + // The input's leading slash decides: normalize_path always prepends + // one, so its result made every relative path absolute and skipped + // the current_path join. It takes `\` for a separator, so a leading + // one is the root too. + if path.starts_with(['/', '\\']) { + return Self::normalize_path(path); } + let base = self.current_path.trim_end_matches('/'); + Self::normalize_path(&format!("{}/{}", base, path)) } /// Split "/a/b/file.txt" → ("/a/b", "file.txt") @@ -810,8 +812,8 @@ impl StorageProvider for KoofrProvider { .map(|f| self.file_to_entry(f, &resolved)) .collect(); - // Update current_path on successful listing - self.current_path = resolved; + // A listing does not move the current folder: `cd` does. Moved, a + // relative path listed twice reached `docs/docs`. Ok(entries) } @@ -2078,6 +2080,28 @@ pub async fn koofr_empty_trash( #[cfg(test)] mod tests { + #[test] + fn a_name_keeps_its_leading_and_trailing_spaces() { + // A name with a leading or trailing space is a name of its own, and a + // name of spaces only is not the current folder. + assert_eq!(super::KoofrProvider::normalize_path("/ a"), "/ a"); + assert_eq!(super::KoofrProvider::normalize_path("/a /b "), "/a /b "); + let mut p = super::KoofrProvider::new(super::KoofrConfig { + email: "user@example.com".to_string(), + password: secrecy::SecretString::from("pass".to_string()), + initial_path: None, + }); + p.current_path = "/Cloud".to_string(); + assert_eq!(p.resolve_path(" "), "/Cloud/ "); + assert_eq!(p.resolve_path(" a"), "/Cloud/ a"); + assert_eq!(p.resolve_path("/a "), "/a "); + // A leading backslash is the root, as normalize_path reads it. + assert_eq!(p.resolve_path("\\docs\\a "), "/docs/a "); + assert_eq!(p.resolve_path("sub\\a"), "/Cloud/sub/a"); + // A relative path joins the current folder. + assert_eq!(p.resolve_path("child"), "/Cloud/child"); + } + #[test] fn probe_contract_koofr_does_not_erase_http_failures() { let body = r#"{"error":{"code":"NotFound","message":"missing"}}"#; @@ -2262,6 +2286,49 @@ mod tests { /// A rename onto its own path is a no-op everywhere else; Koofr refused /// it (live on 2026-09-26). Nothing is sent. + /// A listing does not move the current folder: `cd` does. Koofr's + /// listing moved it, and with a relative path joined to the current + /// folder, a relative path listed twice (the CLI's sync doctor lists the + /// root it was given, then walks it) reached `/docs/docs`. + #[tokio::test] + async fn a_listing_leaves_the_current_folder_where_it_is() { + use std::sync::{Arc, Mutex}; + let listed: Arc>> = Arc::default(); + let seen = Arc::clone(&listed); + let app = axum::Router::new().route( + "/mounts/M/files/list", + axum::routing::get(move |uri: axum::http::Uri| { + let seen = Arc::clone(&seen); + async move { + let url = reqwest::Url::parse(&format!("http://h{uri}")).unwrap(); + let path = url + .query_pairs() + .find(|(k, _)| k == "path") + .map(|(_, v)| v.to_string()) + .unwrap_or_default(); + seen.lock().unwrap().push(path); + axum::Json(serde_json::json!({ "files": [] })) + } + }), + ); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { axum::serve(listener, app).await.ok() }); + let mut provider = KoofrProvider::new(KoofrConfig { + email: "u@example.com".to_string(), + password: secrecy::SecretString::from("p".to_string()), + initial_path: None, + }); + provider.connected = true; + provider.mount_id = "M".to_string(); + provider.api_base_override = Some(format!("http://{addr}")); + + provider.list("docs").await.expect("first listing"); + provider.list("docs").await.expect("second listing"); + assert_eq!(*listed.lock().unwrap(), ["/docs", "/docs"]); + assert_eq!(provider.pwd().await.unwrap(), "/"); + } + #[tokio::test] async fn a_rename_onto_its_own_path_sends_nothing() { let (mut provider, calls) = provider_on_refusing_koofr().await; diff --git a/src-tauri/src/providers/opendrive.rs b/src-tauri/src/providers/opendrive.rs index 664a1570e..c0bab3bd6 100644 --- a/src-tauri/src/providers/opendrive.rs +++ b/src-tauri/src/providers/opendrive.rs @@ -646,7 +646,8 @@ impl OpenDriveProvider { if path.is_empty() || path == "." { return Ok(self.current_path.clone()); } - if path.starts_with('/') { + // `normalize_path` takes `\` for a separator: a leading one is the root too. + if path.starts_with(['/', '\\']) { normalize_path(path) } else { normalize_path(&format!( @@ -3315,6 +3316,21 @@ mod tests { assert_eq!(normalize_path("/a/b/../../..").unwrap(), "/"); } + #[test] + fn a_leading_backslash_is_the_root_and_a_relative_path_joins_the_current_folder() { + let mut p = OpenDriveProvider::new(OpenDriveConfig { + host: "https://dev.opendrive.com".into(), + username: "u".into(), + password: SecretString::from("p".to_string()), + initial_path: None, + default_privacy: None, + }); + p.current_path = "/Cloud".to_string(); + assert_eq!(p.resolve_path("\\docs\\a ").unwrap(), "/docs/a "); + assert_eq!(p.resolve_path("/docs").unwrap(), "/docs"); + assert_eq!(p.resolve_path("sub\\a").unwrap(), "/Cloud/sub/a"); + } + #[test] fn split_parent_child_handles_root_and_nested() { assert_eq!(split_parent_child("/"), ("/".to_string(), String::new())); diff --git a/src-tauri/src/providers/path_resolution_guard.rs b/src-tauri/src/providers/path_resolution_guard.rs index a227d9435..8beaa077d 100644 --- a/src-tauri/src/providers/path_resolution_guard.rs +++ b/src-tauri/src/providers/path_resolution_guard.rs @@ -15,6 +15,10 @@ //! `.resolve_path(` (rustfmt breaks a long call as `self\n .resolve_path(`), //! `current_folder_id` in any form (`&self.current_folder_id` borrows it //! without `.clone()`), and `"root"` in any form (`"root".into()`). +//! +//! Two guards read every provider: a listing does not move the current +//! folder, and the initial folder of a profile is taken as given, spaces +//! included. // SPDX-License-Identifier: GPL-3.0-or-later // Copyright (c) 2024-2026 axpnet: AI-assisted (see AI-TRANSPARENCY.md) @@ -437,3 +441,443 @@ async fn live_cd_then_one_segment_paths_resolve_like_every_provider() { } assert!(ran > 0, "set at least one AEROFTP_LIVE_CD_*_PROFILE_ID"); } + +/// Every provider source but this guard (which names the patterns), as +/// `(file name, source)`. +fn provider_sources() -> Vec<(String, String)> { + let dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src/providers"); + let mut sources: Vec<(String, String)> = std::fs::read_dir(&dir) + .expect("the providers folder") + .filter_map(Result::ok) + .map(|entry| entry.path()) + .filter(|path| path.extension().is_some_and(|ext| ext == "rs")) + .filter(|path| !path.ends_with("path_resolution_guard.rs")) + .map(|path| { + let name = path.file_name().unwrap().to_string_lossy().into_owned(); + ( + name, + std::fs::read_to_string(&path).expect("a provider source"), + ) + }) + .collect(); + sources.sort(); + assert!(sources.len() > 40, "{} sources", sources.len()); + sources +} + +/// Every Rust source of the backend, the CLI and the TUI, by its path under +/// `src`, this guard excepted. +fn backend_sources() -> Vec<(String, String)> { + let src = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let mut pending = vec![src.clone()]; + let mut sources = Vec::new(); + while let Some(dir) = pending.pop() { + for path in std::fs::read_dir(&dir) + .expect("a source folder") + .filter_map(Result::ok) + .map(|entry| entry.path()) + { + if path.is_dir() { + pending.push(path); + } else if path.extension().is_some_and(|ext| ext == "rs") + && !path.ends_with("path_resolution_guard.rs") + { + let name = path + .strip_prefix(&src) + .expect("under src") + .to_string_lossy() + .replace('\\', "/"); + sources.push((name, std::fs::read_to_string(&path).expect("a source"))); + } + } + } + sources.sort(); + assert!( + sources.iter().any(|(name, _)| name == "bin/aeroftp_cli.rs") + && sources.iter().any(|(name, _)| name == "cli_tui/app.rs") + && sources + .iter() + .any(|(name, _)| name == "providers/webdav.rs") + && sources.len() > 300, + "{} sources", + sources.len() + ); + sources +} + +/// [`production`] for a source that may have no test module. +fn production_part(source: &str) -> &str { + [ + "\n#[cfg(test)]\nmod tests {", + "\r\n#[cfg(test)]\r\nmod tests {", + ] + .iter() + .find_map(|marker| source.find(marker)) + .map_or(source, |at| &source[..at]) +} + +/// A listing does not move the current folder: `cd` does. Koofr, kDrive and +/// 4shared moved it in `list`, so a relative path listed twice (the CLI's +/// sync doctor lists the root it was given, then walks it) reached +/// `docs/docs`, and a listing of another folder (the overwrite check's +/// destination) left the session there. +#[test] +fn no_provider_listing_moves_the_current_folder() { + let moves = |line: &str| { + [ + "self.current_path =", + "self.current_dir =", + "self.current_folder_id =", + "self.current_file_id =", + "self.current_fld_id =", + ] + .iter() + .any(|needle| line.contains(needle)) + }; + let (mut in_cd, mut in_list) = (0, Vec::new()); + for (file, source) in provider_sources() { + for (line, function) in sites_where(production_part(&source), moves) { + match function { + "cd" => in_cd += 1, + "list" => in_list.push(format!("{file}:{line}")), + _ => {} + } + } + } + // The guard sees the current folder moved where it belongs. + assert!(in_cd > 10, "{in_cd} moves in cd"); + assert!( + in_list.is_empty(), + "a listing moves the current folder: {in_list:?}" + ); +} + +/// A whitespace trim: `.trim()`, `.trim_start()`, `.trim_end()`, or the +/// function itself handed to a `map` (`str::trim`). `trim_matches` and the +/// other trims that take a pattern are not one: `trim_end_matches('/')` keeps +/// the spaces. +fn trims_whitespace(text: &str) -> bool { + ["trim", "trim_start", "trim_end"].iter().any(|name| { + text.match_indices(name).any(|(at, _)| { + let rest = &text[at + name.len()..]; + match text[..at].chars().next_back() { + Some('.') => rest.starts_with("()"), + Some(':') => !rest + .chars() + .next() + .is_some_and(|c| c.is_alphanumeric() || c == '_'), + _ => false, + } + }) + }) +} + +/// The statements of a source, each with the line it starts on. A chain +/// written over several lines (`.initial_path` / `.as_deref()` / +/// `.map(str::trim)`) is one statement, which a check line by line misses, +/// and so is a call whose arguments run over several lines (`pick(initial,` / +/// `other).trim()`): a comma ends a statement only outside brackets, where it +/// ends a field or an argument of its own. Comments are not part of a +/// statement (a `.trim()` named in one trims nothing), and a literal is read +/// whole: a bracket inside one (`")"`, `')'`) counts for nothing, and `//` +/// inside one (a URL) is not a comment. +fn statements(source: &str) -> Vec<(usize, String)> { + let mut found = Vec::new(); + let mut open: Option<(usize, String)> = None; + let mut depth = 0usize; + let mut scan = Scan::Code; + for (index, line) in source.lines().enumerate() { + let code = code_of(line, &mut scan, &mut depth); + let code = code.trim(); + if code.is_empty() { + continue; + } + let (_, text) = open.get_or_insert_with(|| (index + 1, String::new())); + text.push_str(code); + text.push(' '); + // A line that ends inside a literal or a comment ends no statement. + let ends = code.ends_with([';', '{', '}']) || (code.ends_with(',') && depth == 0); + if scan == Scan::Code && ends { + found.extend(open.take()); + depth = 0; + } + } + found.extend(open); + found +} + +/// Where the end of a line leaves the scan: in code, or inside a string, a +/// raw string (with its `#`s) or a block comment (with its depth) that runs +/// on to the next line. +#[derive(Clone, Copy, PartialEq)] +enum Scan { + Code, + Str, + RawStr(usize), + Comment(usize), +} + +/// The code of one line: its comments taken out, its string and char +/// literals kept whole. Only a bracket outside a literal changes `depth`. +fn code_of(line: &str, scan: &mut Scan, depth: &mut usize) -> String { + let chars: Vec = line.chars().collect(); + let starts_word = + |at: usize| at == 0 || !(chars[at - 1].is_alphanumeric() || chars[at - 1] == '_'); + let mut code = String::new(); + let mut i = 0; + while i < chars.len() { + let (c, next) = (chars[i], chars.get(i + 1).copied()); + match *scan { + Scan::Comment(open) => { + if (c, next) == ('*', Some('/')) { + *scan = if open == 1 { + Scan::Code + } else { + Scan::Comment(open - 1) + }; + i += 2; + } else if (c, next) == ('/', Some('*')) { + *scan = Scan::Comment(open + 1); + i += 2; + } else { + i += 1; + } + continue; + } + Scan::Str => { + code.push(c); + if c == '\\' { + code.extend(next); + i += 2; + continue; + } + if c == '"' { + *scan = Scan::Code; + } + i += 1; + continue; + } + Scan::RawStr(hashes) => { + code.push(c); + i += 1; + let closing = chars[i..].iter().take(hashes).filter(|&&h| h == '#'); + if c == '"' && closing.count() == hashes { + code.extend(&chars[i..i + hashes]); + i += hashes; + *scan = Scan::Code; + } + continue; + } + Scan::Code => {} + } + match (c, next) { + ('/', Some('/')) => break, + ('/', Some('*')) => { + *scan = Scan::Comment(1); + i += 2; + continue; + } + ('"', _) => *scan = Scan::Str, + // `r"..."`, `r#"..."#`, `br"..."`; not a raw identifier (`r#type`). + ('r', _) + if starts_word(i) + || (i > 0 && matches!(chars[i - 1], 'b' | 'c') && starts_word(i - 1)) => + { + let hashes = chars[i + 1..].iter().take_while(|&&h| h == '#').count(); + if chars.get(i + 1 + hashes) == Some(&'"') { + code.extend(&chars[i..i + 2 + hashes]); + i += 2 + hashes; + *scan = Scan::RawStr(hashes); + continue; + } + } + // A char literal (`')'`, `'\''`, `'\u{29}'`); a lifetime (`'a`) is + // not one and is code. + ('\'', _) => { + let end = if next == Some('\\') { + chars + .get(i + 3..) + .and_then(|rest| rest.iter().position(|&q| q == '\'')) + .map(|at| i + 3 + at) + } else if chars.get(i + 2) == Some(&'\'') { + Some(i + 2) + } else { + None + }; + if let Some(end) = end { + code.extend(&chars[i..=end]); + i = end + 1; + continue; + } + } + ('(' | '[', _) => *depth += 1, + (')' | ']', _) => *depth = depth.saturating_sub(1), + _ => {} + } + code.push(c); + i += 1; + } + code +} + +#[test] +fn a_call_over_several_lines_is_one_statement() { + let source = "let a = pick(initial_path,\n other)\n .trim();\n\ + let b = Form {\n x: one(a,\n b),\n y: 2,\n};\n"; + let found: Vec<(usize, String)> = statements(source); + let texts: Vec<&str> = found.iter().map(|(_, text)| text.as_str()).collect(); + assert_eq!( + texts, + [ + "let a = pick(initial_path, other) .trim(); ", + "let b = Form { ", + "x: one(a, b), ", + "y: 2, ", + "}; ", + ] + ); + assert_eq!( + found.iter().map(|(line, _)| *line).collect::>(), + [1, 4, 5, 7, 8] + ); +} + +/// A comment is not code: `.trim()` named in a trailing comment trimmed +/// nothing and failed the check, and a statement whose line ended in a +/// comment ran on into the next one. `//` in a string (a URL) is not a +/// comment, and a bracket in a literal does not end an argument list early. +#[test] +fn comments_are_left_out_and_literals_read_whole() { + let texts = |source: &str| -> Vec { + statements(source) + .into_iter() + .map(|(_, text)| text) + .collect() + }; + assert_eq!( + texts("let base = initial_path.trim_end_matches('/'); // was .trim()\nlet b = 2;\n"), + [ + "let base = initial_path.trim_end_matches('/'); ", + "let b = 2; " + ] + ); + assert_eq!( + texts("let u = format!(\"https://h/{initial}\"); // .trim()\n"), + ["let u = format!(\"https://h/{initial}\"); "] + ); + assert_eq!( + texts("let a = /* .trim() */ initial;\n/* one\n.trim() */\nlet b = 2;\n"), + ["let a = initial; ", "let b = 2; "] + ); + for (open, literal) in [ + ("pick(", "\")\","), + ("pick(", "')',"), + ("pick(", "\"\\\")\","), + ("pick(", "'\\'',"), + ("pick(", "r#\"a \")\" b\"#,"), + ("pick(", "b')',"), + ] { + let source = format!("let a = {open}{literal}\n initial)\n .trim();\n"); + assert_eq!( + texts(&source), + [format!("let a = {open}{literal} initial) .trim(); ")], + "{literal}" + ); + } + // A lifetime is not a char literal; the argument list still closes. + assert_eq!( + texts("fn f<'a>(x: &'a str,\n y: u8) {\nlet z = 1;\n"), + ["fn f<'a>(x: &'a str, y: u8) { ", "let z = 1; "] + ); + // A raw string ends at its first quote: its backslash escapes nothing. + assert_eq!( + texts("let a = pick(r\"C:\\\",\n initial);\nlet b = 2;\n"), + ["let a = pick(r\"C:\\\", initial); ", "let b = 2; "] + ); + // A string over several lines ends no statement inside it. + assert_eq!( + texts("let s = \"a;\nb {\";\nlet t = 1;\n"), + ["let s = \"a; b {\"; ", "let t = 1; "] + ); +} + +/// A path is the name as typed, spaces included, and so is the initial +/// folder of a profile: `/reports ` and `/reports` are two folders. kDrive, +/// Drime, Internxt, Jottacloud and Twake trimmed it on connect, and the +/// session started in the other one; WebDAV did on the root it found under +/// Nextcloud, in a chain over several lines; the CLI did on the profile +/// base, and the TUI on its folder and on the profile it saves. The rclone +/// export, the Zoho folder id, the benchmark's scratch folder and the AI +/// worker's S3 root read it trimmed too. +/// +/// Three statements read a trimmed value on purpose, each named here with the +/// text that finds it: an import reads another application's file by that +/// file's rules, and the AeroCrypt anchor trims on both sides (`norm_anchor` +/// here, `normCryptScope` in the app), so it changes on both sides together. +const TRIMS_ON_PURPOSE: [(&str, &str); 3] = [ + // Dreamweaver's `remoteroot` attribute, read from its site file. + ("dreamweaver_import.rs", "remoteroot"), + // rclone's own `remote = base:subpath`, read from its config: the name + // of the remote, beside the folder it returns. + ( + "rclone_import.rs", + "(base.trim().to_string(), initial_path)", + ), + // The AeroCrypt anchor a binding without a folder takes from the profile. + ( + "crypt_overlay_provider.rs", + ".replace(\"{username}\", &username)", + ), +]; + +#[test] +fn nothing_trims_the_initial_folder() { + let trims = |statement: &str| statement.contains("initial") && trims_whitespace(statement); + for statement in [ + "let initial = initial.trim().to_string();", + r#"if !initial.trim().is_empty() && initial.trim() != "/" {"#, + "let user_initial = self .config .initial_path .as_deref() .map(str::trim) .filter(|p| !p.is_empty());", + "initial_path: form.initial_path.trim_start().to_string(),", + ] { + assert!(trims(statement), "{statement}"); + } + for statement in [ + "let base = initial_path.trim_end_matches('/');", + "let shown = label.trim();", + "let initial = initial.to_string();", + ] { + assert!(!trims(statement), "{statement}"); + } + let on_purpose = |file: &str, statement: &str| { + TRIMS_ON_PURPOSE + .iter() + .any(|(name, text)| file.ends_with(name) && statement.contains(text)) + }; + let mut excused = Vec::new(); + let mut found = Vec::new(); + for (file, source) in backend_sources() { + // A test module of its own: what it trims is a fixture. + if file.ends_with("tests.rs") { + continue; + } + for (line, statement) in statements(production_part(&source)) { + if !trims(&statement) { + continue; + } + if on_purpose(&file, &statement) { + excused.push(file.clone()); + } else { + found.push(format!("{file}:{line}")); + } + } + } + assert!(found.is_empty(), "the initial folder is trimmed: {found:?}"); + // Each exception still finds its statement, and only that one. + excused.sort(); + let mut expected: Vec = TRIMS_ON_PURPOSE + .iter() + .map(|(name, _)| name.to_string()) + .collect(); + expected.sort(); + assert_eq!(excused, expected); +} diff --git a/src-tauri/src/providers/twake.rs b/src-tauri/src/providers/twake.rs index 940f5b60c..df4360f32 100644 --- a/src-tauri/src/providers/twake.rs +++ b/src-tauri/src/providers/twake.rs @@ -816,7 +816,8 @@ impl TwakeProvider { // ─── Paths ─── fn normalize_path(path: &str) -> String { - let replaced = path.trim().replace('\\', "/"); + // Spaces are part of a name: ` a` and `a ` are not `a`. + let replaced = path.replace('\\', "/"); let mut parts: Vec<&str> = Vec::new(); for seg in replaced.split('/') { match seg { @@ -835,14 +836,14 @@ impl TwakeProvider { } fn resolve_path(&self, path: &str) -> String { - let trimmed = path.trim(); - if trimmed.is_empty() || trimmed == "." { + if path.is_empty() || path == "." { return self.current_path.clone(); } - if trimmed.starts_with('/') { - Self::normalize_path(trimmed) + // `normalize_path` takes `\` for a separator: a leading one is the root too. + if path.starts_with(['/', '\\']) { + Self::normalize_path(path) } else { - Self::normalize_path(&format!("{}/{}", self.current_path, trimmed)) + Self::normalize_path(&format!("{}/{}", self.current_path, path)) } } @@ -1048,7 +1049,8 @@ impl StorageProvider for TwakeProvider { self.connected = true; self.current_path = "/".into(); if let Some(initial) = self.config.initial_path.clone() { - if !initial.trim().is_empty() && initial.trim() != "/" { + // As given: a name of spaces is a folder too. + if !initial.is_empty() && initial != "/" { self.cd(&initial).await?; } } @@ -1607,6 +1609,31 @@ impl StorageProvider for TwakeProvider { #[cfg(test)] mod tests { + #[test] + fn a_name_keeps_its_leading_and_trailing_spaces() { + // A name with a leading or trailing space is a name of its own, and a + // name of spaces only is not the current folder. + assert_eq!(super::TwakeProvider::normalize_path("/ a"), "/ a"); + assert_eq!(super::TwakeProvider::normalize_path("/a /b "), "/a /b "); + let mut p = super::TwakeProvider::new(super::TwakeConfig { + credentials: super::TwakeCredentials { + instance: "https://example.test".into(), + client_id: "cid".into(), + client_secret: "s".into(), + registration_access_token: "r".into(), + refresh_token: "t".into(), + }, + initial_path: None, + }); + p.current_path = "/Cloud".to_string(); + assert_eq!(p.resolve_path(" "), "/Cloud/ "); + assert_eq!(p.resolve_path(" a"), "/Cloud/ a"); + assert_eq!(p.resolve_path("/a "), "/a "); + // A leading backslash is the root, as normalize_path reads it. + assert_eq!(p.resolve_path("\\docs\\a "), "/docs/a "); + assert_eq!(p.resolve_path("sub\\a"), "/Cloud/sub/a"); + } + use super::*; #[test] diff --git a/src-tauri/src/providers/webdav.rs b/src-tauri/src/providers/webdav.rs index 7fcb80b10..9b8bff10e 100644 --- a/src-tauri/src/providers/webdav.rs +++ b/src-tauri/src/providers/webdav.rs @@ -3288,17 +3288,31 @@ impl WebDavProvider { // ...) the input has no percent-encoding and decode is a // no-op; on the Filen bridge it un-mangles the name. Issue // #128. - let name = if !displayname.trim().is_empty() { - urlencoding::decode(displayname.trim()) - .unwrap_or_else(|_| displayname.trim().into()) - .into_owned() + // The spaces around a name are part of it: ` a` + // and `a ` are two other files than `a`, and a + // name of spaces is a name. The href names the + // resource, so a displayname that says its name + // with other spaces around it takes the href's: + // the entry's path must reach that resource. A + // displayname that names something else (a + // server whose href is not the name) is the name. + let leaf = decoded_href + .trim_end_matches('/') + .rsplit('/') + .next() + .unwrap_or("") + .to_string(); + let name = if displayname.is_empty() { + leaf } else { - decoded_href - .trim_end_matches('/') - .rsplit('/') - .next() - .unwrap_or("") - .to_string() + let shown = urlencoding::decode(&displayname) + .unwrap_or_else(|_| displayname.as_str().into()) + .into_owned(); + if shown.trim() == leaf.trim() { + leaf + } else { + shown + } }; if name.is_empty() || name == "." || name == ".." { @@ -3390,11 +3404,14 @@ impl WebDavProvider { Ok(Event::CData(ref e)) => { if let Some(ref tag) = current_tag { - let text = e.as_ref().trim().to_string(); - if !text.is_empty() { + // As in the text arm, the name is read as given: + // `` names ` a.txt `. + let raw = e.as_ref().to_string(); + let text = raw.trim().to_string(); + if !text.is_empty() || (tag == "displayname" && !raw.is_empty()) { match tag.as_str() { "href" => href = text, - "displayname" => displayname = text, + "displayname" => displayname.push_str(&raw), "getcontentlength" => getcontentlength = text, "getlastmodified" => getlastmodified = text, "getcontenttype" => getcontenttype = text, @@ -4141,12 +4158,12 @@ impl StorageProvider for WebDavProvider { self.connected = true; // Issue #175: server_root is the auto-detected path. // current_path defaults to it; if the user supplied - // a relative initial_path, append it under the root. + // a relative initial_path, append it under the root, + // as given: `reports ` is its own folder. let user_initial = self .config .initial_path .as_deref() - .map(str::trim) .filter(|p| !p.is_empty() && *p != "/"); let starting_path = match user_initial { Some(rel) => { @@ -6558,6 +6575,33 @@ mod tests { assert!(!local.exists(), "a stalled download leaves no file behind"); } + /// Nextcloud and ownCloud refuse a PROPFIND on `/`; the root found under + /// `/remote.php/dav/files//` takes the initial folder as given. + #[tokio::test] + async fn a_discovered_nextcloud_root_keeps_the_spaces_of_the_initial_folder() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let base = format!("http://{}", listener.local_addr().unwrap()); + let app = axum::Router::new().fallback(axum::routing::any( + |req: axum::extract::Request| async move { + if req.method().as_str() == "PROPFIND" + && req.uri().path() == "/remote.php/dav/files/user/" + { + axum::http::StatusCode::MULTI_STATUS + } else { + axum::http::StatusCode::METHOD_NOT_ALLOWED + } + }, + )); + let server = tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + let mut config = test_config(&base); + config.initial_path = Some("reports ".to_string()); + let mut provider = WebDavProvider::new(config).expect("provider"); + let outcome = provider.connect().await; + server.abort(); + outcome.expect("connect"); + assert_eq!(provider.current_path, "/remote.php/dav/files/user/reports "); + } + fn test_config(url: &str) -> WebDavConfig { WebDavConfig { url: url.to_string(), @@ -7142,6 +7186,115 @@ mod tests { assert_eq!(entries[1].name, "a & b.txt"); } + /// A displayname that begins or ends with a space names that file, in a + /// text node or in a CDATA section: it was trimmed, so ` a.txt ` was + /// listed as `a.txt` at `/dav/a.txt`, and every operation on it reached + /// another file or none. The href names the resource: a displayname that + /// says the href's name with other spaces around it (` b.txt ` for + /// `/dav/b.txt`) takes the href's, so the entry's path reaches the file. + /// A displayname that names something else than the href is the name. + #[test] + fn parse_propfind_keeps_the_spaces_around_a_displayname() { + let provider = WebDavProvider::new(test_config("https://example.com/dav")) + .expect("Failed to create WebDavProvider"); + + let xml = r#" + + + /dav/%20a.txt%20 + + + a.txt + + 3 + + + + + /dav/%20%20/ + + + + + + + + + /dav/%20c.txt%20 + + + + + + + + + /dav/%20%20%20/ + + + + + + + + + /dav/b.txt + + + b.txt + + + + + + /dav/d.txt + + + + + + + + + /dav/%20/ + + + + + + + + + /dav/e7 + + + e.txt + + + + + "#; + + let entries = provider.parse_propfind_response(xml, "/dav").unwrap(); + let named: Vec<(&str, &str)> = entries + .iter() + .map(|e| (e.name.as_str(), e.path.as_str())) + .collect(); + assert_eq!( + named, + [ + (" a.txt ", "/dav/ a.txt "), + (" ", "/dav/ "), + (" c.txt ", "/dav/ c.txt "), + (" ", "/dav/ "), + ("b.txt", "/dav/b.txt"), + ("d.txt", "/dav/d.txt"), + (" ", "/dav/ "), + (" e.txt", "/dav/ e.txt"), + ] + ); + } + /// CR-536 regression: a whitespace-ONLY Text fragment between two entity /// refs (`a& &b.txt`) is part of the name, not indentation, and /// must survive in displayname parsing. diff --git a/src-tauri/src/rclone_import.rs b/src-tauri/src/rclone_import.rs index 5fa5c7ae8..4c5a95136 100644 --- a/src-tauri/src/rclone_import.rs +++ b/src-tauri/src/rclone_import.rs @@ -2127,11 +2127,14 @@ fn sanitize_export_server(server: &RcloneExportServer) -> RcloneExportServer { /// it lies outside it, the crypt wraps the clear remote. Written as it was, /// `vault:/enc` exported over the alias `vault-path = vault:/enc` became /// `vault-path:/enc`, which is `vault:/enc/enc`. The overlay scope is already -/// relative to the start folder. +/// relative to the start folder; when that folder has no alias in the file +/// (`folder`: its name ends in a space, or no name was free), the crypt wraps +/// the clear remote at that folder, written as its alias would hold it. fn crypt_export_remote_target( base_name: &str, clear_name: &str, base_root: Option<&str>, + folder: Option<&str>, options: &serde_json::Value, ) -> String { let trim = |p: &str| { @@ -2167,11 +2170,17 @@ fn crypt_export_remote_target( }, }; } - let scope = options - .get("rcloneCryptOverlayScope") - .and_then(|v| v.as_str()) - .unwrap_or(""); - target(base_name, &trim(scope)) + let scope = trim( + options + .get("rcloneCryptOverlayScope") + .and_then(|v| v.as_str()) + .unwrap_or(""), + ); + match folder.map(|folder| folder.trim_end_matches('/')) { + Some(folder) if scope.is_empty() => format!("{clear_name}:{folder}"), + Some(folder) => format!("{clear_name}:{folder}/{scope}"), + None => target(base_name, &scope), + } } /// Cloudinary's account name. The app's form keeps it in `options.bucket`; @@ -2367,6 +2376,7 @@ fn append_crypt_remote_section( base_name: &str, profile_section: &str, base_root: Option<&str>, + folder: Option<&str>, options: Option<&serde_json::Value>, names: &mut RcloneNamespace, ) -> CryptSection { @@ -2383,6 +2393,13 @@ fn append_crypt_remote_section( { return CryptSection::Refused(problem.to_string()); } + let remote = crypt_export_remote_target(base_name, profile_section, base_root, folder, opts); + // rclone reads `remote = nas:/data ` as `nas:/data`, another folder. + if remote.ends_with(char::is_whitespace) { + return CryptSection::Refused(format!( + "it would wrap '{remote}', whose name ends in a space, which rclone's config cannot hold" + )); + } let requested = sanitize_rclone_remote_name( opts.get("rcloneCryptOverlayName") .and_then(|v| v.as_str()) @@ -2424,15 +2441,7 @@ fn append_crypt_remote_section( } output.push_str(&format!("[{}]\n", section)); output.push_str("type = crypt\n"); - output.push_str(&format!( - "remote = {}\n", - ini_value(&crypt_export_remote_target( - base_name, - profile_section, - base_root, - opts - )) - )); + output.push_str(&format!("remote = {}\n", ini_value(&remote))); if let Some(pw) = password { output.push_str(&format!( "password = {}\n", @@ -2743,6 +2752,9 @@ pub fn export_rclone( // The folder of the clear remote that `crypt_base` points at when it // is an alias (a pinned bucket, the start folder). let mut crypt_base_root: Option = None; + // That folder when its alias could not be written: an overlay scope is + // relative to it, so the crypt wraps the clear remote there. + let mut crypt_folder: Option = None; match proto { "ftp" => { @@ -2989,6 +3001,7 @@ pub fn export_rclone( "# bucket alias for '{}' omitted: no free remote name near '{}'\n", remote_name, desired )); + crypt_folder = Some(bucket.to_string()); } } } @@ -3492,7 +3505,6 @@ pub fn export_rclone( if let Some(path) = options .and_then(|o| o.get("initial_path")) .and_then(|v| v.as_str()) - .map(str::trim) .filter(|p| !p.is_empty() && *p != "/") { // sftp addresses an absolute folder as `remote:/abs`; ftp and @@ -3502,38 +3514,53 @@ pub fn export_rclone( } else { path.trim_start_matches('/').to_string() }; - let desired = sanitize_rclone_remote_name(&format!("{remote_name}-path")); - match names.claim_generated(&desired) { - Some(alias_name) if !alias_name.is_empty() && alias_name != remote_name => { - if alias_name != desired { + // rclone reads a config value without the spaces at its ends + // (and a quoted one with its quotes), so `remote = nas:/data ` + // would open `/data`: a folder whose name ends in a space + // cannot be written there, and the alias is left out. + if path.ends_with(char::is_whitespace) { + notes.push_str(&format!( + "# start-folder alias for '{}' omitted: rclone's config cannot hold '{}', whose name ends in a space\n", + server.name.replace(['\n', '\r'], " "), + path.replace(['\n', '\r'], " ") + )); + crypt_folder = Some(alias_path); + } else { + let desired = sanitize_rclone_remote_name(&format!("{remote_name}-path")); + match names.claim_generated(&desired) { + Some(alias_name) if !alias_name.is_empty() && alias_name != remote_name => { + if alias_name != desired { + trailing.push_str(&format!( + "\n# renamed from '{}': that name is taken by another remote\n", + desired + )); + } trailing.push_str(&format!( - "\n# renamed from '{}': that name is taken by another remote\n", - desired - )); - } - trailing.push_str(&format!( "\n# '{}' opens in '{}' in AeroFTP; address that folder as `{}:`.\n", server.name.replace(['\n', '\r'], " "), alias_path, alias_name )); - trailing.push_str(&format!("[{}]\n", alias_name)); - trailing.push_str("type = alias\n"); - trailing.push_str(&format!("remote = {}:{}\n", remote_name, alias_path)); - // A crypt overlay scoped by path must wrap the alias, - // so `rcloneCryptOverlayScope = /vault` resolves under - // the start folder, not under the account root. An - // imported `rcloneCryptRemote` is taken relative to - // this folder (`crypt_export_remote_target`). - crypt_base = alias_name; - crypt_base_root = Some(alias_path.to_string()); - } - _ => { - notes.push_str(&format!( + trailing.push_str(&format!("[{}]\n", alias_name)); + trailing.push_str("type = alias\n"); + trailing + .push_str(&format!("remote = {}:{}\n", remote_name, alias_path)); + // A crypt overlay scoped by path must wrap the alias, + // so `rcloneCryptOverlayScope = /vault` resolves under + // the start folder, not under the account root. An + // imported `rcloneCryptRemote` is taken relative to + // this folder (`crypt_export_remote_target`). + crypt_base = alias_name; + crypt_base_root = Some(alias_path.to_string()); + } + _ => { + notes.push_str(&format!( "# start-folder alias for '{}' omitted: no free remote name near '{}'\n", server.name.replace(['\n', '\r'], " "), desired )); + crypt_folder = Some(alias_path); + } } } } @@ -3551,6 +3578,7 @@ pub fn export_rclone( &crypt_base, &remote_name, crypt_base_root.as_deref(), + crypt_folder.as_deref(), options, &mut names, ); @@ -4751,6 +4779,43 @@ user = t assert!(alias_at > sftp_at); } + /// rclone reads `remote = nas:/mnt/data ` as `nas:/mnt/data` (measured with + /// rclone 1.75.2; a quoted value keeps its quotes): the alias would open + /// another folder, so it is left out and the config says why. + #[test] + fn test_export_rclone_leaves_out_a_start_folder_that_ends_in_a_space() { + let servers = vec![RcloneExportServer { + id: "nas".to_string(), + name: "nas".to_string(), + host: "nas.example.test".to_string(), + port: 22, + username: "nasuser".to_string(), + protocol: Some("sftp".to_string()), + options: Some(serde_json::json!({ "initial_path": "/mnt/data " })), + provider_id: None, + }]; + let mut passwords = HashMap::new(); + passwords.insert("nas".to_string(), "secret".to_string()); + let tmp = std::env::temp_dir().join("aeroftp-test-export-sftp-path-trailing-space.conf"); + export_rclone(&servers, &passwords, &tmp).expect("should export"); + let conf = std::fs::read_to_string(&tmp).expect("read conf"); + std::fs::remove_file(&tmp).ok(); + + assert!( + conf.contains("[nas]"), + "the remote itself is written:\n{conf}" + ); + assert!( + !conf.contains("[nas-path]"), + "no alias to another folder:\n{conf}" + ); + assert!(!conf.contains("remote = nas:/mnt/data"), "{conf}"); + assert!( + conf.contains("# start-folder alias for 'nas' omitted: rclone's config cannot hold '/mnt/data ', whose name ends in a space"), + "the config says why:\n{conf}" + ); + } + #[test] fn test_export_rclone_scoped_crypt_wraps_the_start_folder_alias() { let servers = vec![RcloneExportServer { @@ -5277,6 +5342,84 @@ user = t } } + /// The `remote =` of every crypt section, in order. + fn crypt_remotes(conf: &str) -> Vec<&str> { + let lines: Vec<&str> = conf.lines().collect(); + lines + .windows(2) + .filter(|pair| pair[0] == "type = crypt") + .filter_map(|pair| pair[1].strip_prefix("remote = ")) + .collect() + } + + /// A crypt overlay's scope is relative to the folder its alias points at + /// (the start folder, a pinned bucket). With that alias left out, the + /// crypt wrapped the clear remote's root: `/vault` of the account, outside + /// the folder. It wraps the clear remote at that folder, and is refused + /// when rclone's config cannot hold the folder. + #[test] + fn test_export_rclone_crypt_wraps_the_folder_whose_alias_is_left_out() { + let crypt = |initial: Option<&str>, scope: &str| { + let mut options = serde_json::json!({ + "rcloneCryptEnabled": true, + "rcloneCryptPassword": "pw", + "rcloneCryptOverlayScope": scope, + }); + if let Some(initial) = initial { + options["initial_path"] = initial.into(); + } + options + }; + // A start folder that ends in a space: no alias can hold it. + let servers = vec![ + export_server("nas", "sftp", Some(crypt(Some("/mnt/data "), "/vault"))), + export_server("pub", "ftp", Some(crypt(Some("/pub "), "vault"))), + export_server("bare", "sftp", Some(crypt(Some("/mnt/data "), ""))), + ]; + let (outcome, conf) = export_to_string(&servers, "crypt-unaliased-space"); + assert_eq!( + crypt_remotes(&conf), + ["nas:/mnt/data /vault", "pub:pub /vault"], + "{conf}" + ); + // The whole folder: `remote = bare:/mnt/data ` would open `/mnt/data`. + let refused: Vec<(&str, &str)> = outcome + .skipped + .iter() + .map(|skip| (skip.name.as_str(), skip.reason.as_str())) + .collect(); + assert_eq!( + refused, + [( + "bare", + "it would wrap 'bare:/mnt/data ', whose name ends in a space, which rclone's config cannot hold" + )] + ); + let (notes, _) = notes_and_body(&conf, "bare"); + assert!( + notes + .join("\n") + .contains("# crypt overlay for 'bare' not written"), + "{conf}" + ); + + // No free name for the alias: the folder is written as it is. + let mut servers = vec![ + export_server("box", "sftp", Some(crypt(Some("/data"), "/vault"))), + export_server("store", "s3", Some(crypt(None, "/vault"))), + ]; + servers[1].options.as_mut().unwrap()["bucket"] = "b".into(); + for base in ["box-path", "store-b"] { + servers.push(export_server(base, "ftp", None)); + for suffix in 2..=MAX_NAME_SUFFIX { + servers.push(export_server(&format!("{base}-{suffix}"), "ftp", None)); + } + } + let (outcome, conf) = export_to_string(&servers, "crypt-unaliased-taken"); + assert_eq!(crypt_remotes(&conf), ["box:/data/vault", "store:b/vault"]); + assert!(outcome.skipped.is_empty(), "{:?}", outcome.skipped); + } + #[test] fn test_import_rclone_internxt_reveals_real_rclone_obscured() { // `pass` below was produced by the real rclone binary (`rclone obscure @@ -7401,7 +7544,7 @@ token = {\"access_token\":\"acc\",\"token_type\":\"Zoho-oauthtoken\",\"refresh_t fn crypt_export_remote_target_is_relative_to_the_alias_folder() { let imported = |target: &str| serde_json::json!({ "rcloneCryptRemote": target }); let at = |target: &str| { - crypt_export_remote_target("nas-path", "nas", Some("/team"), &imported(target)) + crypt_export_remote_target("nas-path", "nas", Some("/team"), None, &imported(target)) }; assert_eq!(at("x:/team"), "nas-path:"); assert_eq!(at("x:team/vault"), "nas-path:/vault"); @@ -7412,7 +7555,7 @@ token = {\"access_token\":\"acc\",\"token_type\":\"Zoho-oauthtoken\",\"refresh_t "a prefix is not a folder" ); assert_eq!( - crypt_export_remote_target("nas", "nas", None, &imported("x:/enc")), + crypt_export_remote_target("nas", "nas", None, None, &imported("x:/enc")), "nas:/enc" ); assert_eq!( @@ -7420,11 +7563,23 @@ token = {\"access_token\":\"acc\",\"token_type\":\"Zoho-oauthtoken\",\"refresh_t "nas-path", "nas", Some("/team"), + None, &serde_json::json!({ "rcloneCryptOverlayScope": "/vault" }) ), "nas-path:/vault", "the overlay scope is already relative to the start folder" ); + // No alias points at the folder: the crypt wraps the clear remote + // there, as the alias would have held it. An imported path is from + // the clear remote's root and does not need it. + let scoped = |scope: &str| serde_json::json!({ "rcloneCryptOverlayScope": scope }); + let unaliased = |folder: &str, options: &serde_json::Value| { + crypt_export_remote_target("nas", "nas", None, Some(folder), options) + }; + assert_eq!(unaliased("/data ", &scoped("/vault")), "nas:/data /vault"); + assert_eq!(unaliased("data/", &scoped("vault/")), "nas:data/vault"); + assert_eq!(unaliased("/data ", &scoped("")), "nas:/data "); + assert_eq!(unaliased("/data ", &imported("x:/enc")), "nas:/enc"); } /// A crypt over an alias imports onto what the alias points at; a chain diff --git a/src-tauri/src/util/mod.rs b/src-tauri/src/util/mod.rs index 63c9ad262..288b74053 100644 --- a/src-tauri/src/util/mod.rs +++ b/src-tauri/src/util/mod.rs @@ -16,6 +16,7 @@ pub mod provider_guard; pub mod scratch; pub mod shutdown; pub mod size; +pub mod start_folder; pub mod temp_name; pub use abort_on_drop::AbortOnDrop; diff --git a/src-tauri/src/util/start_folder.rs b/src-tauri/src/util/start_folder.rs new file mode 100644 index 000000000..54bb9c15f --- /dev/null +++ b/src-tauri/src/util/start_folder.rs @@ -0,0 +1,93 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// Copyright (c) 2024-2026 axpnet: AI-assisted (see AI-TRANSPARENCY.md) + +//! The folder a session stands in when its profile's initial folder is +//! relative. +//! +//! Connect enters a relative initial folder (`docs`) from where the session +//! starts: the root of a cloud drive, the login home on FTP and SFTP. Those +//! providers resolve a relative path against the folder they stand in, so a +//! path built on the relative folder applied it twice: `a.txt` reached +//! `docs/docs/a.txt`, and the benchmark's scratch folder `docs/docs/ +//! aeroftp-bench`. A provider that keeps the folder as a prefix of its own +//! (S3's `bucket/backups/`) stays at its root, and the folder stays as given. + +use crate::providers::StorageProvider; + +/// Whether a session in `cwd` stands in the relative folder `base`: `cwd` is +/// `/docs`, or ends with `/docs` (the login home's `docs`). Compared folder +/// by folder, so `./docs` and `docs//sub` match the `/docs` and `/docs/sub` a +/// provider reports, and a `..` is taken with the folder before it: +/// `docs/../team` is `team`. A `..` with no folder before it goes above the +/// start, which cannot be told from here, and the base is kept. +pub fn stands_in(cwd: &str, base: &str) -> bool { + let (cwd, base) = (folders(cwd), folders(base)); + !base.is_empty() && !base.contains(&"..") && cwd.ends_with(&base) +} + +/// The folders of `path`, without `.` and with each `folder/..` taken out. +fn folders(path: &str) -> Vec<&str> { + let mut folders: Vec<&str> = Vec::new(); + for folder in path.split('/') { + match folder { + "" | "." => {} + ".." if folders.last().is_some_and(|last| *last != "..") => { + folders.pop(); + } + _ => folders.push(folder), + } + } + folders +} + +/// The initial folder `path` as the connected session has entered it: the +/// folder the session stands in when `path` is relative and it stands there, +/// `path` as given otherwise. +pub async fn as_entered(path: &str, provider: &mut dyn StorageProvider) -> String { + if !path.is_empty() && !path.starts_with('/') { + if let Ok(cwd) = provider.pwd().await { + if stands_in(&cwd, path) { + return cwd; + } + } + } + path.to_string() +} + +#[cfg(test)] +mod tests { + use super::stands_in; + + /// The relative base is compared with the folder the session reports one + /// folder at a time: `./docs` stood in `/docs` and was not seen, so it + /// stayed relative and was applied again. + #[test] + fn stands_in_compares_folders_not_spellings() { + for (cwd, base) in [ + ("/docs", "docs"), + ("/home/u/docs/", "docs"), + ("/docs", "./docs"), + ("/./docs", "docs"), + ("/home/u/docs/sub", "docs//sub/"), + ("/docs/sub", "./docs/./sub"), + ("/home/u/docs ", "docs "), + ("/home/team", "docs/../team"), + ("/home/team/sub", "./a/b/../../team/sub/"), + ("/docs", "docs/sub/.."), + ] { + assert!(stands_in(cwd, base), "{cwd:?} {base:?}"); + } + for (cwd, base) in [ + ("/mydocs", "docs"), + ("/docs", "docs "), + ("/docs", "../docs"), + ("/docs", "a/../../docs"), + ("/home/team", "docs/.. /team"), + ("/docs", "docs/.."), + ("/docs", "."), + ("/", ""), + ] { + assert!(!stands_in(cwd, base), "{cwd:?} {base:?}"); + } + } +} diff --git a/src/App.tsx b/src/App.tsx index 77b5f5ffd..78c80074f 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -225,7 +225,7 @@ import { usedScanBoundKey } from './utils/usedScanBound'; import type { UpdateVerificationInfo } from './utils/updateVerification'; import { UpdateVerificationPanel } from './components/UpdateVerificationPanel'; import { safePickerStartDir } from './utils/safePickerDir'; -import { normCryptScope, connectLandsOutsideOverlay, isWithinCryptScope, resolveBoundCryptScope, resolveUsernameTemplate, shouldAutoUnlockOverlayOnConnect } from './utils/cryptScope'; +import { normCryptScope, normLandingPath, connectLandsOutsideOverlay, isWithinCryptScope, resolveBoundCryptScope, resolveUsernameTemplate, shouldAutoUnlockOverlayOnConnect } from './utils/cryptScope'; import { migrateFilenApiKeysToVault } from './utils/filenApiKeyMigration'; import { CustomTitlebar } from './components/CustomTitlebar'; import { ExportImportDialog } from './components/ExportImportDialog'; @@ -3649,11 +3649,11 @@ const App: React.FC = () => { if (prof) { quotaProfile = prof; profileId = prof.id; - if (prof.initialPath?.trim()) { - const resolved = resolveUsernameTemplate(prof.initialPath.trim(), cp.username); + if (prof.initialPath) { + const resolved = resolveUsernameTemplate(prof.initialPath, cp.username); scanRoot = usedScanRoot({ ...scanRootArgs, - profileRoot: stripLegacyNextcloudWebdavRoot(resolved, prof.providerId, cp.username) || resolved || prof.initialPath.trim(), + profileRoot: stripLegacyNextcloudWebdavRoot(resolved, prof.providerId, cp.username) || resolved || prof.initialPath, }); } const m = prof.options?.manualTotalBytes; @@ -4888,7 +4888,7 @@ const App: React.FC = () => { // List storage roots (or profile default remote path) through provider fabric. let files: RemoteFile[] = []; let remotePath = '/'; - const wantedRemote = profile?.initialPath?.trim(); + const wantedRemote = profile?.initialPath; try { if (wantedRemote && wantedRemote !== '/' && wantedRemote !== '.') { try { @@ -4944,7 +4944,7 @@ const App: React.FC = () => { // because this callback sits above loadLocalFiles in the App body. let resolvedLocalPath = currentLocalPath; let resolvedLocalFiles = localFiles; - const wantedLocal = profile?.localInitialPath?.trim(); + const wantedLocal = profile?.localInitialPath; if (wantedLocal) { try { const listed: LocalFile[] = await connectScope.step(() => invoke('get_local_files', { @@ -5970,13 +5970,16 @@ const App: React.FC = () => { // leaves it at "/"). Callers only pass a landing path for non-empty scopes; // empty scope (whole-remote overlay) never reaches here and lands as today. if (opts && opts.initialLandingPath !== undefined) { - const landing = opts.initialLandingPath; - if (normCryptScope(landing) === '') { + // The Remote Path as given: not through the anchor's normCryptScope, + // whose trim would land `/docs ` on `/docs`. + const landing = normLandingPath(opts.initialLandingPath); + if (landing === '') { // Remote Path is the on-wire root. await invoke('provider_change_dir', { path: '/', ...(ownership ? { expectedGeneration: ownership.generation } : {}) }).catch((error) => { if (ownership && String(error).startsWith('CONNECTION_CHANGED')) throw error; }); assertOverlayActivationCurrent(ownership); } else { - await positionWrappedProviderAtScope(landing, ownership); + await invoke('provider_change_dir', { path: landing, ...(ownership ? { expectedGeneration: ownership.generation } : {}) }); + assertOverlayActivationCurrent(ownership); } return; } @@ -6393,7 +6396,7 @@ const App: React.FC = () => { const activeSess = sessions.find((s) => s.id === activeSessionId); const reloadScope = activeSess?.cryptOverlay?.remoteScope || null; const scopeNonEmpty = normCryptScope(reloadScope) !== ''; - const remotePath = activeSess?.serverInitialPath?.trim() || ''; + const remotePath = activeSess?.serverInitialPath || ''; const initialLandingOpts = scopeNonEmpty ? { initialLandingPath: remotePath } : undefined; @@ -10021,7 +10024,7 @@ const App: React.FC = () => { // "Copy to other panel" header chip, and drag-and-drop. setActivePanel('local'); setActiveLocalPanelId(panelId); - const selectedEndpoint = createRemoteEndpoint(profile, profile.initialPath?.trim() || '/'); + const selectedEndpoint = createRemoteEndpoint(profile, profile.initialPath || '/'); // Z.3.5: when the active connection is a saved profile and the user // already has files selected on the remote panel, route the request diff --git a/src/components/CrossProfile/CrossProfilePanel.tsx b/src/components/CrossProfile/CrossProfilePanel.tsx index cb3a1b934..eb65a2f3a 100644 --- a/src/components/CrossProfile/CrossProfilePanel.tsx +++ b/src/components/CrossProfile/CrossProfilePanel.tsx @@ -19,7 +19,7 @@ import { useGuardedClose } from '../../hooks/useGuardedClose'; import { GuardedCloseConfirm } from '../GuardedCloseConfirm'; const getDefaultRemotePath = (profile: ServerProfile | null): string => - profile?.initialPath?.trim() ? profile.initialPath.trim() : '/'; + profile?.initialPath || '/'; interface CrossProfilePlanEntry { source_path: string; @@ -138,7 +138,7 @@ export const CrossProfilePanel: React.FC = ({ onClose, i if (match) { seededSourceRef.current = true; setSourceProfile(match); - setSourcePath(initialSourcePath?.trim() || getDefaultRemotePath(match)); + setSourcePath(initialSourcePath || getDefaultRemotePath(match)); } } if (initialDestProfileId && initialDestProfileId !== initialSourceProfileId) { @@ -146,7 +146,7 @@ export const CrossProfilePanel: React.FC = ({ onClose, i if (match) { seededDestRef.current = true; setDestProfile(match); - setDestPath(initialDestPath?.trim() || getDefaultRemotePath(match)); + setDestPath(initialDestPath || getDefaultRemotePath(match)); } } } diff --git a/src/components/DevTools/AIChat.tsx b/src/components/DevTools/AIChat.tsx index 80a028d6a..44672838b 100644 --- a/src/components/DevTools/AIChat.tsx +++ b/src/components/DevTools/AIChat.tsx @@ -2410,8 +2410,8 @@ export const AIChat: React.FC = ({ className = '', remotePath, loca && !profile.aeroCryptOverlay && !profile.password).map(profile => ({ id: profile.id, name: profile.name, - root: profile.initialPath?.trim() - ? `/${profile.initialPath.trim().replace(/^\/+|\/+$/g, '')}` : '/', + root: profile.initialPath + ? `/${profile.initialPath.replace(/^\/+|\/+$/g, '')}` : '/', })).filter(profile => profile.id && profile.root.split('/').every(part => part !== '.' && part !== '..')); setRemoteWorkerProfiles(eligible); setSelectedRemoteWorkerIds(previous => previous.filter(id => eligible.some(profile => profile.id === id))); diff --git a/src/components/MountManagerDialog.tsx b/src/components/MountManagerDialog.tsx index 712ff6ba1..158eb044f 100644 --- a/src/components/MountManagerDialog.tsx +++ b/src/components/MountManagerDialog.tsx @@ -210,7 +210,7 @@ const MountProfileCarousel: React.FC = ({ title, prof
- {profile.initialPath?.trim() || profile.host || '/'} + {profile.initialPath || profile.host || '/'}
@@ -251,7 +251,7 @@ export function MountManagerDialog({ onClose, initialProfileId, initialRemotePat if (profile) { next.name = profile.name; next.profile = profile.name; - next.remote_path = remotePath?.trim() || profile.initialPath?.trim() || '/'; + next.remote_path = remotePath || profile.initialPath || '/'; try { next.mountpoint = await invoke('mount_suggest_path', { profile: profile.name }); } catch { @@ -408,7 +408,7 @@ export function MountManagerDialog({ onClose, initialProfileId, initialRemotePat const updates: Partial = { profile: profileName, name: profileName, - remote_path: selectedProfile?.initialPath?.trim() || '/', + remote_path: selectedProfile?.initialPath || '/', }; try { diff --git a/src/components/common/ProfileRail.tsx b/src/components/common/ProfileRail.tsx index edb5358ba..64f0de67f 100644 --- a/src/components/common/ProfileRail.tsx +++ b/src/components/common/ProfileRail.tsx @@ -188,9 +188,9 @@ export const ProfileRail: React.FC = ({ {renderBadges?.(profile)} - {profile.initialPath?.trim() && ( + {profile.initialPath && (
- {profile.initialPath.trim()} + {profile.initialPath}
)} diff --git a/src/utils/aeroSyncCliCommand.test.ts b/src/utils/aeroSyncCliCommand.test.ts index 36a24c317..3b2a0025a 100644 --- a/src/utils/aeroSyncCliCommand.test.ts +++ b/src/utils/aeroSyncCliCommand.test.ts @@ -122,10 +122,21 @@ describe('aeroftp-cli sync line for the Plan tab', () => { // No start folder and a bare root: the CLI reads it as the server's // default folder, which may be a home directory. [{ profileInitialPath: '', remotePath: '/' }, 'remote-path'], + // The CLI takes the start folder as given: `/www ` is not `/www`, so + // `/www/r` would be joined under `/www `. + [{ profileInitialPath: '/www ', remotePath: '/www/r' }, 'remote-path'], ])('names the difference instead of a command: %o', (patch, reason) => { expect(buildCliSyncCommand({ ...mirrorUpload, ...patch })).toEqual({ kind: 'none', reason }); }); + it('offers the line for a start folder whose name ends in a space', () => { + const built = buildCliSyncCommand({ ...mirrorUpload, profileInitialPath: '/www ', remotePath: '/www /r' }); + expect(built.kind).toBe('command'); + if (built.kind !== 'command') return; + expect(built.argv.slice(0, 6)).toEqual(['aeroftp-cli', '--profile', 'Srv', 'sync', '/l', '/www /r']); + expect(splitPosix(built.text).slice(0, 6)).toEqual(built.argv.slice(0, 6)); + }); + it.each(["it's", 'it\u2018s', 'it\u2019s', 'it\u201As', 'it\u201Bs', "a\u2019'\u2018b"])( 'keeps a PowerShell argument whole when it contains a quote character: %s', (arg) => { diff --git a/src/utils/aeroSyncCliCommand.ts b/src/utils/aeroSyncCliCommand.ts index 407167946..1b310e8ab 100644 --- a/src/utils/aeroSyncCliCommand.ts +++ b/src/utils/aeroSyncCliCommand.ts @@ -85,7 +85,7 @@ export type CliCommandResult = * different folder from the one the Plan compared. */ function cliReachesSameRemoteFolder(remotePath: string, initialPath: string | null | undefined): boolean { - const base = (initialPath ?? '').trim(); + const base = initialPath ?? ''; if (base === '' || base === '/') return remotePath.replace(/^\/+/, '') !== ''; return remotePath.startsWith(base.replace(/\/+$/, '')); } diff --git a/src/utils/cryptConnectLanding.wiring.test.ts b/src/utils/cryptConnectLanding.wiring.test.ts index 0a5a95766..7dada3553 100644 --- a/src/utils/cryptConnectLanding.wiring.test.ts +++ b/src/utils/cryptConnectLanding.wiring.test.ts @@ -11,7 +11,7 @@ import { transformWithOxc } from 'vite'; import { describe, expect, it, vi } from 'vitest'; import app from '../App.tsx?raw'; -import { connectLandsOutsideOverlay, isWithinCryptScope, normCryptScope, resolveUsernameTemplate } from './cryptScope'; +import { connectLandsOutsideOverlay, isWithinCryptScope, normCryptScope, normLandingPath, resolveUsernameTemplate } from './cryptScope'; import { refusalFor } from './overlayRefusal'; import { assertOverlayActivationCurrent } from './profileOverlayUnlock'; @@ -56,14 +56,13 @@ async function reanchorWith(opts: { initialLandingPath?: string | null } | undef const end = app.indexOf(REANCHOR_END, start); if (start < 0 || end < 0) throw new Error('smartReanchorOverlay initial-landing branch not found in App.tsx'); const invoke = vi.fn().mockResolvedValue(undefined); - const positionWrappedProviderAtScope = vi.fn().mockResolvedValue(undefined); await execute(app.slice(start, end), { opts, invoke, - normCryptScope, - positionWrappedProviderAtScope, + normLandingPath, + assertOverlayActivationCurrent, }, ''); - return { invoke, positionWrappedProviderAtScope }; + return { invoke }; } describe('crypt overlay connect landing (#1211)', () => { @@ -72,9 +71,13 @@ describe('crypt overlay connect landing (#1211)', () => { expect(opts).toEqual({ initialLandingPath: '' }); }); - it('blank Remote Path behaves like an empty one', async () => { + it('the landing is the Remote Path the connect opened, spaces included', async () => { + // The connect enters the Remote Path as given (FTP sends `CWD `), so + // a name of spaces is a folder of that name, not the on-wire root. const opts = await landingFor({ id: 's1', cryptOverlay: { remoteScope: '/Vault' }, serverInitialPath: ' ' }); - expect(opts).toEqual({ initialLandingPath: '' }); + expect(opts).toEqual({ initialLandingPath: ' ' }); + const named = await landingFor({ id: 's1', cryptOverlay: { remoteScope: '/Vault' }, serverInitialPath: '/docs ' }); + expect(named).toEqual({ initialLandingPath: '/docs ' }); }); it('a configured Remote Path remains the landing', async () => { @@ -88,15 +91,24 @@ describe('crypt overlay connect landing (#1211)', () => { }); it('smartReanchorOverlay maps an empty landing to provider_change_dir("/")', async () => { - const { invoke, positionWrappedProviderAtScope } = await reanchorWith({ initialLandingPath: '' }); - expect(invoke).toHaveBeenCalledWith('provider_change_dir', { path: '/' }); - expect(positionWrappedProviderAtScope).not.toHaveBeenCalled(); + for (const initialLandingPath of ['', '/', null]) { + const { invoke } = await reanchorWith({ initialLandingPath }); + expect(invoke.mock.calls).toEqual([['provider_change_dir', { path: '/' }]]); + } }); it('smartReanchorOverlay moves to a non-empty landing path', async () => { - const { invoke, positionWrappedProviderAtScope } = await reanchorWith({ initialLandingPath: '/docs' }); - expect(positionWrappedProviderAtScope).toHaveBeenCalledWith('/docs', undefined); - expect(invoke).not.toHaveBeenCalled(); + const { invoke } = await reanchorWith({ initialLandingPath: '/docs' }); + expect(invoke.mock.calls).toEqual([['provider_change_dir', { path: '/docs' }]]); + }); + + it('smartReanchorOverlay lands on the Remote Path as given, spaces included', async () => { + // The anchor's normCryptScope trims: `/docs ` would land on `/docs`, + // and a name of spaces on the root. + for (const [given, path] of [['/docs ', '/docs '], [' docs/', '/ docs'], [' ', '/ ']]) { + const { invoke } = await reanchorWith({ initialLandingPath: given }); + expect(invoke.mock.calls).toEqual([['provider_change_dir', { path }]]); + } }); }); diff --git a/src/utils/cryptScope.test.ts b/src/utils/cryptScope.test.ts index 8e295b2aa..e12ac66f3 100644 --- a/src/utils/cryptScope.test.ts +++ b/src/utils/cryptScope.test.ts @@ -1,7 +1,7 @@ // Copyright (c) 2024-2026 axpnet: AI-assisted (see AI-TRANSPARENCY.md) // SPDX-License-Identifier: GPL-3.0-or-later import { describe, it, expect } from 'vitest'; -import { normCryptScope, connectLandsOutsideOverlay, isWithinCryptScope, resolveBoundCryptScope, resolveUsernameTemplate, shouldAutoUnlockOverlayOnConnect } from './cryptScope'; +import { normCryptScope, normLandingPath, connectLandsOutsideOverlay, isWithinCryptScope, resolveBoundCryptScope, resolveUsernameTemplate, shouldAutoUnlockOverlayOnConnect } from './cryptScope'; describe('normCryptScope', () => { it('collapses the empty scope and the root onto "whole remote"', () => { @@ -19,6 +19,16 @@ describe('normCryptScope', () => { }); }); +describe('normLandingPath', () => { + it('keeps the spaces of the Remote Path and normalizes only its slashes', () => { + for (const root of [undefined, null, '', '/', '//']) expect(normLandingPath(root)).toBe(''); + expect(normLandingPath('/docs ')).toBe('/docs '); + expect(normLandingPath(' docs/')).toBe('/ docs'); + expect(normLandingPath(' ')).toBe('/ '); + expect(normLandingPath('docs//')).toBe('/docs'); + }); +}); + describe('connectLandsOutsideOverlay', () => { it('opens a root Remote Path outside a subfolder island', () => { expect(connectLandsOutsideOverlay('/', '/Crypt')).toBe(true); @@ -33,6 +43,16 @@ describe('connectLandsOutsideOverlay', () => { expect(connectLandsOutsideOverlay('/Crypt/', '/Crypt')).toBe(false); }); + // The landing is the folder navigation opens (normLandingPath), spaces + // included: read with the anchor's trim, `/Crypt ` was inside `/Crypt` and + // the overlay unlocked on a folder outside it. + it('opens outside the island when the Remote Path is another folder by its spaces', () => { + expect(connectLandsOutsideOverlay('/Crypt ', '/Crypt')).toBe(true); + expect(connectLandsOutsideOverlay(' /Crypt', '/Crypt')).toBe(true); + expect(connectLandsOutsideOverlay('/Crypt /sub', '/Crypt')).toBe(true); + expect(connectLandsOutsideOverlay('/Crypt /sub', '/Crypt ')).toBe(true); + }); + it('treats an empty Overlays Path as the Remote Path itself', () => { expect(connectLandsOutsideOverlay('/', '')).toBe(false); expect(connectLandsOutsideOverlay('/', '/')).toBe(false); diff --git a/src/utils/cryptScope.ts b/src/utils/cryptScope.ts index 15c4db980..9b2df0285 100644 --- a/src/utils/cryptScope.ts +++ b/src/utils/cryptScope.ts @@ -21,6 +21,17 @@ export const normCryptScope = (s?: string | null): string => { return '/' + t.replace(/^\/+|\/+$/g, ''); }; +/** + * The Remote Path a connect lands on, as given: `/docs ` is its own folder. + * Its slashes are normalized as in `normCryptScope`, but not its spaces: the + * anchor's trim would land on another folder. Empty or `/` is the on-wire + * root (''). + */ +export const normLandingPath = (s?: string | null): string => { + const t = (s || '').replace(/^\/+|\/+$/g, ''); + return t ? '/' + t : ''; +}; + export const resolveUsernameTemplate = ( path: string | null | undefined, username: string | null | undefined, @@ -45,7 +56,9 @@ export const connectLandsOutsideOverlay = ( ): boolean => { const island = normCryptScope(overlayPath); if (!island) return false; - const landing = normCryptScope(remotePath); + // The folder the connect opens, read as navigation reads it: `/Crypt ` is + // another folder than an island at `/Crypt`, and opens outside it. + const landing = normLandingPath(remotePath); if (!landing) return true; if (landing === island || landing.startsWith(island + '/')) return false; return true; diff --git a/src/utils/initialFolderAsGiven.test.ts b/src/utils/initialFolderAsGiven.test.ts new file mode 100644 index 000000000..fb4492771 --- /dev/null +++ b/src/utils/initialFolderAsGiven.test.ts @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// Copyright (c) 2024-2026 axpnet: AI-assisted (see AI-TRANSPARENCY.md) + +import { describe, expect, it } from 'vitest'; + +/** + * A profile's initial folder is used as given: `/reports ` is its own folder, + * and the providers, the CLI and the TUI open it by that name. A frontend that + * trims it opens, shows, mounts or compares another folder. + * + * This is a source guard, not a behaviour test: it reads the frontend through + * Vite's raw glob and looks for a trim of a value named after the initial + * folder (`initialPath`, `localInitialPath`, `serverInitialPath`, + * `initialSourcePath`, ...), written on one line or across several. + * + * Outside its reach on purpose: the AeroCrypt overlay anchor + * (`remoteScope?.trim() || initialPath?.trim()`). Its normalization trims on + * both sides, `normCryptScope` here and `norm_anchor` in the backend, and the + * write guard decides inside or outside the vault with it, so it changes on + * both sides together or not at all. + */ +const SOURCES = import.meta.glob(['../**/*.{ts,tsx}', '!../**/*.test.{ts,tsx}'], { + query: '?raw', + import: 'default', + eager: true, +}) as Record; + +const NAME = String.raw`\w*[iI]nitial\w*Path`; +const TRIM = String.raw`trim(?:Start|End)?\(`; +const PATTERNS = [ + // initialPath.trim(), profile?.initialPath?.trim(), initialSourcePath?.trim() + new RegExp(String.raw`\b${NAME}\s*\??\.\s*${TRIM}`, 'g'), + // (initialPath ?? '').trim(), (initialPath || "").trimEnd() + new RegExp(String.raw`\(\s*[\w.?]*\b${NAME}\s*(?:\?\?|\|\|)\s*(?:''|""|\`\`)\s*\)\s*\.\s*${TRIM}`, 'g'), + // initialPath: form.path.trim(), initial_path: value.trim() + new RegExp(String.raw`\b(?:${NAME}|initial_path)\s*:\s*[\w.?]+\s*\??\.\s*${TRIM}`, 'g'), +]; +const ANCHOR = /Scope\s*\??\.\s*trim\(\)\s*\|\|\s*$/; + +function trimsOfTheInitialFolder(source: string): string[] { + // One line per statement is not how this code is written: read it as one. + const text = source.replace(/\/\/[^\n]*/g, ' ').replace(/\s+/g, ' '); + const found: string[] = []; + for (const pattern of PATTERNS) { + for (const match of text.matchAll(pattern)) { + const before = text.slice(Math.max(0, match.index - 80), match.index); + if (ANCHOR.test(before.replace(/[\w.?]*$/, ''))) continue; + found.push(text.slice(Math.max(0, match.index - 40), match.index + match[0].length)); + } + } + return found; +} + +describe('the initial folder is used as given', () => { + it('reads the frontend sources', () => { + const names = Object.keys(SOURCES); + expect(names.some(name => name.endsWith('/App.tsx'))).toBe(true); + expect(names.some(name => name.endsWith('/aeroSyncCliCommand.ts'))).toBe(true); + expect(names.some(name => name.endsWith('.test.ts'))).toBe(false); + }); + + it.each([ + "const wantedRemote = profile?.initialPath?.trim();", + "if (prof.initialPath?.trim()) {", + "{profile.initialPath.trim()}", + "const base = (initialPath ?? '').trim();", + "setSourcePath(initialSourcePath?.trim() || fallback);", + "const wantedLocal = profile?.localInitialPath\n ?.trimEnd();", + "const draft = { initialPath: form.remotePath.trim() };", + "invoke('x', { initial_path: value.trimStart() });", + ])('finds a trim of the initial folder: %s', (sample) => { + expect(trimsOfTheInitialFolder(sample)).toHaveLength(1); + }); + + it.each([ + "const wantedRemote = profile?.initialPath;", + "const base = initialPath.replace(/\\/+$/, '');", + "const label = initialLabel.trim();", + "const name = form.name.trim();", + "const scope = binding.remoteScope?.trim() || profile.initialPath?.trim() || '';", + "const overlayScope = remoteScope?.trim() || initialPath?.trim() || '';", + ])('leaves alone what is not one: %s', (sample) => { + expect(trimsOfTheInitialFolder(sample)).toEqual([]); + }); + + it('nothing in the frontend trims the initial folder', () => { + const found = Object.entries(SOURCES).flatMap(([name, source]) => + trimsOfTheInitialFolder(source).map(snippet => `${name}: ${snippet}`)); + expect(found).toEqual([]); + }); +}); diff --git a/src/utils/overlayRootNavigation.wiring.test.ts b/src/utils/overlayRootNavigation.wiring.test.ts index 220c286da..0dfd5224e 100644 --- a/src/utils/overlayRootNavigation.wiring.test.ts +++ b/src/utils/overlayRootNavigation.wiring.test.ts @@ -4,7 +4,7 @@ import { describe, expect, it, vi } from 'vitest'; import { transformWithOxc } from 'vite'; import app from '../App.tsx?raw'; -import { normCryptScope } from './cryptScope'; +import { normCryptScope, normLandingPath } from './cryptScope'; import { assertOverlayActivationCurrent } from './profileOverlayUnlock'; // Execute the real re-anchor and activation handlers, including their catches @@ -29,7 +29,7 @@ async function handlers(error: unknown, owned: boolean, staleAfterNavigation = f }); const bindSessionCryptOverlay = vi.fn(); const context = { - invoke, normCryptScope, assertOverlayActivationCurrent, + invoke, normCryptScope, normLandingPath, assertOverlayActivationCurrent, positionWrappedProviderAtScope: vi.fn().mockResolvedValue(undefined), applyProviderCryptOverlay: vi.fn().mockResolvedValue(''), cryptOverlaySentinel: () => 'rclone_crypt_provider', diff --git a/src/utils/panelEndpoints.test.ts b/src/utils/panelEndpoints.test.ts index a068c2f34..5319bdaff 100644 --- a/src/utils/panelEndpoints.test.ts +++ b/src/utils/panelEndpoints.test.ts @@ -2,7 +2,7 @@ // Copyright (c) 2024-2026 axpnet -- AI-assisted (see AI-TRANSPARENCY.md) import { describe, expect, it } from 'vitest'; -import { createLocalEndpoint, createRemoteEndpoint, getEndpointCapabilities, getPanelPairKind } from './panelEndpoints'; +import { createLocalEndpoint, createOverlayEndpoint, createRemoteEndpoint, getEndpointCapabilities, getPanelPairKind } from './panelEndpoints'; import type { ServerProfile } from '../types'; const profile = (id: string, protocol: ServerProfile['protocol']): ServerProfile => ({ @@ -37,4 +37,18 @@ describe('panel endpoint helpers', () => { expect(driveCaps.canDeltaSync).toBe(false); expect(driveCaps.canServerSideCopy).toBe(true); }); + + it('keeps a path as given, spaces included: only an empty one takes the fallback', () => { + // `/reports ` is its own folder: a transfer planned on `/reports` + // would reach another one. + const sftp = profile('prod', 'sftp'); + expect(createRemoteEndpoint(sftp, '/reports ').path).toBe('/reports '); + expect(createRemoteEndpoint({ ...sftp, initialPath: '/reports ' }).path).toBe('/reports '); + expect(createRemoteEndpoint({ ...sftp, initialPath: ' ' }, '').path).toBe(' '); + expect(createRemoteEndpoint({ ...sftp, initialPath: '' }).path).toBe('/'); + expect(createLocalEndpoint('local', ' /home/u/a ').path).toBe(' /home/u/a '); + expect(createLocalEndpoint('local', '').path).toBe(''); + const overlay = createOverlayEndpoint({ sessionId: 's1', vaultPath: '/v.aerovault', source: 'local', path: ' a' }); + expect(overlay.path).toBe(' a'); + }); }); diff --git a/src/utils/panelEndpoints.ts b/src/utils/panelEndpoints.ts index 2fabd302d..fce0527f3 100644 --- a/src/utils/panelEndpoints.ts +++ b/src/utils/panelEndpoints.ts @@ -53,11 +53,12 @@ const SERVER_SIDE_COPY_PROTOCOLS = new Set([ 'koofr', ]); -export const normalizePanelPath = (path: string | undefined | null, fallback = '/'): string => { - const trimmed = (path ?? '').trim(); - if (!trimmed) return fallback; - return trimmed; -}; +/** + * A panel's path as given: `/reports ` is its own folder, and a transfer + * planned on `/reports` would reach another one. Only a missing or empty path + * takes the fallback. + */ +export const normalizePanelPath = (path: string | undefined | null, fallback = '/'): string => path || fallback; export const createLocalEndpoint = ( panelId: AeroFileLocalPanelId, diff --git a/src/utils/profileOverlayOwnership.wiring.test.ts b/src/utils/profileOverlayOwnership.wiring.test.ts index 89a822ad2..00ce85f47 100644 --- a/src/utils/profileOverlayOwnership.wiring.test.ts +++ b/src/utils/profileOverlayOwnership.wiring.test.ts @@ -5,7 +5,7 @@ import { describe, expect, it } from 'vitest'; import { transformWithOxc } from 'vite'; import app from '../App.tsx?raw'; import { assertOverlayActivationCurrent, profileOverlayApplyParams } from './profileOverlayUnlock'; -import { normCryptScope, resolveUsernameTemplate } from './cryptScope'; +import { normCryptScope, normLandingPath, resolveUsernameTemplate } from './cryptScope'; function block(start: string, end: string): string { const a = app.indexOf(start); @@ -41,7 +41,7 @@ async function activation(hold?: string, setup?: 'generation-error' | 'missing-u } const context = { crypto: setup === 'missing-uuid' ? {} : setup === 'uuid-error' ? { randomUUID: () => { throw new Error('synthetic UUID failure'); } } : crypto, - assertOverlayActivationCurrent, normCryptScope, resolveUsernameTemplate, profileOverlayApplyParams, + assertOverlayActivationCurrent, normCryptScope, normLandingPath, resolveUsernameTemplate, profileOverlayApplyParams, loadSavedServerProfiles: () => stage('profiles', [{ id: 'old_profile', initialPath: '/', username: 'test', aeroCryptOverlay: { enabled: true, kind, remoteScope: '/Crypt' },