You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by te...@apache.org on 2026/04/27 15:52:34 UTC

(opendal) 01/01: Fix root and FTP temp path normalization

This is an automated email from the ASF dual-hosted git repository.

TennyZhuang pushed a commit to branch avery/rust-path-normalization-ftp-temp
in repository https://gitbox.apache.org/repos/asf/opendal.git

commit 35b4b3ad50087cf4aae6a06596c8c07134c54847
Author: avery-codex-rust-lead-871065 <av...@users.noreply.github.com>
AuthorDate: Mon Apr 27 23:52:18 2026 +0800

    Fix root and FTP temp path normalization
---
 core/core/src/raw/path.rs        |  2 ++
 core/services/ftp/src/backend.rs | 41 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/core/core/src/raw/path.rs b/core/core/src/raw/path.rs
index 55733f47c..0ea35e449 100644
--- a/core/core/src/raw/path.rs
+++ b/core/core/src/raw/path.rs
@@ -134,6 +134,7 @@ pub fn normalize_path(path: &str) -> String {
 /// Finally, we will get path like `/path/to/root/`.
 pub fn normalize_root(v: &str) -> String {
     let mut v = v
+        .trim()
         .split('/')
         .filter(|v| !v.is_empty())
         .collect::<Vec<&str>>()
@@ -303,6 +304,7 @@ mod tests {
             ("abs file path with extra /", "///abc/def", "/abc/def/"),
             ("abs dir path with extra /", "///abc/def/", "/abc/def/"),
             ("dir path contains ///", "abc///def///", "/abc/def/"),
+            ("dir path with whitespace", "  abc/def/  ", "/abc/def/"),
         ];
 
         for (name, input, expect) in cases {
diff --git a/core/services/ftp/src/backend.rs b/core/services/ftp/src/backend.rs
index 048c6b220..c0742af6f 100644
--- a/core/services/ftp/src/backend.rs
+++ b/core/services/ftp/src/backend.rs
@@ -278,7 +278,7 @@ impl Access for FtpBackend {
             }
         }
 
-        let tmp_path = (!op.append()).then_some(build_tmp_path_of(path));
+        let tmp_path = (!op.append()).then_some(build_ftp_tmp_path(path));
         let w = FtpWriter::new(ftp_stream, path.to_string(), tmp_path);
 
         Ok((RpWrite::new(), w))
@@ -332,9 +332,20 @@ impl FtpBackend {
     }
 }
 
+fn build_ftp_tmp_path(path: &str) -> String {
+    let parent = get_parent(path);
+    let tmp_name = build_tmp_path_of(path);
+
+    if parent == "/" {
+        tmp_name
+    } else {
+        format!("{parent}{tmp_name}")
+    }
+}
+
 #[cfg(test)]
 mod build_test {
-    use super::FtpBuilder;
+    use super::{FtpBuilder, build_ftp_tmp_path};
     use crate::FtpConfig;
     use opendal_core::*;
 
@@ -396,4 +407,30 @@ mod build_test {
         assert_eq!(cfg.user.as_deref(), Some("alice"));
         assert_eq!(cfg.password.as_deref(), Some("secret"));
     }
+
+    #[test]
+    fn test_build_ftp_tmp_path() {
+        let cases = vec![
+            ("root file", "example.txt", "example.txt."),
+            ("nested file", "folder/example.txt", "folder/example.txt."),
+            (
+                "deep nested file",
+                "folder/sub/example.txt",
+                "folder/sub/example.txt.",
+            ),
+        ];
+
+        for (name, path, expect_starts_with) in cases {
+            let actual = build_ftp_tmp_path(path);
+            assert!(
+                actual.starts_with(expect_starts_with),
+                "{name}: got `{actual}`, but expect `{expect_starts_with}`"
+            );
+            assert_eq!(
+                actual.len(),
+                expect_starts_with.len() + 8,
+                "{name}: got `{actual}`, but expect `{expect_starts_with}`"
+            );
+        }
+    }
 }