From 8ffc0f5ead8f5a152e58c1d8a32005a8b872cd8e Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Tue, 7 Jul 2026 15:31:06 +0200 Subject: [PATCH] fix(integrations/dav-server): improve WebDAV handling Normalize DavPath values before using them for comparisons against OpenDAL paths. OpenDAL normalizes operation paths internally, so the DAV integration compares paths in that same form when checking lister output and collection moves. Use directory-shaped paths for collection metadata, listing, creation, and deletion. Slashless collection requests now fall back to trailing slash paths. Support moving empty collections by creating the destination collection and deleting the source after a limited listing confirms the source is empty. This is needed so Windows Explorer can create a folder by renaming the initial New folder collection. Directory copies and non-empty directory moves still return NotImplemented. Gate DAV append requests on write_can_append. Honor create_new by using if-not-exists only when write_with_if_not_exists is advertised. Map more OpenDAL errors to WebDAV errors, including Unsupported, path-type errors, and conditional conflicts. Fixes #7881 --- integrations/dav-server/src/file.rs | 23 +++++- integrations/dav-server/src/fs.rs | 117 +++++++++++++++++++++------ integrations/dav-server/src/utils.rs | 10 ++- 3 files changed, 119 insertions(+), 31 deletions(-) diff --git a/integrations/dav-server/src/file.rs b/integrations/dav-server/src/file.rs index 209ac60fdcdb..cf34a93a1060 100644 --- a/integrations/dav-server/src/file.rs +++ b/integrations/dav-server/src/file.rs @@ -70,9 +70,26 @@ impl OpendalFile { .map_err(convert_error)?; State::Read(r) } else if options.write { - let w = op - .writer_with(&path) - .append(options.append) + let capability = op.info().capability(); + let mut writer = op.writer_with(&path); + + if options.append { + if !capability.write_can_append { + // DAV append requires backend support. + return Err(FsError::NotImplemented); + } + writer = writer.append(true); + } + + if options.create_new { + if !capability.write_with_if_not_exists { + // Conditional create requires backend support. + return Err(FsError::NotImplemented); + } + writer = writer.if_not_exists(true); + } + + let w = writer .await .map_err(convert_error)? .into_futures_async_write(); diff --git a/integrations/dav-server/src/fs.rs b/integrations/dav-server/src/fs.rs index 8edcf21b7f51..11d9edbb6025 100644 --- a/integrations/dav-server/src/fs.rs +++ b/integrations/dav-server/src/fs.rs @@ -23,7 +23,8 @@ use dav_server::fs::{DavFile, FsStream}; use dav_server::fs::{DavFileSystem, ReadDirMeta}; use futures::FutureExt; use futures::StreamExt; -use opendal_core::Operator; +use opendal_core::raw::normalize_path; +use opendal_core::{ErrorKind, Operator}; use std::path::Path; use super::dir::OpendalStream; @@ -68,7 +69,24 @@ impl OpendalFs { } fn fs_path(&self, path: &DavPath) -> Result { - String::from_utf8(path.as_bytes().to_vec()).map_err(|_| FsError::GeneralFailure) + String::from_utf8(path.as_bytes().to_vec()) + .map(|path| normalize_path(path.as_str())) + .map_err(|_| FsError::GeneralFailure) + } + + fn fs_dir_path(&self, path: &DavPath) -> Result { + Ok(Self::into_dir_path(self.fs_path(path)?)) + } + + fn into_dir_path(mut path: String) -> String { + if !Self::is_dir_path(&path) { + path.push('/'); + } + path + } + + fn is_dir_path(path: &str) -> bool { + path.is_empty() || path.ends_with('/') } } @@ -92,7 +110,7 @@ impl DavFileSystem for OpendalFs { _meta: ReadDirMeta, ) -> FsFuture<'a, FsStream>> { async move { - let path = self.fs_path(path)?; + let path = self.fs_dir_path(path)?; self.op .lister(path.as_str()) .await @@ -105,21 +123,26 @@ impl DavFileSystem for OpendalFs { fn metadata<'a>(&'a self, path: &'a DavPath) -> FsFuture<'a, Box> { async move { 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); - Ok(Box::new(webdav_metadata) as Box) + let metadata = match self.op.stat(path.as_str()).await { + Ok(metadata) => metadata, + Err(e) + if !Self::is_dir_path(&path) + && matches!(e.kind(), ErrorKind::NotFound | ErrorKind::IsADirectory) => + { + let path = Self::into_dir_path(path); + self.op.stat(path.as_str()).await.map_err(convert_error)? } - Err(e) => Err(convert_error(e)), - } + Err(e) => return Err(convert_error(e)), + }; + + Ok(Box::new(OpendalMetaData::new(metadata)) as Box) } .boxed() } fn create_dir<'a>(&'a self, path: &'a DavPath) -> FsFuture<'a, ()> { async move { - let path = self.fs_path(path)?; + let path = self.fs_dir_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. @@ -161,7 +184,11 @@ impl DavFileSystem for OpendalFs { } fn remove_dir<'a>(&'a self, path: &'a DavPath) -> FsFuture<'a, ()> { - self.remove_file(path) + async move { + let path = self.fs_dir_path(path)?; + self.op.delete(&path).await.map_err(convert_error) + } + .boxed() } fn remove_file<'a>(&'a self, path: &'a DavPath) -> FsFuture<'a, ()> { @@ -174,16 +201,54 @@ impl DavFileSystem for OpendalFs { fn rename<'a>(&'a self, from: &'a DavPath, to: &'a DavPath) -> FsFuture<'a, ()> { async move { - let from_path = from - .as_rel_ospath() - .to_str() - .ok_or(FsError::GeneralFailure)?; - let to_path = to.as_rel_ospath().to_str().ok_or(FsError::GeneralFailure)?; if from.is_collection() { - let _ = self.remove_file(to).await; + let from_path = self.fs_dir_path(from)?; + let to_path = self.fs_dir_path(to)?; + + if self + .op + .exists(to_path.as_str()) + .await + .map_err(convert_error)? + { + return Err(FsError::Exists); + } + + let mut lister = self + .op + .lister_with(from_path.as_str()) + .limit(2) + .await + .map_err(convert_error)?; + let mut found = false; + while let Some(entry) = lister.next().await { + let entry = entry.map_err(convert_error)?; + if entry.path() == from_path { + found = true; + } else { + // Only empty directory moves are supported. + return Err(FsError::NotImplemented); + } + } + if !found { + return Err(FsError::NotFound); + } + + self.op + .create_dir(to_path.as_str()) + .await + .map_err(convert_error)?; + return self + .op + .delete(from_path.as_str()) + .await + .map_err(convert_error); } + + let from_path = self.fs_path(from)?; + let to_path = self.fs_path(to)?; self.op - .rename(from_path, to_path) + .rename(from_path.as_str(), to_path.as_str()) .await .map_err(convert_error) } @@ -192,13 +257,15 @@ impl DavFileSystem for OpendalFs { fn copy<'a>(&'a self, from: &'a DavPath, to: &'a DavPath) -> FsFuture<'a, ()> { async move { - let from_path = from - .as_rel_ospath() - .to_str() - .ok_or(FsError::GeneralFailure)?; - let to_path = to.as_rel_ospath().to_str().ok_or(FsError::GeneralFailure)?; + if from.is_collection() || to.is_collection() { + // Directory copies are handled by dav-server. + return Err(FsError::NotImplemented); + } + + let from_path = self.fs_path(from)?; + let to_path = self.fs_path(to)?; self.op - .copy(from_path, to_path) + .copy(from_path.as_str(), to_path.as_str()) .await .map(|_| ()) .map_err(convert_error) diff --git a/integrations/dav-server/src/utils.rs b/integrations/dav-server/src/utils.rs index fbd45842998e..6a6ee4b82905 100644 --- a/integrations/dav-server/src/utils.rs +++ b/integrations/dav-server/src/utils.rs @@ -17,10 +17,14 @@ pub fn convert_error(opendal_error: opendal_core::Error) -> dav_server::fs::FsError { match opendal_error.kind() { - opendal_core::ErrorKind::AlreadyExists | opendal_core::ErrorKind::IsSameFile => { - dav_server::fs::FsError::Exists - } + opendal_core::ErrorKind::Unsupported => dav_server::fs::FsError::NotImplemented, opendal_core::ErrorKind::NotFound => dav_server::fs::FsError::NotFound, + opendal_core::ErrorKind::PermissionDenied + | opendal_core::ErrorKind::IsADirectory + | opendal_core::ErrorKind::NotADirectory => dav_server::fs::FsError::Forbidden, + opendal_core::ErrorKind::AlreadyExists + | opendal_core::ErrorKind::IsSameFile + | opendal_core::ErrorKind::ConditionNotMatch => dav_server::fs::FsError::Exists, _ => dav_server::fs::FsError::GeneralFailure, } }