diff --git a/src/application/adapters/webdav_adapter.rs b/src/application/adapters/webdav_adapter.rs index 59f5aa15..9cd063b1 100644 --- a/src/application/adapters/webdav_adapter.rs +++ b/src/application/adapters/webdav_adapter.rs @@ -96,6 +96,13 @@ pub struct PropValue { pub value: Option, } +/// A single PROPPATCH operation (preserves document order per RFC 4918 §9.2). +#[derive(Debug, Clone)] +pub enum PropPatchOp { + Set(PropValue), + Remove(QualifiedName), +} + /// WebDAV lock information #[derive(Debug, Clone)] pub struct LockInfo { @@ -191,10 +198,31 @@ impl WebDavAdapter { if let Some(prefix) = key.strip_prefix("xmlns:") { let uri = attr.unescape_value().unwrap_or_default().to_string(); ns_map.insert(prefix.to_string(), uri); + } else if key == "xmlns" { + // Default namespace declaration: xmlns="uri" + let uri = attr.unescape_value().unwrap_or_default().to_string(); + ns_map.insert(String::new(), uri); } } } + /// Reject `xmlns:prefix=""` declarations — binding a prefix to an empty URI + /// is forbidden by the XML Namespaces 1.0 spec (RFC 4918 §8.1 requires 400). + fn check_ns_decls_valid(e: &BytesStart) -> Result<()> { + for attr in e.attributes().flatten() { + let key = std::str::from_utf8(attr.key.as_ref()).unwrap_or(""); + if key.starts_with("xmlns:") { + let uri = attr.unescape_value().unwrap_or_default(); + if uri.is_empty() { + return Err(WebDavError::ParseError( + "Invalid namespace declaration: prefix bound to empty URI".to_string(), + )); + } + } + } + Ok(()) + } + /// Resolve a prefixed element name (e.g. `D:resourcetype`) to a /// `QualifiedName` using the accumulated namespace declarations. pub fn resolve_name( @@ -208,6 +236,11 @@ impl WebDavAdapter { return QualifiedName::new(uri.clone(), local.to_string()); } } + // No prefix: check for a default namespace (xmlns="..."). + // An empty string means xmlns="" — null namespace override, which is valid. + if let Some(default_ns) = ns_map.get("") { + return QualifiedName::new(default_ns.clone(), name_str.to_string()); + } // Fallback: no prefix or unknown prefix → use legacy extraction QualifiedName::new( Self::extract_namespace(name_str), @@ -233,6 +266,7 @@ impl WebDavAdapter { match xml_reader.read_event_into(&mut buffer) { Ok(Event::Start(ref e)) => { Self::collect_ns_decls(e, &mut ns_map); + Self::check_ns_decls_valid(e)?; let name = e.name(); let name_str = std::str::from_utf8(name.as_ref()).unwrap_or(""); @@ -270,6 +304,7 @@ impl WebDavAdapter { } Ok(Event::Empty(ref e)) => { Self::collect_ns_decls(e, &mut ns_map); + Self::check_ns_decls_valid(e)?; let name = e.name(); let name_str = std::str::from_utf8(name.as_ref()).unwrap_or(""); @@ -294,11 +329,10 @@ impl WebDavAdapter { // RFC 4918 §8.1: non-well-formed XML MUST produce 400. quick-xml is // lenient about EOF-inside-element (no XmlError on unclosed tags), so - // check explicitly: if we opened a but never closed it, - // the body was not well-formed. - if in_propfind && !saw_propfind_close { + // check explicitly: body must contain a complete …. + if !saw_propfind_close { return Err(WebDavError::ParseError( - "PROPFIND body is not well-formed XML: element was never closed" + "PROPFIND body is not well-formed XML: missing or unclosed element" .to_string(), )); } @@ -342,6 +376,24 @@ impl WebDavAdapter { ) } + /// Write a single qualified name as an empty XML element with proper namespace declaration. + /// + /// DAV: props use the `D:` prefix (already declared on the root element). + /// All other namespaces get a local `xmlns:X` declaration on the element itself. + fn write_qname_empty(xml_writer: &mut Writer, prop: &QualifiedName) -> Result<()> { + if prop.namespace.is_empty() { + xml_writer.write_event(Event::Empty(BytesStart::new(prop.name.as_str())))?; + } else if prop.namespace == "DAV:" { + xml_writer.write_event(Event::Empty(BytesStart::new(format!("D:{}", prop.name))))?; + } else { + let tag = format!("X:{}", prop.name); + let mut start = BytesStart::new(tag.as_str()); + start.push_attribute(("xmlns:X", prop.namespace.as_str())); + xml_writer.write_event(Event::Empty(start))?; + } + Ok(()) + } + /// Write a 404 propstat block for unknown properties (RFC 4918 §9.2). fn write_unknown_props_404( xml_writer: &mut Writer, @@ -353,15 +405,7 @@ impl WebDavAdapter { xml_writer.write_event(Event::Start(BytesStart::new("D:propstat")))?; xml_writer.write_event(Event::Start(BytesStart::new("D:prop")))?; for prop in unknown { - if prop.namespace == "DAV:" { - xml_writer - .write_event(Event::Empty(BytesStart::new(format!("D:{}", prop.name))))?; - } else { - xml_writer.write_event(Event::Empty(BytesStart::new(format!( - "{}:{}", - prop.namespace, prop.name - ))))?; - } + Self::write_qname_empty(xml_writer, prop)?; } xml_writer.write_event(Event::End(BytesEnd::new("D:prop")))?; xml_writer.write_event(Event::Start(BytesStart::new("D:status")))?; @@ -436,11 +480,30 @@ impl WebDavAdapter { xml_writer.write_event(Event::Text(BytesText::new(href)))?; xml_writer.write_event(Event::End(BytesEnd::new("D:href")))?; + // Compute dead props first so we can exclude them from the 404 propstat. + let relevant_dead: Vec<_> = match &request.prop_find_type { + PropFindType::Prop(requested) => dead_props + .iter() + .filter(|(name, _)| requested.iter().any(|r| r == name)) + .cloned() + .collect(), + PropFindType::AllProp => dead_props.to_vec(), + PropFindType::PropName => vec![], + }; + let dead_name_set: std::collections::HashSet<&QualifiedName> = + relevant_dead.iter().map(|(n, _)| n).collect(); + match &request.prop_find_type { PropFindType::Prop(props) => { // RFC 4918 §9.2: known props → 200 propstat; unknown → 404 propstat. + // Props found in the dead store are returned in the dead 200 propstat, + // so exclude them from the 404 propstat to avoid duplicate reporting. let (known, unknown): (Vec<_>, Vec<_>) = props.iter().partition(|p| Self::folder_prop_is_known(p)); + let truly_unknown: Vec<_> = unknown + .into_iter() + .filter(|p| !dead_name_set.contains(*p)) + .collect(); xml_writer.write_event(Event::Start(BytesStart::new("D:propstat")))?; xml_writer.write_event(Event::Start(BytesStart::new("D:prop")))?; @@ -451,7 +514,7 @@ impl WebDavAdapter { xml_writer.write_event(Event::End(BytesEnd::new("D:status")))?; xml_writer.write_event(Event::End(BytesEnd::new("D:propstat")))?; - Self::write_unknown_props_404(xml_writer, &unknown)?; + Self::write_unknown_props_404(xml_writer, &truly_unknown)?; } other => { xml_writer.write_event(Event::Start(BytesStart::new("D:propstat")))?; @@ -474,16 +537,6 @@ impl WebDavAdapter { } // Dead properties — written as a separate 200 propstat (RFC 4918 §4.2). - // For Prop requests, only include dead props that were explicitly requested. - let relevant_dead: Vec<_> = match &request.prop_find_type { - PropFindType::Prop(requested) => dead_props - .iter() - .filter(|(name, _)| requested.iter().any(|r| r == name)) - .cloned() - .collect(), - PropFindType::AllProp => dead_props.to_vec(), - PropFindType::PropName => vec![], - }; Self::write_dead_props_propstat(xml_writer, &relevant_dead)?; xml_writer.write_event(Event::End(BytesEnd::new("D:response")))?; @@ -513,11 +566,30 @@ impl WebDavAdapter { xml_writer.write_event(Event::Text(BytesText::new(href)))?; xml_writer.write_event(Event::End(BytesEnd::new("D:href")))?; + // Compute dead props first so we can exclude them from the 404 propstat. + let relevant_dead: Vec<_> = match &request.prop_find_type { + PropFindType::Prop(requested) => dead_props + .iter() + .filter(|(name, _)| requested.iter().any(|r| r == name)) + .cloned() + .collect(), + PropFindType::AllProp => dead_props.to_vec(), + PropFindType::PropName => vec![], + }; + let dead_name_set: std::collections::HashSet<&QualifiedName> = + relevant_dead.iter().map(|(n, _)| n).collect(); + match &request.prop_find_type { PropFindType::Prop(props) => { // RFC 4918 §9.2: known props → 200 propstat; unknown → 404 propstat. + // Props found in the dead store are returned in the dead 200 propstat, + // so exclude them from the 404 propstat to avoid duplicate reporting. let (known, unknown): (Vec<_>, Vec<_>) = props.iter().partition(|p| Self::file_prop_is_known(p)); + let truly_unknown: Vec<_> = unknown + .into_iter() + .filter(|p| !dead_name_set.contains(*p)) + .collect(); xml_writer.write_event(Event::Start(BytesStart::new("D:propstat")))?; xml_writer.write_event(Event::Start(BytesStart::new("D:prop")))?; @@ -528,7 +600,7 @@ impl WebDavAdapter { xml_writer.write_event(Event::End(BytesEnd::new("D:status")))?; xml_writer.write_event(Event::End(BytesEnd::new("D:propstat")))?; - Self::write_unknown_props_404(xml_writer, &unknown)?; + Self::write_unknown_props_404(xml_writer, &truly_unknown)?; } other => { xml_writer.write_event(Event::Start(BytesStart::new("D:propstat")))?; @@ -551,15 +623,6 @@ impl WebDavAdapter { } // Dead properties (RFC 4918 §4.2). - let relevant_dead: Vec<_> = match &request.prop_find_type { - PropFindType::Prop(requested) => dead_props - .iter() - .filter(|(name, _)| requested.iter().any(|r| r == name)) - .cloned() - .collect(), - PropFindType::AllProp => dead_props.to_vec(), - PropFindType::PropName => vec![], - }; Self::write_dead_props_propstat(xml_writer, &relevant_dead)?; xml_writer.write_event(Event::End(BytesEnd::new("D:response")))?; @@ -852,8 +915,11 @@ impl WebDavAdapter { Ok(()) } - /// Parse a PROPPATCH XML request - pub fn parse_proppatch(reader: R) -> Result<(Vec, Vec)> { + /// Parse a PROPPATCH XML request. + /// + /// Returns operations in document order (RFC 4918 §9.2 requires document-order + /// processing so that remove-then-set and set-then-remove yield different results). + pub fn parse_proppatch(reader: R) -> Result> { let mut xml_reader = Reader::from_reader(BufReader::new(reader)); xml_reader.config_mut().trim_text(true); @@ -863,8 +929,7 @@ impl WebDavAdapter { let mut in_remove = false; let mut in_prop = false; let mut current_prop: Option = None; - let mut props_to_set = Vec::new(); - let mut props_to_remove = Vec::new(); + let mut ops: Vec = Vec::new(); let mut current_text = String::new(); let mut ns_map = std::collections::HashMap::::new(); @@ -896,7 +961,30 @@ impl WebDavAdapter { } } Ok(Event::Text(e)) if current_prop.is_some() => { - current_text.push_str(&e.decode().unwrap_or_default()); + let raw = e.decode().unwrap_or_default(); + let unescaped = + quick_xml::escape::unescape(&raw).unwrap_or_else(|_| raw.clone()); + current_text.push_str(&unescaped); + } + Ok(Event::GeneralRef(ref e)) if current_prop.is_some() => { + // quick-xml 0.39 emits GeneralRef for character references like 𐀀 + // and named entity references like &. Resolve them to actual chars. + match e.resolve_char_ref() { + Ok(Some(ch)) => current_text.push(ch), + Ok(None) => { + if let Ok(name) = e.decode() { + match name.as_ref() { + "amp" => current_text.push('&'), + "lt" => current_text.push('<'), + "gt" => current_text.push('>'), + "apos" => current_text.push('\''), + "quot" => current_text.push('"'), + _ => {} + } + } + } + Err(_) => {} + } } Ok(Event::End(ref e)) => { let name = e.name(); @@ -910,19 +998,18 @@ impl WebDavAdapter { s if s == "remove" || s.ends_with(":remove") => in_remove = false, s if s == "prop" || s.ends_with(":prop") => in_prop = false, _ if in_prop => { - // End of property element if let Some(prop_name) = current_prop.take() { if in_set { - props_to_set.push(PropValue { + ops.push(PropPatchOp::Set(PropValue { name: prop_name, value: if current_text.is_empty() { None } else { Some(current_text.clone()) }, - }); + })); } else if in_remove { - props_to_remove.push(prop_name); + ops.push(PropPatchOp::Remove(prop_name)); } } current_text.clear(); @@ -939,12 +1026,12 @@ impl WebDavAdapter { let qname = Self::resolve_name(name_str, &ns_map); if in_set { - props_to_set.push(PropValue { + ops.push(PropPatchOp::Set(PropValue { name: qname, value: None, - }); + })); } else if in_remove { - props_to_remove.push(qname); + ops.push(PropPatchOp::Remove(qname)); } } } @@ -956,7 +1043,7 @@ impl WebDavAdapter { buffer.clear(); } - Ok((props_to_set, props_to_remove)) + Ok(ops) } /// Generate a PROPPATCH response @@ -1001,12 +1088,7 @@ impl WebDavAdapter { // Write property names for prop in success_props { - let prop_name = if prop.namespace == "DAV:" { - format!("D:{}", prop.name) - } else { - format!("{}:{}", prop.namespace, prop.name) - }; - xml_writer.write_event(Event::Empty(BytesStart::new(&prop_name)))?; + Self::write_qname_empty(&mut xml_writer, prop)?; } // End prop @@ -1030,12 +1112,7 @@ impl WebDavAdapter { // Write property names for prop in failed_props { - let prop_name = if prop.namespace == "DAV:" { - format!("D:{}", prop.name) - } else { - format!("{}:{}", prop.namespace, prop.name) - }; - xml_writer.write_event(Event::Empty(BytesStart::new(&prop_name)))?; + Self::write_qname_empty(&mut xml_writer, prop)?; } // End prop diff --git a/src/interfaces/api/handlers/caldav_handler.rs b/src/interfaces/api/handlers/caldav_handler.rs index a4f6e22b..d0468e08 100644 --- a/src/interfaces/api/handlers/caldav_handler.rs +++ b/src/interfaces/api/handlers/caldav_handler.rs @@ -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); diff --git a/src/interfaces/api/handlers/carddav_handler.rs b/src/interfaces/api/handlers/carddav_handler.rs index 29a8260a..47c2e97a 100644 --- a/src/interfaces/api/handlers/carddav_handler.rs +++ b/src/interfaces/api/handlers/carddav_handler.rs @@ -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); diff --git a/src/interfaces/api/handlers/webdav_handler.rs b/src/interfaces/api/handlers/webdav_handler.rs index a10b5042..23fde19e 100644 --- a/src/interfaces/api/handlers/webdav_handler.rs +++ b/src/interfaces/api/handlers/webdav_handler.rs @@ -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