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/cli/migrate.rs b/src/cli/migrate.rs index 2ff35f4d..89f6e06a 100644 --- a/src/cli/migrate.rs +++ b/src/cli/migrate.rs @@ -6,22 +6,44 @@ //! runtime, or historical schema-drift cleanup). //! //! Currently ships one action: `nfc-filenames` — cleans up NFD/NFC -//! filename collisions in databases populated before the June 2026 -//! write-time fix at `src/domain/services/path_service.rs::normalize_storage_name` -//! (called from `src/infrastructure/repositories/pg/file_blob_read_repository.rs` -//! during file operations). New installs never need this migration; -//! only pre-June-2026 databases do. +//! name collisions in databases populated before the write-time fix +//! landed at the repository layer (see +//! `folder_db_repository::create_folder`, `file_blob_write_repository` +//! ingest paths, `drive_pg_repository::create_shared_drive_atomic`). +//! New installs get NFC on every ingest and never accumulate drift. +//! +//! Covers BOTH `storage.files.name` and `storage.folders.name`. The +//! folder pass was added 2026-09-04 in response to +//! AtalayaLabs/OxiCloud#706 (macOS Finder folder upload landed NFD; +//! the file-only migrate did nothing for the reporter). Folders have +//! no `blob_hash`, so the collision branch is "older keeps NFC name, +//! newer becomes `.duplicate[-N]`" only — no dedup-by-trash arm, +//! because trashing a folder strands its subtree. //! //! Previously lived in a standalone `migrate-nfc-filenames` binary //! before the v0.9.0 CLI/server merge — see docs/plan/bundled-binary.md § 1b. //! The 149-line body of `main()` moved here as `run_nfc_filenames()` //! with `env::args()` parsing replaced by clap. //! -//! Future removal target: v1.0. Databases upgraded through v0.9.0 -//! will have run this migration (or been unaffected because they were -//! post-fix installs); by v1.0 no user should still need it. Drop -//! the `NfcFilenames` variant + this module's `run_nfc_filenames()` -//! function together at that point. +//! **Retention: indefinite.** An earlier version of this doc set a +//! "future removal target: v1.0" — retracted 2026-09-04 for three +//! reasons: +//! +//! 1. The pre-2026-09 write-side normalization was DEAD CODE +//! (invariants at `File::new` / `Folder::new_folder` entity +//! constructors that the create path bypassed), so every +//! OxiCloud version shipped before that date accumulated NFD +//! content and has a real remediation need. Many self-hosters +//! won't upgrade for months. +//! 2. Prior versions of THIS migrate command referenced the D7- +//! dropped `user_id` column and errored on first run, so users +//! who tried to apply it never got anywhere. The 2026-09-04 fix +//! makes it work again — but re-applying to instances that were +//! "already migrated" (they weren't) is now the only remediation +//! path for their historical NFD content. +//! 3. Post-fix installs run it as a no-op (all `already_nfc`), so +//! the cost of shipping it forever is zero and the safety it +//! offers for late-upgraders is real. use std::env; @@ -34,17 +56,25 @@ use crate::domain::services::path_service::normalize_storage_name; #[derive(Subcommand)] pub enum Action { - /// NFC-normalize storage.files.name across the instance. + /// NFC-normalize `storage.files.name` AND `storage.folders.name` + /// across the instance. /// - /// Historical cleanup for databases populated before June 2026. - /// New installs (post-`normalize_storage_name` write-time fix) - /// never need this — file operations already write NFC form. + /// Historical cleanup for databases with rows written before the + /// repo-level write-time normalization landed (see module doc for + /// the exact repo methods). Post-fix installs run this as a + /// harmless no-op — every row reports `already_nfc`. /// - /// Collision handling: + /// Collision handling (files): /// * No collision → UPDATE row name to NFC. /// * Same blob content → trash the newer row. /// * Different content → rename the newer to `{name}.duplicate[-N]`. /// + /// Collision handling (folders): + /// * No collision → UPDATE row name to NFC. + /// * Collision → rename the newer to `{name}.duplicate[-N]`; the + /// dedup-by-trash arm from the file path is deliberately absent + /// because trashing a folder strands its subtree. + /// /// In all collision cases, the surviving (older) row's name is /// also normalized to NFC. NfcFilenames { @@ -64,12 +94,36 @@ pub async fn run(action: Action) -> u8 { struct FileRow { id: Uuid, folder_id: Option, - user_id: Uuid, + /// §14 provenance — the user who created the row. Pre-D7 this + /// lived on `user_id`; post-D7 it's `created_by` and `user_id` + /// no longer exists. Not part of the collision scope (the DB + /// unique index is `(folder_id, name) WHERE NOT is_trashed` — + /// no user column in it), but surfaced in the log lines so an + /// operator triaging a large migration output can spot rows + /// owned by a specific principal without a separate query. + created_by: Option, name: String, blob_hash: String, created_at: DateTime, } +/// Structural sibling of [`FileRow`] for `storage.folders`. Folders +/// have no `blob_hash` — there is no "same content dedup" branch on +/// collision, only "keep older, rename newer to .duplicate". Added to +/// close the AtalayaLabs/OxiCloud#706 recovery gap: pre-fix DBs with +/// NFD-named folders (macOS Finder / NC desktop upload from macOS) +/// were unreachable via NFC-normalizing clients, and the file-only +/// migrate did nothing for them. +#[derive(Debug, Clone)] +struct FolderRow { + id: Uuid, + parent_id: Option, + /// §14 provenance — see [`FileRow::created_by`]. + created_by: Option, + name: String, + created_at: DateTime, +} + #[derive(Default)] struct Stats { scanned: u64, @@ -77,6 +131,13 @@ struct Stats { normalized_in_place: u64, deduped_same_content: u64, renamed_duplicate: u64, + // Folder stats — deliberately separate so operators reading the + // summary see "X files, Y folders" instead of one blended count + // that hides the fact that a run touched both scopes. + folders_scanned: u64, + folders_already_nfc: u64, + folders_normalized_in_place: u64, + folders_renamed_duplicate: u64, } async fn run_nfc_filenames(dry_run: bool) -> u8 { @@ -129,8 +190,12 @@ async fn run_nfc_filenames(dry_run: bool) -> u8 { } // Row is in non-NFC form. Look for a collision in the same - // (folder_id, user_id) scope, including rows that may also - // be non-NFC but happen to normalize to the same NFC value. + // folder scope (the DB's unique-index scope for storage.files — + // `(folder_id, name) WHERE NOT is_trashed`), including rows + // that may also be non-NFC but happen to normalize to the same + // NFC value. Pre-D7 this scope included user_id; the column + // has since been dropped (`docs/plan/drive.md` §D7), so the + // scope now matches today's unique constraint verbatim. let collision = match find_collision(&pool, row, &nfc_name).await { Ok(c) => c, Err(e) => { @@ -145,8 +210,14 @@ async fn run_nfc_filenames(dry_run: bool) -> u8 { match collision { None => { println!( - "NORMALIZE {} user={} '{}' → '{}'", - row.id, row.user_id, row.name, nfc_name + "NORMALIZE file={} folder={:?} created_by={:?} '{}' ({}B) → '{}' ({}B)", + row.id, + row.folder_id, + row.created_by, + row.name, + row.name.len(), + nfc_name, + nfc_name.len(), ); if !dry_run && let Err(e) = sqlx::query("UPDATE storage.files SET name = $1 WHERE id = $2") @@ -172,10 +243,11 @@ async fn run_nfc_filenames(dry_run: bool) -> u8 { // Same content → trash the newer; promote older's // name to NFC if it isn't already. println!( - "DEDUP newer={} (trash, same blob) older={} user={} hash={}", + "DEDUP newer={} (trash, same blob) older={} folder={:?} created_by={:?} hash={}", newer.id, older.id, - older.user_id, + older.folder_id, + older.created_by, &older.blob_hash[..16.min(older.blob_hash.len())] ); if !dry_run { @@ -217,8 +289,14 @@ async fn run_nfc_filenames(dry_run: bool) -> u8 { } }; println!( - "RENAME newer={} (different blob) older={} '{}' → '{}'", - newer.id, older.id, newer.name, disambiguated + "RENAME newer={} (different blob) older={} created_by={:?} '{}' ({}B) → '{}' ({}B)", + newer.id, + older.id, + newer.created_by, + newer.name, + newer.name.len(), + disambiguated, + disambiguated.len(), ); if !dry_run { if let Err(e) = @@ -248,8 +326,18 @@ async fn run_nfc_filenames(dry_run: bool) -> u8 { } } + // Second pass: folders. Same shape as the file loop but no dedup + // branch (folders have no `blob_hash`). Added to close + // AtalayaLabs/OxiCloud#706 — a reported macOS-Finder folder upload + // with an NFD name was unreachable via NFC-normalizing clients and + // this migration was the operator's documented recovery path. + if let Err(code) = run_folders(&pool, dry_run, &mut stats).await { + return code; + } + println!(); println!("=== Summary ==="); + println!(" --- storage.files ---"); println!(" scanned : {}", stats.scanned); println!( " already in NFC : {}", @@ -267,6 +355,23 @@ async fn run_nfc_filenames(dry_run: bool) -> u8 { " renamed to .duplicate : {}", stats.renamed_duplicate ); + println!(" --- storage.folders ---"); + println!( + " scanned : {}", + stats.folders_scanned + ); + println!( + " already in NFC : {}", + stats.folders_already_nfc + ); + println!( + " normalized in place (no collision) : {}", + stats.folders_normalized_in_place + ); + println!( + " renamed to .duplicate : {}", + stats.folders_renamed_duplicate + ); if dry_run { println!(); println!("DRY RUN — no rows were written. Re-run without --dry-run to apply."); @@ -277,7 +382,7 @@ async fn run_nfc_filenames(dry_run: bool) -> u8 { async fn load_non_trashed_files(pool: &PgPool) -> Result, Box> { let raw = sqlx::query( - "SELECT id, folder_id, user_id, name, blob_hash, created_at + "SELECT id, folder_id, created_by, name, blob_hash, created_at FROM storage.files WHERE NOT is_trashed ORDER BY created_at", @@ -290,7 +395,7 @@ async fn load_non_trashed_files(pool: &PgPool) -> Result, Box Result, Box Result, Box> { + let raw = sqlx::query( + "SELECT id, parent_id, created_by, name, created_at + FROM storage.folders + WHERE NOT is_trashed + ORDER BY created_at", + ) + .fetch_all(pool) + .await?; + + let mut out = Vec::with_capacity(raw.len()); + for r in raw { + out.push(FolderRow { + id: r.try_get("id")?, + parent_id: r.try_get("parent_id")?, + created_by: r.try_get("created_by")?, + name: r.try_get("name")?, + created_at: r.try_get("created_at")?, + }); + } + Ok(out) +} + +/// Looks for a file in the same folder scope whose CURRENT name +/// equals `nfc_name`, excluding the row being processed. The other +/// row may itself be in non-NFC form whose normalized representation +/// happens to differ from `nfc_name`; the collision check is +/// intentionally based on stored bytes (matching the UNIQUE-index +/// semantics that this migration is repairing). async fn find_collision( pool: &PgPool, row: &FileRow, nfc_name: &str, ) -> Result, Box> { let result = sqlx::query( - "SELECT id, folder_id, user_id, name, blob_hash, created_at + "SELECT id, folder_id, created_by, name, blob_hash, created_at FROM storage.files WHERE name = $1 - AND user_id = $2 - AND ($3::uuid IS NULL AND folder_id IS NULL - OR folder_id = $3::uuid) - AND id <> $4 + AND ($2::uuid IS NULL AND folder_id IS NULL + OR folder_id = $2::uuid) + AND id <> $3 AND NOT is_trashed LIMIT 1", ) .bind(nfc_name) - .bind(row.user_id) .bind(row.folder_id) .bind(row.id) .fetch_optional(pool) @@ -331,17 +464,49 @@ async fn find_collision( Ok(result.map(|r| FileRow { id: r.get("id"), folder_id: r.get("folder_id"), - user_id: r.get("user_id"), + created_by: r.get("created_by"), name: r.get("name"), blob_hash: r.get("blob_hash"), created_at: r.get("created_at"), })) } +/// Folder-side sibling of [`find_collision`]. Same shape but keyed on +/// `parent_id` — the natural uniqueness scope for `storage.folders`. +async fn find_folder_collision( + pool: &PgPool, + row: &FolderRow, + nfc_name: &str, +) -> Result, Box> { + let result = sqlx::query( + "SELECT id, parent_id, created_by, name, created_at + FROM storage.folders + WHERE name = $1 + AND ($2::uuid IS NULL AND parent_id IS NULL + OR parent_id = $2::uuid) + AND id <> $3 + AND NOT is_trashed + LIMIT 1", + ) + .bind(nfc_name) + .bind(row.parent_id) + .bind(row.id) + .fetch_optional(pool) + .await?; + + Ok(result.map(|r| FolderRow { + id: r.get("id"), + parent_id: r.get("parent_id"), + created_by: r.get("created_by"), + name: r.get("name"), + created_at: r.get("created_at"), + })) +} + /// Finds a free name in the form `{nfc_name}.duplicate` or /// `{nfc_name}.duplicate-N` for `N >= 1`, scoped to the row's -/// `(folder_id, user_id)`. Returns the first candidate that does -/// not currently exist as a non-trashed row. +/// folder. Returns the first candidate that does not currently +/// exist as a non-trashed row. async fn find_free_duplicate_name( pool: &PgPool, row: &FileRow, @@ -359,14 +524,12 @@ async fn find_free_duplicate_name( "SELECT EXISTS( SELECT 1 FROM storage.files WHERE name = $1 - AND user_id = $2 - AND ($3::uuid IS NULL AND folder_id IS NULL - OR folder_id = $3::uuid) - AND id <> $4 + AND ($2::uuid IS NULL AND folder_id IS NULL + OR folder_id = $2::uuid) + AND id <> $3 AND NOT is_trashed)", ) .bind(&candidate) - .bind(row.user_id) .bind(row.folder_id) .bind(row.id) .fetch_one(pool) @@ -379,8 +542,52 @@ async fn find_free_duplicate_name( // Safety bound — should never trigger under realistic data. if suffix > 10_000 { return Err(format!( - "Exhausted .duplicate-N suffixes for '{}' in scope (user={}, folder_id={:?})", - nfc_name, row.user_id, row.folder_id + "Exhausted .duplicate-N suffixes for '{}' in scope (folder_id={:?})", + nfc_name, row.folder_id + ) + .into()); + } + } +} + +/// Folder-side sibling. Same shape as [`find_free_duplicate_name`] +/// but keyed on `parent_id`. +async fn find_free_folder_duplicate_name( + pool: &PgPool, + row: &FolderRow, + nfc_name: &str, +) -> Result> { + let mut suffix: u32 = 0; + loop { + let candidate = if suffix == 0 { + format!("{}.duplicate", nfc_name) + } else { + format!("{}.duplicate-{}", nfc_name, suffix) + }; + + let taken: bool = sqlx::query_scalar( + "SELECT EXISTS( + SELECT 1 FROM storage.folders + WHERE name = $1 + AND ($2::uuid IS NULL AND parent_id IS NULL + OR parent_id = $2::uuid) + AND id <> $3 + AND NOT is_trashed)", + ) + .bind(&candidate) + .bind(row.parent_id) + .bind(row.id) + .fetch_one(pool) + .await?; + + if !taken { + return Ok(candidate); + } + suffix = suffix.saturating_add(1); + if suffix > 10_000 { + return Err(format!( + "Exhausted .duplicate-N suffixes for '{}' in scope (parent_id={:?})", + nfc_name, row.parent_id ) .into()); } @@ -404,3 +611,150 @@ async fn normalize_survivor_name( .await?; Ok(()) } + +/// Folder-side sibling of [`normalize_survivor_name`]. If the older +/// folder we kept was itself in non-NFC form, promote it to the NFC +/// name we just picked as canonical. +async fn normalize_folder_survivor_name( + pool: &PgPool, + survivor: &FolderRow, + nfc_name: &str, +) -> Result<(), Box> { + if survivor.name == nfc_name { + return Ok(()); + } + sqlx::query("UPDATE storage.folders SET name = $1 WHERE id = $2") + .bind(nfc_name) + .bind(survivor.id) + .execute(pool) + .await?; + Ok(()) +} + +/// Process every non-trashed folder, mirroring the file loop's shape. +/// Folders have no `blob_hash` so the "same content → dedup" branch is +/// absent: on collision the older folder wins its NFC name, the newer +/// gets renamed to `{nfc_name}.duplicate[-N]`. Never trashes a folder +/// — trashing would strand its subtree, and we cannot know without +/// inspection whether the newer folder was a broken second attempt +/// or an intentional sibling containing different files. Renaming is +/// the conservative choice. +async fn run_folders(pool: &PgPool, dry_run: bool, stats: &mut Stats) -> Result<(), u8> { + let rows = match load_non_trashed_folders(pool).await { + Ok(r) => r, + Err(e) => { + eprintln!("migrate nfc-filenames: folder scan failed: {e}"); + return Err(1); + } + }; + println!("Loaded {} non-trashed folder rows", rows.len()); + println!(); + + stats.folders_scanned = rows.len() as u64; + + for row in &rows { + let nfc_name = normalize_storage_name(&row.name); + if nfc_name == row.name { + stats.folders_already_nfc += 1; + continue; + } + + let collision = match find_folder_collision(pool, row, &nfc_name).await { + Ok(c) => c, + Err(e) => { + eprintln!( + "migrate nfc-filenames: folder collision query failed for {}: {e}", + row.id + ); + return Err(1); + } + }; + + match collision { + None => { + println!( + "NORMALIZE folder={} parent={:?} created_by={:?} '{}' ({}B) → '{}' ({}B)", + row.id, + row.parent_id, + row.created_by, + row.name, + row.name.len(), + nfc_name, + nfc_name.len(), + ); + if !dry_run + && let Err(e) = + sqlx::query("UPDATE storage.folders SET name = $1 WHERE id = $2") + .bind(&nfc_name) + .bind(row.id) + .execute(pool) + .await + { + eprintln!( + "migrate nfc-filenames: folder rename failed for {}: {e}", + row.id + ); + return Err(1); + } + stats.folders_normalized_in_place += 1; + } + Some(other) => { + // Older wins the canonical NFC slot; newer gets a + // `.duplicate[-N]` suffix. No dedup branch here — see + // the doc comment above. + let (older, newer) = if row.created_at <= other.created_at { + (row, &other) + } else { + (&other, row) + }; + + let disambiguated = match find_free_folder_duplicate_name(pool, newer, &nfc_name) + .await + { + Ok(n) => n, + Err(e) => { + eprintln!( + "migrate nfc-filenames: folder duplicate-name search failed for {}: {e}", + newer.id + ); + return Err(1); + } + }; + println!( + "RENAME folder-newer={} older={} created_by={:?} '{}' ({}B) → '{}' ({}B)", + newer.id, + older.id, + newer.created_by, + newer.name, + newer.name.len(), + disambiguated, + disambiguated.len(), + ); + if !dry_run { + if let Err(e) = + sqlx::query("UPDATE storage.folders SET name = $1 WHERE id = $2") + .bind(&disambiguated) + .bind(newer.id) + .execute(pool) + .await + { + eprintln!( + "migrate nfc-filenames: folder disambiguation rename failed for {}: {e}", + newer.id + ); + return Err(1); + } + if let Err(e) = normalize_folder_survivor_name(pool, older, &nfc_name).await { + eprintln!( + "migrate nfc-filenames: folder survivor rename failed for {}: {e}", + older.id + ); + return Err(1); + } + } + stats.folders_renamed_duplicate += 1; + } + } + } + Ok(()) +} 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/src/main.rs b/src/main.rs index 0fc27f83..792cfa12 100644 --- a/src/main.rs +++ b/src/main.rs @@ -141,6 +141,24 @@ fn main() -> Result<(), Box> { if let Some(first) = std::env::args().nth(1) && matches!(first.as_str(), "opaque" | "migrate" | "storage") { + // Load `.env` from CWD before dispatching so subcommands see the + // same `DATABASE_URL` / `OXICLOUD_*` variables the server-startup + // path does. Without this, `oxicloud migrate nfc-filenames` + // errors out with `DATABASE_URL not set` for any operator who + // keeps their config in `.env` (i.e. every self-host on a + // homelab, per the standard project layout). + // + // Non-overriding `dotenv()` — a live shell export still wins, + // matching the server path's default-branch behaviour at + // line ~219 below. `--config ` (line ~177+ below) is + // NOT yet supported for subcommands — that would require + // hoisting the `--config` parse above this dispatch and is + // tracked as follow-up work. Operators needing pinned config + // for a subcommand today: `env $(cat prod.env | xargs) + // oxicloud migrate nfc-filenames`, or run under a systemd + // EnvironmentFile= directive. + dotenvy::dotenv().ok(); + // `oxicloud::cli::run()` returns a plain `u8` exit-code, which // widens exactly into `i32` for `std::process::exit`. Values are // 0/1/2 today; the widening is loss-free by construction. 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` \