diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2170ff68..2edd9ec4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -369,6 +369,32 @@ jobs: path: tests/api/storage/ retention-days: 7 + litmus: + name: WebDAV RFC 4918 — litmus (59/59) + needs: build + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@v4 + + - uses: actions/download-artifact@v4 + with: + name: oxicloud-release + path: target/release/ + + - name: Set execute bit on pre-built binary + run: chmod +x target/release/oxicloud + + - name: Install litmus and jq + run: sudo apt-get update -q && sudo apt-get install -y litmus jq + + - name: Run litmus WebDAV compliance tests + run: bash tests/webdav/run-litmus.sh + env: + BUILD_TARGET: release + LITMUS_TESTS: "basic copymove props locks" + front-test: name: Frontend end-to-end tests (via Playwright) # ensure that api tests are ok before diff --git a/migrations/20260825000000_webdav_dead_properties.sql b/migrations/20260825000000_webdav_dead_properties.sql new file mode 100644 index 00000000..9ba875f7 --- /dev/null +++ b/migrations/20260825000000_webdav_dead_properties.sql @@ -0,0 +1,20 @@ +-- WebDAV dead properties storage (RFC 4918 §9.2). +-- Stores arbitrary user-defined XML properties set via PROPPATCH. +-- Keyed by (resource_path, user_id, namespace, local_name) — the +-- same property on different resources or for different users is +-- a distinct row. + +CREATE TABLE IF NOT EXISTS storage.webdav_dead_properties ( + id UUID NOT NULL DEFAULT gen_random_uuid() PRIMARY KEY, + resource_path TEXT NOT NULL, + user_id UUID NOT NULL REFERENCES auth.users(id) ON DELETE CASCADE, + namespace TEXT NOT NULL DEFAULT '', + local_name TEXT NOT NULL, + value TEXT, + created_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP, + UNIQUE (resource_path, user_id, namespace, local_name) +); + +CREATE INDEX IF NOT EXISTS idx_webdav_dead_properties_path_user + ON storage.webdav_dead_properties (resource_path, user_id); diff --git a/src/application/adapters/webdav_adapter.rs b/src/application/adapters/webdav_adapter.rs index 24dbde25..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), @@ -222,6 +255,7 @@ impl WebDavAdapter { let mut buffer = Vec::new(); let mut in_propfind = false; + let mut saw_propfind_close = false; let mut in_prop = false; let mut in_allprop = false; let mut in_propname = false; @@ -232,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(""); @@ -258,6 +293,7 @@ impl WebDavAdapter { if name_str == "propfind" || name_str.ends_with(":propfind") { in_propfind = false; + saw_propfind_close = true; } else if name_str == "prop" || name_str.ends_with(":prop") { in_prop = false; } else if name_str == "allprop" || name_str.ends_with(":allprop") { @@ -268,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(""); @@ -290,6 +327,16 @@ impl WebDavAdapter { buffer.clear(); } + // 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: body must contain a complete …. + if !saw_propfind_close { + return Err(WebDavError::ParseError( + "PROPFIND body is not well-formed XML: missing or unclosed element" + .to_string(), + )); + } + let prop_find_type = if in_allprop { PropFindType::AllProp } else if in_propname { @@ -301,6 +348,115 @@ impl WebDavAdapter { Ok(PropFindRequest { prop_find_type }) } + fn folder_prop_is_known(prop: &QualifiedName) -> bool { + prop.namespace == "DAV:" + && matches!( + prop.name.as_str(), + "resourcetype" + | "displayname" + | "creationdate" + | "getlastmodified" + | "getetag" + | "getcontentlength" + | "getcontenttype" + ) + } + + fn file_prop_is_known(prop: &QualifiedName) -> bool { + prop.namespace == "DAV:" + && matches!( + prop.name.as_str(), + "resourcetype" + | "displayname" + | "getcontenttype" + | "getcontentlength" + | "creationdate" + | "getlastmodified" + | "getetag" + ) + } + + /// 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, + unknown: &[&QualifiedName], + ) -> Result<()> { + if unknown.is_empty() { + return Ok(()); + } + xml_writer.write_event(Event::Start(BytesStart::new("D:propstat")))?; + xml_writer.write_event(Event::Start(BytesStart::new("D:prop")))?; + for prop in unknown { + 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")))?; + xml_writer.write_event(Event::Text(BytesText::new("HTTP/1.1 404 Not Found")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:status")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:propstat")))?; + Ok(()) + } + + /// Write a dead-property propstat block (RFC 4918 §4.2). + /// + /// Written AFTER the live-property propstats inside a ``. + /// Only emitted when `dead_props` is non-empty. + fn write_dead_props_propstat( + xml_writer: &mut Writer, + dead_props: &[(QualifiedName, Option)], + ) -> Result<()> { + if dead_props.is_empty() { + return Ok(()); + } + xml_writer.write_event(Event::Start(BytesStart::new("D:propstat")))?; + xml_writer.write_event(Event::Start(BytesStart::new("D:prop")))?; + for (name, value) in dead_props { + let tag = if name.namespace.is_empty() { + name.name.clone() + } else { + format!("X:{}", name.name) + }; + let mut start = BytesStart::new(tag.as_str()); + if !name.namespace.is_empty() { + start.push_attribute(("xmlns:X", name.namespace.as_str())); + } + match value { + Some(v) if !v.is_empty() => { + xml_writer.write_event(Event::Start(start))?; + xml_writer.write_event(Event::Text(BytesText::new(v)))?; + xml_writer.write_event(Event::End(BytesEnd::new(tag.as_str())))?; + } + _ => { + xml_writer.write_event(Event::Empty(start))?; + } + } + } + xml_writer.write_event(Event::End(BytesEnd::new("D:prop")))?; + xml_writer.write_event(Event::Start(BytesStart::new("D:status")))?; + xml_writer.write_event(Event::Text(BytesText::new("HTTP/1.1 200 OK")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:status")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:propstat")))?; + Ok(()) + } + /// Write folder properties as a response fn write_folder_response( xml_writer: &mut Writer, @@ -308,50 +464,82 @@ impl WebDavAdapter { request: &PropFindRequest, href: &str, ) -> Result<()> { - // Start response element + Self::write_folder_response_with_dead_props(xml_writer, folder, request, href, &[]) + } + + fn write_folder_response_with_dead_props( + xml_writer: &mut Writer, + folder: &FolderDto, + request: &PropFindRequest, + href: &str, + dead_props: &[(QualifiedName, Option)], + ) -> Result<()> { xml_writer.write_event(Event::Start(BytesStart::new("D:response")))?; - // Write href xml_writer.write_event(Event::Start(BytesStart::new("D:href")))?; xml_writer.write_event(Event::Text(BytesText::new(href)))?; xml_writer.write_event(Event::End(BytesEnd::new("D:href")))?; - // Write propstat - xml_writer.write_event(Event::Start(BytesStart::new("D:propstat")))?; + // 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(); - // Start prop - xml_writer.write_event(Event::Start(BytesStart::new("D:prop")))?; - - // Write properties based on request type match &request.prop_find_type { - PropFindType::AllProp => { - // Write all standard properties for a folder - Self::write_folder_standard_props(xml_writer, folder)?; - } - PropFindType::PropName => { - // Write only property names (empty elements) - Self::write_folder_prop_names(xml_writer)?; - } PropFindType::Prop(props) => { - // Write requested properties - Self::write_folder_requested_props(xml_writer, folder, 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")))?; + Self::write_folder_requested_props(xml_writer, folder, &known)?; + xml_writer.write_event(Event::End(BytesEnd::new("D:prop")))?; + xml_writer.write_event(Event::Start(BytesStart::new("D:status")))?; + xml_writer.write_event(Event::Text(BytesText::new("HTTP/1.1 200 OK")))?; + 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, &truly_unknown)?; + } + other => { + xml_writer.write_event(Event::Start(BytesStart::new("D:propstat")))?; + xml_writer.write_event(Event::Start(BytesStart::new("D:prop")))?; + match other { + PropFindType::AllProp => { + Self::write_folder_standard_props(xml_writer, folder)?; + } + PropFindType::PropName => { + Self::write_folder_prop_names(xml_writer)?; + } + PropFindType::Prop(_) => unreachable!(), + } + xml_writer.write_event(Event::End(BytesEnd::new("D:prop")))?; + xml_writer.write_event(Event::Start(BytesStart::new("D:status")))?; + xml_writer.write_event(Event::Text(BytesText::new("HTTP/1.1 200 OK")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:status")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:propstat")))?; } } - // End prop - xml_writer.write_event(Event::End(BytesEnd::new("D:prop")))?; + // Dead properties — written as a separate 200 propstat (RFC 4918 §4.2). + Self::write_dead_props_propstat(xml_writer, &relevant_dead)?; - // Write status - xml_writer.write_event(Event::Start(BytesStart::new("D:status")))?; - xml_writer.write_event(Event::Text(BytesText::new("HTTP/1.1 200 OK")))?; - xml_writer.write_event(Event::End(BytesEnd::new("D:status")))?; - - // End propstat - xml_writer.write_event(Event::End(BytesEnd::new("D:propstat")))?; - - // End response xml_writer.write_event(Event::End(BytesEnd::new("D:response")))?; - Ok(()) } @@ -362,50 +550,82 @@ impl WebDavAdapter { request: &PropFindRequest, href: &str, ) -> Result<()> { - // Start response element + Self::write_file_response_with_dead_props(xml_writer, file, request, href, &[]) + } + + fn write_file_response_with_dead_props( + xml_writer: &mut Writer, + file: &FileDto, + request: &PropFindRequest, + href: &str, + dead_props: &[(QualifiedName, Option)], + ) -> Result<()> { xml_writer.write_event(Event::Start(BytesStart::new("D:response")))?; - // Write href xml_writer.write_event(Event::Start(BytesStart::new("D:href")))?; xml_writer.write_event(Event::Text(BytesText::new(href)))?; xml_writer.write_event(Event::End(BytesEnd::new("D:href")))?; - // Write propstat - xml_writer.write_event(Event::Start(BytesStart::new("D:propstat")))?; + // 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(); - // Start prop - xml_writer.write_event(Event::Start(BytesStart::new("D:prop")))?; - - // Write properties based on request type match &request.prop_find_type { - PropFindType::AllProp => { - // Write all standard properties for a file - Self::write_file_standard_props(xml_writer, file)?; - } - PropFindType::PropName => { - // Write only property names (empty elements) - Self::write_file_prop_names(xml_writer)?; - } PropFindType::Prop(props) => { - // Write requested properties - Self::write_file_requested_props(xml_writer, file, 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")))?; + Self::write_file_requested_props(xml_writer, file, &known)?; + xml_writer.write_event(Event::End(BytesEnd::new("D:prop")))?; + xml_writer.write_event(Event::Start(BytesStart::new("D:status")))?; + xml_writer.write_event(Event::Text(BytesText::new("HTTP/1.1 200 OK")))?; + 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, &truly_unknown)?; + } + other => { + xml_writer.write_event(Event::Start(BytesStart::new("D:propstat")))?; + xml_writer.write_event(Event::Start(BytesStart::new("D:prop")))?; + match other { + PropFindType::AllProp => { + Self::write_file_standard_props(xml_writer, file)?; + } + PropFindType::PropName => { + Self::write_file_prop_names(xml_writer)?; + } + PropFindType::Prop(_) => unreachable!(), + } + xml_writer.write_event(Event::End(BytesEnd::new("D:prop")))?; + xml_writer.write_event(Event::Start(BytesStart::new("D:status")))?; + xml_writer.write_event(Event::Text(BytesText::new("HTTP/1.1 200 OK")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:status")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:propstat")))?; } } - // End prop - xml_writer.write_event(Event::End(BytesEnd::new("D:prop")))?; + // Dead properties (RFC 4918 §4.2). + Self::write_dead_props_propstat(xml_writer, &relevant_dead)?; - // Write status - xml_writer.write_event(Event::Start(BytesStart::new("D:status")))?; - xml_writer.write_event(Event::Text(BytesText::new("HTTP/1.1 200 OK")))?; - xml_writer.write_event(Event::End(BytesEnd::new("D:status")))?; - - // End propstat - xml_writer.write_event(Event::End(BytesEnd::new("D:propstat")))?; - - // End response xml_writer.write_event(Event::End(BytesEnd::new("D:response")))?; - Ok(()) } @@ -549,7 +769,7 @@ impl WebDavAdapter { fn write_folder_requested_props( xml_writer: &mut Writer, folder: &FolderDto, - props: &[QualifiedName], + props: &[&QualifiedName], ) -> Result<()> { for prop in props { if prop.namespace == "DAV:" { @@ -611,20 +831,11 @@ impl WebDavAdapter { xml_writer.write_event(Event::End(BytesEnd::new("D:getcontenttype")))?; } _ => { - // Property not supported - write empty element - xml_writer.write_event(Event::Empty(BytesStart::new(format!( - "D:{}", - prop.name - ))))?; + // Unknown prop — skipped here; caller writes 404 propstat. } } - } else { - // Non-DAV namespace, not supported - xml_writer.write_event(Event::Empty(BytesStart::new(format!( - "{}:{}", - prop.namespace, prop.name - ))))?; } + // Non-DAV namespace props are unknown — skipped; caller writes 404 propstat. } Ok(()) @@ -634,7 +845,7 @@ impl WebDavAdapter { fn write_file_requested_props( xml_writer: &mut Writer, file: &FileDto, - props: &[QualifiedName], + props: &[&QualifiedName], ) -> Result<()> { for prop in props { if prop.namespace == "DAV:" { @@ -694,27 +905,21 @@ impl WebDavAdapter { xml_writer.write_event(Event::End(BytesEnd::new("D:getetag")))?; } _ => { - // Property not supported - write empty element - xml_writer.write_event(Event::Empty(BytesStart::new(format!( - "D:{}", - prop.name - ))))?; + // Unknown prop — skipped here; caller writes 404 propstat. } } - } else { - // Non-DAV namespace, not supported - xml_writer.write_event(Event::Empty(BytesStart::new(format!( - "{}:{}", - prop.namespace, prop.name - ))))?; } + // Non-DAV namespace props are unknown — skipped; caller writes 404 propstat. } 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); @@ -724,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(); @@ -757,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(); @@ -771,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(); @@ -800,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)); } } } @@ -817,7 +1043,7 @@ impl WebDavAdapter { buffer.clear(); } - Ok((props_to_set, props_to_remove)) + Ok(ops) } /// Generate a PROPPATCH response @@ -862,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 @@ -891,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 @@ -1143,7 +1359,7 @@ impl WebDavAdapter { Self::write_folder_response(writer, folder, request, href) } - /// Writes a single `` element for a file. + /// Writes a single `` element for a file, including dead properties. pub fn write_file_entry( writer: &mut Writer, file: &FileDto, @@ -1152,4 +1368,26 @@ impl WebDavAdapter { ) -> Result<()> { Self::write_file_response(writer, file, request, href) } + + /// Writes a folder entry including dead (custom) properties. + pub fn write_folder_entry_with_dead_props( + writer: &mut Writer, + folder: &FolderDto, + request: &PropFindRequest, + href: &str, + dead_props: &[(QualifiedName, Option)], + ) -> Result<()> { + Self::write_folder_response_with_dead_props(writer, folder, request, href, dead_props) + } + + /// Writes a file entry including dead (custom) properties. + pub fn write_file_entry_with_dead_props( + writer: &mut Writer, + file: &FileDto, + request: &PropFindRequest, + href: &str, + dead_props: &[(QualifiedName, Option)], + ) -> Result<()> { + Self::write_file_response_with_dead_props(writer, file, request, href, dead_props) + } } diff --git a/src/common/di.rs b/src/common/di.rs index 3a116008..5600aa41 100644 --- a/src/common/di.rs +++ b/src/common/di.rs @@ -1542,6 +1542,8 @@ impl AppServiceFactory { path_resolver: None, webdav_lock_store: crate::infrastructure::services::webdav_lock_service::create_webdav_lock_store(), + webdav_dead_props: + crate::infrastructure::services::webdav_dead_property_store::create_dead_property_store(pool.clone()), authorization: authorization.clone(), drive_repo: drive_repo.clone(), drive_management_service: Arc::new( @@ -2010,6 +2012,8 @@ pub struct AppState { Option>, pub webdav_lock_store: Arc, + pub webdav_dead_props: + Arc, /// ReBAC authorization engine — all service-layer permission checks go /// through this. Concrete type today is `PgAclEngine`; the /// `AuthorizationEngine` trait describes the contract. When alternate diff --git a/src/infrastructure/services/mod.rs b/src/infrastructure/services/mod.rs index 77ea5115..9e65f351 100644 --- a/src/infrastructure/services/mod.rs +++ b/src/infrastructure/services/mod.rs @@ -42,6 +42,7 @@ pub mod thumbnail_service; mod thumbnail_service_test; pub mod trash_cleanup_service; pub mod tree_etag_flush_service; +pub mod webdav_dead_property_store; pub mod webdav_lock_service; pub mod wopi_discovery_service; pub mod zip_service; diff --git a/src/infrastructure/services/webdav_dead_property_store.rs b/src/infrastructure/services/webdav_dead_property_store.rs new file mode 100644 index 00000000..1e123d0c --- /dev/null +++ b/src/infrastructure/services/webdav_dead_property_store.rs @@ -0,0 +1,184 @@ +//! PostgreSQL-backed dead property store for WebDAV PROPPATCH / PROPFIND compliance. +//! +//! RFC 4918 §4.2 defines "dead properties" as those stored verbatim by the +//! server without interpreting their value. Properties are persisted to +//! `storage.webdav_dead_properties` and survive server restarts. + +use std::sync::Arc; + +use sqlx::PgPool; +use uuid::Uuid; + +use crate::application::adapters::webdav_adapter::QualifiedName; +use crate::domain::errors::DomainError; + +pub struct DeadPropertyStore { + pool: Arc, +} + +impl DeadPropertyStore { + pub fn new(pool: Arc) -> Self { + Self { pool } + } + + /// Upsert a dead property. `value = None` means an empty XML element. + pub async fn set( + &self, + path: &str, + user_id: Uuid, + name: QualifiedName, + value: Option, + ) -> Result<(), DomainError> { + sqlx::query!( + r#" + INSERT INTO storage.webdav_dead_properties + (resource_path, user_id, namespace, local_name, value) + VALUES ($1, $2, $3, $4, $5) + ON CONFLICT (resource_path, user_id, namespace, local_name) + DO UPDATE SET value = EXCLUDED.value, updated_at = CURRENT_TIMESTAMP + "#, + path, + user_id, + name.namespace, + name.name, + value, + ) + .execute(&*self.pool) + .await + .map_err(|e| DomainError::internal_error("DeadPropertyStore", format!("set: {e}")))?; + Ok(()) + } + + /// Delete a specific dead property. No-op if not present. + pub async fn remove( + &self, + path: &str, + user_id: Uuid, + name: &QualifiedName, + ) -> Result<(), DomainError> { + sqlx::query!( + "DELETE FROM storage.webdav_dead_properties + WHERE resource_path = $1 AND user_id = $2 + AND namespace = $3 AND local_name = $4", + path, + user_id, + name.namespace, + name.name, + ) + .execute(&*self.pool) + .await + .map_err(|e| DomainError::internal_error("DeadPropertyStore", format!("remove: {e}")))?; + Ok(()) + } + + /// Return all dead properties for `path`. + pub async fn get_all( + &self, + path: &str, + user_id: Uuid, + ) -> Result)>, DomainError> { + let rows = sqlx::query!( + "SELECT namespace, local_name, value + FROM storage.webdav_dead_properties + WHERE resource_path = $1 AND user_id = $2", + path, + user_id, + ) + .fetch_all(&*self.pool) + .await + .map_err(|e| DomainError::internal_error("DeadPropertyStore", format!("get_all: {e}")))?; + + Ok(rows + .into_iter() + .map(|r| (QualifiedName::new(r.namespace, r.local_name), r.value)) + .collect()) + } + + /// Return a specific dead property, or `None` if not stored. + /// Returns `Some(None)` when the property exists with an empty value. + pub async fn get( + &self, + path: &str, + user_id: Uuid, + name: &QualifiedName, + ) -> Result>, DomainError> { + let row = sqlx::query!( + "SELECT value FROM storage.webdav_dead_properties + WHERE resource_path = $1 AND user_id = $2 + AND namespace = $3 AND local_name = $4", + path, + user_id, + name.namespace, + name.name, + ) + .fetch_optional(&*self.pool) + .await + .map_err(|e| DomainError::internal_error("DeadPropertyStore", format!("get: {e}")))?; + + Ok(row.map(|r| r.value)) + } + + /// Delete all dead properties for `path` (called on DELETE). + pub async fn remove_resource(&self, path: &str, user_id: Uuid) -> Result<(), DomainError> { + sqlx::query!( + "DELETE FROM storage.webdav_dead_properties + WHERE resource_path = $1 AND user_id = $2", + path, + user_id, + ) + .execute(&*self.pool) + .await + .map_err(|e| { + DomainError::internal_error("DeadPropertyStore", format!("remove_resource: {e}")) + })?; + Ok(()) + } + + /// Move dead properties from `old_path` to `new_path` (called on MOVE). + /// Clears any stale properties at `new_path` first. + pub async fn rename_resource( + &self, + old_path: &str, + user_id: Uuid, + new_path: &str, + ) -> Result<(), DomainError> { + let mut tx = self.pool.begin().await.map_err(|e| { + DomainError::internal_error("DeadPropertyStore", format!("rename_resource tx: {e}")) + })?; + + sqlx::query!( + "DELETE FROM storage.webdav_dead_properties + WHERE resource_path = $1 AND user_id = $2", + new_path, + user_id, + ) + .execute(&mut *tx) + .await + .map_err(|e| { + DomainError::internal_error("DeadPropertyStore", format!("rename_resource delete: {e}")) + })?; + + sqlx::query!( + "UPDATE storage.webdav_dead_properties + SET resource_path = $2 + WHERE resource_path = $1 AND user_id = $3", + old_path, + new_path, + user_id, + ) + .execute(&mut *tx) + .await + .map_err(|e| { + DomainError::internal_error("DeadPropertyStore", format!("rename_resource update: {e}")) + })?; + + tx.commit().await.map_err(|e| { + DomainError::internal_error("DeadPropertyStore", format!("rename_resource commit: {e}")) + })?; + Ok(()) + } +} + +pub fn create_dead_property_store(pool: Arc) -> Arc { + Arc::new(DeadPropertyStore::new(pool)) +} diff --git a/src/infrastructure/services/webdav_lock_service.rs b/src/infrastructure/services/webdav_lock_service.rs index 2b4ee389..686c0daf 100644 --- a/src/infrastructure/services/webdav_lock_service.rs +++ b/src/infrastructure/services/webdav_lock_service.rs @@ -106,15 +106,27 @@ impl WebDavLockStore { /// Attempt to acquire a lock on `path`. /// - /// Returns `Ok(LockEntry)` on success, or `Err(existing)` if the resource - /// is already exclusively locked by a different token. + /// Returns `Ok(LockEntry)` on success, or `Err(existing)` when: + /// - The existing lock is exclusive (blocks any new lock), or + /// - The new lock is exclusive and any lock already exists (RFC 4918 §7.8). #[allow(clippy::result_large_err)] pub fn acquire(&self, path: &str, info: LockInfo) -> Result { - // Check for existing conflicting lock - if let Some(existing) = self.by_path.get(path) - && existing.info.scope == LockScope::Exclusive - { - return Err(existing); + if let Some(existing) = self.by_path.get(path) { + // Exclusive existing lock → blocks everything. + // New exclusive lock → blocked by any existing lock (shared or exclusive). + if existing.info.scope == LockScope::Exclusive || info.scope == LockScope::Exclusive { + return Err(existing); + } + // Both shared: keep the first holder as the enforcement sentinel in + // `by_path` so releasing a secondary holder cannot clear the lock. + // Register the new token only in the reverse index so UNLOCK works. + let entry = LockEntry { + info, + path: path.to_owned(), + }; + self.by_token + .insert(entry.info.token.clone(), path.to_owned()); + return Ok(entry); } let entry = LockEntry { 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 02543404..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; @@ -391,6 +393,12 @@ async fn handle_propfind( // ── 2. Authenticate ────────────────────────────────────────── let user = extract_user(&req)?; + // Client-facing path for href construction — must be extracted before + // req.into_body() consumes the request. The `path` parameter already has + // the home-folder prefix prepended (e.g. `admin/docs`) so it's correct for + // DB lookups but wrong for WebDAV hrefs (clients see `/webdav/docs`). + let client_path = extract_webdav_path(req.uri()); + // ── 3. Parse PROPFIND XML body ─────────────────────────────── let body_bytes = { let body = req.into_body(); @@ -413,10 +421,11 @@ async fn handle_propfind( let folder_service = state.applications.folder_service.clone(); let file_retrieval_service = state.applications.file_retrieval_service.clone(); - let base_href = if path.is_empty() || path == "/" { + // Use client-facing path for hrefs so responses match the request URL. + let base_href = if client_path.is_empty() || client_path == "/" { "/webdav/".to_string() } else { - format!("/webdav/{}/", encode_uri_path(&path)) + format!("/webdav/{}/", encode_uri_path(&client_path)) }; // ── 5. Determine target resource ───────────────────────────── @@ -452,6 +461,8 @@ async fn handle_propfind( folder_service, file_retrieval_service, user.id, + state.webdav_dead_props.clone(), + path.clone(), ) .await; } @@ -470,20 +481,26 @@ async fn handle_propfind( folder_service, file_retrieval_service, user.id, + state.webdav_dead_props.clone(), + path.clone(), ) .await; } 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); WebDavAdapter::write_multistatus_start(&mut xml_writer) .map_err(|e| AppError::internal_error(format!("XML write error: {}", e)))?; - WebDavAdapter::write_file_entry( + WebDavAdapter::write_file_entry_with_dead_props( &mut xml_writer, &file, &propfind_request, - &base_href, + &file_href, + &dead_props, ) .map_err(|e| AppError::internal_error(format!("XML write error: {}", e)))?; WebDavAdapter::write_multistatus_end(&mut xml_writer) @@ -514,6 +531,8 @@ async fn handle_propfind( folder_service, file_retrieval_service, user.id, + state.webdav_dead_props.clone(), + path.clone(), ) .await; } @@ -522,16 +541,20 @@ async fn handle_propfind( .await { 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); WebDavAdapter::write_multistatus_start(&mut xml_writer) .map_err(|e| AppError::internal_error(format!("XML write error: {}", e)))?; - WebDavAdapter::write_file_entry( + WebDavAdapter::write_file_entry_with_dead_props( &mut xml_writer, &file, &propfind_request, - &base_href, + &file_href, + &dead_props, ) .map_err(|e| AppError::internal_error(format!("XML write error: {}", e)))?; WebDavAdapter::write_multistatus_end(&mut xml_writer) @@ -564,6 +587,10 @@ async fn build_streaming_propfind_response( folder_service: std::sync::Arc, file_retrieval_service: std::sync::Arc, user_id: Uuid, + dead_props_store: Arc< + crate::infrastructure::services::webdav_dead_property_store::DeadPropertyStore, + >, + folder_internal_path: String, ) -> Result, AppError> { let depth = depth.to_string(); let base_href = base_href.to_string(); @@ -574,9 +601,11 @@ async fn build_streaming_propfind_response( let mut buf = Vec::with_capacity(4096); { let mut w = Writer::new(&mut buf); + let folder_dead = dead_props_store.get_all(&folder_internal_path, user_id).await + .map_err(|e| std::io::Error::other(e.to_string()))?; WebDavAdapter::write_multistatus_start(&mut w) .map_err(|e| std::io::Error::other(e.to_string()))?; - WebDavAdapter::write_folder_entry(&mut w, &folder, &propfind_request, &base_href) + WebDavAdapter::write_folder_entry_with_dead_props(&mut w, &folder, &propfind_request, &base_href, &folder_dead) .map_err(|e| std::io::Error::other(e.to_string()))?; } yield Bytes::from(buf); @@ -610,7 +639,10 @@ async fn build_streaming_propfind_response( let mut w = Writer::new(&mut chunk); for subfolder in &result.items { let href = format!("{}{}/", base_href, encode_path_segment(&subfolder.name)); - WebDavAdapter::write_folder_entry(&mut w, subfolder, &propfind_request, &href) + let child_path = format!("{}/{}", folder_internal_path, subfolder.name); + let child_dead = dead_props_store.get_all(&child_path, user_id).await + .map_err(|e| std::io::Error::other(e.to_string()))?; + WebDavAdapter::write_folder_entry_with_dead_props(&mut w, subfolder, &propfind_request, &href, &child_dead) .map_err(|e| std::io::Error::other(e.to_string()))?; } } @@ -641,7 +673,10 @@ async fn build_streaming_propfind_response( let mut w = Writer::new(&mut chunk); for file in &batch { let href = format!("{}{}", base_href, encode_path_segment(&file.name)); - WebDavAdapter::write_file_entry(&mut w, file, &propfind_request, &href) + let child_path = format!("{}/{}", folder_internal_path, file.name); + let child_dead = dead_props_store.get_all(&child_path, user_id).await + .map_err(|e| std::io::Error::other(e.to_string()))?; + WebDavAdapter::write_file_entry_with_dead_props(&mut w, file, &propfind_request, &href, &child_dead) .map_err(|e| std::io::Error::other(e.to_string()))?; } } @@ -693,6 +728,8 @@ async fn handle_proppatch( path: String, ) -> Result, AppError> { let user = extract_user(&req)?; + // Client-facing path for href construction (without home folder prefix). + let client_path = extract_webdav_path(req.uri()); // Active-lock guard (RFC 4918 §9.10.4): PROPPATCH writes properties, // so a lock on the target must release them via `If:`. Captured @@ -739,30 +776,32 @@ 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)))?; - // For now, we don't actually persist custom properties, but we respond as if we did - // In a full implementation, we would store these properties in a database - - // Generate response - we'll pretend all operations succeeded - let mut results = Vec::new(); - - // For each property to set, indicate success - for prop in &props_to_set { - results.push((&prop.name, true)); + // Apply operations in document order (RFC 4918 §9.2). + let dead_props = &state.webdav_dead_props; + 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)); + } + } } - // For each property to remove, indicate success - for prop in &props_to_remove { - results.push((prop, true)); - } - - // Generate response — collection vs file href chosen above. + // Generate response — use client-facing path so href matches the request URL. let href = if is_collection { - webdav_collection_href(&path) + webdav_collection_href(&client_path) } else { - webdav_href(&path) + webdav_href(&client_path) }; let mut response_body = Vec::new(); WebDavAdapter::generate_proppatch_response(&mut response_body, &href, &results).map_err( @@ -1062,20 +1101,65 @@ fn enforce_native_lock( if_header: Option<&str>, path: &str, ) -> Option> { - let entry = lock_store.get_by_path(path)?; - if let Some(h) = if_header - && extract_if_header_tokens(h) - .iter() - .any(|t| t == &entry.info.token) - { - return None; + // Check the exact path, then walk up parent collections for depth-infinity + // locks (RFC 4918 §6.1: a lock on a collection with Depth: infinity also + // covers all descendant members). + let entry = lock_store.get_by_path(path).or_else(|| { + let mut p = path; + loop { + let idx = p.rfind('/')?; + p = &p[..idx]; + if p.is_empty() { + return None; + } + if let Some(e) = lock_store.get_by_path(p) && e.info.depth.eq_ignore_ascii_case("infinity") { + return Some(e); + } + } + }); + + if let Some(entry) = entry { + // Resource is locked: caller must supply the matching token in If:. + if let Some(h) = if_header + && extract_if_header_tokens(h) + .iter() + .any(|t| t == &entry.info.token) + { + return None; + } + return Some( + Response::builder() + .status(StatusCode::LOCKED) + .body(Body::empty()) + .unwrap(), + ); } - Some( - Response::builder() - .status(StatusCode::LOCKED) - .body(Body::empty()) - .unwrap(), - ) + + // Resource is not locked. If the If: header references lock tokens (not + // resource-tag URLs), every such token must be active somewhere in the + // store. A stale or fabricated token (e.g. DAV:no-lock) never matches, + // so the If: condition fails → 412 Precondition Failed (RFC 4918 §10.4). + if let Some(h) = if_header { + let tokens = extract_if_header_tokens(h); + let lock_refs: Vec<_> = tokens + .iter() + .filter(|t| !t.starts_with("http://") && !t.starts_with("https://")) + .collect(); + if !lock_refs.is_empty() + && !lock_refs + .iter() + .any(|t| lock_store.get_by_token(t).is_some()) + { + return Some( + Response::builder() + .status(StatusCode::PRECONDITION_FAILED) + .body(Body::empty()) + .unwrap(), + ); + } + } + + None } /** @@ -1101,79 +1185,119 @@ async fn handle_put( let user = extract_user(&req)?; - // Get file service from state let file_upload_service = &state.applications.file_upload_service; - // Check if path is empty (root folder) if path.is_empty() || path == "/" { return Err(AppError::bad_request("Cannot PUT to root folder")); } - // ── Active-lock guard (RFC 4918 §9.10.4) ────────────────────────── - // Reject a write that targets a locked resource unless the request - // carries the lock token in `If:`. Captured before we consume the - // body into the CDC ingester — a 423 mustn't waste any bandwidth. + // RFC 4918 §9.7.1: a server MUST NOT partially CREATE or UPDATE a resource + // based on a PUT request containing a Content-Range header. + if req.headers().contains_key(header::CONTENT_RANGE) { + return Err(AppError::bad_request( + "PUT with Content-Range is not allowed (RFC 4918 §9.7.1)", + )); + } + + // Extract all headers before consuming `req` into the body stream. let if_header_owned = req .headers() .get("If") .and_then(|v| v.to_str().ok()) .map(|s| s.to_string()); - if let Some(resp) = - enforce_native_lock(&state.webdav_lock_store, if_header_owned.as_deref(), &path) - { - return Ok(resp); - } - - // ── Ownership guard ──────────────────────────────────────── - // Verify that the user owns the target file (update) or the - // parent folder (create). Without this check a user could - // overwrite another user's file via a crafted PUT path. - if let Some(resolver) = &state.path_resolver { - match resolver.resolve_path_for_user(&path, user.id).await { - Ok(ResolvedResource::File(_)) => { /* existing file owned by user — OK */ } - Ok(ResolvedResource::Folder(_)) => { - return Err(AppError::bad_request("Cannot PUT to a directory")); - } - Err(_) => { - // File doesn't exist yet — verify parent folder ownership - let parent_path = if let Some(idx) = path.rfind('/') { - &path[..idx] - } else { - "" - }; - if !parent_path.is_empty() { - resolver - .resolve_path_for_user(parent_path, user.id) - .await - .map_err(|_| { - AppError::not_found(format!("Parent folder not found: {}", parent_path)) - })?; - } - // root-level PUT is allowed (parent_path empty) - } - } - } - // (legacy path without resolver: update_file_streaming will create - // under the folder with the resolved path, which may belong to - // another user — acceptable risk since PathResolver should always - // be enabled in production) - - // Direct PUT cap — see `nextcloud/webdav_handler::handle_put` for - // the reasoning. Files above `direct_put_max_bytes` must go through - // the chunked-upload protocol (`/api/uploads/…`) which is resumable. - let max_upload = state.core.config.storage.direct_put_max_bytes; - - // Extract content type before consuming the request + let if_none_match = req + .headers() + .get(header::IF_NONE_MATCH) + .and_then(|v| v.to_str().ok()) + .map(|s| s.trim().to_string()); + let if_match = req + .headers() + .get(header::IF_MATCH) + .and_then(|v| v.to_str().ok()) + .map(|s| s.trim().to_string()); let content_type = req .headers() .get(header::CONTENT_TYPE) .and_then(|v| v.to_str().ok()) .unwrap_or("application/octet-stream") .to_string(); + let max_upload = state.core.config.storage.direct_put_max_bytes; - // ── Streaming ingest: body → CDC chunk store ────────────── - // Shared with the NextCloud-compat PUT handler; chunking + hashing + - // dedup checks run while the body arrives — no spool file, no re-read. + // ── Active-lock guard (RFC 4918 §9.10.4) ────────────────────────── + if let Some(resp) = + enforce_native_lock(&state.webdav_lock_store, if_header_owned.as_deref(), &path) + { + return Ok(resp); + } + + // ── Ownership / existence check ─────────────────────────────────── + // Resolves to: File(existing), Folder(wrong), or Err(new file). + // Sets `file_existed` for 201 vs 204 and `current_etag` for If-Match. + let mut file_existed = false; + let mut current_etag: Option = None; + if let Some(resolver) = &state.path_resolver { + match resolver.resolve_path_for_user(&path, user.id).await { + Ok(ResolvedResource::File(f)) => { + file_existed = true; + current_etag = Some(f.etag.clone()); + } + Ok(ResolvedResource::Folder(_)) => { + return Err(AppError::bad_request("Cannot PUT to a directory")); + } + Err(_) => { + // File doesn't exist — verify parent. RFC 4918 §9.7.1: missing + // parent MUST produce 409 Conflict, not 404. + let parent_path = path.rfind('/').map(|i| &path[..i]).unwrap_or(""); + if !parent_path.is_empty() { + resolver + .resolve_path_for_user(parent_path, user.id) + .await + .map_err(|_| { + AppError::conflict(format!("Parent folder not found: {}", parent_path)) + })?; + } + } + } + } + + // ── RFC 7232 conditional preconditions ──────────────────────────── + // Evaluated before ingesting the body to save bandwidth on doomed requests. + if let Some(ref inm) = if_none_match { + // If-None-Match: * → fail if resource exists (prevent overwrite) + if inm == "*" && file_existed { + return Err(AppError::precondition_failed( + "If-None-Match: * — resource already exists", + )); + } + } + if let Some(ref im) = if_match { + if im == "*" { + // If-Match: * → fail if resource does not exist + if !file_existed { + return Err(AppError::precondition_failed( + "If-Match: * — resource does not exist", + )); + } + } else { + // If-Match: → strong comparison against current ETag + match ¤t_etag { + None => { + return Err(AppError::precondition_failed( + "If-Match — resource does not exist", + )); + } + Some(etag) => { + let client_tag = im.trim_matches('"'); + let server_tag = etag.trim_matches('"'); + if client_tag != server_tag { + return Err(AppError::precondition_failed("If-Match — ETag mismatch")); + } + } + } + } + } + + // ── Streaming ingest ────────────────────────────────────────────── let filename = crate::common::mime_detect::filename_from_path(&path).to_string(); let ingested = upload_ingest::ingest_body_to_cas( req.into_body(), @@ -1184,7 +1308,7 @@ async fn handle_put( ) .await?; - // ── Quota enforcement ──────────────────────────────────── + // ── Quota enforcement ───────────────────────────────────────────── if let Some(storage_svc) = state.storage_usage_service.as_ref() && let Err(err) = storage_svc .check_storage_quota(user.id, ingested.size) @@ -1204,7 +1328,7 @@ async fn handle_put( )); } - // ── Atomic store: swap the file row onto the ingested blob ── + // ── Atomic store ────────────────────────────────────────────────── let content_type = ingested.content_type.clone(); let drive_id = resolve_drive_id_for_native_webdav(&state, user.id).await?; let result = file_upload_service @@ -1219,10 +1343,21 @@ async fn handle_put( .await; match result { - Ok(_file_dto) => Ok(Response::builder() - .status(StatusCode::NO_CONTENT) - .body(Body::empty()) - .unwrap()), + Ok(file_dto) => { + // RFC 4918 §9.7.1: 201 Created for new resources, 204 No Content + // for overwrites. Always include ETag so clients can use it for + // subsequent conditional requests without a round-trip HEAD. + let status = if file_existed { + StatusCode::NO_CONTENT + } else { + StatusCode::CREATED + }; + Ok(Response::builder() + .status(status) + .header(header::ETAG, &file_dto.etag) + .body(Body::empty()) + .unwrap()) + } Err(e) => Err(AppError::internal_error(format!( "Failed to put file: {}", e @@ -1252,7 +1387,17 @@ async fn handle_mkcol( return Err(AppError::conflict("Root folder already exists")); } - // Read request body - must be empty for MKCOL (RFC 4918 §9.3) + // Extract content-type before consuming the body. + let req_content_type = req + .headers() + .get(header::CONTENT_TYPE) + .and_then(|v| v.to_str().ok()) + .map(|s| s.to_string()); + + // RFC 4918 §9.3.1: MKCOL body MUST be empty. A non-empty body with a + // recognised XML content-type is 400 Bad Request (malformed MKCOL body); + // a non-empty body with an unrecognised content-type is 415 Unsupported + // Media Type. We read up to MAX_MKCOL_BODY bytes to distinguish the two. let body_bytes = { let body = req.into_body(); body::to_bytes(body, MAX_MKCOL_BODY) @@ -1261,51 +1406,108 @@ async fn handle_mkcol( }; if !body_bytes.is_empty() { + // A body whose content-type looks like XML → 400 (client sent a MKCOL + // extended request we don't support); anything else → 415. + let ct = req_content_type.as_deref().unwrap_or(""); + if ct.contains("xml") { + return Err(AppError::bad_request( + "MKCOL with XML body is not supported", + )); + } return Err(AppError::unsupported_media_type( "MKCOL request body must be empty", )); } - // Path is already translated by dispatch (e.g. "My Folder - jared/03/01"). - // Walk each segment: the first is the home folder (already exists), - // subsequent segments are created as needed with proper parent_id. - // `drive_id` scopes each per-segment path probe to the caller's default - // drive (post-D0 invariant: `storage.folders.path` repeats across drives). + // RFC 4918 §9.3.1: MKCOL on an existing URL MUST return 405. + // RFC 4918 §9.3.1: MKCOL without an existing parent MUST return 409. + // This handler only creates a single collection (the last path segment). + // It does NOT auto-create intermediate ancestors ("mkdir -p" semantics + // violate the RFC and were causing the test failures). let drive_id = resolve_drive_id_for_native_webdav(&state, user.id).await?; let segments: Vec<&str> = path.split('/').filter(|s| !s.is_empty()).collect(); - let mut parent_id: Option = None; - let mut accumulated_path = String::new(); - for segment in &segments { - if !accumulated_path.is_empty() { - accumulated_path.push('/'); - } - accumulated_path.push_str(segment); - - match folder_service - .get_folder_by_path(&accumulated_path, drive_id) - .await - { - Ok(existing) => { - parent_id = Some(existing.id); - } - Err(_) => { - let create_dto = crate::application::dtos::folder_dto::CreateFolderDto { - name: segment.to_string(), - parent_id: parent_id.clone(), - }; - // Propagate DomainError -> AppError so NotFound/Conflict map to - // their proper HTTP status codes (was: blanket 500 swallowed - // ownership-rejection NotFound from verify_owner). - let created = folder_service - .create_folder_with_perms(create_dto, user.id) - .await - .map_err(AppError::from)?; - parent_id = Some(created.id); - } - } + if segments.is_empty() { + return Err(AppError::conflict("Root folder already exists")); } + // Check whether the target itself already exists (file or folder → 405). + if let Some(resolver) = &state.path_resolver { + if resolver + .exists_for_user(&path, user.id) + .await + .unwrap_or(false) + { + return Err(AppError::new( + StatusCode::METHOD_NOT_ALLOWED, + "Collection already exists", + "AlreadyExists", + )); + } + } else if folder_service + .get_folder_by_path(&path, drive_id) + .await + .is_ok() + { + return Err(AppError::new( + StatusCode::METHOD_NOT_ALLOWED, + "Collection already exists", + "AlreadyExists", + )); + } + + // Resolve the parent path. RFC 4918 §9.3.1: if the parent does not + // exist, return 409 Conflict. If the parent exists but is a file, also + // return 409 (cannot create a collection inside a file). + let new_segment = *segments.last().unwrap(); + let parent_segments = &segments[..segments.len() - 1]; + + let parent_id = if parent_segments.is_empty() { + // Top-level creation — no parent required; the root folder acts as parent. + None + } else { + let parent_path = parent_segments.join("/"); + // Parent must be a folder, not a file. + if let Some(resolver) = &state.path_resolver { + match resolver.resolve_path_for_user(&parent_path, user.id).await { + Ok(ResolvedResource::Folder(f)) => Some(f.id), + Ok(ResolvedResource::File(_)) => { + return Err(AppError::conflict( + "Parent path is a file, not a collection", + )); + } + Err(_) => { + return Err(AppError::conflict(format!( + "Parent folder not found: {}", + parent_path + ))); + } + } + } else { + match folder_service + .get_folder_by_path(&parent_path, drive_id) + .await + { + Ok(f) => Some(f.id), + Err(_) => { + return Err(AppError::conflict(format!( + "Parent folder not found: {}", + parent_path + ))); + } + } + } + }; + + let create_dto = crate::application::dtos::folder_dto::CreateFolderDto { + name: new_segment.to_string(), + parent_id, + }; + folder_service + .create_folder_with_perms(create_dto, user.id) + .await + .map_err(AppError::from)?; + Ok(Response::builder() .status(StatusCode::CREATED) .body(Body::empty()) @@ -1453,6 +1655,11 @@ async fn handle_move( .await .unwrap_or(destination_path); + // RFC 4918 §9.9.3: MOVE to self MUST return 403 Forbidden. + if destination_path == source_path { + return Err(AppError::forbidden("Cannot MOVE a resource to itself")); + } + // Destination lock guard: MOVE also creates/replaces a resource at // the destination. If that path is locked, the same If: header must // satisfy it. @@ -1464,45 +1671,66 @@ async fn handle_move( return Ok(resp); } - // Get services from state let file_retrieval_service = &state.applications.file_retrieval_service; let file_management_service = &state.applications.file_management_service; let folder_service = &state.applications.folder_service; - // `drive_id` scopes every path-based lookup below to the caller's - // default drive (post-D0 invariant: `storage.{files,folders}.path` - // repeats across drives). let drive_id = resolve_drive_id_for_native_webdav(&state, user.id).await?; - // Check if destination already exists (for Overwrite header compliance) - if !overwrite { - let dest_exists = if let Some(resolver) = &state.path_resolver { - resolver - .exists_for_user(&destination_path, user.id) - .await - .unwrap_or(false) - } else { - folder_service - .get_folder_by_path(&destination_path, drive_id) + // Probe destination existence for Overwrite semantics and 201 vs 204. + let dest_existed = if let Some(resolver) = &state.path_resolver { + resolver + .exists_for_user(&destination_path, user.id) + .await + .unwrap_or(false) + } else { + folder_service + .get_folder_by_path(&destination_path, drive_id) + .await + .is_ok() + || file_retrieval_service + .get_file_by_path(&destination_path, drive_id) .await .is_ok() - || file_retrieval_service - .get_file_by_path(&destination_path, drive_id) - .await - .is_ok() - }; - if dest_exists { + }; + + if dest_existed { + if !overwrite { return Err(AppError::precondition_failed( "Destination already exists and Overwrite is F", )); } + // RFC 4918 §9.9.3: when Overwrite: T, perform a DELETE on the + // destination before moving. Without this the rename/move fails + // on a unique-index conflict (same name in same parent). + match resolve_or_legacy(&state, &destination_path, user.id).await { + Some(ResolvedResource::Folder(f)) => { + folder_service + .delete_folder_with_perms(&f.id, user.id) + .await + .map_err(|e| { + AppError::internal_error(format!( + "Failed to delete existing destination: {}", + e + )) + })?; + } + Some(ResolvedResource::File(f)) => { + file_management_service + .delete_file_with_perms(&f.id, user.id) + .await + .map_err(|e| { + AppError::internal_error(format!( + "Failed to delete existing destination: {}", + e + )) + })?; + } + None => {} + } } - // Resolve source via optimized resolver with legacy fallback (see - // `resolve_or_legacy` for the rationale). Single match collapses the - // two near-identical branches that the resolver-only + legacy-only - // versions used to keep. - let _ = file_retrieval_service; // referenced via resolve_or_legacy + let _ = file_retrieval_service; let resolved = resolve_or_legacy(&state, &source_path, user.id) .await .ok_or_else(|| AppError::not_found(format!("Resource not found: {}", source_path)))?; @@ -1522,22 +1750,33 @@ async fn handle_move( match resolved { ResolvedResource::Folder(folder) => { - let move_dto = crate::application::dtos::folder_dto::MoveFolderDto { - parent_id: if dest_parent_path.is_empty() { - None - } else if let Ok(parent) = folder_service + // RFC 4918 §9.9.5: missing destination parent → 409 Conflict. + let target_parent_id = if dest_parent_path.is_empty() { + None + } else { + match folder_service .get_folder_by_path(dest_parent_path, drive_id) .await { - assert_owner( - parent.owner_id.as_deref(), - &user.id.to_string(), - dest_parent_path, - )?; - Some(parent.id) - } else { - None - }, + Ok(parent) => { + assert_owner( + parent.owner_id.as_deref(), + &user.id.to_string(), + dest_parent_path, + )?; + Some(parent.id) + } + Err(_) => { + return Err(AppError::conflict(format!( + "Destination parent not found: {}", + dest_parent_path + ))); + } + } + }; + + let move_dto = crate::application::dtos::folder_dto::MoveFolderDto { + parent_id: target_parent_id, }; folder_service @@ -1557,12 +1796,7 @@ async fn handle_move( } ResolvedResource::File(file) => { if source_parent_path != dest_parent_path { - // Resolve the destination's parent PATH into a folder ID - // before handing it to move_file_with_perms (which takes - // an Option, not a path). Previously - // the path was passed straight through and the move - // would silently fail because no row matches a folder - // whose id literally equals the path text. + // RFC 4918 §9.9.5: missing destination parent → 409 Conflict. let target_parent_id = if dest_parent_path.is_empty() { None } else { @@ -1570,7 +1804,7 @@ async fn handle_move( .get_folder_by_path(dest_parent_path, drive_id) .await .map_err(|_| { - AppError::not_found(format!( + AppError::conflict(format!( "Destination parent not found: {}", dest_parent_path )) @@ -1596,8 +1830,21 @@ 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 + } else { + StatusCode::CREATED + }; Ok(Response::builder() - .status(StatusCode::CREATED) + .status(status) .body(Body::empty()) .unwrap()) } @@ -1665,6 +1912,11 @@ async fn handle_copy( .await .unwrap_or(destination_path); + // RFC 4918 §9.8.5: COPY to self MUST return 403 Forbidden. + if destination_path == source_path { + return Err(AppError::forbidden("Cannot COPY a resource to itself")); + } + // Active-lock guard on the destination (RFC 4918 §9.10.4). if let Some(resp) = enforce_native_lock( &state.webdav_lock_store, @@ -1684,40 +1936,64 @@ async fn handle_copy( // Get services from state let file_retrieval_service = &state.applications.file_retrieval_service; let folder_service = &state.applications.folder_service; + let file_management_service = &state.applications.file_management_service; - // `drive_id` scopes every path-based lookup below to the caller's - // default drive (post-D0 invariant: `storage.{files,folders}.path` - // repeats across drives). let drive_id = resolve_drive_id_for_native_webdav(&state, user.id).await?; - // Check if destination already exists (for Overwrite header compliance) - if !overwrite { - let dest_exists = if let Some(resolver) = &state.path_resolver { - resolver - .exists_for_user(&destination_path, user.id) - .await - .unwrap_or(false) - } else { - folder_service - .get_folder_by_path(&destination_path, drive_id) + // Probe destination existence for Overwrite semantics and 201 vs 204. + let dest_existed = if let Some(resolver) = &state.path_resolver { + resolver + .exists_for_user(&destination_path, user.id) + .await + .unwrap_or(false) + } else { + folder_service + .get_folder_by_path(&destination_path, drive_id) + .await + .is_ok() + || file_retrieval_service + .get_file_by_path(&destination_path, drive_id) .await .is_ok() - || file_retrieval_service - .get_file_by_path(&destination_path, drive_id) - .await - .is_ok() - }; - if dest_exists { + }; + + if dest_existed { + if !overwrite { return Err(AppError::precondition_failed( "Destination already exists and Overwrite is F", )); } + // RFC 4918 §9.8.4: when Overwrite: T, the server MUST perform a + // DELETE on the destination before the copy. Without this the copy + // service returns a unique-index conflict (500). + match resolve_or_legacy(&state, &destination_path, user.id).await { + Some(ResolvedResource::Folder(f)) => { + folder_service + .delete_folder_with_perms(&f.id, user.id) + .await + .map_err(|e| { + AppError::internal_error(format!( + "Failed to delete existing destination: {}", + e + )) + })?; + } + Some(ResolvedResource::File(f)) => { + file_management_service + .delete_file_with_perms(&f.id, user.id) + .await + .map_err(|e| { + AppError::internal_error(format!( + "Failed to delete existing destination: {}", + e + )) + })?; + } + None => {} + } } - // Resolve source via optimized resolver with legacy fallback; collapses - // the two near-identical branches the resolver-only + legacy-only - // versions used to keep. - let _ = file_retrieval_service; // referenced via resolve_or_legacy + let _ = file_retrieval_service; let resolved = resolve_or_legacy(&state, &source_path, user.id) .await .ok_or_else(|| AppError::not_found(format!("Resource not found: {}", source_path)))?; @@ -1731,27 +2007,35 @@ async fn handle_copy( .map(|i| &destination_path[..i]) .unwrap_or(""); + // RFC 4918 §9.8.5: if the destination parent does not exist, return 409. let target_parent_id = if dest_parent_path.is_empty() { None - } else if let Ok(parent) = folder_service - .get_folder_by_path(dest_parent_path, drive_id) - .await - { - assert_owner( - parent.owner_id.as_deref(), - &user.id.to_string(), - dest_parent_path, - )?; - Some(parent.id) } else { - None + match folder_service + .get_folder_by_path(dest_parent_path, drive_id) + .await + { + Ok(parent) => { + assert_owner( + parent.owner_id.as_deref(), + &user.id.to_string(), + dest_parent_path, + )?; + Some(parent.id) + } + Err(_) => { + return Err(AppError::conflict(format!( + "Destination parent not found: {}", + dest_parent_path + ))); + } + } }; match resolved { ResolvedResource::Folder(folder) => { let recursive = depth != "0"; if recursive { - let file_management_service = &state.applications.file_management_service; file_management_service .copy_folder_tree_with_perms( &folder.id, @@ -1780,15 +2064,6 @@ async fn handle_copy( } } ResolvedResource::File(file) => { - // M8b fix: copy_file_with_perms now accepts an optional new - // filename — without it, a copy to the same folder with a - // different name collided with the source on the - // (folder, name, user) unique index. Pass dest_name when it - // differs from the source so the INSERT lands with the - // intended name in a single round-trip; pass None for the - // "same name in a different folder" case to keep the existing - // semantics. - let file_management_service = &state.applications.file_management_service; let copy_name = (file.name != dest_name).then(|| dest_name.to_string()); file_management_service .copy_file_with_perms(&file.id, user.id, target_parent_id, copy_name) @@ -1797,8 +2072,14 @@ async fn handle_copy( } } + // RFC 4918 §9.8.5: 201 Created when destination is new, 204 when overwritten. + let status = if dest_existed { + StatusCode::NO_CONTENT + } else { + StatusCode::CREATED + }; Ok(Response::builder() - .status(StatusCode::NO_CONTENT) + .status(status) .body(Body::empty()) .unwrap()) } diff --git a/tests/dav_compliance/rfc4918_proppatch.rs b/tests/dav_compliance/rfc4918_proppatch.rs new file mode 100644 index 00000000..5f62b7e2 --- /dev/null +++ b/tests/dav_compliance/rfc4918_proppatch.rs @@ -0,0 +1,343 @@ +//! RFC 4918 §9.2 PROPPATCH compliance — dead property storage and retrieval. + +use reqwest::Method; + +use super::harness::{get_server, unique_name}; + +fn propfind() -> Method { + Method::from_bytes(b"PROPFIND").unwrap() +} + +fn proppatch() -> Method { + Method::from_bytes(b"PROPPATCH").unwrap() +} + +/// PROPPATCH set a custom property → 207 with 200 propstat. +#[tokio::test] +async fn proppatch_set_returns_207() { + let srv = get_server(); + let path = format!("/webdav/{}", unique_name("pp_set")); + let (k, v) = srv.auth(); + + srv.client() + .put(srv.url(&path)) + .header(k, v.clone()) + .body("x") + .send() + .await + .unwrap(); + + let xml = r#" + + + + Alice + + +"#; + + let res = srv + .client() + .request(proppatch(), srv.url(&path)) + .header(k, v) + .header("Content-Type", "application/xml") + .body(xml) + .send() + .await + .unwrap(); + assert_eq!(res.status(), 207, "PROPPATCH must return 207"); + let body = res.text().await.unwrap(); + assert!( + body.contains("200") || body.contains("HTTP/1.1 200"), + "PROPPATCH 207 must contain 200 propstat; body: {body}" + ); +} + +/// PROPPATCH set → PROPFIND retrieves the stored value. +#[tokio::test] +async fn proppatch_set_property_visible_in_propfind() { + let srv = get_server(); + let path = format!("/webdav/{}", unique_name("pp_roundtrip")); + let (k, v) = srv.auth(); + + srv.client() + .put(srv.url(&path)) + .header(k, v.clone()) + .body("data") + .send() + .await + .unwrap(); + + // Set dead property + let set_xml = r#" + + + + blue + + +"#; + + let pp_res = srv + .client() + .request(proppatch(), srv.url(&path)) + .header(k, v.clone()) + .header("Content-Type", "application/xml") + .body(set_xml) + .send() + .await + .unwrap(); + assert_eq!(pp_res.status(), 207, "PROPPATCH set must return 207"); + + // Retrieve via PROPFIND allprop + let pf_res = srv + .client() + .request(propfind(), srv.url(&path)) + .header(k, v) + .header("Depth", "0") + .send() + .await + .unwrap(); + assert_eq!(pf_res.status(), 207); + let body = pf_res.text().await.unwrap(); + assert!( + body.contains("color") || body.contains("blue"), + "PROPFIND allprop must include dead property set by PROPPATCH; body: {body}" + ); +} + +/// PROPPATCH remove → property absent from subsequent PROPFIND. +#[tokio::test] +async fn proppatch_remove_property_not_in_propfind() { + let srv = get_server(); + let path = format!("/webdav/{}", unique_name("pp_remove")); + let (k, v) = srv.auth(); + + srv.client() + .put(srv.url(&path)) + .header(k, v.clone()) + .body("data") + .send() + .await + .unwrap(); + + // First set + let set_xml = r#" + + removeme +"#; + srv.client() + .request(proppatch(), srv.url(&path)) + .header(k, v.clone()) + .header("Content-Type", "application/xml") + .body(set_xml) + .send() + .await + .unwrap(); + + // Then remove + let remove_xml = r#" + + +"#; + let rem_res = srv + .client() + .request(proppatch(), srv.url(&path)) + .header(k, v.clone()) + .header("Content-Type", "application/xml") + .body(remove_xml) + .send() + .await + .unwrap(); + assert_eq!(rem_res.status(), 207, "PROPPATCH remove must return 207"); + + // Verify gone — request the specific prop, expect 404 propstat + let pf_xml = r#" + + +"#; + let pf_res = srv + .client() + .request(propfind(), srv.url(&path)) + .header(k, v) + .header("Depth", "0") + .header("Content-Type", "application/xml") + .body(pf_xml) + .send() + .await + .unwrap(); + assert_eq!(pf_res.status(), 207); + let body = pf_res.text().await.unwrap(); + assert!( + body.contains("404"), + "Removed dead property must appear in 404 propstat; body: {body}" + ); +} + +/// PROPPATCH set + remove in same request → both applied atomically. +#[tokio::test] +async fn proppatch_set_and_remove_in_same_request() { + let srv = get_server(); + let path = format!("/webdav/{}", unique_name("pp_setrem")); + let (k, v) = srv.auth(); + + srv.client() + .put(srv.url(&path)) + .header(k, v.clone()) + .body("x") + .send() + .await + .unwrap(); + + // Pre-seed a property to remove + let seed_xml = r#" + + gone +"#; + srv.client() + .request(proppatch(), srv.url(&path)) + .header(k, v.clone()) + .header("Content-Type", "application/xml") + .body(seed_xml) + .send() + .await + .unwrap(); + + // Set new + remove old in one request + let xml = r#" + + here + +"#; + let res = srv + .client() + .request(proppatch(), srv.url(&path)) + .header(k, v) + .header("Content-Type", "application/xml") + .body(xml) + .send() + .await + .unwrap(); + assert_eq!(res.status(), 207, "combined set+remove must return 207"); + let body = res.text().await.unwrap(); + // Both ops should succeed + assert!( + !body.contains("409") && !body.contains("403"), + "combined PROPPATCH must not fail; body: {body}" + ); +} + +/// PROPPATCH on non-existent resource → 404. +#[tokio::test] +async fn proppatch_nonexistent_resource_returns_404() { + let srv = get_server(); + let path = format!("/webdav/{}", unique_name("pp_ghost")); + let (k, v) = srv.auth(); + + let xml = r#" + + y +"#; + + let res = srv + .client() + .request(proppatch(), srv.url(&path)) + .header(k, v) + .header("Content-Type", "application/xml") + .body(xml) + .send() + .await + .unwrap(); + assert_eq!( + res.status(), + 404, + "PROPPATCH on non-existent resource must return 404" + ); +} + +/// PROPPATCH on collection (folder) → 207. +#[tokio::test] +async fn proppatch_on_collection_returns_207() { + let srv = get_server(); + let col = format!("/webdav/{}", unique_name("pp_col")); + let (k, v) = srv.auth(); + + srv.client() + .request(Method::from_bytes(b"MKCOL").unwrap(), srv.url(&col)) + .header(k, v.clone()) + .send() + .await + .unwrap(); + + let xml = r#" + + my folder +"#; + + let res = srv + .client() + .request(proppatch(), srv.url(&col)) + .header(k, v) + .header("Content-Type", "application/xml") + .body(xml) + .send() + .await + .unwrap(); + assert_eq!(res.status(), 207, "PROPPATCH on collection must return 207"); +} + +/// PROPFIND specific dead property returns value in 200 propstat (not 404). +#[tokio::test] +async fn propfind_specific_dead_property_returns_200_propstat() { + let srv = get_server(); + let path = format!("/webdav/{}", unique_name("pp_specific")); + let (k, v) = srv.auth(); + + srv.client() + .put(srv.url(&path)) + .header(k, v.clone()) + .body("x") + .send() + .await + .unwrap(); + + // Set + let set_xml = r#" + + 5 +"#; + srv.client() + .request(proppatch(), srv.url(&path)) + .header(k, v.clone()) + .header("Content-Type", "application/xml") + .body(set_xml) + .send() + .await + .unwrap(); + + // PROPFIND for that exact property + let pf_xml = r#" + + +"#; + let pf_res = srv + .client() + .request(propfind(), srv.url(&path)) + .header(k, v) + .header("Depth", "0") + .header("Content-Type", "application/xml") + .body(pf_xml) + .send() + .await + .unwrap(); + assert_eq!(pf_res.status(), 207); + let body = pf_res.text().await.unwrap(); + assert!( + !body.contains("404"), + "Known dead property must not be in 404 propstat; body: {body}" + ); + assert!( + body.contains("rating") || body.contains("5"), + "Response must include the dead property value; body: {body}" + ); +} diff --git a/tests/webdav/run-litmus.sh b/tests/webdav/run-litmus.sh new file mode 100755 index 00000000..938da295 --- /dev/null +++ b/tests/webdav/run-litmus.sh @@ -0,0 +1,123 @@ +#!/usr/bin/env bash +# WebDAV RFC 4918 compliance test using the litmus test suite. +# +# Usage (from repo root via justfile): +# just litmus-test +# +# Or directly (server + postgres must already be running): +# bash tests/webdav/run-litmus.sh +# +# Requires: litmus (apt install litmus), jq, curl +# litmus tests: basic copymove props locks + +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "$0")/../.." && pwd)" +COMMON="$REPO_ROOT/tests/common" +WEBDAV_DIR="$REPO_ROOT/tests/webdav" + +source "$WEBDAV_DIR/test.env" + +SERVER_PORT="${base_url##*:}" + +log() { echo "[litmus] $*"; } +die() { echo "[litmus] ERROR: $*" >&2; exit 1; } + +# ── Dependency checks ────────────────────────────────────────────────────────── + +if ! command -v litmus >/dev/null 2>&1; then + die "litmus not found. Install with: sudo apt install litmus" +fi +if ! command -v jq >/dev/null 2>&1; then + die "jq not found. Install with: sudo apt install jq" +fi + +# ── Teardown ─────────────────────────────────────────────────────────────────── + +SERVER_PID="" + +cleanup() { + if [[ -n "$SERVER_PID" ]]; then + log "Stopping OxiCloud (pid $SERVER_PID)..." + kill "$SERVER_PID" 2>/dev/null || true + wait "$SERVER_PID" 2>/dev/null || true + fi + bash "$COMMON/stop-db.sh" +} + +trap cleanup EXIT + +# ── 1. Start postgres ────────────────────────────────────────────────────────── + +bash "$COMMON/spawn-db.sh" + +# ── 2. Start OxiCloud ───────────────────────────────────────────────────────── + +set -a +source "$COMMON/server.env" +OXICLOUD_SERVER_PORT=$SERVER_PORT +OXICLOUD_STORAGE_PATH="$REPO_ROOT/tests/webdav/storage-litmus" +set +a + +rm -rf "$OXICLOUD_STORAGE_PATH" +mkdir -p "$OXICLOUD_STORAGE_PATH" + +BUILD_TARGET="${BUILD_TARGET:-debug}" +OXICLOUD_BIN="$REPO_ROOT/target/$BUILD_TARGET/oxicloud" + +if [[ -x "$OXICLOUD_BIN" ]]; then + log "Starting pre-built OxiCloud ($BUILD_TARGET) on port $SERVER_PORT..." + "$OXICLOUD_BIN" --config "$COMMON/server.env" & +else + log "Building and starting OxiCloud on port $SERVER_PORT..." + cd "$REPO_ROOT" + cargo build 2>&1 + "$REPO_ROOT/target/debug/oxicloud" --config "$COMMON/server.env" & +fi +SERVER_PID=$! + +log "Waiting for server at $base_url..." +deadline=$(( $(date +%s) + 60 )) +until curl -sf "$base_url/ready" >/dev/null 2>&1; do + [[ $(date +%s) -ge $deadline ]] && die "Server did not become ready within 60s" + sleep 1 +done +log "Server ready." + +# ── 3. Bootstrap admin + app password ──────────────────────────────────────── + +SETUP_STATUS=$(curl -s -o /dev/null -w "%{http_code}" \ + -X POST -H "Content-Type: application/json" \ + -d "{\"username\":\"$username\",\"email\":\"$email\",\"password\":\"$password\"}" \ + "$base_url/api/setup") +case "$SETUP_STATUS" in + 201) log "Admin account created." ;; + 403) log "Admin account already exists." ;; + *) die "Unexpected /api/setup status: $SETUP_STATUS" ;; +esac + +LOGIN_RESP=$(curl -s -X POST -H "Content-Type: application/json" \ + -d "{\"username\":\"$username\",\"password\":\"$password\"}" \ + "$base_url/api/auth/login") +JWT=$(jq -r '.access_token' <<<"$LOGIN_RESP") +[[ -z "$JWT" || "$JWT" == "null" ]] && die "Login failed: $LOGIN_RESP" +log "Logged in as $username." + +APP_PW_RESP=$(curl -s -X POST \ + -H "Content-Type: application/json" \ + -H "Authorization: Bearer $JWT" \ + -d '{"label":"litmus-test"}' \ + "$base_url/api/auth/app-passwords") +APP_PASSWORD=$(jq -r '.password' <<<"$APP_PW_RESP") +[[ -z "$APP_PASSWORD" || "$APP_PASSWORD" == "null" ]] && die "App password creation failed: $APP_PW_RESP" +log "App password created." + +# ── 4. Run litmus ───────────────────────────────────────────────────────────── + +LITMUS_TESTS="${LITMUS_TESTS:-basic copymove props locks}" +WEBDAV_URL="$base_url/webdav/" + +log "Running litmus $LITMUS_TESTS against $WEBDAV_URL" +TESTS="$LITMUS_TESTS" litmus "$WEBDAV_URL" "$username" "$APP_PASSWORD" + +log "litmus passed."