diff --git a/migrations/20261024000000_nfc_name_column_comments.sql b/migrations/20261024000000_nfc_name_column_comments.sql new file mode 100644 index 00000000..6b4055d7 --- /dev/null +++ b/migrations/20261024000000_nfc_name_column_comments.sql @@ -0,0 +1,52 @@ +-- COMMENT ON COLUMN for every user-visible-name column whose invariant +-- ("stored bytes are NFC") is enforced by the write repository, not by +-- the DB itself. +-- +-- Why this migration exists +-- ───────────────────────── +-- Before 2026-09-04 the NFC invariant lived at `File::new` / +-- `Folder::new_folder` entity constructors — plausible-looking but +-- DEAD CODE for the create path, because every real caller went +-- straight from a DTO string to `sqlx::bind()` inside the repos +-- without ever constructing the entity first. Result: 22 audited +-- entry points, every single one shipped raw client input to the DB. +-- macOS Finder / DAVX5 / NC-desktop uploads landed NFD; NFC- +-- normalizing clients then failed to find their own content by URL +-- (AtalayaLabs/OxiCloud#706). +-- +-- The fix moved normalization to the repository methods that own the +-- INSERT / UPDATE. The next contributor writing a new write surface +-- may reasonably wonder where to enforce the invariant — this comment +-- puts the answer next to the column so grep-hunting the codebase is +-- not required. Purely documentation; no runtime effect. A stronger +-- form (CHECK CONSTRAINT `name = normalize(name, NFC)`) was +-- considered and rejected for now — that would rely on every +-- historical row already being NFC (which we deliberately do NOT +-- migrate on read, so pre-fix rows stay in place until an operator +-- runs `oxicloud migrate nfc-filenames`), and would fail-boot any +-- upgrade path where the migrate has not yet been applied. +-- +-- Idempotent. COMMENT ON COLUMN replaces any prior comment on the +-- same target, so re-running has no effect. + +COMMENT ON COLUMN storage.files.name IS + 'User-visible file name. MUST be NFC (Unicode Normalization Form C). ' + 'Invariant enforced at write time by ' + 'src/infrastructure/repositories/pg/file_blob_write_repository.rs — the ' + '`save_file_with_blob_impl`, `copy_file`, `rename_file`, ' + '`register_file_deferred`, and `copy_folder_tree` methods each call ' + '`normalize_storage_name(_owned)` before binding. No DB-level CHECK ' + 'constraint (historical NFD rows may still exist on pre-2026-09-04 ' + 'databases until `oxicloud migrate nfc-filenames` is run). New write ' + 'surfaces MUST land in one of those repo methods; direct INSERT ' + 'bypasses the invariant.'; + +COMMENT ON COLUMN storage.folders.name IS + 'User-visible folder name. MUST be NFC (Unicode Normalization Form C). ' + 'Invariant enforced at write time by ' + 'src/infrastructure/repositories/pg/folder_db_repository.rs — the ' + '`create_folder` and `rename_folder` methods each call ' + '`normalize_storage_name_owned` before binding. See also ' + 'storage.files.name — identical contract, different table. No DB-level ' + 'CHECK (see that column comment). New write surfaces MUST land in one of ' + 'those repo methods; direct INSERT bypasses the invariant.'; diff --git a/src/infrastructure/repositories/pg/address_book_pg_repository.rs b/src/infrastructure/repositories/pg/address_book_pg_repository.rs index cd9ead3c..a061e630 100644 --- a/src/infrastructure/repositories/pg/address_book_pg_repository.rs +++ b/src/infrastructure/repositories/pg/address_book_pg_repository.rs @@ -7,6 +7,7 @@ use crate::domain::entities::contact::AddressBook; use crate::domain::repositories::address_book_repository::{ AddressBookRepository, AddressBookRepositoryResult, }; +use crate::domain::services::path_service::normalize_storage_name; pub struct AddressBookPgRepository { pool: Arc, @@ -40,6 +41,15 @@ impl AddressBookRepository for AddressBookPgRepository { &self, address_book: AddressBook, ) -> AddressBookRepositoryResult { + // NFC-normalize the caller-supplied display name at the last + // touch before bind. Same choke-point pattern as the storage.* + // repos — the entity constructor's normalization is bypassed by + // every real production path (`AddressBook::from_raw` + // reconstructs from DB bytes; `AddressBook::new` goes through + // an inbound DTO that may or may not have been touched). + // Enforcing here means every carddav write surface — DAV + // `MKCOL`, REST create — lands in NFC regardless. + let normalized_name = normalize_storage_name(address_book.name()); let row = sqlx::query( r#" INSERT INTO carddav.address_books (id, name, owner_id, description, color, is_public, created_at, updated_at) @@ -48,7 +58,7 @@ impl AddressBookRepository for AddressBookPgRepository { "# ) .bind(address_book.id()) - .bind(address_book.name()) + .bind(&normalized_name) .bind(address_book.owner_id()) .bind(address_book.description()) .bind(address_book.color()) @@ -76,6 +86,8 @@ impl AddressBookRepository for AddressBookPgRepository { &self, address_book: AddressBook, ) -> AddressBookRepositoryResult { + // NFC-normalize on rename — see `create_address_book` for the why. + let normalized_name = normalize_storage_name(address_book.name()); let now = Utc::now(); let row = sqlx::query( r#" @@ -85,7 +97,7 @@ impl AddressBookRepository for AddressBookPgRepository { RETURNING id, name, owner_id, description, color, is_public, created_at, updated_at "#, ) - .bind(address_book.name()) + .bind(&normalized_name) .bind(address_book.description()) .bind(address_book.color()) .bind(address_book.is_public()) diff --git a/src/infrastructure/repositories/pg/calendar_pg_repository.rs b/src/infrastructure/repositories/pg/calendar_pg_repository.rs index fe0c213f..423c3ee1 100644 --- a/src/infrastructure/repositories/pg/calendar_pg_repository.rs +++ b/src/infrastructure/repositories/pg/calendar_pg_repository.rs @@ -7,6 +7,7 @@ use crate::domain::entities::calendar::Calendar; use crate::domain::repositories::calendar_repository::{ CalendarRepository, CalendarRepositoryResult, }; +use crate::domain::services::path_service::normalize_storage_name; pub struct CalendarPgRepository { pool: Arc, @@ -38,6 +39,15 @@ impl CalendarPgRepository { impl CalendarRepository for CalendarPgRepository { async fn create_calendar(&self, calendar: Calendar) -> CalendarRepositoryResult { + // NFC-normalize the caller-supplied display name at the last + // touch before bind — same choke-point pattern the storage.files + // / storage.folders repos use (see docs/plan/nfc-normalization.md + // / migrate.rs module doc). macOS CalDAV clients emit NFD in the + // display-name field just as Finder does in the filename field; + // NC-desktop / Thunderbird would then miss the calendar on their + // NFC-normalized lookup path. Same class of bug as + // AtalayaLabs/OxiCloud#706, different table. + let normalized_name = normalize_storage_name(calendar.name()); let row = sqlx::query( r#" INSERT INTO caldav.calendars (id, name, owner_id, description, color, is_public, created_at, updated_at) @@ -46,7 +56,7 @@ impl CalendarRepository for CalendarPgRepository { "# ) .bind(calendar.id()) - .bind(calendar.name()) + .bind(&normalized_name) .bind(calendar.owner_id()) .bind(calendar.description()) .bind(calendar.color()) @@ -75,6 +85,8 @@ impl CalendarRepository for CalendarPgRepository { } async fn update_calendar(&self, calendar: Calendar) -> CalendarRepositoryResult { + // NFC-normalize on rename — see `create_calendar` for the why. + let normalized_name = normalize_storage_name(calendar.name()); let now = Utc::now(); let row = sqlx::query( r#" @@ -84,7 +96,7 @@ impl CalendarRepository for CalendarPgRepository { RETURNING id, name, owner_id, description, color, is_public, created_at, updated_at "#, ) - .bind(calendar.name()) + .bind(&normalized_name) .bind(calendar.description()) .bind(calendar.color()) .bind(false) // is_public doesn't exist as a field diff --git a/src/infrastructure/repositories/pg/contact_group_pg_repository.rs b/src/infrastructure/repositories/pg/contact_group_pg_repository.rs index 02383cc0..9e0dd3ee 100644 --- a/src/infrastructure/repositories/pg/contact_group_pg_repository.rs +++ b/src/infrastructure/repositories/pg/contact_group_pg_repository.rs @@ -11,6 +11,7 @@ use crate::domain::entities::contact::{Contact, ContactGroup}; use crate::domain::repositories::contact_repository::{ ContactGroupRepository, ContactRepositoryResult, }; +use crate::domain::services::path_service::normalize_storage_name; pub struct ContactGroupPgRepository { pool: Arc, @@ -24,12 +25,19 @@ impl ContactGroupPgRepository { impl ContactGroupRepository for ContactGroupPgRepository { async fn create_group(&self, group: ContactGroup) -> ContactRepositoryResult { + // NFC-normalize at the last touch before bind — same choke-point + // pattern as the storage.* / caldav.* / carddav.address_books + // repos. Group display names on macOS Contacts sync as NFD + // (Address Book pushes decomposed forms in vCard KIND=group); + // NC-desktop / Thunderbird would then miss the group on their + // NFC-normalized lookup. + let normalized_name = normalize_storage_name(group.name()); sqlx::query( "INSERT INTO carddav.contact_groups (id, address_book_id, name, created_at, updated_at) VALUES ($1, $2, $3, $4, $5)" ) .bind(group.id()) .bind(group.address_book_id()) - .bind(group.name()) + .bind(&normalized_name) .bind(group.created_at()) .bind(group.updated_at()) .execute(self.pool.as_ref()) @@ -40,8 +48,10 @@ impl ContactGroupRepository for ContactGroupPgRepository { } async fn update_group(&self, group: ContactGroup) -> ContactRepositoryResult { + // NFC-normalize on rename — see `create_group` for the why. + let normalized_name = normalize_storage_name(group.name()); sqlx::query("UPDATE carddav.contact_groups SET name = $1, updated_at = $2 WHERE id = $3") - .bind(group.name()) + .bind(&normalized_name) .bind(Utc::now()) .bind(group.id()) .execute(self.pool.as_ref()) diff --git a/src/infrastructure/repositories/pg/drive_pg_repository.rs b/src/infrastructure/repositories/pg/drive_pg_repository.rs index e7f51ede..e207bc00 100644 --- a/src/infrastructure/repositories/pg/drive_pg_repository.rs +++ b/src/infrastructure/repositories/pg/drive_pg_repository.rs @@ -19,6 +19,7 @@ use crate::domain::entities::drive::{Drive, DriveKind}; use crate::domain::repositories::drive_repository::{ DriveRepository, DriveRepositoryError, DriveWithRootName, }; +use crate::domain::services::path_service::normalize_storage_name; /// Decode a `d.policies` JSONB column straight into `DrivePolicies` via /// `sqlx::types::Json` — a single `serde_json::from_slice` over the raw JSONB @@ -395,6 +396,13 @@ impl DriveRepository for DrivePgRepository { quota_bytes: Option, granted_by: Uuid, ) -> Result { + // NFC-normalize the admin-supplied shared-drive root name — same + // reasoning as `folder_db_repository::create_folder`. Even though + // this write is admin-only (not end-user-driven), the field feeds + // straight into `storage.folders.name` and WebDAV path lookups + // against it must match what NFC-normalizing clients send. + let name = normalize_storage_name(name); + // Same four-write transaction shape as `create_personal_drive_atomic` // (see that method for the why-not-CTE explanation). Differences: // - `kind='shared'`, `default_for_user=NULL`. diff --git a/src/infrastructure/repositories/pg/external_mount_repository.rs b/src/infrastructure/repositories/pg/external_mount_repository.rs index 366f1294..90dc683c 100644 --- a/src/infrastructure/repositories/pg/external_mount_repository.rs +++ b/src/infrastructure/repositories/pg/external_mount_repository.rs @@ -13,6 +13,7 @@ use crate::application::ports::external_mount_ports::{ ExternalMountRecord, ExternalMountRepositoryPort, NewExternalMount, }; use crate::domain::errors::DomainError; +use crate::domain::services::path_service::normalize_storage_name; /// PostgreSQL implementation of [`ExternalMountRepositoryPort`]. pub struct ExternalMountPgRepository { @@ -93,6 +94,14 @@ impl ExternalMountRepositoryPort for ExternalMountPgRepository { } async fn create(&self, mount: &NewExternalMount) -> Result<(), DomainError> { + // NFC-normalize the admin-supplied display label at the last + // touch before bind. Admin-facing (not end-user drag-drop) so + // NFD is unlikely, but the invariant matches the storage.* + // pattern — the sibling folder row (created via + // `folder_db_repository::create_folder`, which already + // normalizes) and this admin label should stay byte-consistent + // on any table. + let normalized_name = normalize_storage_name(&mount.name); sqlx::query( "INSERT INTO storage.external_mounts (mount_folder_id, kind, config, name, owner_id, read_only) @@ -101,7 +110,7 @@ impl ExternalMountRepositoryPort for ExternalMountPgRepository { .bind(mount.mount_folder_id) .bind(&mount.kind) .bind(&mount.config) - .bind(&mount.name) + .bind(&normalized_name) .bind(mount.owner_id) .bind(mount.read_only) .execute(self.pool.as_ref()) diff --git a/src/infrastructure/repositories/pg/file_blob_write_repository.rs b/src/infrastructure/repositories/pg/file_blob_write_repository.rs index dca03852..591b224d 100644 --- a/src/infrastructure/repositories/pg/file_blob_write_repository.rs +++ b/src/infrastructure/repositories/pg/file_blob_write_repository.rs @@ -17,6 +17,7 @@ use crate::application::dtos::display_helpers::category_order_for; use crate::application::ports::storage_ports::{CopyFolderTreeResult, FileWritePort}; use crate::common::errors::DomainError; use crate::domain::entities::file::File; +use crate::domain::services::path_service::{normalize_storage_name, normalize_storage_name_owned}; use super::transaction_utils::retry_on_deadlock; use crate::infrastructure::services::dedup_service::DedupService; @@ -281,6 +282,16 @@ impl FileBlobWriteRepository { size: u64, caller_id: Uuid, ) -> Result { + // NFC-normalize at the last touch before the DB bind. Same + // reasoning as `folder_db_repository::create_folder`: every write + // surface that lands here — REST multipart upload, by-hash instant + // upload, chunked-upload complete, WOPI create-fallback, WebDAV + // PUT, NC PUT, NC chunked-upload assemble — passes raw client + // bytes. macOS Finder emits NFD; canonicalising once here closes + // every audited entry-point at one choke-point. `is_nfc_quick` + // fast path is one table-lookup for the ~99% of names already NFC. + let name = normalize_storage_name_owned(name); + // Root files have no parent folder to derive an owner from — keep the // previous resolve_user_id(None) contract (release the ref, error out). let Some(fid) = folder_id.as_deref() else { @@ -630,7 +641,14 @@ impl FileWritePort for FileBlobWriteRepository { // folder's owner as the author when Adam copied a file into // Alice's folder. let target_fid = target_folder_id.clone(); - let rename_to = new_name.map(|s| s.to_string()); + // NFC-normalize the destination name at the last touch before the + // bind. `new_name = None` means "keep the source's stored name" — + // that path is already normalized (either by an earlier write here + // or, for pre-fix rows, deliberately left as-is per operator + // decision to not touch historical NFD content). Only fresh + // client-supplied `new_name` needs the pass; WebDAV `COPY` with a + // Destination header renaming a file is the canonical caller. + let rename_to = new_name.map(normalize_storage_name); let row = retry_on_deadlock("files.copy", || async { let mut tx = self.pool.begin().await?; @@ -764,6 +782,11 @@ impl FileWritePort for FileBlobWriteRepository { new_name: &str, caller_id: Uuid, ) -> Result { + // NFC-normalize the client-supplied name at the last touch — same + // reasoning as `save_file_with_blob_impl`. REST rename, WebDAV + // MOVE-with-rename, NC MOVE-with-rename all funnel here. + let new_name = normalize_storage_name(new_name); + // §14: `updated_by = $3` (caller_id), see move_file. let row = sqlx::query_as::< _, @@ -789,7 +812,7 @@ impl FileWritePort for FileBlobWriteRepository { created_by, updated_by "#, ) - .bind(new_name) + .bind(&new_name) .bind(file_id) .bind(caller_id) .fetch_optional(self.pool.as_ref()) @@ -877,6 +900,14 @@ impl FileWritePort for FileBlobWriteRepository { size: u64, caller_id: Uuid, ) -> Result<(File, PathBuf), DomainError> { + // NFC-normalize at the last touch before the DB bind — same + // reasoning as `save_file_with_blob_impl`. Deferred registration + // is the write-behind cache's fast-path (row up first, blob + // hash filled in on the async callback); it takes fresh client + // input via chunked-upload finalize among others, so NFD is + // reachable here too. + let name = normalize_storage_name_owned(name); + // For deferred registration we use a placeholder hash. // The write-behind cache will call update_file_content later. let placeholder_hash = "0000000000000000000000000000000000000000000000000000000000000000"; @@ -1074,6 +1105,13 @@ impl FileWritePort for FileBlobWriteRepository { target_parent_id: Option, dest_name: Option, ) -> Result { + // NFC-normalize the caller-supplied rename before handing off to + // the PG stored function. `dest_name = None` keeps the source's + // stored name (already normalized on ingest for post-fix rows; + // pre-fix historical NFD deliberately preserved). Only WebDAV + // COPY-a-folder-tree-with-rename passes a fresh client string. + let dest_name = dest_name.map(normalize_storage_name_owned); + let row = sqlx::query_as::<_, (String, i64, i64)>( "SELECT new_root_id, folders_copied, files_copied \ FROM storage.copy_folder_tree($1::uuid, $2::uuid, $3)", diff --git a/src/infrastructure/repositories/pg/folder_db_repository.rs b/src/infrastructure/repositories/pg/folder_db_repository.rs index d407ea05..e6ce6167 100644 --- a/src/infrastructure/repositories/pg/folder_db_repository.rs +++ b/src/infrastructure/repositories/pg/folder_db_repository.rs @@ -18,7 +18,7 @@ use crate::common::errors::DomainError; use crate::domain::entities::folder::Folder; use crate::domain::repositories::folder_repository::FolderRepository; use crate::domain::services::authorization::ResourceKind; -use crate::domain::services::path_service::StoragePath; +use crate::domain::services::path_service::{StoragePath, normalize_storage_name_owned}; /// Type alias for folder metadata rows from SQL queries. /// Tuple order: id, name, path, parent_id, drive_id, created_at, @@ -261,6 +261,21 @@ impl FolderRepository for FolderDbRepository { parent_id: Option, caller_id: Uuid, ) -> Result { + // Belt-and-suspenders NFC normalization at the last touch before the + // DB write. Every caller that lands here — REST `POST /api/folders`, + // WebDAV `MKCOL`, NextCloud `MKCOL`, batch create, WebDAV/NC `COPY` + // fall-through — passes the raw client-supplied name. macOS Finder / + // Android sync clients emit NFD path segments; if we bind them raw + // the DB row's bytes don't match NFC-normalizing clients' subsequent + // PROPFINDs (see AtalayaLabs/OxiCloud#706). Canonicalising here is + // the single choke-point that closes every entry-point audited on + // 2026-09-03 without asking each handler to remember. Fast-path + // `is_nfc_quick` inside `normalize_storage_name_owned` returns the + // owned string unchanged for names already in NFC — every visible + // ASCII name, every browser-composed non-ASCII name — so this costs + // one table-lookup on the hot path. + let name = normalize_storage_name_owned(name); + // Derive `drive_id` from the parent folder. Root-level folders // are reserved for the atomic drive-creation transaction in // `DrivePgRepository::create_personal_drive_atomic` (see @@ -689,6 +704,11 @@ impl FolderRepository for FolderDbRepository { new_name: String, caller_id: Uuid, ) -> Result { + // NFC-normalize the client-supplied new name — same reasoning as + // `create_folder` above (WebDAV `MOVE`, NC `MOVE`, REST rename all + // funnel here with raw client bytes). Cheap on the common path. + let new_name = normalize_storage_name_owned(new_name); + // The BEFORE UPDATE trigger recomputes path/lpath for this row; // the AFTER UPDATE cascade trigger then batch-updates all // descendants in a single UPDATE using the GiST lpath index. diff --git a/src/interfaces/api/handlers/webdav_handler.rs b/src/interfaces/api/handlers/webdav_handler.rs index 3700ea6c..22fd0374 100644 --- a/src/interfaces/api/handlers/webdav_handler.rs +++ b/src/interfaces/api/handlers/webdav_handler.rs @@ -36,6 +36,7 @@ use crate::application::services::folder_service::FolderService; use crate::common::di::AppState; use crate::domain::repositories::drive_repository::DriveRepository; use crate::domain::services::authorization::{Permission, Resource, Subject}; +use crate::domain::services::path_service::normalize_storage_name; use crate::infrastructure::services::path_resolver_service::ResolvedResource; use crate::infrastructure::services::webdav_dead_property_store::{DeadPropertyStore, ResourceRef}; use crate::interfaces::errors::AppError; @@ -2452,6 +2453,18 @@ async fn handle_mkcol( )); } + // Capture the URL-path segments BEFORE scope-resolution rewrites `path` + // to `scope.db_path` — we need the original request URL to reconstruct + // the canonical `Content-Location` when the last segment gets NFC- + // normalized. The last segment is the same either way (it's the target + // resource name), but the URL prefix (including any `@drive/` + // routing tokens the client used) is only preserved here. + let request_url_segments: Vec = path + .split('/') + .filter(|s| !s.is_empty()) + .map(str::to_owned) + .collect(); + // 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). @@ -2556,8 +2569,15 @@ async fn handle_mkcol( } }; + // NFC-normalize the client-supplied last segment so we can emit + // `Content-Location` when the canonical URL differs from what the + // client sent. The repo layer normalizes again on the way to the DB + // (idempotently — `is_nfc_quick` returns immediately for already-NFC + // input); doing it here too gives the handler a cheap way to know + // whether the URL changed. See AtalayaLabs/OxiCloud#706. + let normalized_segment = normalize_storage_name(new_segment); let create_dto = crate::application::dtos::folder_dto::CreateFolderDto { - name: new_segment.to_string(), + name: normalized_segment.clone(), parent_id, }; folder_service @@ -2565,10 +2585,57 @@ async fn handle_mkcol( .await .map_err(AppError::from)?; - Ok(Response::builder() - .status(StatusCode::CREATED) - .body(Body::empty()) - .unwrap()) + // If the client sent an NFD name (macOS Finder, some Android sync + // clients) and we canonicalised it, tell them the authoritative URL + // via `Content-Location` (RFC 7231 §3.1.4.2). Well-behaved clients + // (NextCloud desktop, rclone) update their local index; naive + // clients ignore the header (safely — status stays 201). Emitting + // only when the segment actually changed keeps the wire clean on + // the common ASCII / already-NFC path. + let mut response = Response::builder().status(StatusCode::CREATED); + if normalized_segment != new_segment { + response = response.header( + "Content-Location", + canonical_collection_url(&request_url_segments, &normalized_segment), + ); + } + Ok(response.body(Body::empty()).unwrap()) +} + +/// Reconstruct the canonical `Content-Location` value for a WebDAV +/// resource whose last URL segment was NFC-normalized server-side. +/// +/// Takes the original request-URL segments (as split by `/` after the +/// `/webdav/` prefix) and the canonical last-segment string, and +/// returns a full `/webdav/…/` URL with each segment individually +/// percent-encoded. Collection responses append a trailing `/` per +/// RFC 4918 §5.2. +fn canonical_collection_url(request_url_segments: &[String], canonical_last: &str) -> String { + let mut out = String::with_capacity( + request_url_segments.iter().map(|s| s.len()).sum::() + canonical_last.len() + 16, + ); + out.push_str("/webdav/"); + // Walk all segments except the last; the last is replaced with the + // canonical (normalized) form. + let prefix = if request_url_segments.len() > 1 { + &request_url_segments[..request_url_segments.len() - 1] + } else { + &[][..] + }; + for seg in prefix { + let _ = std::fmt::Write::write_fmt( + &mut out, + format_args!("{}/", utf8_percent_encode(seg, PATH_SEGMENT_ENCODE_SET)), + ); + } + let _ = std::fmt::Write::write_fmt( + &mut out, + format_args!( + "{}/", + utf8_percent_encode(canonical_last, PATH_SEGMENT_ENCODE_SET) + ), + ); + out } /** diff --git a/src/interfaces/nextcloud/webdav_handler.rs b/src/interfaces/nextcloud/webdav_handler.rs index 0185ac2d..769cf179 100644 --- a/src/interfaces/nextcloud/webdav_handler.rs +++ b/src/interfaces/nextcloud/webdav_handler.rs @@ -27,6 +27,7 @@ use crate::application::ports::trash_ports::TrashUseCase; use crate::common::di::AppState; use crate::common::mime_detect::filename_from_path; use crate::domain::services::authorization::{Permission, Resource, Subject}; +use crate::domain::services::path_service::normalize_storage_name; use crate::infrastructure::services::path_resolver_service::ResolvedResource; use crate::infrastructure::services::webdav_dead_property_store::ResourceRef; use crate::interfaces::api::handlers::webdav_handler::{ @@ -1295,6 +1296,13 @@ async fn handle_mkcol( } let (target_name, parent_segments) = segments.split_last().expect("checked non-empty above"); + // NFC-normalize the client-supplied last segment so we can emit + // `Content-Location` if the canonical URL differs. Repo also + // normalizes (idempotent — `is_nfc_quick` fast path). See + // AtalayaLabs/OxiCloud#706 for the class of bug this closes on the + // NC surface (macOS Finder / NC desktop client emit NFD on macOS). + let normalized_target = normalize_storage_name(target_name); + // Take POC's `chroot`-based root resolution (drive-aware mount // point) but keep HEAD's parent_path lookup pattern — the // continuation below uses `get_folder_by_path(&parent_path, @@ -1320,7 +1328,7 @@ async fn handle_mkcol( }; let dto = CreateFolderDto { - name: target_name.to_string(), + name: normalized_target.clone(), parent_id: Some(parent_folder.id.clone()), }; // AuthZ audit #7 (2026-07-12): route `_with_perms` errors through @@ -1332,10 +1340,25 @@ async fn handle_mkcol( .await .map_err(AppError::from)?; - Ok(Response::builder() - .status(StatusCode::CREATED) - .body(Body::empty()) - .unwrap()) + // Emit Content-Location (RFC 7231 §3.1.4.2) only when the URL + // canonicalization actually changed something — keeps the common + // ASCII / already-NFC path clean. Well-behaved clients (NC desktop, + // rclone) update their local index; naive clients ignore the + // header safely (status stays 201). + let mut response = Response::builder().status(StatusCode::CREATED); + if normalized_target != *target_name { + let mut canonical_subpath = String::with_capacity(subpath.len()); + for seg in parent_segments { + canonical_subpath.push_str(seg); + canonical_subpath.push('/'); + } + canonical_subpath.push_str(&normalized_target); + response = response.header( + "Content-Location", + nc_collection_href(&user.username, &canonical_subpath), + ); + } + Ok(response.body(Body::empty()).unwrap()) } // ──────────────────── DELETE ──────────────────── diff --git a/tests/api/nfc_normalization.hurl b/tests/api/nfc_normalization.hurl new file mode 100644 index 00000000..cbf9758c --- /dev/null +++ b/tests/api/nfc_normalization.hurl @@ -0,0 +1,284 @@ +# ============================================================= +# OxiCloud — NFC normalization on write (regression pin) +# ============================================================= +# Regression pin for AtalayaLabs/OxiCloud#706. The bug: macOS Finder +# and other NFD-emitting clients uploaded folder names in decomposed +# form ("à" = "a" + U+0300 combining grave, 2 codepoints), which +# landed raw in storage.folders.name / storage.files.name. Clients +# doing NFC-normalized lookups (NextCloud desktop, DAVX5, well- +# behaved sync clients — the exact clients the reporter used) then +# failed to descend into or match their own uploads. +# +# The fix normalizes at the repository layer (folder_db_repository, +# file_blob_write_repository) so every write surface — REST, WebDAV, +# NextCloud DAV, batch, chunked, WOPI-fallback — is covered at one +# choke point. WebDAV/NC MKCOL/PUT handlers additionally emit +# `Content-Location` (RFC 7231 §3.1.4.2) when canonicalisation +# actually changed the URL, so well-behaved clients update their +# local index immediately without waiting for the next PROPFIND +# cycle. +# +# This file pins the three critical write paths from the reporter's +# scenario: +# 1. REST `POST /api/folders` — the browser-side upload path +# 2. WebDAV `MKCOL` — davfs / macOS Finder +# 3. NextCloud `MKCOL` — NC desktop client / DAVX5 +# +# For each: post NFD, expect the DB row to hold NFC, and for the +# DAV surfaces expect `Content-Location` pointing at the canonical +# NFC URL, plus a follow-up NFC-URL lookup that succeeds (proving +# the two clients-and-server sides agree on the canonical form +# after the fix). +# +# Byte encoding conventions used below: +# * NFD `à` = U+0061 U+0300 → UTF-8 `61 CC 80` → URL `a%CC%80` +# * NFC `à` = U+00E0 → UTF-8 `C3 A0` → URL `%C3%A0` +# JSON bodies use `̀` (JSON-standard Unicode escape, always +# interpreted by the server's JSON parser). Assertions use Hurl's +# `\u{HHHH}` escape for the expected NFC codepoint. +# ============================================================= + + +# ───────────────────────────────────────────────────────────── +# Step 1 — Admin login. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ "username": "{{username}}", "password": "{{password}}" } + +HTTP 200 +[Captures] +admin_token: jsonpath "$.access_token" + + +# ───────────────────────────────────────────────────────────── +# Step 2 — Resolve admin's home folder id (the default parent +# for REST folder create when no parent_id is supplied, but we +# pass it explicitly so this test doesn't depend on the auto- +# resolve fallback path). +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/folders +Authorization: Bearer {{admin_token}} + +HTTP 200 +[Captures] +home_id: jsonpath "$[0].id" + + +# ───────────────────────────────────────────────────────────── +# Step 3 — REST `POST /api/folders` with an NFD name in the +# JSON body. The name field carries "nfc-rest-à" — that +# is the 11-byte NFD form ("nfc-rest-a" + U+0300). Post-fix, +# the repo NFC-normalizes at bind time, so the returned name +# must be the 10-byte NFC form "nfc-rest-\u{00e0}". +# +# Pre-fix: the response would echo the NFD input verbatim +# (`nfc-rest-à`), the DB would store 11 bytes, and a +# subsequent PROPFIND from an NFC-normalizing client would +# miss. That's the class of bug #706 reports. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/folders +Authorization: Bearer {{admin_token}} +Content-Type: application/json +{ + "name": "nfc-rest-à", + "parent_id": "{{home_id}}" +} + +HTTP 201 +[Captures] +rest_folder_id: jsonpath "$.id" +[Asserts] +# The stored (and returned) name must be NFC. If this fails, the +# repo-level normalize call was skipped or bypassed by a new code +# path — see folder_db_repository::create_folder. +jsonpath "$.name" == "nfc-rest-\u{00e0}" + + +# ───────────────────────────────────────────────────────────── +# Step 4 — Follow-up GET verifies the DB persisted NFC (not just +# that the create response happened to canonicalise before +# echoing). Reads from the same row a client's PROPFIND would. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/folders/{{rest_folder_id}} +Authorization: Bearer {{admin_token}} + +HTTP 200 +[Asserts] +jsonpath "$.name" == "nfc-rest-\u{00e0}" + + +# ───────────────────────────────────────────────────────────── +# Step 5 — Cleanup the REST-created folder before moving on to +# the DAV surfaces. Ed's memory feedback_hurl_teardown_shared_db: +# hurl files share the DB across the suite; each file must clean +# up what it created. +# ───────────────────────────────────────────────────────────── +DELETE {{base_url}}/api/folders/{{rest_folder_id}} +Authorization: Bearer {{admin_token}} + +HTTP * + + +# ───────────────────────────────────────────────────────────── +# Step 6 — WebDAV `MKCOL` with an NFD folder name in the URL +# path. Sends `nfc-dav-a%CC%80/` — the URL-encoded NFD form +# ("nfc-dav-a" + %CC%80 for U+0300). Post-fix, the handler +# canonicalises and emits `Content-Location` pointing at the +# NFC URL. Naive clients ignore the header (status stays 201); +# well-behaved clients update their local index to the +# canonical URL immediately. +# +# Bare `/webdav//` (no `@drive//` prefix) maps +# to the caller's default drive contents — matches the shape +# `webdav_drive_root.hurl` Step 4 documents. Simpler than the +# picker, and covers the same code path (`handle_mkcol` runs +# either way; the last URL segment is what the normalize +# operates on). +# ───────────────────────────────────────────────────────────── +MKCOL {{base_url}}/webdav/nfc-dav-a%CC%80/ +Authorization: Bearer {{admin_token}} + +HTTP 201 +[Asserts] +# The canonical URL substitutes the NFC form (%C3%A0) for the +# NFD segment in the request. If Content-Location is missing or +# still contains %CC%80, either the handler skipped the +# normalize-and-diff or repo-level canonicalization didn't fire. +header "Content-Location" contains "%C3%A0" +header "Content-Location" not contains "%CC%80" + + +# ───────────────────────────────────────────────────────────── +# Step 7 — PROPFIND on the CANONICAL (NFC) URL confirms the +# folder is reachable there. This is what a well-behaved sync +# client does on its next cycle after consuming Content-Location +# — and what the reporter's macOS/Android clients were doing +# already, hence their failure to find their own NFD uploads. +# Post-fix, this must return a matching 207. +# ───────────────────────────────────────────────────────────── +PROPFIND {{base_url}}/webdav/nfc-dav-%C3%A0/ +Authorization: Bearer {{admin_token}} +Depth: 0 +Content-Type: application/xml +``` + + + + +``` + +HTTP 207 +[Asserts] +# The response body must reference the canonical URL exactly. +body contains "nfc-dav-%C3%A0" + + +# ───────────────────────────────────────────────────────────── +# Step 8 — PROPFIND on the ORIGINAL (NFD) URL returns 404. The +# server does not maintain a legacy-NFD-alias for post-fix rows +# — the canonical URL is the only one that resolves. This pins +# the intended one-way behaviour (write NFD → stored NFC → +# only NFC URL matches), which is exactly what NFC-normalizing +# clients want. +# +# (Pre-existing NFD rows in the DB, deliberately not touched by +# this fix per operator decision, remain reachable via their +# NFD URL. That's a separate scenario — historic content, not +# the write-time regression this file covers.) +# ───────────────────────────────────────────────────────────── +PROPFIND {{base_url}}/webdav/nfc-dav-a%CC%80/ +Authorization: Bearer {{admin_token}} +Depth: 0 +Content-Type: application/xml +``` + + + + +``` + +HTTP 404 + + +# ───────────────────────────────────────────────────────────── +# Step 9 — Cleanup the WebDAV-created folder. +# ───────────────────────────────────────────────────────────── +DELETE {{base_url}}/webdav/nfc-dav-%C3%A0/ +Authorization: Bearer {{admin_token}} + +HTTP * + + +# ───────────────────────────────────────────────────────────── +# Step 10 — Mint an app-password for NextCloud Basic Auth. +# +# NextCloud DAV endpoints (`/remote.php/dav/…`) never accept a +# plain JWT — they use HTTP Basic Auth with an app-password, +# same pattern as every existing `nc_*.hurl` test. Mint one +# here so steps 11-13 below can authenticate. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/auth/app-passwords +Authorization: Bearer {{admin_token}} +Content-Type: application/json +{ "label": "nfc_normalization hurl test" } + +HTTP 200 +[Captures] +nc_user: jsonpath "$.username" +nc_pw: jsonpath "$.password" +nc_pw_id: jsonpath "$.id" + + +# ───────────────────────────────────────────────────────────── +# Step 11 — NextCloud `MKCOL` with NFD name in the URL path. +# Same shape as WebDAV MKCOL but on the /remote.php/dav/files/ +# surface — this is the code path macOS-based NC desktop +# clients hit. Same Content-Location contract. +# ───────────────────────────────────────────────────────────── +MKCOL {{base_url}}/remote.php/dav/files/{{username}}/nfc-nc-a%CC%80/ +[BasicAuth] +{{nc_user}}: {{nc_pw}} + +HTTP 201 +[Asserts] +header "Content-Location" contains "%C3%A0" +header "Content-Location" not contains "%CC%80" + + +# ───────────────────────────────────────────────────────────── +# Step 12 — PROPFIND via NextCloud DAV on the canonical URL. +# ───────────────────────────────────────────────────────────── +PROPFIND {{base_url}}/remote.php/dav/files/{{username}}/nfc-nc-%C3%A0/ +Depth: 0 +Content-Type: application/xml +[BasicAuth] +{{nc_user}}: {{nc_pw}} +``` + + + + +``` + +HTTP 207 +[Asserts] +body contains "nfc-nc-%C3%A0" + + +# ───────────────────────────────────────────────────────────── +# Step 13 — Cleanup the NC-created folder + retire the +# app-password so this test file leaves no side effects +# behind (per feedback_hurl_teardown_shared_db). +# ───────────────────────────────────────────────────────────── +DELETE {{base_url}}/remote.php/dav/files/{{username}}/nfc-nc-%C3%A0/ +[BasicAuth] +{{nc_user}}: {{nc_pw}} + +HTTP * + + +DELETE {{base_url}}/api/auth/app-passwords/{{nc_pw_id}} +Authorization: Bearer {{admin_token}} + +HTTP * diff --git a/tests/api/run.sh b/tests/api/run.sh index 9afd51bf..10643e8d 100755 --- a/tests/api/run.sh +++ b/tests/api/run.sh @@ -224,6 +224,7 @@ hurl --variables-file "$API_DIR/test.env" --file-root "$REPO_ROOT/tests" --test "$API_DIR/webdav_drive_root.hurl" \ "$API_DIR/webdav_permissions.hurl" \ "$API_DIR/webdav_nested_move_cascade.hurl" \ + "$API_DIR/nfc_normalization.hurl" \ "$API_DIR/wopi_authz.hurl" \ "$API_DIR/wopi_shared_drive.hurl" \ `# LAST, deliberately — and kept last even though it no longer cuts` \