refactor: extract confined_join

core/core/src/raw/path.rs

@@ -17,6 +17,7 @@
 
 use crate::*;
 use std::hash::{BuildHasher, Hasher};
+use std::path::{Component, Path, PathBuf};
 
 /// build_abs_path will build an absolute path with root.
 ///
@@ -147,6 +148,35 @@ pub fn normalize_root(v: &str) -> String {
     v
 }
 
+/// Join a path to a service's root. Useful for local filesystem services to
+/// reject `..` traversal in a key so path joins cannot escape `root`.
+///
+/// Services must confine operations within configured root. Local filesystem services
+/// could use this function to confine paths within configured root.
+///
+/// Expect `path` comes from `normalize_path` before calling this function.
+/// OpenDAL dispatches path to services after `normalize_path`, so services can
+/// safely assume that `path` is normalized.
+pub fn confined_join(root: &Path, path: &str) -> Result<PathBuf> {
+    let trimmed = path.trim_end_matches('/');
+
+    // `normalize_path` strips leading `/` and empty segments but
+    // intentionally does NOT resolve `.`/`..`, and `PathBuf::join` is purely
+    // lexical, so a key such as `../../etc/passwd` would otherwise escape the
+    // configured `root` at syscall time.
+    if Path::new(trimmed)
+        .components()
+        .any(|c| matches!(c, Component::ParentDir))
+    {
+        return Err(Error::new(
+            ErrorKind::NotFound,
+            "path escapes the configured root via `..`",
+        )
+        .with_context("path", path));
+    }
+    Ok(root.join(trimmed))
+}
+
 /// Get basename from path.
 pub fn get_basename(path: &str) -> &str {
     // Handle root case
@@ -380,6 +410,49 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_confined_join() {
+        let root = Path::new("/root");
+        let cases = vec![
+            ("root path", "/", root.to_path_buf()),
+            ("empty path", "", root.to_path_buf()),
+            ("current path", ".", root.to_path_buf()),
+            ("input file", "def", root.join("def")),
+            ("input nested file", "abc/def", root.join("abc/def")),
+            ("input dir", "def/", root.join("def")),
+            ("input nested dir", "abc/def/", root.join("abc/def")),
+            (". in path", "dir/file.txt", root.join("dir/file.txt")),
+            (
+                ". in path with dir",
+                "dir/./file.txt",
+                root.join("dir/file.txt"),
+            ),
+            (".. in path", "a..b", root.join("a..b")),
+        ];
+
+        for (name, input, expect) in cases {
+            let actual = confined_join(root, input).expect(name);
+            assert_eq!(actual, expect, "{name}");
+        }
+    }
+
+    #[test]
+    fn test_confined_join_rejects_parent_dir() {
+        let root = Path::new("/root");
+        let cases = vec![
+            ("parent dir", ".."),
+            ("parent dir file", "../def"),
+            ("nested parent dir", "abc/../def"),
+            ("trailing parent dir", "abc/.."),
+            ("trailing parent dir slash", "abc/../"),
+        ];
+
+        for (name, input) in cases {
+            let err = confined_join(root, input).expect_err(name);
+            assert_eq!(err.kind(), ErrorKind::NotFound, "{name}");
+        }
+    }
+
     #[test]
     fn test_build_rel_path() {
         let cases = vec![

core/services/compfs/src/backend.rs

@@ -136,7 +136,7 @@ impl Service for CompfsBackend {
         path: &str,
         _: OpCreateDir,
     ) -> Result<RpCreateDir> {
-        let path = self.core.prepare_path(path)?;
+        let path = self.core.root_join(path)?;
 
         self.core
             .exec(move || async move { compio::fs::create_dir_all(path).await })
@@ -146,7 +146,7 @@ impl Service for CompfsBackend {
     }
 
     async fn stat(&self, _ctx: &OperationContext, path: &str, _: OpStat) -> Result<RpStat> {
-        let path = self.core.prepare_path(path)?;
+        let path = self.core.root_join(path)?;
         let meta = self
             .core
             .exec(move || async move { compio::fs::metadata(path).await })
@@ -185,8 +185,8 @@ impl Service for CompfsBackend {
         _opts: OpCopier,
     ) -> Result<Self::Copier> {
         let core = self.core.clone();
-        let from = self.core.prepare_path(from)?;
-        let to = self.core.prepare_path(to)?;
+        let from = self.core.root_join(from)?;
+        let to = self.core.root_join(to)?;
 
         Ok(oio::OneShotCopier::new(async move {
             core.exec(move || async move {
@@ -217,8 +217,8 @@ impl Service for CompfsBackend {
         to: &str,
         _: OpRename,
     ) -> Result<RpRename> {
-        let from = self.core.prepare_path(from)?;
-        let to = self.core.prepare_path(to)?;
+        let from = self.core.root_join(from)?;
+        let to = self.core.root_join(to)?;
 
         self.core
             .exec(move || async move {
@@ -234,22 +234,22 @@ impl Service for CompfsBackend {
     fn read(&self, _ctx: &OperationContext, path: &str, _: OpRead) -> Result<Self::Reader> {
         Ok(oio::PositionReader::new(CompfsReader::new(
             self.core.clone(),
-            self.core.prepare_path(path)?,
+            self.core.root_join(path)?,
         )))
     }
 
     fn write(&self, _ctx: &OperationContext, path: &str, args: OpWrite) -> Result<Self::Writer> {
         Ok(CompfsLazyWriter::new(
             self.core.clone(),
-            self.core.prepare_path(path)?,
+            self.core.root_join(path)?,
             args,
         ))
     }
 
     fn list(&self, _ctx: &OperationContext, path: &str, _: OpList) -> Result<Self::Lister> {
         Ok(CompfsLazyLister::new(
             self.core.clone(),
-            self.core.prepare_path(path)?,
+            self.core.root_join(path)?,
         ))
     }
 

core/services/compfs/src/core.rs

@@ -16,7 +16,6 @@
 // under the License.
 
 use std::future::Future;
-use std::path::Path;
 use std::path::PathBuf;
 
 use compio::buf::{IoBuf, IoVectoredBuf};
@@ -64,23 +63,10 @@ pub(super) struct CompfsCore {
 }
 
 impl CompfsCore {
-    /// Reject `..` traversal in a key so it cannot escape `root`, matching the
-    /// `fs` backend (#7684). Confinement lives here because `normalize_path`
-    /// deliberately leaves `.`/`..` unresolved (RFC 0112).
-    pub fn prepare_path(&self, path: &str) -> Result<PathBuf> {
-        use std::path::Component;
-        let trimmed = path.trim_end_matches('/');
-        if Path::new(trimmed)
-            .components()
-            .any(|c| matches!(c, Component::ParentDir))
-        {
-            return Err(Error::new(
-                ErrorKind::NotFound,
-                "path escapes the configured root via `..`",
-            )
-            .with_context("path", path));
-        }
-        Ok(self.root.join(trimmed))
+    /// Join a path to root safely. Rejects `..` traversal beyond root.
+    #[inline]
+    pub fn root_join(&self, path: &str) -> Result<PathBuf> {
+        confined_join(&self.root, path)
     }
 
     pub async fn exec<Fn, Fut, R>(&self, f: Fn) -> opendal_core::Result<R>
@@ -152,50 +138,4 @@ mod tests {
         assert_eq!(buf.total_len(), len);
         assert_eq!(collected, bytes);
     }
-
-    fn new_test_core() -> CompfsCore {
-        CompfsCore {
-            info: ServiceInfo::new("compfs", "", ""),
-            capability: Capability::default(),
-            root: PathBuf::from("/data/root"),
-            dispatcher: Dispatcher::new().unwrap(),
-            buf_pool: oio::PooledBuf::new(16),
-        }
-    }
-
-    #[test]
-    fn test_prepare_path_rejects_parent_dir() {
-        let core = new_test_core();
-        for key in ["../etc/passwd", "../../etc/passwd", "a/../../b", "a/.."] {
-            let err = core.prepare_path(key).unwrap_err();
-            assert_eq!(
-                err.kind(),
-                ErrorKind::NotFound,
-                "key should be rejected: {key}"
-            );
-        }
-    }
-
-    #[test]
-    fn test_prepare_path_allows_normal_keys() {
-        let core = new_test_core();
-        // Normal keys, `.` (CurDir), and trailing slashes resolve unchanged.
-        assert_eq!(
-            core.prepare_path("a/b.txt").unwrap(),
-            PathBuf::from("/data/root/a/b.txt")
-        );
-        assert_eq!(
-            core.prepare_path("a/b/").unwrap(),
-            PathBuf::from("/data/root/a/b")
-        );
-        assert_eq!(
-            core.prepare_path("./a/b").unwrap(),
-            PathBuf::from("/data/root/a/b")
-        );
-        // A key containing `..` only as a substring of a name is not a traversal.
-        assert_eq!(
-            core.prepare_path("a..b").unwrap(),
-            PathBuf::from("/data/root/a..b")
-        );
-    }
 }

core/services/compfs/src/deleter.rs

@@ -34,12 +34,12 @@ impl CompfsDeleter {
 impl oio::OneShotDelete for CompfsDeleter {
     async fn delete_once(&self, path: String, _: OpDelete) -> Result<()> {
         let res = if path.ends_with('/') {
-            let path = self.core.prepare_path(&path)?;
+            let path = self.core.root_join(&path)?;
             self.core
                 .exec(move || async move { compio::fs::remove_dir(path).await })
                 .await
         } else {
-            let path = self.core.prepare_path(&path)?;
+            let path = self.core.root_join(&path)?;
             self.core
                 .exec(move || async move { compio::fs::remove_file(path).await })
                 .await

core/services/fs/src/core.rs

@@ -33,40 +33,15 @@ pub struct FsCore {
 }
 
 impl FsCore {
-    /// Join a caller-supplied key onto a base directory while keeping the result
-    /// confined to that base.
-    ///
-    /// `normalize_path` (opendal-core) strips leading `/` and empty segments but
-    /// intentionally does NOT resolve `.`/`..`, and `PathBuf::join` is purely
-    /// lexical, so a key such as `../../etc/passwd` would otherwise escape the
-    /// configured `root` at syscall time. The fs backend documents that "all
-    /// operations will happen under this root", so we reject any key whose
-    /// components include a `..` (parent-dir) traversal.
-    pub fn confined_join(base: &Path, path: &str) -> Result<PathBuf> {
-        use std::path::Component;
-        let trimmed = path.trim_end_matches('/');
-        if Path::new(trimmed)
-            .components()
-            .any(|c| matches!(c, Component::ParentDir))
-        {
-            return Err(Error::new(
-                ErrorKind::NotFound,
-                "path escapes the configured root via `..`",
-            )
-            .with_context("path", path));
-        }
-        Ok(base.join(trimmed))
-    }
-
-    /// Join a caller-supplied key onto `self.root`, rejecting `..` traversal.
+    /// Join a path to root safely. Rejects `..` traversal beyond root.
     #[inline]
     pub fn root_join(&self, path: &str) -> Result<PathBuf> {
-        Self::confined_join(&self.root, path)
+        confined_join(&self.root, path)
     }
 
     // Build write path and ensure the parent dirs created
     pub async fn ensure_write_abs_path(&self, parent: &Path, path: &str) -> Result<PathBuf> {
-        let p = Self::confined_join(parent, path)?;
+        let p = confined_join(parent, path)?;
 
         // Create dir before write path.
         //

core/services/monoiofs/src/backend.rs

@@ -107,7 +107,7 @@ impl Service for MonoiofsBackend {
     }
 
     async fn stat(&self, _ctx: &OperationContext, path: &str, _args: OpStat) -> Result<RpStat> {
-        let path = self.core.prepare_path(path)?;
+        let path = self.core.root_join(path)?;
         let meta = self
             .core
             .dispatch(move || monoio::fs::metadata(path))
@@ -128,7 +128,7 @@ impl Service for MonoiofsBackend {
         Ok(RpStat::new(m))
     }
     fn read(&self, _ctx: &OperationContext, path: &str, _args: OpRead) -> Result<Self::Reader> {
-        let path = self.core.prepare_path(path)?;
+        let path = self.core.root_join(path)?;
         Ok(oio::PositionReader::new(MonoiofsPositionReader::new(
             self.core.clone(),
             path,
@@ -163,7 +163,7 @@ impl Service for MonoiofsBackend {
         to: &str,
         _args: OpRename,
     ) -> Result<RpRename> {
-        let from = self.core.prepare_path(from)?;
+        let from = self.core.root_join(from)?;
         // ensure file exists
         self.core
             .dispatch({
@@ -186,7 +186,7 @@ impl Service for MonoiofsBackend {
         path: &str,
         _args: OpCreateDir,
     ) -> Result<RpCreateDir> {
-        let path = self.core.prepare_path(path)?;
+        let path = self.core.root_join(path)?;
         self.core
             .dispatch(move || monoio::fs::create_dir_all(path))
             .await
@@ -203,7 +203,7 @@ impl Service for MonoiofsBackend {
         _opts: OpCopier,
     ) -> Result<Self::Copier> {
         let core = self.core.clone();
-        let from = self.core.prepare_path(from)?;
+        let from = self.core.root_join(from)?;
         let to = to.to_string();
 
         let copier = oio::OneShotCopier::new(async move {