fix(filename): fix uniform encoding encoding (NFC)
This commit is contained in:
@@ -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.';
|
||||
@@ -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<PgPool>,
|
||||
@@ -40,6 +41,15 @@ impl AddressBookRepository for AddressBookPgRepository {
|
||||
&self,
|
||||
address_book: AddressBook,
|
||||
) -> AddressBookRepositoryResult<AddressBook> {
|
||||
// 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<AddressBook> {
|
||||
// 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())
|
||||
|
||||
@@ -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<PgPool>,
|
||||
@@ -38,6 +39,15 @@ impl CalendarPgRepository {
|
||||
|
||||
impl CalendarRepository for CalendarPgRepository {
|
||||
async fn create_calendar(&self, calendar: Calendar) -> CalendarRepositoryResult<Calendar> {
|
||||
// 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<Calendar> {
|
||||
// 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
|
||||
|
||||
@@ -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<PgPool>,
|
||||
@@ -24,12 +25,19 @@ impl ContactGroupPgRepository {
|
||||
|
||||
impl ContactGroupRepository for ContactGroupPgRepository {
|
||||
async fn create_group(&self, group: ContactGroup) -> ContactRepositoryResult<ContactGroup> {
|
||||
// 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<ContactGroup> {
|
||||
// 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())
|
||||
|
||||
@@ -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<T>` — a single `serde_json::from_slice` over the raw JSONB
|
||||
@@ -395,6 +396,13 @@ impl DriveRepository for DrivePgRepository {
|
||||
quota_bytes: Option<i64>,
|
||||
granted_by: Uuid,
|
||||
) -> Result<DriveWithRootName, DriveRepositoryError> {
|
||||
// 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`.
|
||||
|
||||
@@ -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())
|
||||
|
||||
@@ -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<File, DomainError> {
|
||||
// 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<File, DomainError> {
|
||||
// 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<String>,
|
||||
dest_name: Option<String>,
|
||||
) -> Result<CopyFolderTreeResult, DomainError> {
|
||||
// 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)",
|
||||
|
||||
@@ -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<String>,
|
||||
caller_id: Uuid,
|
||||
) -> Result<Folder, DomainError> {
|
||||
// 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<Folder, DomainError> {
|
||||
// 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.
|
||||
|
||||
@@ -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/<selector>`
|
||||
// routing tokens the client used) is only preserved here.
|
||||
let request_url_segments: Vec<String> = 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::<usize>() + 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
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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 ────────────────────
|
||||
|
||||
@@ -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/<name>/` (no `@drive/<selector>/` 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
|
||||
```
|
||||
<?xml version="1.0" encoding="UTF-8"?>
|
||||
<D:propfind xmlns:D="DAV:">
|
||||
<D:prop><D:displayname/><D:resourcetype/></D:prop>
|
||||
</D:propfind>
|
||||
```
|
||||
|
||||
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
|
||||
```
|
||||
<?xml version="1.0" encoding="UTF-8"?>
|
||||
<D:propfind xmlns:D="DAV:">
|
||||
<D:prop><D:resourcetype/></D:prop>
|
||||
</D:propfind>
|
||||
```
|
||||
|
||||
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}}
|
||||
```
|
||||
<?xml version="1.0" encoding="UTF-8"?>
|
||||
<D:propfind xmlns:D="DAV:">
|
||||
<D:prop><D:displayname/><D:resourcetype/></D:prop>
|
||||
</D:propfind>
|
||||
```
|
||||
|
||||
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 *
|
||||
@@ -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` \
|
||||
|
||||
Reference in New Issue
Block a user