From 37565d6d59b5a794fddc19e69d19ea2df6ecf1b6 Mon Sep 17 00:00:00 2001 From: Jamie Pine Date: Fri, 25 Sep 2026 15:41:47 -0700 Subject: [PATCH] Give SMB share entries their own ids and answer search patterns Every entry reported file id 0, and list_dir ignored the search pattern even though the server crate leaves filtering to the backend. macOS looks a name up by searching for it and takes the first entry back as that name's answer, so after a listing it read one file's size and bytes under another file's name. Entries now carry a hash of their share path as their id, the same one a listing and an open report, and a pattern answers with only the entries it names, reading the one entry directly when the pattern is an exact name. --- core/src/service/mounts/smb.rs | 247 ++++++++++++++++++++++++++++++--- 1 file changed, 228 insertions(+), 19 deletions(-) diff --git a/core/src/service/mounts/smb.rs b/core/src/service/mounts/smb.rs index 15658aa5b..b844176a9 100644 --- a/core/src/service/mounts/smb.rs +++ b/core/src/service/mounts/smb.rs @@ -13,6 +13,7 @@ use super::provider::{self, remote_share_name, share_name, ByteProvider, ByteTarget, MountTarget}; use crate::context::CoreContext; +use crate::ops::indexing::metadata::EntryMetadata; use crate::ops::indexing::state::EntryKind; use async_trait::async_trait; use smb_server::{ @@ -79,7 +80,92 @@ fn to_filetime(time: Option) -> u64 { (since.as_secs() + FILETIME_EPOCH_OFFSET) * 10_000_000 + (since.subsec_nanos() as u64) / 100 } -fn dir_info(name: String, modified: Option) -> FileInfo { +/// The file id the share reports for a share-relative path. +/// +/// The macOS client keys the nodes it builds from a directory listing on this +/// id, so two entries sharing one would read as a single file. Hashing the +/// path gives every entry its own id, and the same one whether the client +/// learns it from a listing or from opening the path. Zero means "no id" to +/// clients. +fn file_id(rel: &str) -> u64 { + let hash = blake3::hash(rel.as_bytes()); + let mut id = [0u8; 8]; + id.copy_from_slice(&hash.as_bytes()[..8]); + u64::from_le_bytes(id).max(1) +} + +/// Whether a directory entry named `name` answers a client's search +/// `pattern`. +/// +/// A client lists a directory with `*`, which reaches the backend as no +/// pattern at all, and looks one name up by searching for that name alone and +/// taking the first entry back as its answer. So an entry the pattern does not +/// name must never be returned. `*` matches any run of characters and `?` any +/// one; the DOS forms `<`, `>` and `"` are read as `*`, `?` and `.`. Case is +/// ignored, as the share declares itself case-insensitive. +fn name_matches(pattern: &str, name: &str) -> bool { + let pattern: Vec = pattern + .chars() + .flat_map(char::to_lowercase) + .map(|c| match c { + '<' => '*', + '>' => '?', + '"' => '.', + c => c, + }) + .collect(); + let name: Vec = name.chars().flat_map(char::to_lowercase).collect(); + + let (mut p, mut n) = (0, 0); + // The last `*` seen, and where in the name it started matching, so a + // mismatch after it retries with the star taking one more character. + let mut star: Option<(usize, usize)> = None; + while n < name.len() { + match pattern.get(p) { + Some('?') => { + p += 1; + n += 1; + } + Some('*') => { + star = Some((p, n)); + p += 1; + } + Some(c) if *c == name[n] => { + p += 1; + n += 1; + } + _ => match star { + Some((star_at, from)) => { + p = star_at + 1; + n = from + 1; + star = Some((star_at, from + 1)); + } + None => return false, + }, + } + } + pattern[p..].iter().all(|c| *c == '*') +} + +/// Whether a search pattern names one entry rather than matching several. +fn is_exact(pattern: &str) -> bool { + !pattern.contains(['*', '?', '<', '>', '"']) + && Path::new(pattern) + .components() + .map(|component| matches!(component, std::path::Component::Normal(_))) + .eq([true]) +} + +/// The share-relative path of `name` inside the directory at `parent`. +fn child_rel(parent: &str, name: &str) -> String { + if parent.is_empty() { + name.to_string() + } else { + format!("{parent}/{name}") + } +} + +fn dir_info(name: String, id: u64, modified: Option) -> FileInfo { let ft = to_filetime(modified); FileInfo { name, @@ -90,11 +176,11 @@ fn dir_info(name: String, modified: Option) -> FileInfo { last_write_time: ft, change_time: ft, is_directory: true, - file_index: 0, + file_index: id, } } -fn file_info(name: String, size: u64, modified: Option) -> FileInfo { +fn file_info(name: String, id: u64, size: u64, modified: Option) -> FileInfo { let ft = to_filetime(modified); FileInfo { name, @@ -105,7 +191,7 @@ fn file_info(name: String, size: u64, modified: Option) -> FileInfo last_write_time: ft, change_time: ft, is_directory: false, - file_index: 0, + file_index: id, } } @@ -197,12 +283,14 @@ impl ShareBackend for LibraryShare { } let name = display_name(&abs, &rel); + let id = file_id(&rel); if is_directory { return Ok(Box::new(DirHandle { index, abs, - info: dir_info(name, meta.as_ref().and_then(|m| m.modified)), + info: dir_info(name, id, meta.as_ref().and_then(|m| m.modified)), + rel, })); } @@ -220,6 +308,7 @@ impl ShareBackend for LibraryShare { target: byte_target, info: file_info( name, + id, stat.size, stat.modified.or(meta.and_then(|m| m.modified)), ), @@ -284,7 +373,7 @@ impl Handle for RootHandle { } async fn stat(&self) -> SmbResult { - Ok(dir_info(SHARE.to_string(), None)) + Ok(dir_info(SHARE.to_string(), file_id(""), None)) } async fn set_times(&self, _times: FileTimes) -> SmbResult<()> { @@ -295,14 +384,15 @@ impl Handle for RootHandle { Err(SmbError::AccessDenied) } - async fn list_dir(&self, _pattern: Option<&str>) -> SmbResult> { + async fn list_dir(&self, pattern: Option<&str>) -> SmbResult> { Ok(self .share .source_names() .await .into_iter() + .filter(|(name, _)| pattern.is_none_or(|pattern| name_matches(pattern, name))) .map(|(name, modified)| DirEntry { - info: dir_info(name, modified), + info: dir_info(name.clone(), file_id(&name), modified), }) .collect()) } @@ -316,6 +406,22 @@ struct DirHandle { index: Arc>, abs: PathBuf, info: FileInfo, + /// The directory's share-relative path, which its entries' ids hash under. + rel: String, +} + +impl DirHandle { + /// The listing entry for the child called `name`. + fn entry(&self, name: String, meta: &EntryMetadata) -> DirEntry { + let id = file_id(&child_rel(&self.rel, &name)); + DirEntry { + info: if meta.kind == EntryKind::Directory { + dir_info(name, id, meta.modified) + } else { + file_info(name, id, meta.size, meta.modified) + }, + } + } } #[async_trait] @@ -344,8 +450,14 @@ impl Handle for DirHandle { Err(SmbError::AccessDenied) } - async fn list_dir(&self, _pattern: Option<&str>) -> SmbResult> { + async fn list_dir(&self, pattern: Option<&str>) -> SmbResult> { let index = self.index.read().await; + // A lookup of one name reads that entry rather than the directory. + if let Some(name) = pattern.filter(|pattern| is_exact(pattern)) { + if let Some(meta) = index.get_entry_ref(&self.abs.join(name)) { + return Ok(vec![self.entry(name.to_string(), &meta)]); + } + } let Some(children) = index.list_directory(&self.abs) else { return Ok(Vec::new()); }; @@ -358,13 +470,10 @@ impl Handle for DirHandle { .file_name() .map(|n| n.to_string_lossy().into_owned()) .unwrap_or_default(); - out.push(DirEntry { - info: if meta.kind == EntryKind::Directory { - dir_info(name, meta.modified) - } else { - file_info(name, meta.size, meta.modified) - }, - }); + if pattern.is_some_and(|pattern| !name_matches(pattern, &name)) { + continue; + } + out.push(self.entry(name, &meta)); } Ok(out) } @@ -521,7 +630,7 @@ mod tests { source_id: uuid::Uuid::nil(), path: PathBuf::from("/src/a001.braw"), }, - info: file_info("a001.braw".into(), size, None), + info: file_info("a001.braw".into(), file_id("src/a001.braw"), size, None), } } @@ -594,12 +703,17 @@ mod tests { #[test] fn directories_and_files_carry_the_right_attributes() { - let dir = dir_info("Footage".into(), None); + let dir = dir_info("Footage".into(), file_id("Footage"), None); assert!(dir.is_directory); assert_eq!(dir.end_of_file, 0); assert_eq!(dir.attributes(), 0x10); - let file = file_info("a001.braw".into(), 24_300_000_000, None); + let file = file_info( + "a001.braw".into(), + file_id("Footage/a001.braw"), + 24_300_000_000, + None, + ); assert!(!file.is_directory); assert_eq!(file.end_of_file, 24_300_000_000); // Allocation size matching end-of-file is what makes a streamed file @@ -608,6 +722,101 @@ mod tests { assert_eq!(file.attributes(), 0x80); } + #[test] + fn search_patterns_match_as_the_protocol_asks() { + assert!(name_matches("clip.mp4", "clip.mp4")); + assert!(name_matches("CLIP.MP4", "clip.mp4")); + assert!(!name_matches("clip.mp4", "tail-moov.mov")); + assert!(!name_matches("clip", "clip.mp4")); + assert!(name_matches("*", "clip.mp4")); + assert!(name_matches("*.mov", "tail-moov.MOV")); + assert!(!name_matches("*.mov", "clip.mp4")); + assert!(name_matches("c?ip.*", "clip.mp4")); + assert!(name_matches("*moov*", "tail-moov.mov")); + assert!(name_matches("<.mov", "tail-moov.mov")); + assert!(is_exact("clip.mp4")); + assert!(!is_exact("*.mov")); + assert!(!is_exact("../clip.mp4")); + assert!(!is_exact("a/clip.mp4")); + assert!(!is_exact("")); + } + + fn listed_names(entries: &[DirEntry]) -> Vec<&str> { + entries + .iter() + .map(|entry| entry.info.name.as_str()) + .collect() + } + + /// A lookup of one name answers with that entry alone. macOS takes the + /// first entry back as the name's, so answering with the directory's first + /// entry would give one file another's id, size and bytes. + #[tokio::test] + async fn a_name_lookup_answers_with_that_entry_alone() { + let root = PathBuf::from("/src/clips"); + let mut arena = crate::ops::indexing::arena::Arena::new().expect("arena"); + for (name, size) in [("tail-moov.mov", 10), ("clip.mp4", 60)] { + let path = root.join(name); + let metadata = EntryMetadata { + path: path.clone(), + kind: EntryKind::File, + size, + modified: None, + accessed: None, + created: None, + inode: None, + permissions: None, + uid: None, + gid: None, + link_target: None, + is_hidden: false, + }; + arena + .add_entry(path, uuid::Uuid::now_v7(), metadata) + .expect("entry"); + } + let dir = DirHandle { + index: Arc::new(tokio::sync::RwLock::new(arena)), + abs: root, + info: dir_info("clips".into(), file_id("src/clips"), None), + rel: "src/clips".into(), + }; + + let found = dir.list_dir(Some("clip.mp4")).await.unwrap(); + assert_eq!(listed_names(&found), ["clip.mp4"]); + assert_eq!(found[0].info.end_of_file, 60); + assert_eq!(found[0].info.file_index, file_id("src/clips/clip.mp4")); + + assert_eq!( + listed_names(&dir.list_dir(Some("CLIP.MP4")).await.unwrap()), + ["clip.mp4"] + ); + assert!(dir.list_dir(Some("._clip.mp4")).await.unwrap().is_empty()); + assert_eq!( + listed_names(&dir.list_dir(Some("*.mov")).await.unwrap()), + ["tail-moov.mov"] + ); + + let all = dir.list_dir(None).await.unwrap(); + assert_eq!(all.len(), 2); + assert_ne!(all[0].info.file_index, all[1].info.file_index); + } + + /// Every entry gets its own id, the same one a listing and an open of its + /// path report, and never the zero clients read as "no id". + #[test] + fn file_ids_follow_the_path() { + let listed = file_id(&child_rel("Footage-a1b2c3/day1", "a001.braw")); + let opened = file_id("Footage-a1b2c3/day1/a001.braw"); + assert_eq!(listed, opened); + assert_ne!(opened, file_id("Footage-a1b2c3/day1/a002.braw")); + assert_ne!(file_id(""), file_id("Footage-a1b2c3")); + assert_eq!(child_rel("", "Footage-a1b2c3"), "Footage-a1b2c3"); + for rel in ["", "Footage-a1b2c3", "Footage-a1b2c3/day1/a001.braw"] { + assert_ne!(file_id(rel), 0); + } + } + #[test] fn detached_and_unreachable_map_to_retryable_errors() { // A drive in a drawer is not "not found" — telling a client the file