diff --git a/src/application/adapters/webdav_adapter.rs b/src/application/adapters/webdav_adapter.rs index 24dbde25..f8a77096 100644 --- a/src/application/adapters/webdav_adapter.rs +++ b/src/application/adapters/webdav_adapter.rs @@ -301,6 +301,63 @@ 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 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 { + 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 + ))))?; + } + } + 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 folder properties as a response fn write_folder_response( xml_writer: &mut Writer, @@ -308,50 +365,50 @@ impl WebDavAdapter { request: &PropFindRequest, href: &str, ) -> Result<()> { - // Start response element 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")))?; - - // 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. + let (known, unknown): (Vec<_>, Vec<_>) = + props.iter().partition(|p| Self::folder_prop_is_known(p)); + + 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, &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")))?; - - // 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 +419,50 @@ impl WebDavAdapter { request: &PropFindRequest, href: &str, ) -> Result<()> { - // Start response element 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")))?; - - // 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. + let (known, unknown): (Vec<_>, Vec<_>) = + props.iter().partition(|p| Self::file_prop_is_known(p)); + + 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, &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")))?; - - // 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(()) } @@ -611,20 +668,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(()) @@ -694,20 +742,11 @@ 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(()) diff --git a/src/interfaces/api/handlers/webdav_handler.rs b/src/interfaces/api/handlers/webdav_handler.rs index b2958f18..898d9a03 100644 --- a/src/interfaces/api/handlers/webdav_handler.rs +++ b/src/interfaces/api/handlers/webdav_handler.rs @@ -1148,79 +1148,122 @@ 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(), @@ -1231,7 +1274,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) @@ -1251,7 +1294,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 @@ -1266,10 +1309,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 @@ -1299,7 +1353,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) @@ -1308,51 +1372,101 @@ 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()) @@ -1500,6 +1614,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. @@ -1511,45 +1630,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)))?; @@ -1569,22 +1709,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 @@ -1604,12 +1755,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 { @@ -1617,7 +1763,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 )) @@ -1643,8 +1789,14 @@ async fn handle_move( } } + // 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()) } @@ -1712,6 +1864,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, @@ -1731,40 +1888,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)))?; @@ -1778,27 +1959,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, @@ -1827,15 +2016,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) @@ -1844,8 +2024,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/src/interfaces/api/routes.rs b/src/interfaces/api/routes.rs index 7cc52917..8276c4e9 100644 --- a/src/interfaces/api/routes.rs +++ b/src/interfaces/api/routes.rs @@ -592,8 +592,6 @@ pub fn create_api_routes(app_state: &Arc) -> Router> { tracing::info!("Contacts REST API routes initialized"); } - - // NOTE: WebDAV routes are mounted at top-level (/webdav) in main.rs // for client compatibility, NOT under /api.