From 3fbc2b909cd8de3f4be67c864caa2d33d1ec1cfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADtor=20Vasconcellos?= Date: Tue, 18 Apr 2023 06:12:02 +0000 Subject: [PATCH] [ENG-461] Location creation fails for '/' (root) and 'C:' ([A-Z] drives) (#710) * Fix issue with root locations failing to be added - Add `normpath` dependency to the core - Improve automatic name resolution for new locations * Add `MetadataExt` trait to extend the `created` and `modified` methods of `Metadata` - Prevent failure in the Indexer job caused by paths without associated created or modified times; assume current time for both when unavailable - Rename possible location in root path from `/` to `Root` * Improve hadling of location path on Windows - Normalize location's path to ensure a valid format on Windows - Improve error handling when adding a path - Avoid consuming symlinks on Linux by not normalizing location's path - Improve location naming logic --- Cargo.lock | 86 ++++++++++++++++++++-- core/Cargo.toml | 19 ++--- core/src/location/file_path_helper.rs | 17 +++++ core/src/location/indexer/walk.rs | 14 ++-- core/src/location/manager/watcher/utils.rs | 14 ++-- core/src/location/mod.rs | 68 ++++++++++++++--- 6 files changed, 178 insertions(+), 40 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cd2dd2fa8..4f7c317cc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4608,6 +4608,15 @@ dependencies = [ "minimal-lexical", ] +[[package]] +name = "normpath" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec60c60a693226186f5d6edf073232bfb6464ed97eb22cf3b01c1e8198fd97f5" +dependencies = [ + "windows-sys 0.48.0", +] + [[package]] name = "notify" version = "5.0.0" @@ -6644,6 +6653,7 @@ dependencies = [ "int-enum", "itertools", "mini-moka", + "normpath", "notify", "once_cell", "prisma-client-rust", @@ -9364,12 +9374,12 @@ version = "0.42.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5a3e1820f08b8513f676f7ab6c1f99ff312fb97b553d30ff4dd86f9f15728aa7" dependencies = [ - "windows_aarch64_gnullvm", + "windows_aarch64_gnullvm 0.42.1", "windows_aarch64_msvc 0.42.1", "windows_i686_gnu 0.42.1", "windows_i686_msvc 0.42.1", "windows_x86_64_gnu 0.42.1", - "windows_x86_64_gnullvm", + "windows_x86_64_gnullvm 0.42.1", "windows_x86_64_msvc 0.42.1", ] @@ -9379,7 +9389,16 @@ version = "0.45.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "75283be5efb2831d37ea142365f009c02ec203cd29a3ebecbc093d52315b66d0" dependencies = [ - "windows-targets", + "windows-targets 0.42.1", +] + +[[package]] +name = "windows-sys" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "677d2418bec65e3338edb076e806bc1ec15693c5d0104683f2efe857f61056a9" +dependencies = [ + "windows-targets 0.48.0", ] [[package]] @@ -9388,15 +9407,30 @@ version = "0.42.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8e2522491fbfcd58cc84d47aeb2958948c4b8982e9a2d8a2a35bbaed431390e7" dependencies = [ - "windows_aarch64_gnullvm", + "windows_aarch64_gnullvm 0.42.1", "windows_aarch64_msvc 0.42.1", "windows_i686_gnu 0.42.1", "windows_i686_msvc 0.42.1", "windows_x86_64_gnu 0.42.1", - "windows_x86_64_gnullvm", + "windows_x86_64_gnullvm 0.42.1", "windows_x86_64_msvc 0.42.1", ] +[[package]] +name = "windows-targets" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7b1eb6f0cd7c80c79759c929114ef071b87354ce476d9d94271031c0497adfd5" +dependencies = [ + "windows_aarch64_gnullvm 0.48.0", + "windows_aarch64_msvc 0.48.0", + "windows_i686_gnu 0.48.0", + "windows_i686_msvc 0.48.0", + "windows_x86_64_gnu 0.48.0", + "windows_x86_64_gnullvm 0.48.0", + "windows_x86_64_msvc 0.48.0", +] + [[package]] name = "windows-tokens" version = "0.39.0" @@ -9409,6 +9443,12 @@ version = "0.42.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8c9864e83243fdec7fc9c5444389dcbbfd258f745e7853198f365e3c4968a608" +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "91ae572e1b79dba883e0d315474df7305d12f569b400fcf90581b06062f7e1bc" + [[package]] name = "windows_aarch64_msvc" version = "0.32.0" @@ -9445,6 +9485,12 @@ version = "0.42.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4c8b1b673ffc16c47a9ff48570a9d85e25d265735c503681332589af6253c6c7" +[[package]] +name = "windows_aarch64_msvc" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b2ef27e0d7bdfcfc7b868b317c1d32c641a6fe4629c171b8928c7b08d98d7cf3" + [[package]] name = "windows_i686_gnu" version = "0.32.0" @@ -9481,6 +9527,12 @@ version = "0.42.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "de3887528ad530ba7bdbb1faa8275ec7a1155a45ffa57c37993960277145d640" +[[package]] +name = "windows_i686_gnu" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "622a1962a7db830d6fd0a69683c80a18fda201879f0f447f065a3b7467daa241" + [[package]] name = "windows_i686_msvc" version = "0.32.0" @@ -9517,6 +9569,12 @@ version = "0.42.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bf4d1122317eddd6ff351aa852118a2418ad4214e6613a50e0191f7004372605" +[[package]] +name = "windows_i686_msvc" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4542c6e364ce21bf45d69fdd2a8e455fa38d316158cfd43b3ac1c5b1b19f8e00" + [[package]] name = "windows_x86_64_gnu" version = "0.32.0" @@ -9553,12 +9611,24 @@ version = "0.42.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c1040f221285e17ebccbc2591ffdc2d44ee1f9186324dd3e84e99ac68d699c45" +[[package]] +name = "windows_x86_64_gnu" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ca2b8a661f7628cbd23440e50b05d705db3686f894fc9580820623656af974b1" + [[package]] name = "windows_x86_64_gnullvm" version = "0.42.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "628bfdf232daa22b0d64fdb62b09fcc36bb01f05a3939e20ab73aaf9470d0463" +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7896dbc1f41e08872e9d5e8f8baa8fdd2677f29468c4e156210174edc7f7b953" + [[package]] name = "windows_x86_64_msvc" version = "0.32.0" @@ -9595,6 +9665,12 @@ version = "0.42.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "447660ad36a13288b1db4d4248e857b510e8c3a225c822ba4fb748c0aafecffd" +[[package]] +name = "windows_x86_64_msvc" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1a515f5799fe4961cb532f983ce2b23082366b898e52ffbce459c86f67c8378a" + [[package]] name = "winreg" version = "0.10.1" diff --git a/core/Cargo.toml b/core/Cargo.toml index 6adf9383d..7f1bf4a1e 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -18,9 +18,9 @@ sync-messages = [] [dependencies] sd-ffmpeg = { path = "../crates/ffmpeg", optional = true } sd-crypto = { path = "../crates/crypto", features = [ - "rspc", - "serde", - "keymanager", + "rspc", + "serde", + "keymanager", ] } sd-file-ext = { path = "../crates/file-ext" } sd-sync = { path = "../crates/sync" } @@ -31,11 +31,11 @@ httpz = { workspace = true } prisma-client-rust = { workspace = true } specta = { workspace = true } tokio = { workspace = true, features = [ - "sync", - "rt-multi-thread", - "io-util", - "macros", - "time", + "sync", + "rt-multi-thread", + "io-util", + "macros", + "time", ] } base64 = "0.21.0" @@ -70,10 +70,11 @@ serde_with = "2.2.0" dashmap = { version = "5.4.0", features = ["serde"] } ffmpeg-next = { version = "6.0.0", optional = true, features = [] } notify = { version = "5.0.0", default-features = false, features = [ - "macos_fsevent", + "macos_fsevent", ], optional = true } static_assertions = "1.1.0" serde-hashkey = "0.4.5" +normpath = { version = "1.1.1", features = ["localization"] } [target.'cfg(windows)'.dependencies.winapi-util] version = "0.1.5" diff --git a/core/src/location/file_path_helper.rs b/core/src/location/file_path_helper.rs index 7670c838d..9be0d1af6 100644 --- a/core/src/location/file_path_helper.rs +++ b/core/src/location/file_path_helper.rs @@ -5,6 +5,7 @@ use std::{ fmt::{Display, Formatter}, fs::Metadata, path::{Path, PathBuf, MAIN_SEPARATOR, MAIN_SEPARATOR_STR}, + time::SystemTime, }; use chrono::{DateTime, Utc}; @@ -724,3 +725,19 @@ pub async fn get_inode_and_device_from_path( Ok((info.file_index(), info.volume_serial_number())) } } + +pub trait MetadataExt { + fn created_or_now(&self) -> SystemTime; + + fn modified_or_now(&self) -> SystemTime; +} + +impl MetadataExt for Metadata { + fn created_or_now(&self) -> SystemTime { + self.created().unwrap_or_else(|_| SystemTime::now()) + } + + fn modified_or_now(&self) -> SystemTime { + self.modified().unwrap_or_else(|_| SystemTime::now()) + } +} diff --git a/core/src/location/indexer/walk.rs b/core/src/location/indexer/walk.rs index f3a31beff..ae6e1a495 100644 --- a/core/src/location/indexer/walk.rs +++ b/core/src/location/indexer/walk.rs @@ -1,4 +1,4 @@ -use crate::location::file_path_helper::FilePathMetadata; +use crate::location::file_path_helper::{FilePathMetadata, MetadataExt}; #[cfg(target_family = "unix")] use crate::location::file_path_helper::get_inode_and_device; @@ -291,8 +291,8 @@ async fn inner_walk_single_dir( inode, device, size_in_bytes: metadata.len(), - created_at: metadata.created()?.into(), - modified_at: metadata.modified()?.into(), + created_at: metadata.created_or_now().into(), + modified_at: metadata.modified_or_now().into(), }, }, ); @@ -327,8 +327,8 @@ async fn inner_walk_single_dir( inode, device, size_in_bytes: metadata.len(), - created_at: metadata.created()?.into(), - modified_at: metadata.modified()?.into(), + created_at: metadata.created_or_now().into(), + modified_at: metadata.modified_or_now().into(), }, }, ); @@ -372,8 +372,8 @@ async fn prepared_indexed_paths( inode, device, size_in_bytes: metadata.len(), - created_at: metadata.created()?.into(), - modified_at: metadata.modified()?.into(), + created_at: metadata.created_or_now().into(), + modified_at: metadata.modified_or_now().into(), }, }); } diff --git a/core/src/location/manager/watcher/utils.rs b/core/src/location/manager/watcher/utils.rs index 910508287..425e566e9 100644 --- a/core/src/location/manager/watcher/utils.rs +++ b/core/src/location/manager/watcher/utils.rs @@ -6,7 +6,7 @@ use crate::{ file_path_helper::{ extract_materialized_path, file_path_with_object, filter_existing_file_path_params, get_parent_dir, get_parent_dir_id, loose_find_existing_file_path_params, FilePathError, - FilePathMetadata, MaterializedPath, + FilePathMetadata, MaterializedPath, MetadataExt, }, find_location, location_with_indexer_rules, manager::LocationManagerError, @@ -117,8 +117,8 @@ pub(super) async fn create_dir( inode, device, size_in_bytes: metadata.len(), - created_at: metadata.created()?.into(), - modified_at: metadata.modified()?.into(), + created_at: metadata.created_or_now().into(), + modified_at: metadata.modified_or_now().into(), }, ) .await?; @@ -191,8 +191,8 @@ pub(super) async fn create_file( inode, device, size_in_bytes: metadata.len(), - created_at: metadata.created()?.into(), - modified_at: metadata.modified()?.into(), + created_at: metadata.created_or_now().into(), + modified_at: metadata.modified_or_now().into(), }, ) .await?; @@ -217,7 +217,7 @@ pub(super) async fn create_file( Uuid::new_v4().as_bytes().to_vec(), vec![ object::date_created::set( - DateTime::::from(fs_metadata.created().unwrap()).into(), + DateTime::::from(fs_metadata.created_or_now()).into(), ), object::kind::set(kind.int_value()), ], @@ -373,7 +373,7 @@ async fn inner_update_file( file_path::size_in_bytes::set(fs_metadata.len().to_string()), ), { - let date = DateTime::::from(fs_metadata.modified()?).into(); + let date = DateTime::::from(fs_metadata.modified_or_now()).into(); ( ("date_modified", json!(date)), diff --git a/core/src/location/mod.rs b/core/src/location/mod.rs index 286ee3560..69ed772f9 100644 --- a/core/src/location/mod.rs +++ b/core/src/location/mod.rs @@ -17,11 +17,11 @@ use crate::{ use std::{ collections::HashSet, - ffi::OsStr, - path::{Path, PathBuf}, + path::{Component, Path, PathBuf}, }; use futures::future::TryFutureExt; +use normpath::PathExt; use prisma_client_rust::QueryError; use rspc::Type; use serde::Deserialize; @@ -452,18 +452,62 @@ async fn create_location( ) -> Result { let Library { db, sync, .. } = &library; - let location_path = location_path.as_ref(); + let mut location_path = location_path.as_ref().to_path_buf(); - let name = location_path - .file_name() - .and_then(OsStr::to_str) - .map(str::to_string) - .unwrap(); + let (path, normalized_path) = location_path + // Normalize path and also check if it exists + .normalize() + .and_then(|normalized_path| { + if cfg!(windows) { + // Use normalized path as location path on Windows + // This ensures we always receive a valid windows formated path + // ex: /Users/JohnDoe/Downloads will become C:\Users\JohnDoe\Downloads + // Internally `normalize` calls `GetFullPathNameW` on Windows + // https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getfullpathnamew + location_path = normalized_path.as_path().to_path_buf(); + } - let path = location_path - .to_str() - .map(str::to_string) - .expect("Found non-UTF-8 path"); + Ok(( + // TODO: Maybe save the path bytes instead of the string representation to avoid depending on UTF-8 + location_path + .to_str() + .map(str::to_string) + .ok_or(io::Error::new( + io::ErrorKind::InvalidInput, + "Found non-UTF-8 path", + ))?, + normalized_path, + )) + }) + .map_err(|_| { + LocationError::DirectoryNotFound(location_path.to_string_lossy().to_string()) + })?; + + // Not needed on Windows because the normalization already handles it + if cfg!(not(windows)) { + // Replace location_path with normalize_path, when the first one ends in `.` or `..` + // This is required so localize_name doesn't panic + if let Some(component) = location_path.components().next_back() { + match component { + Component::CurDir | Component::ParentDir => { + location_path = normalized_path.as_path().to_path_buf(); + } + _ => {} + } + } + } + + // Use `to_string_lossy` because a partially corrupted but identifiable name is better than nothing + let mut name = location_path.localize_name().to_string_lossy().to_string(); + + // Windows doesn't have a root directory + if cfg!(not(windows)) && name == "/" { + name = "Root".to_string() + } + + if name.replace(char::REPLACEMENT_CHARACTER, "") == "" { + name = "Unknown".to_string() + } let location = sync .write_op(