From 512df6386fa484ed4767de33f59ff4efd49d3f1b Mon Sep 17 00:00:00 2001 From: axpnet <45786925+axpnet@users.noreply.github.com> Date: Sat, 31 Oct 2026 07:53:54 +0100 Subject: [PATCH 1/8] fix(providers): a name with a leading or trailing space is its own name (Requested 128) Ten providers trimmed the path they were given before resolving it: 4shared, kDrive, Koofr, Drime, FileLu, Internxt, Jottacloud and Twake in their path normalization, ImageKit and Cloudinary in `resolve_path`. ` a` and `a ` reached `a`, and a name of spaces only reached the current folder, for every command: listing, download, upload and delete included. The other providers already keep the spaces; these now do too. - Koofr's `resolve_path` checked the leading slash on the normalized path, which always has one, so a relative path never joined the current folder. It now checks the input, as kDrive and Drime already did. - The CLI refused a remote or local name of spaces only as a missing argument in `get`, `put`, `rcat` and `versions list/get/restore/purge`. An empty argument is still refused. - Drime and Jottacloud had tests asserting `normalize_path(" ") == "/"`, the defect itself; they now assert `"/ "`. Session: L2 016retSy Signed-off-by: axpnet <45786925+axpnet@users.noreply.github.com> --- src-tauri/src/bin/aeroftp_cli.rs | 48 +++++++++++++++++++++----- src-tauri/src/providers/cloudinary.rs | 22 +++++++++++- src-tauri/src/providers/drime_cloud.rs | 35 +++++++++++++------ src-tauri/src/providers/filelu.rs | 18 +++++++++- src-tauri/src/providers/fourshared.rs | 28 ++++++++++----- src-tauri/src/providers/imagekit.rs | 20 ++++++++++- src-tauri/src/providers/internxt.rs | 19 ++++++++-- src-tauri/src/providers/jottacloud.rs | 27 +++++++++++---- src-tauri/src/providers/kdrive.rs | 33 ++++++++++++------ src-tauri/src/providers/koofr.rs | 46 +++++++++++++++++------- src-tauri/src/providers/twake.rs | 34 ++++++++++++++---- 11 files changed, 261 insertions(+), 69 deletions(-) diff --git a/src-tauri/src/bin/aeroftp_cli.rs b/src-tauri/src/bin/aeroftp_cli.rs index 9b3181ed9..a37edb6a9 100644 --- a/src-tauri/src/bin/aeroftp_cli.rs +++ b/src-tauri/src/bin/aeroftp_cli.rs @@ -35538,7 +35538,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 +37688,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 +40145,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 +40185,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 +40197,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 +40269,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 +40325,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; } @@ -40703,7 +40703,7 @@ fn rcat_staging_file() -> Result { } 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; } @@ -89619,6 +89619,38 @@ mod tests { assert_eq!(code, 130, "-1 means the put was still running"); } + /// A remote name of spaces only is a name: `versions list` and `get` + /// refused it as a missing argument (exit 5) before connecting. + #[test] + fn a_remote_name_of_spaces_is_not_a_missing_path() { + let code = against_stalling(StallingProvider::default(), |_| async { + cmd_versions_list("memory://", " ", &quiet_cli(), OutputFormat::Json).await + }); + assert_eq!(code, 7, "it reached the provider, which keeps no versions"); + + let dir = tempfile::tempdir().unwrap(); + let local = dir.path().join("f.bin").to_string_lossy().into_owned(); + let cli = Cli { + quiet: true, + ..test_cli() + }; + let code = interrupted_after_a_moment(|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"); + } + /// `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. diff --git a/src-tauri/src/providers/cloudinary.rs b/src-tauri/src/providers/cloudinary.rs index 6171ac88d..da5c9be2e 100644 --- a/src-tauri/src/providers/cloudinary.rs +++ b/src-tauri/src/providers/cloudinary.rs @@ -342,7 +342,8 @@ impl CloudinaryProvider { } 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('/') { @@ -2133,6 +2134,25 @@ 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. 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 diff --git a/src-tauri/src/providers/drime_cloud.rs b/src-tauri/src/providers/drime_cloud.rs index f5f81a9ec..d4bd38f2f 100644 --- a/src-tauri/src/providers/drime_cloud.rs +++ b/src-tauri/src/providers/drime_cloud.rs @@ -393,31 +393,31 @@ 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); + 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) { @@ -2963,6 +2963,19 @@ 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 "); + } + fn test_provider() -> DrimeCloudProvider { let config = DrimeCloudConfig { api_token: secrecy::SecretString::from("test-token".to_string()), @@ -3723,7 +3736,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..fe4b896d8 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(); } @@ -2569,6 +2570,21 @@ 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 "); + } struct CapFixture { provider: FileLuProvider, diff --git a/src-tauri/src/providers/fourshared.rs b/src-tauri/src/providers/fourshared.rs index a1194d6c8..c8d94c4ce 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 @@ -1986,6 +1985,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/imagekit.rs b/src-tauri/src/providers/imagekit.rs index fa4b9fb6b..808a53f5a 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('/') { @@ -1712,6 +1713,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; diff --git a/src-tauri/src/providers/internxt.rs b/src-tauri/src/providers/internxt.rs index 1778049c8..e94cbc88c 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, ".." => { @@ -3830,6 +3830,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..e945961b0 100644 --- a/src-tauri/src/providers/jottacloud.rs +++ b/src-tauri/src/providers/jottacloud.rs @@ -881,20 +881,20 @@ 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(); } @@ -3743,6 +3743,19 @@ 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 "); + } + fn test_provider() -> JottacloudProvider { let config = JottacloudConfig { login_token: SecretString::from("test-token".to_string()), @@ -3769,7 +3782,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"); diff --git a/src-tauri/src/providers/kdrive.rs b/src-tauri/src/providers/kdrive.rs index 4831d97a1..755074005 100644 --- a/src-tauri/src/providers/kdrive.rs +++ b/src-tauri/src/providers/kdrive.rs @@ -555,32 +555,32 @@ 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); + 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) { @@ -2579,6 +2579,19 @@ 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 "); + } + 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..a8f9c0b6f 100644 --- a/src-tauri/src/providers/koofr.rs +++ b/src-tauri/src/providers/koofr.rs @@ -524,30 +524,31 @@ 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. + 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") @@ -2078,6 +2079,25 @@ 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 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"}}"#; diff --git a/src-tauri/src/providers/twake.rs b/src-tauri/src/providers/twake.rs index 940f5b60c..04a6a0b52 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,13 @@ 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) + 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)) } } @@ -1607,6 +1607,28 @@ 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 "); + } + use super::*; #[test] From fe4ce1666a9300fc4b911c5d6ad9e755442f4277 Mon Sep 17 00:00:00 2001 From: axpnet <45786925+axpnet@users.noreply.github.com> Date: Sat, 31 Oct 2026 08:46:32 +0100 Subject: [PATCH 2/8] test(cli): every argument check takes a name of spaces, and the provider gets it as given The CLI test covered `versions list` and `get` by exit code only: a trim after the argument check would have kept both codes and lost the name. It now covers all eight checks the fix changed, and asserts the name the provider receives: - `versions list`, `get`, `restore` and `purge`, and `rcat`, of a remote ` `: the provider is asked for ` `; - `get` of a remote ` `: the provider is asked for ` `, the download starts and is stopped; - `versions get` to a local ` `: the version is downloaded to ` `; - `put` of a local ` `: it is looked up (exit 2, none there) instead of refused as missing (exit 5). The test provider can keep versions and complete an upload, and records the paths it is asked for. `rcat` reads its input through a test seam: a test reading stdin would wait on the terminal it runs in. Session: L2 016retSy Signed-off-by: axpnet <45786925+axpnet@users.noreply.github.com> --- src-tauri/src/bin/aeroftp_cli.rs | 187 +++++++++++++++++++++++++++++-- 1 file changed, 177 insertions(+), 10 deletions(-) diff --git a/src-tauri/src/bin/aeroftp_cli.rs b/src-tauri/src/bin/aeroftp_cli.rs index a37edb6a9..357058297 100644 --- a/src-tauri/src/bin/aeroftp_cli.rs +++ b/src-tauri/src/bin/aeroftp_cli.rs @@ -40702,6 +40702,22 @@ 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.is_empty() { print_error(format, "Missing remote path for rcat", 5); @@ -40728,7 +40744,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); @@ -89223,6 +89239,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 +89388,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 +89450,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,22 +89682,71 @@ mod tests { assert_eq!(code, 130, "-1 means the put was still running"); } - /// A remote name of spaces only is a name: `versions list` and `get` - /// refused it as a missing argument (exit 5) before connecting. + /// 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() { - let code = against_stalling(StallingProvider::default(), |_| async { - cmd_versions_list("memory://", " ", &quiet_cli(), OutputFormat::Json).await - }); - assert_eq!(code, 7, "it reached the provider, which keeps no versions"); - + // 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 = interrupted_after_a_moment(|cancelled| async move { + let (code, asked) = asked_of(|cancelled| async move { cmd_get( "memory://", " ", @@ -89649,6 +89761,61 @@ mod tests { .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 From 6e9efdbf29823cd484b5964a3e82e3b321e0dba9 Mon Sep 17 00:00:00 2001 From: axpnet <45786925+axpnet@users.noreply.github.com> Date: Sat, 31 Oct 2026 13:34:34 +0100 Subject: [PATCH 3/8] fix(providers): the initial folder keeps its spaces, a listing leaves the current folder, Cloudinary resolves a path once Follow-up of the review on fe4ce1666: the places the first change missed, each counted across every provider and the CLI. - The initial folder of a profile is taken as given. kDrive, Drime, Internxt (both login paths), Jottacloud and Twake trimmed it on connect, and the CLI trimmed the profile base it joins paths to and the MTP initial folder: with `/reports ` saved, the session started in `/reports` and `a.txt` went to `/reports/a.txt`. Only an exactly empty value or `/` means the root now. - Jottacloud took the original parent of a trash entry from a trimmed ``, so an entry deleted from `reports ` would be restored to `reports`. - A listing does not move the current folder; `cd` does. Koofr, kDrive and 4shared moved it in `list`. With the relative path now joined to the current folder (the first change, for Koofr), a relative path listed twice reached `docs/docs`: the CLI sync doctor lists the root it was given, then walks it. A listing of another folder (the GUI's overwrite check lists the destination) also left the session there. The GUI moves with `provider_change_dir`, which calls `cd`. - Cloudinary's `delete` and move resolved the path, then handed the result to `stat` and `rmdir`, which resolved it again against the current folder. The resolved path has no leading slash, so after `cd /photos`, `/photos/a.jpg` became `photos/photos/a.jpg`: a delete or a rename in a folder failed as not found, and a folder delete targeted another folder. They now pass the input, resolved once. Tests, each failing on fe4ce1666: - koofr: a relative path listed twice asks for `/docs` both times and the current folder stays `/`. - cloudinary: after `cd /photos`, a file delete, a folder delete and a rename reach `photos/a`, `photos/sub` and `photos/b`, and no request names `photos/photos`. - jottacloud: a trash entry under `reports ` keeps the space in its path. - path_resolution_guard: no provider's `list` moves the current folder (the guard sees the moves in `cd`), and no provider trims its initial folder. - CLI: the profile base keeps its spaces; no `initial_path` is trimmed in the CLI. Session: L2 016retSy Signed-off-by: axpnet <45786925+axpnet@users.noreply.github.com> --- src-tauri/src/bin/aeroftp_cli.rs | 51 +++++++++- src-tauri/src/providers/cloudinary.rs | 75 ++++++++++++-- src-tauri/src/providers/drime_cloud.rs | 3 +- src-tauri/src/providers/fourshared.rs | 5 +- src-tauri/src/providers/internxt.rs | 6 +- src-tauri/src/providers/jottacloud.rs | 38 +++++-- src-tauri/src/providers/kdrive.rs | 9 +- src-tauri/src/providers/koofr.rs | 47 ++++++++- .../src/providers/path_resolution_guard.rs | 98 +++++++++++++++++++ src-tauri/src/providers/twake.rs | 3 +- 10 files changed, 304 insertions(+), 31 deletions(-) diff --git a/src-tauri/src/bin/aeroftp_cli.rs b/src-tauri/src/bin/aeroftp_cli.rs index 357058297..64547a6ef 100644 --- a/src-tauri/src/bin/aeroftp_cli.rs +++ b/src-tauri/src/bin/aeroftp_cli.rs @@ -30815,7 +30815,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 +30826,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 +30872,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 +30882,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 +32388,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 @@ -86159,6 +86160,46 @@ 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"); + } + + /// The profile's initial folder is taken as given, spaces included, in + /// every place the CLI reads it (the resolver, the MTP open). + #[test] + fn test_no_initial_path_is_trimmed() { + let source = include_str!("aeroftp_cli.rs"); + // Cut at the test module, LF or CRLF (Git for Windows' default). + let at = [ + "\n#[cfg(test)]\nmod tests {", + "\r\n#[cfg(test)]\r\nmod tests {", + ] + .iter() + .find_map(|marker| source.find(marker)) + .expect("the CLI has a test module to cut at"); + // Built, so this test does not name it. + let needle = ["initial_path", ".trim()"].concat(); + let found: Vec = source[..at] + .lines() + .enumerate() + .filter(|(_, line)| !line.trim_start().starts_with("//")) + .filter(|(_, line)| line.contains(&needle)) + .map(|(index, _)| index + 1) + .collect(); + assert!(found.is_empty(), "initial_path trimmed at lines {found:?}"); + } + #[test] fn test_resolve_cli_remote_path_user_already_includes_base() { assert_eq!( diff --git a/src-tauri/src/providers/cloudinary.rs b/src-tauri/src/providers/cloudinary.rs index da5c9be2e..c92151fce 100644 --- a/src-tauri/src/providers/cloudinary.rs +++ b/src-tauri/src/providers/cloudinary.rs @@ -674,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, @@ -1359,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()) @@ -2625,8 +2631,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], @@ -2641,6 +2647,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(); @@ -2711,6 +2722,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 d4bd38f2f..4231b4627 100644 --- a/src-tauri/src/providers/drime_cloud.rs +++ b/src-tauri/src/providers/drime_cloud.rs @@ -1226,7 +1226,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)); diff --git a/src-tauri/src/providers/fourshared.rs b/src-tauri/src/providers/fourshared.rs index c8d94c4ce..b09361273 100644 --- a/src-tauri/src/providers/fourshared.rs +++ b/src-tauri/src/providers/fourshared.rs @@ -1312,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) } diff --git a/src-tauri/src/providers/internxt.rs b/src-tauri/src/providers/internxt.rs index e94cbc88c..fbd3de941 100644 --- a/src-tauri/src/providers/internxt.rs +++ b/src-tauri/src/providers/internxt.rs @@ -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!( diff --git a/src-tauri/src/providers/jottacloud.rs b/src-tauri/src/providers/jottacloud.rs index e945961b0..fc1da5262 100644 --- a/src-tauri/src/providers/jottacloud.rs +++ b/src-tauri/src/providers/jottacloud.rs @@ -1525,7 +1525,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); } @@ -3120,13 +3121,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 +3275,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 +3314,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, @@ -4017,6 +4018,31 @@ 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 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 755074005..2573a4bce 100644 --- a/src-tauri/src/providers/kdrive.rs +++ b/src-tauri/src/providers/kdrive.rs @@ -900,7 +900,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 +1061,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) } diff --git a/src-tauri/src/providers/koofr.rs b/src-tauri/src/providers/koofr.rs index a8f9c0b6f..db9d0270a 100644 --- a/src-tauri/src/providers/koofr.rs +++ b/src-tauri/src/providers/koofr.rs @@ -811,8 +811,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) } @@ -2282,6 +2282,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/path_resolution_guard.rs b/src-tauri/src/providers/path_resolution_guard.rs index a227d9435..5bb63f61d 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,97 @@ 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 +} + +/// [`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 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. +#[test] +fn no_provider_trims_the_initial_folder() { + let trims = |line: &str| line.contains("initial") && line.contains(".trim"); + assert!(trims( + " let initial = initial.trim().to_string();" + )); + assert!(trims( + r#" if !initial.trim().is_empty() && initial.trim() != "/" {"# + )); + let found: Vec = provider_sources() + .iter() + .flat_map(|(file, source)| { + sites_where(production_part(source), trims) + .into_iter() + .map(move |(line, _)| format!("{file}:{line}")) + }) + .collect(); + assert!(found.is_empty(), "the initial folder is trimmed: {found:?}"); +} diff --git a/src-tauri/src/providers/twake.rs b/src-tauri/src/providers/twake.rs index 04a6a0b52..527f34a06 100644 --- a/src-tauri/src/providers/twake.rs +++ b/src-tauri/src/providers/twake.rs @@ -1048,7 +1048,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?; } } From 4b33e94950de367f4f07bff78dad40c631466c68 Mon Sep 17 00:00:00 2001 From: axpnet <45786925+axpnet@users.noreply.github.com> Date: Sat, 31 Oct 2026 14:52:48 +0100 Subject: [PATCH 4/8] fix(providers): ImageKit refuses a name it would store as another, names keep their spaces in listings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review of 6e9efdbf2 found three more places, and the class reaches further. ImageKit stores every character of a name that is not a letter, a mark or a digit (of any script), `-`, `_` or, in a file name, `.`, as `_`. Measured on the API: `put "a b.txt"` reported success, stored `a_b.txt` and replaced the file of that name; ` lead.txt` became `_lead.txt`; `mkdir "a b"` and `mkdir "a.b"` created `a_b`; a rename to `r a.txt` gave `r_a.txt`. Its upload documentation lists fewer characters than it keeps (`é`, `ß`, `日`, `١`, `²` stay). Such a name is now refused before any request, naming what ImageKit would store: upload, folder creation, file rename, and the copy and move that rename after their first call. The folder-rename check that existed is the same rule now, and keeps a combining mark, which ImageKit keeps. - Cloudinary: a display name of spaces was read as absent, and the asset went by its public id. - Jottacloud: the trash parent's names are the stored ones (`reports％` for `reports%`); they are decoded like listed names, since `jfs_url` encodes the path again and restore or purge reached another name. - WebDAV: the displayname was trimmed, and the entry's path is built from it: ` a.txt ` was listed as `a.txt` at `/dav/a.txt`. - FTP: the MLSD name was trimmed. RFC 3659 puts it after one space, its own spaces included. suppaftp trims the MLST reply line, so `stat` gives an entry the spaces its name ended with back when that is all it lost. - Cross-profile transfer: a plan showed ` a` as `a`, took a path of spaces for the provider's root, and ` .aerocrypt.tsv` for the vault marker. - AeroCrypt: a vault created from the dialog on `/reports ` was rooted at `/reports`. Tests: one per place; the ImageKit one checks that no request leaves for upload, mkdir, rename, move and copy of such names and that kept names go. 15 mutations (each fix undone, the ImageKit checks one by one, the combining mark dropped) each turn a test red. Session: L2 016retSy Signed-off-by: axpnet <45786925+axpnet@users.noreply.github.com> --- src-tauri/src/aerocrypt_provider.rs | 7 +- src-tauri/src/cross_profile_transfer.rs | 45 ++++- src-tauri/src/providers/cloudinary.rs | 19 +- src-tauri/src/providers/ftp.rs | 1 + src-tauri/src/providers/ftp_listing.rs | 51 ++++- src-tauri/src/providers/imagekit.rs | 238 ++++++++++++++++++++++-- src-tauri/src/providers/jottacloud.rs | 40 +++- src-tauri/src/providers/webdav.rs | 49 ++++- 8 files changed, 417 insertions(+), 33 deletions(-) diff --git a/src-tauri/src/aerocrypt_provider.rs b/src-tauri/src/aerocrypt_provider.rs index 4fa8aeaa8..e823fa474 100644 --- a/src-tauri/src/aerocrypt_provider.rs +++ b/src-tauri/src/aerocrypt_provider.rs @@ -537,7 +537,7 @@ pub struct AeroCryptCreatedVault { /// 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 as given: `/reports ` is not `/reports`. let base = if base.is_empty() { "/" } else { base }; match subpath .map(|s| s.trim().trim_matches('/')) @@ -1696,6 +1696,11 @@ mod tests { 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("/reports ", None), "/reports "); + assert_eq!( + overlay_create_root("/reports ", Some("New")), + "/reports /New" + ); assert_eq!(overlay_create_root("", None), "/"); } 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 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 @@ -2140,6 +2141,22 @@ 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 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..f6c2db210 100644 --- a/src-tauri/src/providers/ftp_listing.rs +++ b/src-tauri/src/providers/ftp_listing.rs @@ -542,12 +542,30 @@ 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. +pub(crate) fn name_as_asked(entry: &mut RemoteEntry, asked: &str) { + let Some(asked_name) = asked.trim_end_matches('/').rsplit('/').next() else { + return; + }; + if asked_name == entry.name || asked_name.trim_end() != entry.name { + return; + } + if let Some(parent) = entry.path.strip_suffix(entry.name.as_str()) { + entry.path = format!("{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 +873,37 @@ 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") + ); + } + #[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 808a53f5a..edb34c258 100644 --- a/src-tauri/src/providers/imagekit.rs +++ b/src-tauri/src/providers/imagekit.rs @@ -405,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 @@ -551,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 { @@ -577,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()))) @@ -639,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(()) @@ -712,6 +724,16 @@ impl ImageKitProvider { 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 @@ -725,7 +747,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; @@ -752,7 +773,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) { @@ -1284,6 +1304,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 @@ -1614,28 +1635,63 @@ fn folder_copy_destination(source: &str, target: &str) -> Result Result<(), ProviderError> { - let rewritten: String = name - .chars() +/// What ImageKit does with a name it is given: it keeps letters, marks and +/// digits of any script, `-`, `_` and, in a file name, `.`, and stores every +/// other character as `_`. Measured on the API on 2026-10-31: space, no-break +/// space, `(`, `&`, `'`, `’`, `·` and an emoji became `_`; `é` (composed or +/// with a combining accent), `ß`, `日`, `١` and `²` stayed; 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 c.is_alphanumeric() || c == '-' { + if c.is_alphanumeric() + || unicode_normalization::char::is_combining_mark(c) + || c == '-' + || c == '_' + || (c == '.' && !folder) + { c } else { '_' } }) - .collect(); - if rewritten == name { + .collect() +} + +/// 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, digits, '-' and '_'" + } else { + "a file name there keeps only letters, digits, '.', '-' 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}" ))) } @@ -2374,12 +2430,158 @@ 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 any script, `-`, `_`, and `.` in a file name only. + #[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", + ] { + 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"), + ] { + assert_eq!(imagekit_stored_name(sent, false), stored, "{sent:?}"); + } + 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, + ), + ("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}"); + + 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] diff --git a/src-tauri/src/providers/jottacloud.rs b/src-tauri/src/providers/jottacloud.rs index fc1da5262..96522b7ef 100644 --- a/src-tauri/src/providers/jottacloud.rs +++ b/src-tauri/src/providers/jottacloud.rs @@ -2342,6 +2342,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('/') @@ -2350,7 +2353,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() }; @@ -4043,6 +4050,37 @@ mod tests { 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/webdav.rs b/src-tauri/src/providers/webdav.rs index 7fcb80b10..cb317e0c7 100644 --- a/src-tauri/src/providers/webdav.rs +++ b/src-tauri/src/providers/webdav.rs @@ -3288,9 +3288,13 @@ 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()) + // As given, like the rest of the name: ` a` and + // `a ` are two other files than `a`, and a name + // of spaces is a name. XML writers indent around + // the element, never inside its text. + let name = if !displayname.is_empty() { + urlencoding::decode(&displayname) + .unwrap_or_else(|_| displayname.as_str().into()) .into_owned() } else { decoded_href @@ -7142,6 +7146,45 @@ mod tests { assert_eq!(entries[1].name, "a & b.txt"); } + /// A displayname that begins or ends with a space names that file: 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. + #[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/ + + + + + + + + "#; + + 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/ ")]); + } + /// 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. From 880cd8c4bfbba2fe8ba53eaac701f9b734f61a60 Mon Sep 17 00:00:00 2001 From: axpnet <45786925+axpnet@users.noreply.github.com> Date: Sat, 31 Oct 2026 16:29:52 +0100 Subject: [PATCH 5/8] fix(paths): the initial folder is used as given everywhere it is read, a leading backslash is the root, a relative base is applied once Closes the three findings of the review on 6e9efdbf2, and the siblings each one has. A leading backslash is the root. Koofr, Drime, FileLu, Jottacloud, kDrive, OpenDrive and Twake normalize `\` to `/`, but resolve_path tested only `/` for an absolute path, so `\docs\a` was joined under the current folder. All seven now take a leading `\` for the root, each with a test. A relative initial folder is applied once. Connect enters `docs` from where the session starts (the drive root, or the login home on FTP and SFTP) and the provider resolves a relative path against the folder it stands in, so the CLI base left relative reached `docs/docs`. CliBase::for_provider now makes the base the folder the session stands in when it ends with that relative folder on a boundary; a provider that keeps the folder as a prefix of its own (S3's `bucket/backups/`) stands at its root and keeps the base. Both cases are tested on an in-memory provider that now answers pwd and resolves a relative path. Nothing trims the initial folder. WebDAV trimmed it in a chain over several lines, which the line-based guard missed; the guard now reads statements and scans every backend source, not only the providers and the CLI. It found, and this fixes: the TUI's folder and the profile form it saves, the rclone export (the CLI path option and the start-folder alias), the Zoho folder id, the benchmark's scratch folder and the AI worker's S3 root. rclone reads a config value without its edge spaces (measured with rclone 1.75.2; a quoted value keeps its quotes), so an alias to a folder ending in a space is left out with a note instead of pointing at another folder. Three statements trim on purpose and are named in the guard: two imports that read another application's file, and the AeroCrypt anchor, which trims on both sides (norm_anchor, normCryptScope). The frontend did the same with 20 trims on 18 lines in 6 files (App, the mount dialog, the profile rail, the cross-profile panel, the AI remote worker roots, the AeroSync CLI line); a source guard covers them, the AeroSync mirror has executing cases, and the crypt landing test now expects the Remote Path the connect opened, spaces included. The CLI's line-based trim test is removed: the guard covers it. Session: L2 016retSy Signed-off-by: axpnet <45786925+axpnet@users.noreply.github.com> --- src-tauri/src/ai_core/runner/worker_remote.rs | 4 +- src-tauri/src/bin/aeroftp_cli.rs | 148 ++++++++++++--- src-tauri/src/bridge_commands.rs | 18 +- src-tauri/src/cli_tui/app.rs | 36 +++- src-tauri/src/cli_tui/session.rs | 25 ++- src-tauri/src/community_benchmark/mod.rs | 3 +- src-tauri/src/community_benchmark/tests.rs | 5 +- src-tauri/src/providers/drime_cloud.rs | 6 +- src-tauri/src/providers/filelu.rs | 6 +- src-tauri/src/providers/jottacloud.rs | 6 +- src-tauri/src/providers/kdrive.rs | 6 +- src-tauri/src/providers/koofr.rs | 8 +- src-tauri/src/providers/opendrive.rs | 18 +- .../src/providers/path_resolution_guard.rs | 171 ++++++++++++++++-- src-tauri/src/providers/twake.rs | 6 +- src-tauri/src/providers/webdav.rs | 31 +++- src-tauri/src/rclone_import.rs | 107 ++++++++--- src/App.tsx | 14 +- .../CrossProfile/CrossProfilePanel.tsx | 6 +- src/components/DevTools/AIChat.tsx | 4 +- src/components/MountManagerDialog.tsx | 6 +- src/components/common/ProfileRail.tsx | 4 +- src/utils/aeroSyncCliCommand.test.ts | 8 + src/utils/aeroSyncCliCommand.ts | 2 +- src/utils/cryptConnectLanding.wiring.test.ts | 8 +- src/utils/initialFolderAsGiven.test.ts | 91 ++++++++++ 26 files changed, 625 insertions(+), 122 deletions(-) create mode 100644 src/utils/initialFolderAsGiven.test.ts 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 64547a6ef..bec220906 100644 --- a/src-tauri/src/bin/aeroftp_cli.rs +++ b/src-tauri/src/bin/aeroftp_cli.rs @@ -9640,7 +9640,23 @@ impl CliBase { /// The base of a connected provider: Proton Drive's start folder is where /// its session stands right after connecting. + /// + /// A relative initial folder (`docs`) is entered by connect 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 base left relative was applied twice and `a.txt` reached + /// `docs/docs/a.txt`. When the session stands in that folder, the base is + /// where it stands. A provider that keeps the folder as a prefix of its + /// own (S3's `bucket/backups/`) stays at its root and keeps the base. async fn for_provider(path: impl Into, provider: &mut dyn StorageProvider) -> Self { + let mut path = path.into(); + if !path.is_empty() && !path.starts_with('/') { + if let Ok(cwd) = provider.pwd().await { + if stands_in(&cwd, &path) { + path = cwd; + } + } + } let start_folder = if provider.provider_type() == ProviderType::Proton { Some(provider.pwd().await.unwrap_or_else(|_| "/".to_string())) } else { @@ -9650,6 +9666,14 @@ impl CliBase { } } +/// Whether a session in `cwd` stands in the relative folder `base`: `cwd` is +/// `/docs`, or ends with `/docs` (the login home's `docs`). +fn stands_in(cwd: &str, base: &str) -> bool { + let base = base.trim_end_matches('/'); + let cwd = cwd.trim_end_matches('/'); + !base.is_empty() && cwd.ends_with(base) && cwd[..cwd.len() - base.len()].ends_with('/') +} + impl std::ops::Deref for CliBase { type Target = str; fn deref(&self) -> &str { @@ -86175,31 +86199,6 @@ mod tests { assert_eq!(resolve_cli_remote_path("/ reports", ""), "/ reports"); } - /// The profile's initial folder is taken as given, spaces included, in - /// every place the CLI reads it (the resolver, the MTP open). - #[test] - fn test_no_initial_path_is_trimmed() { - let source = include_str!("aeroftp_cli.rs"); - // Cut at the test module, LF or CRLF (Git for Windows' default). - let at = [ - "\n#[cfg(test)]\nmod tests {", - "\r\n#[cfg(test)]\r\nmod tests {", - ] - .iter() - .find_map(|marker| source.find(marker)) - .expect("the CLI has a test module to cut at"); - // Built, so this test does not name it. - let needle = ["initial_path", ".trim()"].concat(); - let found: Vec = source[..at] - .lines() - .enumerate() - .filter(|(_, line)| !line.trim_start().starts_with("//")) - .filter(|(_, line)| line.contains(&needle)) - .map(|(index, _)| index + 1) - .collect(); - assert!(found.is_empty(), "initial_path trimmed at lines {found:?}"); - } - #[test] fn test_resolve_cli_remote_path_user_already_includes_base() { assert_eq!( @@ -88997,6 +88996,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 { @@ -89064,16 +89067,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(()) @@ -96708,6 +96717,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/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..7a21b71c7 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] diff --git a/src-tauri/src/providers/drime_cloud.rs b/src-tauri/src/providers/drime_cloud.rs index 4231b4627..518dfa388 100644 --- a/src-tauri/src/providers/drime_cloud.rs +++ b/src-tauri/src/providers/drime_cloud.rs @@ -413,7 +413,8 @@ impl DrimeCloudProvider { // 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 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('/'); @@ -2975,6 +2976,9 @@ mod tests { 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 { diff --git a/src-tauri/src/providers/filelu.rs b/src-tauri/src/providers/filelu.rs index fe4b896d8..12b26a8fb 100644 --- a/src-tauri/src/providers/filelu.rs +++ b/src-tauri/src/providers/filelu.rs @@ -494,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(); @@ -2584,6 +2585,9 @@ mod tests { 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 { diff --git a/src-tauri/src/providers/jottacloud.rs b/src-tauri/src/providers/jottacloud.rs index 96522b7ef..942323ad8 100644 --- a/src-tauri/src/providers/jottacloud.rs +++ b/src-tauri/src/providers/jottacloud.rs @@ -898,7 +898,8 @@ impl JottacloudProvider { 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('/'); @@ -3762,6 +3763,9 @@ mod tests { 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 { diff --git a/src-tauri/src/providers/kdrive.rs b/src-tauri/src/providers/kdrive.rs index 2573a4bce..3a26dbd8e 100644 --- a/src-tauri/src/providers/kdrive.rs +++ b/src-tauri/src/providers/kdrive.rs @@ -576,7 +576,8 @@ impl KDriveProvider { // 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 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('/'); @@ -2589,6 +2590,9 @@ mod tests { 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 { diff --git a/src-tauri/src/providers/koofr.rs b/src-tauri/src/providers/koofr.rs index db9d0270a..87628e49c 100644 --- a/src-tauri/src/providers/koofr.rs +++ b/src-tauri/src/providers/koofr.rs @@ -543,8 +543,9 @@ impl KoofrProvider { } // 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. - if path.starts_with('/') { + // 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('/'); @@ -2094,6 +2095,9 @@ mod tests { 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"); } 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 5bb63f61d..d2f2332bc 100644 --- a/src-tauri/src/providers/path_resolution_guard.rs +++ b/src-tauri/src/providers/path_resolution_guard.rs @@ -465,6 +465,46 @@ fn provider_sources() -> Vec<(String, String)> { 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 { [ @@ -512,26 +552,125 @@ fn no_provider_listing_moves_the_current_folder() { ); } +/// 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. +fn statements(source: &str) -> Vec<(usize, String)> { + let mut found = Vec::new(); + let mut open: Option<(usize, String)> = None; + for (index, line) in source.lines().enumerate() { + let code = line.trim_start(); + if code.starts_with("//") { + continue; + } + let (_, text) = open.get_or_insert_with(|| (index + 1, String::new())); + text.push_str(code); + text.push(' '); + if code.ends_with([';', '{', '}', ',']) { + found.extend(open.take()); + } + } + found.extend(open); + found +} + /// 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. +/// 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 no_provider_trims_the_initial_folder() { - let trims = |line: &str| line.contains("initial") && line.contains(".trim"); - assert!(trims( - " let initial = initial.trim().to_string();" - )); - assert!(trims( - r#" if !initial.trim().is_empty() && initial.trim() != "/" {"# - )); - let found: Vec = provider_sources() +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() - .flat_map(|(file, source)| { - sites_where(production_part(source), trims) - .into_iter() - .map(move |(line, _)| format!("{file}:{line}")) - }) + .map(|(name, _)| name.to_string()) .collect(); - assert!(found.is_empty(), "the initial folder is trimmed: {found:?}"); + expected.sort(); + assert_eq!(excused, expected); } diff --git a/src-tauri/src/providers/twake.rs b/src-tauri/src/providers/twake.rs index 527f34a06..df4360f32 100644 --- a/src-tauri/src/providers/twake.rs +++ b/src-tauri/src/providers/twake.rs @@ -839,7 +839,8 @@ impl TwakeProvider { 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(['/', '\\']) { Self::normalize_path(path) } else { Self::normalize_path(&format!("{}/{}", self.current_path, path)) @@ -1628,6 +1629,9 @@ mod tests { 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::*; diff --git a/src-tauri/src/providers/webdav.rs b/src-tauri/src/providers/webdav.rs index cb317e0c7..90201e63d 100644 --- a/src-tauri/src/providers/webdav.rs +++ b/src-tauri/src/providers/webdav.rs @@ -4145,12 +4145,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) => { @@ -6562,6 +6562,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(), diff --git a/src-tauri/src/rclone_import.rs b/src-tauri/src/rclone_import.rs index 5fa5c7ae8..15651368c 100644 --- a/src-tauri/src/rclone_import.rs +++ b/src-tauri/src/rclone_import.rs @@ -3492,48 +3492,60 @@ 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 - // webdav paths are relative to the account/URL root. - let alias_path = if proto == "sftp" { - path.to_string() + // 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'], " ") + )); } 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 { + // sftp addresses an absolute folder as `remote:/abs`; ftp and + // webdav paths are relative to the account/URL root. + let alias_path = if proto == "sftp" { + path.to_string() + } 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 { + 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 )); + } } } } @@ -4751,6 +4763,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 { diff --git a/src/App.tsx b/src/App.tsx index 77b5f5ffd..ec4af8a43 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -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', { @@ -6393,7 +6393,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 +10021,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..22deca31e 100644 --- a/src/utils/aeroSyncCliCommand.test.ts +++ b/src/utils/aeroSyncCliCommand.test.ts @@ -122,10 +122,18 @@ 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'); + }); + 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..b3233b031 100644 --- a/src/utils/cryptConnectLanding.wiring.test.ts +++ b/src/utils/cryptConnectLanding.wiring.test.ts @@ -72,9 +72,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 () => { 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([]); + }); +}); From b6cbf3e83b03f83eda9f7545c1160cd0711e5a5e Mon Sep 17 00:00:00 2001 From: axpnet <45786925+axpnet@users.noreply.github.com> Date: Sat, 31 Oct 2026 16:42:06 +0100 Subject: [PATCH 6/8] fix(aerocrypt): a vault root that begins or ends with a space is refused, since its anchor would name another folder The create dialog now roots a vault at its folder as given (`/reports `), but the overlay anchor trims the ends of its folder (`norm_anchor`, and `normCryptScope` in the app): the vault at `/reports ` would be anchored at `/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. Both create commands (AeroCrypt and rclone crypt) go through `overlay_create_root`, which now refuses such a root with a message that names it; a folder below it (`/reports /New`) keeps its spaced name inside the path and is anchored as created. The comparison with the dialog's folder no longer trims it either. Test: `a_vault_root_the_anchor_would_change_is_refused` refuses `/reports `, ` /reports` and `/home/u/ `, and checks with `norm_anchor` that `/reports /New` and `/reports /` are anchored at their own folder; without the refusal it fails. Session: L2 016retSy Signed-off-by: axpnet <45786925+axpnet@users.noreply.github.com> --- src-tauri/src/aerocrypt_provider.rs | 72 +++++++++++++++++++++-------- src-tauri/src/lib.rs | 4 +- 2 files changed, 56 insertions(+), 20 deletions(-) diff --git a/src-tauri/src/aerocrypt_provider.rs b/src-tauri/src/aerocrypt_provider.rs index e823fa474..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 { - // The folder as given: `/reports ` is not `/reports`. +/// +/// 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,19 +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("/reports ", None), "/reports "); - assert_eq!( - overlay_create_root("/reports ", Some("New")), - "/reports /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/lib.rs b/src-tauri/src/lib.rs index 776cd2280..433ee5253 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -1463,8 +1463,8 @@ async fn rclone_crypt_provider_create_remote( // Same rule as the AeroCrypt create: the folder is absolute, taken from // the dialog, never from the provider's pwd, which the overlay is then // applied at by the caller. rclone crypt has no marker to write. - let root = aerocrypt_provider::overlay_create_root(&base_path, target_subpath.as_deref()); - if root != base_path.trim() { + let root = aerocrypt_provider::overlay_create_root(&base_path, target_subpath.as_deref())?; + if root != base_path { let mut provider_lock = provider_state.provider.lock().await; let provider = provider_lock .as_mut() From 13f1c4a828bd2fd00d06bc9d0ed3da9e37a88ee4 Mon Sep 17 00:00:00 2001 From: axpnet <45786925+axpnet@users.noreply.github.com> Date: Sat, 31 Oct 2026 17:35:53 +0100 Subject: [PATCH 7/8] fix(paths): panel paths and the crypt landing as given, names of spaces in LIST, MLST and CDATA, a crypt without its alias wraps its folder, ImageKit's measured rule Closes the findings of the review on 880cd8c4b, with their siblings. Panel paths are used as given. normalizePanelPath trimmed every endpoint path (local, remote, overlay), so a cross-profile transfer planned on `/reports ` reached `/reports`. Only a missing or empty path takes the fallback now. The crypt connect lands on the Remote Path as given. The phase-2 landing went through normCryptScope, the anchor's normalization, whose trim landed `/docs ` on `/docs` and a name of spaces on the root. normLandingPath normalizes only its slashes. The anchor itself still trims on both sides, as before. Names of spaces are read in three more places. WebDAV's CDATA arm trimmed the displayname, which the text arm no longer did. An FTP LIST row whose name is only spaces has eight fields and was dropped, so stat without MLST read the file as missing: the name is what follows the one separator after the time or the year, and only when the date reads as one. suppaftp trims the MLST reply, and the reply about `/x/ ` names `/x/`: stat returned the folder `x` as the file. The entry takes the name and path asked. A crypt overlay whose alias is left out wraps its folder. The overlay scope is relative to the start folder or pinned bucket its alias points at, and when the alias could not be written (a start folder ending in a space, or no free name for it or for the bucket alias) the crypt wrapped the clear remote's root: `/vault` of the account. It now wraps the clear remote at that folder, as the alias would hold it, and is refused with a note when even that path ends in a space. A relative initial folder is compared folder by folder. `./docs` stood in `/docs` and was not seen, so the CLI applied it again. The check moved to util::start_folder, and the benchmark uses it too: its scratch folder joined a relative start path to the folder the session already stood in (`team /team /aeroftp-bench`). ImageKit. A queued moveFolder job answered a later move of the same folder to another name in that folder; only a move that keeps the name resumes it (the renameFolder and copyFolder resumes already carry the name). The name rule kept every Unicode alphanumeric: 34 more characters measured on the API show that the circled letters U+24B6 to U+24E9 and every character above U+FFFF (letters, digits and marks included) become `_`, one each. The rule is now letters, marks and digits up to U+FFFF without the circled letters, and the test holds every measured character. The Rust guard reads a call whose arguments run over several lines as one statement: a comma ends a statement only outside brackets. Session: L2 016retSy Signed-off-by: axpnet <45786925+axpnet@users.noreply.github.com> --- src-tauri/src/bin/aeroftp_cli.rs | 30 +--- src-tauri/src/community_benchmark/engine.rs | 11 +- src-tauri/src/community_benchmark/tests.rs | 53 +++++- src-tauri/src/providers/ftp_listing.rs | 82 +++++++++- src-tauri/src/providers/imagekit.rs | 120 ++++++++++---- .../src/providers/path_resolution_guard.rs | 38 ++++- src-tauri/src/providers/webdav.rs | 44 ++++- src-tauri/src/rclone_import.rs | 154 +++++++++++++++--- src-tauri/src/util/mod.rs | 1 + src-tauri/src/util/start_folder.rs | 76 +++++++++ src/App.tsx | 11 +- src/utils/aeroSyncCliCommand.test.ts | 3 + src/utils/cryptConnectLanding.wiring.test.ts | 30 ++-- src/utils/cryptScope.test.ts | 12 +- src/utils/cryptScope.ts | 11 ++ .../overlayRootNavigation.wiring.test.ts | 4 +- src/utils/panelEndpoints.test.ts | 16 +- src/utils/panelEndpoints.ts | 11 +- .../profileOverlayOwnership.wiring.test.ts | 4 +- 19 files changed, 589 insertions(+), 122 deletions(-) create mode 100644 src-tauri/src/util/start_folder.rs diff --git a/src-tauri/src/bin/aeroftp_cli.rs b/src-tauri/src/bin/aeroftp_cli.rs index bec220906..1e7881d70 100644 --- a/src-tauri/src/bin/aeroftp_cli.rs +++ b/src-tauri/src/bin/aeroftp_cli.rs @@ -9639,24 +9639,12 @@ impl CliBase { } /// The base of a connected provider: Proton Drive's start folder is where - /// its session stands right after connecting. - /// - /// A relative initial folder (`docs`) is entered by connect 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 base left relative was applied twice and `a.txt` reached - /// `docs/docs/a.txt`. When the session stands in that folder, the base is - /// where it stands. A provider that keeps the folder as a prefix of its - /// own (S3's `bucket/backups/`) stays at its root and keeps the base. + /// 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 mut path = path.into(); - if !path.is_empty() && !path.starts_with('/') { - if let Ok(cwd) = provider.pwd().await { - if stands_in(&cwd, &path) { - path = cwd; - } - } - } + 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 { @@ -9666,14 +9654,6 @@ impl CliBase { } } -/// Whether a session in `cwd` stands in the relative folder `base`: `cwd` is -/// `/docs`, or ends with `/docs` (the login home's `docs`). -fn stands_in(cwd: &str, base: &str) -> bool { - let base = base.trim_end_matches('/'); - let cwd = cwd.trim_end_matches('/'); - !base.is_empty() && cwd.ends_with(base) && cwd[..cwd.len() - base.len()].ends_with('/') -} - impl std::ops::Deref for CliBase { type Target = str; fn deref(&self) -> &str { diff --git a/src-tauri/src/community_benchmark/engine.rs b/src-tauri/src/community_benchmark/engine.rs index c2ae79413..a2c78c18c 100644 --- a/src-tauri/src/community_benchmark/engine.rs +++ b/src-tauri/src/community_benchmark/engine.rs @@ -389,8 +389,8 @@ 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. let prefix = match opts.test_root_prefix.as_deref() { Some(prefix) => { let kind = provider.provider_type(); @@ -405,7 +405,12 @@ 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 => benchmark_remote_roots( + &crate::util::start_folder::as_entered(initial_path, &mut *provider).await, + &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/tests.rs b/src-tauri/src/community_benchmark/tests.rs index 7a21b71c7..af4b1d9bf 100644 --- a/src-tauri/src/community_benchmark/tests.rs +++ b/src-tauri/src/community_benchmark/tests.rs @@ -631,6 +631,10 @@ 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, + /// Every folder asked for, in order. + mkdirs: Vec, } struct MemRemote(Arc>); @@ -677,7 +681,13 @@ mod engine_runs { Ok(out) } async fn pwd(&mut self) -> Result { - Ok("/".into()) + Ok(self + .0 + .lock() + .unwrap() + .cwd + .clone() + .unwrap_or_else(|| "/".into())) } async fn cd(&mut self, _p: &str) -> Result<(), ProviderError> { Ok(()) @@ -729,6 +739,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())); } @@ -894,6 +905,46 @@ 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. + #[tokio::test] + async fn a_relative_start_path_is_applied_once() { + 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, "team ", opts, &Silent).await.unwrap(); + assert!( + outcome.report.summary.errors.is_empty(), + "{:?}", + 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"); + assert!( + r.mkdirs + .iter() + .all(|dir| dir.starts_with('/') && !dir.contains("team /team ")), + "{:?}", + r.mkdirs + ); + assert!( + r.mkdirs + .iter() + .any(|dir| dir.starts_with("/home/u/team /aeroftp-bench/")), + "{:?}", + r.mkdirs + ); + assert!(r.files.is_empty(), "{:?}", r.files.keys()); + assert_eq!(r.dirs, BTreeSet::from(home)); + } + #[tokio::test] async fn a_run_measures_and_leaves_nothing_behind() { let (state, provider) = remote(); diff --git a/src-tauri/src/providers/ftp_listing.rs b/src-tauri/src/providers/ftp_listing.rs index f6c2db210..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 @@ -545,15 +566,24 @@ 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 Some(asked_name) = asked.trim_end_matches('/').rsplit('/').next() else { + let asked = asked.trim_end_matches('/'); + let Some(asked_name) = asked.rsplit('/').next() else { return; }; - if asked_name == entry.name || asked_name.trim_end() != entry.name { + if asked_name == entry.name { return; } - if let Some(parent) = entry.path.strip_suffix(entry.name.as_str()) { - entry.path = format!("{parent}{asked_name}"); + 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(); } } @@ -904,6 +934,46 @@ mod tests { ); } + /// 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 edb34c258..827491463 100644 --- a/src-tauri/src/providers/imagekit.rs +++ b/src-tauri/src/providers/imagekit.rs @@ -716,12 +716,15 @@ 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 @@ -1635,30 +1638,39 @@ fn folder_copy_destination(source: &str, target: &str) -> Result String { name.chars() - .map(|c| { - if c.is_alphanumeric() - || unicode_normalization::char::is_combining_mark(c) - || c == '-' - || c == '_' - || (c == '.' && !folder) - { - c - } else { - '_' - } - }) + .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 { @@ -1685,9 +1697,9 @@ fn refuse_rewritten_name(name: &str, use_: NameUse) -> Result<(), ProviderError> NameUse::RenameFolder => ("rename the folder to", "Nothing was changed"), }; let keeps = if folder { - "a folder name there keeps only letters, digits, '-' and '_'" + "a folder name there keeps only letters, marks and digits up to U+FFFF, '-' and '_'" } else { - "a file name there keeps only letters, digits, '.', '-' and '_'" + "a file name there keeps only letters, marks and digits up to U+FFFF, '.', '-' and '_'" }; Err(ProviderError::NotSupported(format!( "ImageKit would {would} \"{stored}\", not \"{name}\": {keeps}, and every other \ @@ -2439,7 +2451,9 @@ mod tests { } /// What ImageKit keeps of a name, as measured on its API: letters, marks - /// and digits of any script, `-`, `_`, and `.` in a file name only. + /// 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 [ @@ -2450,6 +2464,13 @@ mod tests { "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); } @@ -2461,9 +2482,21 @@ mod tests { ("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}"); } @@ -2864,6 +2897,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/path_resolution_guard.rs b/src-tauri/src/providers/path_resolution_guard.rs index d2f2332bc..fa6cc4f60 100644 --- a/src-tauri/src/providers/path_resolution_guard.rs +++ b/src-tauri/src/providers/path_resolution_guard.rs @@ -574,10 +574,14 @@ fn trims_whitespace(text: &str) -> bool { /// 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. +/// `.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. fn statements(source: &str) -> Vec<(usize, String)> { let mut found = Vec::new(); let mut open: Option<(usize, String)> = None; + let mut depth = 0usize; for (index, line) in source.lines().enumerate() { let code = line.trim_start(); if code.starts_with("//") { @@ -586,14 +590,44 @@ fn statements(source: &str) -> Vec<(usize, String)> { let (_, text) = open.get_or_insert_with(|| (index + 1, String::new())); text.push_str(code); text.push(' '); - if code.ends_with([';', '{', '}', ',']) { + for c in code.chars() { + match c { + '(' | '[' => depth += 1, + ')' | ']' => depth = depth.saturating_sub(1), + _ => {} + } + } + if code.ends_with([';', '{', '}']) || (code.ends_with(',') && depth == 0) { found.extend(open.take()); + depth = 0; } } found.extend(open); found } +#[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 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 diff --git a/src-tauri/src/providers/webdav.rs b/src-tauri/src/providers/webdav.rs index 90201e63d..074005d52 100644 --- a/src-tauri/src/providers/webdav.rs +++ b/src-tauri/src/providers/webdav.rs @@ -3394,11 +3394,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, @@ -7173,9 +7176,10 @@ mod tests { assert_eq!(entries[1].name, "a & b.txt"); } - /// A displayname that begins or ends with a space names that file: 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. + /// 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. #[test] fn parse_propfind_keeps_the_spaces_around_a_displayname() { let provider = WebDavProvider::new(test_config("https://example.com/dav")) @@ -7202,6 +7206,24 @@ mod tests { + + /dav/%20c.txt%20 + + + + + + + + + /dav/%20%20%20/ + + + + + + + "#; let entries = provider.parse_propfind_response(xml, "/dav").unwrap(); @@ -7209,7 +7231,15 @@ mod tests { .iter() .map(|e| (e.name.as_str(), e.path.as_str())) .collect(); - assert_eq!(named, [(" a.txt ", "/dav/ a.txt "), (" ", "/dav/ ")]); + assert_eq!( + named, + [ + (" a.txt ", "/dav/ a.txt "), + (" ", "/dav/ "), + (" c.txt ", "/dav/ c.txt "), + (" ", "/dav/ "), + ] + ); } /// CR-536 regression: a whitespace-ONLY Text fragment between two entity diff --git a/src-tauri/src/rclone_import.rs b/src-tauri/src/rclone_import.rs index 15651368c..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()); } } } @@ -3494,6 +3507,13 @@ pub fn export_rclone( .and_then(|v| v.as_str()) .filter(|p| !p.is_empty() && *p != "/") { + // sftp addresses an absolute folder as `remote:/abs`; ftp and + // webdav paths are relative to the account/URL root. + let alias_path = if proto == "sftp" { + path.to_string() + } else { + path.trim_start_matches('/').to_string() + }; // 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 @@ -3504,14 +3524,8 @@ pub fn export_rclone( server.name.replace(['\n', '\r'], " "), path.replace(['\n', '\r'], " ") )); + crypt_folder = Some(alias_path); } else { - // sftp addresses an absolute folder as `remote:/abs`; ftp and - // webdav paths are relative to the account/URL root. - let alias_path = if proto == "sftp" { - path.to_string() - } 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 => { @@ -3545,6 +3559,7 @@ pub fn export_rclone( server.name.replace(['\n', '\r'], " "), desired )); + crypt_folder = Some(alias_path); } } } @@ -3563,6 +3578,7 @@ pub fn export_rclone( &crypt_base, &remote_name, crypt_base_root.as_deref(), + crypt_folder.as_deref(), options, &mut names, ); @@ -5326,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 @@ -7450,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"); @@ -7461,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!( @@ -7469,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..46c7bea91 --- /dev/null +++ b/src-tauri/src/util/start_folder.rs @@ -0,0 +1,76 @@ +// 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. A base with `..` cannot be told from here and is kept. +pub fn stands_in(cwd: &str, base: &str) -> bool { + let folders = |path: &str| -> Vec { + path.split('/') + .filter(|folder| !folder.is_empty() && *folder != ".") + .map(str::to_string) + .collect() + }; + let (cwd, base) = (folders(cwd), folders(base)); + !base.is_empty() && !base.iter().any(|folder| folder == "..") && cwd.ends_with(&base) +} + +/// 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 "), + ] { + assert!(stands_in(cwd, base), "{cwd:?} {base:?}"); + } + for (cwd, base) in [ + ("/mydocs", "docs"), + ("/docs", "docs "), + ("/docs", "../docs"), + ("/docs", "."), + ("/", ""), + ] { + assert!(!stands_in(cwd, base), "{cwd:?} {base:?}"); + } + } +} diff --git a/src/App.tsx b/src/App.tsx index ec4af8a43..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'; @@ -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; } diff --git a/src/utils/aeroSyncCliCommand.test.ts b/src/utils/aeroSyncCliCommand.test.ts index 22deca31e..3b2a0025a 100644 --- a/src/utils/aeroSyncCliCommand.test.ts +++ b/src/utils/aeroSyncCliCommand.test.ts @@ -132,6 +132,9 @@ describe('aeroftp-cli sync line for the Plan tab', () => { 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"])( diff --git a/src/utils/cryptConnectLanding.wiring.test.ts b/src/utils/cryptConnectLanding.wiring.test.ts index b3233b031..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)', () => { @@ -92,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..9a5c6ec0a 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); diff --git a/src/utils/cryptScope.ts b/src/utils/cryptScope.ts index 15c4db980..95678a291 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, 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' }, From d35590ffba81166c3ccfa6572b72ab0f167bdb28 Mon Sep 17 00:00:00 2001 From: axpnet <45786925+axpnet@users.noreply.github.com> Date: Sat, 31 Oct 2026 18:32:52 +0100 Subject: [PATCH 8/8] fix(paths): the connect landing and WebDAV names follow the resource, the benchmark asks pwd under its stop signal, the guard reads code only Closes the findings of the review on 13f1c4a82, with their siblings. The connect landing check reads the Remote Path as navigation does. connectLandsOutsideOverlay classified it with normCryptScope, the anchor's trim, while navigation opens it with normLandingPath: `/Crypt ` read as inside an island at `/Crypt`, and the overlay unlocked on a folder outside it. It was the only landing read through the anchor's normalization; isWithinCryptScope already keeps the spaces of the folder shown. The href names a WebDAV resource. An entry's name and path come from its displayname when it has one, and a displayname now keeps its spaces: one that says the href's name with other spaces around it (` b.txt ` for `/dav/b.txt`) made a path that reached no file. Such a displayname takes the href's name. A displayname that names something else than the href is the name, as before. The benchmark asks where the session stands under the run's stop signal. Its two pwd calls before the scratch folder (a relative start path, and a relative prefix on Proton Drive) were the only provider calls of the run outside it: a pwd that never answered held the run past its profile timeout and its cancel. A `..` in a relative start folder is taken with the folder before it when the session's folder is compared: `docs/../team` stood in `/home/u/team` unseen, and the benchmark put its scratch folder under it as `docs/../team/aeroftp-bench`. A `..` with no folder before it still cannot be told and is kept. The Rust guard reads code only. Comments are left out of a statement (a `.trim()` named in a trailing comment failed the check, and a statement whose line ended in a comment ran into the next one), and string, raw string and char literals are read whole: a bracket inside one no longer moves the depth that decides where a statement ends, and `//` inside one is not a comment. The ImageKit refusal test uploads a name with a letter above U+FFFF. Session: L2 016retSy Signed-off-by: axpnet <45786925+axpnet@users.noreply.github.com> --- src-tauri/src/community_benchmark/engine.rs | 22 +- src-tauri/src/community_benchmark/tests.rs | 116 ++++++++--- src-tauri/src/providers/imagekit.rs | 7 + .../src/providers/path_resolution_guard.rs | 195 +++++++++++++++++- src-tauri/src/providers/webdav.rs | 83 ++++++-- src-tauri/src/util/start_folder.rs | 33 ++- src/utils/cryptScope.test.ts | 10 + src/utils/cryptScope.ts | 4 +- 8 files changed, 395 insertions(+), 75 deletions(-) diff --git a/src-tauri/src/community_benchmark/engine.rs b/src-tauri/src/community_benchmark/engine.rs index a2c78c18c..b085f5228 100644 --- a/src-tauri/src/community_benchmark/engine.rs +++ b/src-tauri/src/community_benchmark/engine.rs @@ -390,12 +390,17 @@ 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 under the - // start path. + // 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() }; @@ -407,10 +412,15 @@ pub async fn run( Some(prefix) => benchmark_remote_roots_from_prefix(prefix, &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 => benchmark_remote_roots( - &crate::util::start_folder::as_entered(initial_path, &mut *provider).await, - &report_id, - ), + 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/tests.rs b/src-tauri/src/community_benchmark/tests.rs index af4b1d9bf..402f06b57 100644 --- a/src-tauri/src/community_benchmark/tests.rs +++ b/src-tauri/src/community_benchmark/tests.rs @@ -633,6 +633,10 @@ mod engine_runs { 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, } @@ -649,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() @@ -681,6 +685,9 @@ mod engine_runs { Ok(out) } async fn pwd(&mut self) -> Result { + if self.0.lock().unwrap().hang_pwd { + std::future::pending::<()>().await; + } Ok(self .0 .lock() @@ -907,42 +914,83 @@ mod engine_runs { /// 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. + /// 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() { - 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()); + 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:?}"); } - let opts = options(BenchmarkLevel::Quick, "1K", None); - let outcome = run(provider, "team ", opts, &Silent).await.unwrap(); - assert!( - outcome.report.summary.errors.is_empty(), - "{:?}", - 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"); - assert!( - r.mkdirs - .iter() - .all(|dir| dir.starts_with('/') && !dir.contains("team /team ")), - "{:?}", - r.mkdirs - ); - assert!( - r.mkdirs - .iter() - .any(|dir| dir.starts_with("/home/u/team /aeroftp-bench/")), - "{:?}", - r.mkdirs - ); - assert!(r.files.is_empty(), "{:?}", r.files.keys()); - assert_eq!(r.dirs, BTreeSet::from(home)); } #[tokio::test] diff --git a/src-tauri/src/providers/imagekit.rs b/src-tauri/src/providers/imagekit.rs index 827491463..19c2557ac 100644 --- a/src-tauri/src/providers/imagekit.rs +++ b/src-tauri/src/providers/imagekit.rs @@ -2576,6 +2576,11 @@ mod tests { "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), ( @@ -2605,6 +2610,8 @@ mod tests { ); 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) diff --git a/src-tauri/src/providers/path_resolution_guard.rs b/src-tauri/src/providers/path_resolution_guard.rs index fa6cc4f60..8beaa077d 100644 --- a/src-tauri/src/providers/path_resolution_guard.rs +++ b/src-tauri/src/providers/path_resolution_guard.rs @@ -577,27 +577,27 @@ fn trims_whitespace(text: &str) -> bool { /// `.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. +/// 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 = line.trim_start(); - if code.starts_with("//") { + 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(' '); - for c in code.chars() { - match c { - '(' | '[' => depth += 1, - ')' | ']' => depth = depth.saturating_sub(1), - _ => {} - } - } - if code.ends_with([';', '{', '}']) || (code.ends_with(',') && depth == 0) { + // 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; } @@ -606,6 +606,120 @@ fn statements(source: &str) -> Vec<(usize, String)> { 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\ @@ -628,6 +742,65 @@ fn a_call_over_several_lines_is_one_statement() { ); } +/// 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 diff --git a/src-tauri/src/providers/webdav.rs b/src-tauri/src/providers/webdav.rs index 074005d52..9b8bff10e 100644 --- a/src-tauri/src/providers/webdav.rs +++ b/src-tauri/src/providers/webdav.rs @@ -3288,21 +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. - // As given, like the rest of the name: ` a` and - // `a ` are two other files than `a`, and a name - // of spaces is a name. XML writers indent around - // the element, never inside its text. - let name = if !displayname.is_empty() { - urlencoding::decode(&displayname) - .unwrap_or_else(|_| displayname.as_str().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 == ".." { @@ -7179,7 +7189,10 @@ mod tests { /// 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. + /// 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")) @@ -7224,6 +7237,42 @@ mod tests { + + /dav/b.txt + + + b.txt + + + + + + /dav/d.txt + + + + + + + + + /dav/%20/ + + + + + + + + + /dav/e7 + + + e.txt + + + + "#; let entries = provider.parse_propfind_response(xml, "/dav").unwrap(); @@ -7238,6 +7287,10 @@ mod tests { (" ", "/dav/ "), (" c.txt ", "/dav/ c.txt "), (" ", "/dav/ "), + ("b.txt", "/dav/b.txt"), + ("d.txt", "/dav/d.txt"), + (" ", "/dav/ "), + (" e.txt", "/dav/ e.txt"), ] ); } diff --git a/src-tauri/src/util/start_folder.rs b/src-tauri/src/util/start_folder.rs index 46c7bea91..54bb9c15f 100644 --- a/src-tauri/src/util/start_folder.rs +++ b/src-tauri/src/util/start_folder.rs @@ -17,16 +17,27 @@ 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. A base with `..` cannot be told from here and is kept. +/// 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 folders = |path: &str| -> Vec { - path.split('/') - .filter(|folder| !folder.is_empty() && *folder != ".") - .map(str::to_string) - .collect() - }; let (cwd, base) = (folders(cwd), folders(base)); - !base.is_empty() && !base.iter().any(|folder| folder == "..") && cwd.ends_with(&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 @@ -60,6 +71,9 @@ mod tests { ("/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:?}"); } @@ -67,6 +81,9 @@ mod tests { ("/mydocs", "docs"), ("/docs", "docs "), ("/docs", "../docs"), + ("/docs", "a/../../docs"), + ("/home/team", "docs/.. /team"), + ("/docs", "docs/.."), ("/docs", "."), ("/", ""), ] { diff --git a/src/utils/cryptScope.test.ts b/src/utils/cryptScope.test.ts index 9a5c6ec0a..e12ac66f3 100644 --- a/src/utils/cryptScope.test.ts +++ b/src/utils/cryptScope.test.ts @@ -43,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 95678a291..9b2df0285 100644 --- a/src/utils/cryptScope.ts +++ b/src/utils/cryptScope.ts @@ -56,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;