cjw/Oxicloud
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

fix(webdav): achieve 100% litmus compliance (59/59 tests)

Browse Source 
Complete RFC 4918 litmus test suite compliance (basic 16/16,
copymove 13/13, props 30/30) by fixing six categories of failures:

- PropPatchOp: parse PROPPATCH in document order (RFC 4918 §9.2)
  so that remove-then-set and set-then-remove yield different results
- Null namespace: resolve_name now correctly maps xmlns="" to empty-ns
  QualifiedName instead of falling through to DAV: namespace
- Unicode chars: handle quick-xml 0.39's Event::GeneralRef for character
  references (𐀀) — the parser emits these as GeneralRef, not Text
- MOVE preserves dead props: call rename_resource on MOVE, clearing
  stale destination data even when source has no registered properties
- Malformed PROPFIND: return 400 when body lacks a complete <propfind>
  element (test 2: bare <foo>, test 3: invalid xmlns:prefix="" binding)
- PROPPATCH document order: process ops via Vec<PropPatchOp> instead of
  separate (sets, removes) to honour interleaved remove/set sequences
This commit is contained in:
M.Schmidt
2026-06-30 08:58:13 +02:00
parent 43cf4a2b2b
commit 6ca6649e50
4 changed files with 210 additions and 115 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/interfaces/api/handlers/caldav_handler.rs
+21 -16
View File
@@ -850,11 +850,10 @@ async fn handle_proppatch(
.await
.map_err(|e| AppError::bad_request(format!("Failed to read request body: {}", e)))?;
let (props_to_set, props_to_remove) =
crate::application::adapters::webdav_adapter::WebDavAdapter::parse_proppatch(
body_bytes.reader(),
)
.map_err(|e| AppError::bad_request(format!("Failed to parse PROPPATCH: {}", e)))?;
let ops = crate::application::adapters::webdav_adapter::WebDavAdapter::parse_proppatch(
body_bytes.reader(),
)
.map_err(|e| AppError::bad_request(format!("Failed to parse PROPPATCH: {}", e)))?;
let effective_path = strip_username_prefix(path);
let calendar_id = effective_path.split('/').next().unwrap_or(effective_path);
@@ -870,12 +869,14 @@ async fn handle_proppatch(
is_public: None,
};
for prop in &props_to_set {
match prop.name.name.as_str() {
"displayname" => update.name = Some(prop.value.clone().unwrap_or_default()),
"calendar-description" => update.description = prop.value.clone(),
"calendar-color" => update.color = prop.value.clone(),
_ => {}
for op in &ops {
if let crate::application::adapters::webdav_adapter::PropPatchOp::Set(prop) = op {
match prop.name.name.as_str() {
"displayname" => update.name = Some(prop.value.clone().unwrap_or_default()),
"calendar-description" => update.description = prop.value.clone(),
"calendar-color" => update.color = prop.value.clone(),
_ => {}
}
}
}
@@ -887,11 +888,15 @@ async fn handle_proppatch(
}
let mut results = Vec::new();
for prop in &props_to_set {
results.push((&prop.name, true));
}
for prop in &props_to_remove {
results.push((prop, true));
for op in &ops {
match op {
crate::application::adapters::webdav_adapter::PropPatchOp::Set(prop) => {
results.push((&prop.name, true));
}
crate::application::adapters::webdav_adapter::PropPatchOp::Remove(name) => {
results.push((name, true));
}
}
}
let href = format!("/caldav/{}", path);
src/interfaces/api/handlers/carddav_handler.rs
+21 -16
View File
@@ -733,11 +733,10 @@ async fn handle_proppatch(
.await
.map_err(|e| AppError::bad_request(format!("Failed to read request body: {}", e)))?;
let (props_to_set, props_to_remove) =
crate::application::adapters::webdav_adapter::WebDavAdapter::parse_proppatch(
body_bytes.reader(),
)
.map_err(|e| AppError::bad_request(format!("Failed to parse PROPPATCH: {}", e)))?;
let ops = crate::application::adapters::webdav_adapter::WebDavAdapter::parse_proppatch(
body_bytes.reader(),
)
.map_err(|e| AppError::bad_request(format!("Failed to parse PROPPATCH: {}", e)))?;
let effective_path = strip_username_prefix(path);
let address_book_id = effective_path.split('/').next().unwrap_or(effective_path);
@@ -754,12 +753,14 @@ async fn handle_proppatch(
user_id: user.id.to_string(),
};
for prop in &props_to_set {
match prop.name.name.as_str() {
"displayname" => update.name = Some(prop.value.clone().unwrap_or_default()),
"addressbook-description" => update.description = prop.value.clone(),
"calendar-color" | "addressbook-color" => update.color = prop.value.clone(),
_ => {}
for op in &ops {
if let crate::application::adapters::webdav_adapter::PropPatchOp::Set(prop) = op {
match prop.name.name.as_str() {
"displayname" => update.name = Some(prop.value.clone().unwrap_or_default()),
"addressbook-description" => update.description = prop.value.clone(),
"calendar-color" | "addressbook-color" => update.color = prop.value.clone(),
_ => {}
}
}
}
@@ -773,11 +774,15 @@ async fn handle_proppatch(
}
let mut results = Vec::new();
for prop in &props_to_set {
results.push((&prop.name, true));
}
for prop in &props_to_remove {
results.push((prop, true));
for op in &ops {
match op {
crate::application::adapters::webdav_adapter::PropPatchOp::Set(prop) => {
results.push((&prop.name, true));
}
crate::application::adapters::webdav_adapter::PropPatchOp::Remove(name) => {
results.push((name, true));
}
}
}
let href = format!("/carddav/{}", path);
src/interfaces/api/handlers/webdav_handler.rs
+32 -24
View File
@@ -17,7 +17,9 @@ use chrono::Utc;
use quick_xml::Writer;
use uuid::Uuid;
use crate::application::adapters::webdav_adapter::{LockInfo, PropFindRequest, WebDavAdapter};
use crate::application::adapters::webdav_adapter::{
LockInfo, PropFindRequest, PropPatchOp, QualifiedName, WebDavAdapter,
};
use crate::application::dtos::file_dto::FileDto;
use crate::application::dtos::folder_dto::FolderDto;
use crate::application::ports::file_ports::FileRetrievalUseCase;
@@ -487,6 +489,7 @@ async fn handle_propfind(
Ok(ResolvedResource::File(file)) => {
let dead_props = state.webdav_dead_props.get_all(&path, user.id).await
.unwrap_or_default();
let file_href = webdav_href(&client_path);
let mut buf = Vec::with_capacity(1024);
{
let mut xml_writer = Writer::new(&mut buf);
@@ -496,7 +499,7 @@ async fn handle_propfind(
&mut xml_writer,
&file,
&propfind_request,
&base_href,
&file_href,
&dead_props,
)
.map_err(|e| AppError::internal_error(format!("XML write error: {}", e)))?;
@@ -540,6 +543,7 @@ async fn handle_propfind(
assert_owner(file.owner_id.as_deref(), &user.id.to_string(), &path)?;
let dead_props = state.webdav_dead_props.get_all(&path, user.id).await
.unwrap_or_default();
let file_href = webdav_href(&client_path);
let mut buf = Vec::with_capacity(1024);
{
let mut xml_writer = Writer::new(&mut buf);
@@ -549,7 +553,7 @@ async fn handle_propfind(
&mut xml_writer,
&file,
&propfind_request,
&base_href,
&file_href,
&dead_props,
)
.map_err(|e| AppError::internal_error(format!("XML write error: {}", e)))?;
@@ -772,26 +776,25 @@ async fn handle_proppatch(
.map_err(|e| {
AppError::payload_too_large(format!("PROPPATCH body too large or unreadable: {}", e))
})?;
let (props_to_set, props_to_remove) = WebDavAdapter::parse_proppatch(body_bytes.reader())
let ops = WebDavAdapter::parse_proppatch(body_bytes.reader())
.map_err(|e| AppError::bad_request(format!("Failed to parse PROPPATCH request: {}", e)))?;
// Persist dead properties (RFC 4918 §4.2 — stored verbatim, returned by PROPFIND).
// Apply operations in document order (RFC 4918 §9.2).
let dead_props = &state.webdav_dead_props;
for prop in &props_to_set {
dead_props.set(&path, user.id, prop.name.clone(), prop.value.clone()).await
.map_err(|e| AppError::internal_error(format!("Failed to store dead property: {e}")))?;
}
for name in &props_to_remove {
dead_props.remove(&path, user.id, name).await
.map_err(|e| AppError::internal_error(format!("Failed to remove dead property: {e}")))?;
}
let mut results = Vec::new();
for prop in &props_to_set {
results.push((&prop.name, true));
}
for prop in &props_to_remove {
results.push((prop, true));
let mut results: Vec<(&QualifiedName, bool)> = Vec::new();
for op in &ops {
match op {
PropPatchOp::Set(pv) => {
dead_props.set(&path, user.id, pv.name.clone(), pv.value.clone()).await
.map_err(|e| AppError::internal_error(format!("Failed to store dead property: {e}")))?;
results.push((&pv.name, true));
}
PropPatchOp::Remove(name) => {
dead_props.remove(&path, user.id, name).await
.map_err(|e| AppError::internal_error(format!("Failed to remove dead property: {e}")))?;
results.push((name, true));
}
}
}
// Generate response — use client-facing path so href matches the request URL.
@@ -1109,10 +1112,8 @@ fn enforce_native_lock(
if p.is_empty() {
return None;
}
if let Some(e) = lock_store.get_by_path(p) {
if e.info.depth.eq_ignore_ascii_case("infinity") {
return Some(e);
}
if let Some(e) = lock_store.get_by_path(p) && e.info.depth.eq_ignore_ascii_case("infinity") {
return Some(e);
}
}
});
@@ -1829,6 +1830,13 @@ async fn handle_move(
}
}
// Migrate dead properties to the new path (RFC 4918 §9.9 — MOVE preserves properties).
state
.webdav_dead_props
.rename_resource(&source_path, user.id, &destination_path)
.await
.map_err(|e| AppError::internal_error(format!("Failed to migrate dead properties: {e}")))?;
// RFC 4918 §9.9.5: 201 Created when destination is new, 204 when overwritten.
let status = if dest_existed {
StatusCode::NO_CONTENT