Repository: apache/opendal (language: rust)

Issue:
bug: Integration dav-server-opendalfs can not handle encoded path correctly
### Describe the bug

The currently implementation of dav-server-opendalfs will internally encode the url passed in by dav-server into %+ hex format and pass it to the dependent opendal Operator, thus accessing the wrong path.

For example, accessing '%CE%B1%CE%BB%CF%86%CE%AC%CE%B2%CE%B7%C' in dav-server-opendalfs should access decoded 'αλφάβητο' in underlying opendal service, however it actually access the encoded path currently.


### Steps to Reproduce

code to reproduce:
```rust
use dav_server::fakels::FakeLs;
use dav_server::DavHandler;
use dav_server_opendalfs::OpendalFs;
use hyper::body::Bytes;
use hyper::http;
use opendal::layers::LoggingLayer;
use opendal::{services, Operator};
use std::fs;
use tempfile::TempDir;

#[tokio::main]
async fn main() {
    env_logger::builder().parse_filters("debug").init();
    let tmp_dir = TempDir::with_prefix("test").unwrap();
    let builder = services::Fs::default().root(tmp_dir.path().to_str().unwrap());
    let op = Operator::new(builder)
        .unwrap()
        .layer(LoggingLayer::default())
        .finish();
    let webdavfs = OpendalFs::new(op);
    let dav_server = DavHandler::builder()
        .filesystem(webdavfs)
        .locksystem(FakeLs::new())
        .build_handler();
    // http://localhost:8080/test_αλφάβητο/
    let url_encoded = "http://localhost:8080/test_%CE%B1%CE%BB%CF%86%CE%AC%CE%B2%CE%B7%C/";
    let req = http::request::Request::builder()
        .method("MKCOL")
        .uri(url_encoded)
        .body(http_body_util::Empty::<Bytes>::new())
        .unwrap();
    dav_server.handle(req).await;
    let entries = fs::read_dir(tmp_dir.path())
        .unwrap()
        .collect::<Result<Vec<_>, _>>()
        .unwrap();
    println!("{:?}", entries);
    assert!(entries
        .iter()
        .any(|entry| entry.file_name() == "test_αλφάβητο"));
}
```

### Expected Behavior

dav-server-opendalfs should handle encoded path correctly

### Additional Context

_No response_

### Are you willing to submit a PR to fix this bug?

- [x] Yes, I would like to submit a PR.


Discussion / hints:

Fix patch:
diff --git a/integrations/dav-server/src/file.rs b/integrations/dav-server/src/file.rs
index d20a32c30e39..7c344552ecea 100644
--- a/integrations/dav-server/src/file.rs
+++ b/integrations/dav-server/src/file.rs
@@ -19,7 +19,6 @@ use std::fmt::{Debug, Formatter};
 use std::io::SeekFrom;
 
 use bytes::{Buf, Bytes, BytesMut};
-use dav_server::davpath::DavPath;
 use dav_server::fs::{DavFile, OpenOptions};
 use dav_server::fs::{DavMetaData, FsResult};
 use dav_server::fs::{FsError, FsFuture};
@@ -33,7 +32,7 @@ use super::utils::*;
 /// OpendalFile is a `DavFile` implementation for opendal.
 pub struct OpendalFile {
     op: Operator,
-    path: DavPath,
+    path: String,
     state: State,
     buf: BytesMut,
 }
@@ -60,10 +59,10 @@ enum State {
 
 impl OpendalFile {
     /// Create a new opendal file.
-    pub async fn open(op: Operator, path: DavPath, options: OpenOptions) -> FsResult<Self> {
+    pub async fn open(op: Operator, path: String, options: OpenOptions) -> FsResult<Self> {
         let state = if options.read {
             let r = op
-                .reader(path.as_url_string().as_str())
+                .reader(&path)
                 .await
                 .map_err(convert_error)?
                 .into_futures_async_read(..)
@@ -72,7 +71,7 @@ impl OpendalFile {
             State::Read(r)
         } else if options.write {
             let w = op
-                .writer_with(path.as_url_string().as_str())
+                .writer_with(&path)
                 .append(options.append)
                 .await
                 .map_err(convert_error)?
@@ -95,7 +94,7 @@ impl DavFile for OpendalFile {
     fn metadata(&mut self) -> FsFuture<Box<dyn DavMetaData>> {
         async move {
             self.op
-                .stat(self.path.as_url_string().as_str())
+                .stat(&self.path)
                 .await
                 .map(|opendal_metadata| {
                     Box::new(OpendalMetaData::new(opendal_metadata)) as Box<dyn DavMetaData>
diff --git a/integrations/dav-server/src/fs.rs b/integrations/dav-server/src/fs.rs
index 449b562c1186..49dc3a086b09 100644
--- a/integrations/dav-server/src/fs.rs
+++ b/integrations/dav-server/src/fs.rs
@@ -66,6 +66,10 @@ impl OpendalFs {
     pub fn new(op: Operator) -> Box<OpendalFs> {
         Box::new(OpendalFs { op })
     }
+
+    fn fs_path(&self, path: &DavPath) -> Result<String, FsError> {
+        String::from_utf8(path.as_bytes().to_vec()).map_err(|_| FsError::GeneralFailure)
+    }
 }
 
 impl DavFileSystem for OpendalFs {
@@ -75,7 +79,8 @@ impl DavFileSystem for OpendalFs {
         options: dav_server::fs::OpenOptions,
     ) -> FsFuture<'a, Box<dyn DavFile>> {
         async move {
-            let file = OpendalFile::open(self.op.clone(), path.clone(), options).await?;
+            let path = self.fs_path(path)?;
+            let file = OpendalFile::open(self.op.clone(), path, options).await?;
             Ok(Box::new(file) as Box<dyn DavFile>)
         }
         .boxed()
@@ -87,7 +92,7 @@ impl DavFileSystem for OpendalFs {
         _meta: ReadDirMeta,
     ) -> FsFuture<'a, FsStream<Box<dyn DavDirEntry>>> {
         async move {
-            let path = path.as_url_string();
+            let path = self.fs_path(path)?;
             self.op
                 .lister(path.as_str())
                 .await
@@ -99,7 +104,8 @@ impl DavFileSystem for OpendalFs {
 
     fn metadata<'a>(&'a self, path: &'a DavPath) -> FsFuture<'a, Box<dyn DavMetaData>> {
         async move {
-            let opendal_metadata = self.op.stat(path.as_url_string().as_str()).await;
+            let path = self.fs_path(path)?;
+            let opendal_metadata = self.op.stat(path.as_str()).await;
             match opendal_metadata {
                 Ok(metadata) => {
                     let webdav_metadata = OpendalMetaData::new(metadata);
@@ -113,7 +119,7 @@ impl DavFileSystem for OpendalFs {
 
     fn create_dir<'a>(&'a self, path: &'a DavPath) -> FsFuture<'a, ()> {
         async move {
-            let path = path.as_url_string();
+            let path = self.fs_path(path)?;
 
             // check if the parent path is exist.
             // During MKCOL processing, a server MUST make the Request-URI a member of its parent collection, unless the Request-URI is "/".  If no such ancestor exists, the method MUST fail.
@@ -156,10 +162,8 @@ impl DavFileSystem for OpendalFs {
 
     fn remove_file<'a>(&'a self, path: &'a DavPath) -> FsFuture<'a, ()> {
         async move {
-            self.op
-                .delete(path.as_url_string().as_str())
-                .await
-                .map_err(convert_error)
+            let path = self.fs_path(path)?;
+            self.op.delete(&path).await.map_err(convert_error)
         }
         .boxed()
     }


Test patch:
diff --git a/integrations/dav-server/tests/test.rs b/integrations/dav-server/tests/test.rs
index 62c113e6cae1..44d03708f2ed 100644
--- a/integrations/dav-server/tests/test.rs
+++ b/integrations/dav-server/tests/test.rs
@@ -16,11 +16,16 @@
 // under the License.
 
 use anyhow::Result;
+use bytes::Bytes;
 use dav_server::davpath::DavPath;
-use dav_server::fs::DavFileSystem;
+use dav_server::fs::OpenOptions;
+use dav_server::fs::{DavFileSystem, ReadDirMeta};
 use dav_server_opendalfs::OpendalFs;
+use futures::StreamExt;
 use opendal::services::Fs;
 use opendal::Operator;
+use std::fs;
+use std::path::Path;
 
 #[tokio::test]
 async fn test() -> Result<()> {
@@ -38,3 +43,154 @@ async fn test() -> Result<()> {
 
     Ok(())
 }
+
+fn setup_temp(path: &str) -> Box<OpendalFs> {
+    let _ = fs::remove_dir_all(path);
+    let builder = Fs::default().root(path);
+    let op = Operator::new(builder).unwrap().finish();
+    OpendalFs::new(op)
+}
+
+const TEST_PATH_ENCODED_1: &str = "test_%CE%B1%CE%BB%CF%86%CE%AC";
+const TEST_PATH_DECODED_1: &str = "test_αλφά";
+const TEST_PATH_ENCODED_2: &str = "test_%CE%B2%CE%B7%CF%84%CE%BF";
+const TEST_PATH_DECODED_2: &str = "test_βητο";
+
+#[tokio::test]
+async fn test_create_dir_metadata() {
+    const TMP_PATH: &str = "/tmp/test_create_dir_metadata";
+    let webdavfs = setup_temp(TMP_PATH);
+    webdavfs
+        .create_dir(&DavPath::new(&format!("/{TEST_PATH_ENCODED_1}/")).unwrap())
+        .await
+        .unwrap();
+    assert!(fs::exists(Path::new(TMP_PATH).join(TEST_PATH_DECODED_1)).unwrap());
+    assert!(!fs::exists(Path::new(TMP_PATH).join(TEST_PATH_ENCODED_1)).unwrap());
+
+    webdavfs
+        .metadata(&DavPath::new(&format!("/{TEST_PATH_ENCODED_1}/")).unwrap())
+        .await
+        .unwrap();
+    fs::remove_dir_all(TMP_PATH).unwrap();
+}
+
+#[tokio::test]
+async fn test_remove_dir() {
+    const TMP_PATH: &str = "/tmp/test_remove_dir";
+    let webdavfs = setup_temp(TMP_PATH);
+    webdavfs
+        .create_dir(&DavPath::new(&format!("/{TEST_PATH_ENCODED_1}/")).unwrap())
+        .await
+        .unwrap();
+    webdavfs
+        .remove_dir(&DavPath::new(&format!("/{TEST_PATH_ENCODED_1}/")).unwrap())
+        .await
+        .unwrap();
+    assert!(!fs::exists(Path::new(TMP_PATH).join(TEST_PATH_DECODED_1)).unwrap());
+    assert!(!fs::exists(Path::new(TMP_PATH).join(TEST_PATH_ENCODED_1)).unwrap());
+    fs::remove_dir_all("/tmp/test_remove_dir").unwrap();
+}
+#[tokio::test]
+async fn test_file() {
+    const TMP_PATH: &str = "/tmp/test_file";
+    let webdavfs = setup_temp(TMP_PATH);
+
+    let mut f1 = webdavfs
+        .open(
+            &DavPath::new(&format!("/{TEST_PATH_ENCODED_1}")).unwrap(),
+            OpenOptions {
+                write: true,
+                create_new: true,
+                ..OpenOptions::default()
+            },
+        )
+        .await
+        .unwrap();
+    f1.write_buf(Box::new(Bytes::from("test"))).await.unwrap();
+    f1.flush().await.unwrap();
+    drop(f1);
+    assert!(fs::exists(Path::new(TMP_PATH).join(TEST_PATH_DECODED_1)).unwrap());
+
+    let mut f1 = webdavfs
+        .open(
+            &DavPath::new(&format!("/{TEST_PATH_ENCODED_1}")).unwrap(),
+            OpenOptions {
+                read: true,
+                ..OpenOptions::default()
+            },
+        )
+        .await
+        .unwrap();
+    assert_eq!(f1.read_bytes(4).await.unwrap(), Bytes::from("test"));
+    drop(f1);
+
+    webdavfs
+        .rename(
+            &DavPath::new(&format!("/{TEST_PATH_ENCODED_1}")).unwrap(),
+            &DavPath::new(&format!("/{TEST_PATH_ENCODED_2}")).unwrap(),
+        )
+        .await
+        .unwrap();
+    assert!(!fs::exists(Path::new(TMP_PATH).join(TEST_PATH_DECODED_1)).unwrap());
+    assert!(fs::exists(Path::new(TMP_PATH).join(TEST_PATH_DECODED_2)).unwrap());
+
+    webdavfs
+        .copy(
+            &DavPath::new(&format!("/{TEST_PATH_ENCODED_2}")).unwrap(),
+            &DavPath::new(&format!("/{TEST_PATH_ENCODED_1}")).unwrap(),
+        )
+        .await
+        .unwrap();
+    assert!(fs::exists(Path::new(TMP_PATH).join(TEST_PATH_DECODED_1)).unwrap());
+    assert!(fs::exists(Path::new(TMP_PATH).join(TEST_PATH_DECODED_2)).unwrap());
+
+    webdavfs
+        .remove_file(&DavPath::new(&format!("/{TEST_PATH_ENCODED_1}")).unwrap())
+        .await
+        .unwrap();
+    assert!(!fs::exists(Path::new(TMP_PATH).join(TEST_PATH_DECODED_1)).unwrap());
+    assert!(fs::exists(Path::new(TMP_PATH).join(TEST_PATH_DECODED_2)).unwrap());
+
+    fs::remove_dir_all(TMP_PATH).unwrap();
+}
+
+#[tokio::test]
+async fn test_read_dir() {
+    const TMP_PATH: &str = "/tmp/test_read_dir";
+    let webdavfs = setup_temp(TMP_PATH);
+    webdavfs
+        .create_dir(&DavPath::new(&format!("/{TEST_PATH_ENCODED_1}/")).unwrap())
+        .await
+        .unwrap();
+    webdavfs
+        .create_dir(
+            &DavPath::new(&format!("/{TEST_PATH_ENCODED_1}/{TEST_PATH_ENCODED_1}/")).unwrap(),
+        )
+        .await
+        .unwrap();
+    webdavfs
+        .create_dir(
+            &DavPath::new(&format!("/{TEST_PATH_ENCODED_1}/{TEST_PATH_ENCODED_2}/")).unwrap(),
+        )
+        .await
+        .unwrap();
+    let entries = webdavfs
+        .read_dir(
+            &DavPath::new(&format!("/{TEST_PATH_ENCODED_1}/")).unwrap(),
+            ReadDirMeta::None,
+        )
+        .await
+        .unwrap();
+    let entries = entries
+        .collect::<Vec<_>>()
+        .await
+        .iter()
+        .map(|entry| String::from_utf8(entry.as_ref().unwrap().name()).unwrap())
+        .collect::<Vec<_>>();
+    println!("{:?}", entries);
+    assert_eq!(entries.len(), 2);
+    assert!(entries.contains(&format!("{TEST_PATH_DECODED_1}/")));
+    assert!(entries.contains(&format!("{TEST_PATH_DECODED_2}/")));
+
+    fs::remove_dir_all(TMP_PATH).unwrap();
+}