diff --git a/crates/preview/src/disk.rs b/crates/preview/src/disk.rs index 7dbdda1e..7ec2dcfe 100644 --- a/crates/preview/src/disk.rs +++ b/crates/preview/src/disk.rs @@ -4,6 +4,11 @@ //! files) but not fsynced; unreadable files are deleted and re-rendered. When the total size //! exceeds the budget, the least recently used files (by modification time, refreshed on read) //! are removed down to 80 % of the budget. +//! +//! The cache only ever lists, counts or deletes files of its own name shape +//! (`<2 hex>/<32 hex>.jpg` and the `<32 hex>.tmp…` temp files of [`DiskCache::put`], see +//! [`is_cache_file`]): the directory may be shared with other files (e.g. a library opened on a +//! photo folder that already has a `thumbs/` folder), and those are never touched (issue #98). use std::path::{Path, PathBuf}; use std::sync::Mutex; @@ -96,13 +101,25 @@ impl DiskCache { } } - /// All cache files: (path, size, modified). + /// All cache files (only names of the cache's own shape, see [`is_cache_file`]): + /// (path, size, modified). fn scan(&self) -> Vec<(PathBuf, u64, std::time::SystemTime)> { let mut out = Vec::new(); let Ok(rd) = std::fs::read_dir(&self.dir) else { return out }; for sub in rd.flatten() { + let sub_name = sub.file_name(); + let Some(sub_name) = sub_name.to_str() else { continue }; + if !is_shard_name(sub_name) { + continue; + } let Ok(files) = std::fs::read_dir(sub.path()) else { continue }; for f in files.flatten() { + let name = f.file_name(); + let Some(name) = name.to_str() else { continue }; + if !is_cache_file(sub_name, name) { + continue; + } + // `DirEntry::metadata` does not follow symlinks: a link is never a cache file if let Ok(m) = f.metadata() && m.is_file() { @@ -144,6 +161,29 @@ impl DiskCache { } } +fn is_lower_hex(s: &str) -> bool { + s.bytes().all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)) +} + +/// A shard directory name: two lowercase hex digits. +fn is_shard_name(s: &str) -> bool { + s.len() == 2 && is_lower_hex(s) +} + +/// Whether `name` inside shard directory `shard` is a file this cache wrote: `<32 hex>.jpg` +/// (whose first two digits are the shard) or a `<32 hex>.tmp` temp file of +/// [`DiskCache::put`]. Anything else in the cache directory belongs to someone else. +pub fn is_cache_file(shard: &str, name: &str) -> bool { + if !is_shard_name(shard) { + return false; + } + let Some((stem, ext)) = name.split_once('.') else { return false }; + if stem.len() != 32 || !is_lower_hex(stem) || !stem.starts_with(shard) { + return false; + } + ext == "jpg" || ext.strip_prefix("tmp").is_some_and(|t| !t.is_empty() && t.bytes().all(|b| b.is_ascii_alphanumeric())) +} + /// Encode a thumbnail as the cache stores it (JPEG, RGB, 4:2:0). pub fn encode_jpeg(img: &Rgba8) -> Option> { if img.width == 0 || img.height == 0 || img.width > u16::MAX as usize || img.height > u16::MAX as usize { diff --git a/crates/preview/src/tests.rs b/crates/preview/src/tests.rs index 038f935a..ee5bd36f 100644 --- a/crates/preview/src/tests.rs +++ b/crates/preview/src/tests.rs @@ -154,3 +154,41 @@ fn pool_runs_inline() { assert_eq!(p.run_inline(10), 1); assert_eq!(p.try_recv().map(|d| d.result), Some(1)); } + +#[test] +fn disk_cache_never_touches_foreign_files() { + // issue #98: a library opened on a folder that already has a `thumbs/` folder + let dir = temp_dir("foreign"); + let foreign = [ + dir.join("readme.txt"), + dir.join("ab").join("holiday.jpg"), + dir.join("ab").join("0123456789abcdef0123456789abcdef.jpg"), // wrong shard + dir.join("small").join("0123456789abcdef0123456789abcdef.jpg"), + dir.join("01").join("0123456789ABCDEF0123456789ABCDEF.jpg"), // upper case + dir.join("01").join("0123456789abcdef0123456789abcdef.jpeg"), + dir.join("01").join("0123456789abcdef0123456789abcdef.tmp"), + ]; + for p in &foreign { + std::fs::create_dir_all(p.parent().unwrap()).unwrap(); + std::fs::write(p, vec![7u8; 50_000]).unwrap(); + } + // a stale temp file of our own is cleaned up + let own_tmp = dir.join("01").join("0123456789abcdef0123456789abcdef.tmpThreadId9"); + std::fs::write(&own_tmp, b"partial").unwrap(); + let d = DiskCache::new(&dir, 40_000); + assert_eq!(d.size(), 7, "only the cache's own temp file counts"); + // fill past the budget (forces prunes), then clear + for i in 0..40u8 { + d.put(hash_bytes(&[i, 2]), &gradient(160, 100, i)); + } + assert!(d.size() <= 40_000); + d.clear(); + assert_eq!(d.size(), 0); + assert!(!own_tmp.exists()); + for p in &foreign { + assert!(p.exists(), "foreign file deleted: {}", p.display()); + } + assert!(disk::is_cache_file("01", "0123456789abcdef0123456789abcdef.jpg")); + assert!(!disk::is_cache_file("01", "0123456789abcdef0123456789abcdef.jpg.bak")); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/docs/whats-new.md b/docs/whats-new.md index dbbadf90..b606889d 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -19,6 +19,8 @@ in short pieces so slow integrated GPUs aren't reset by their watchdog. `ui.inspect` → `perf` (`gpuReason`, `gpuFallback`), Help ▸ System Info and Settings ▸ Performance say why the GPU isn't used (e.g. a skipped software adapter such as llvmpipe) and why the last render fell back. +- The thumbnail cache only ever counts and deletes its own files (issue #98): a library opened on a folder that + already has a `thumbs/` folder of other pictures no longer loses them when the cache is trimmed or cleared. ### Library - Smart albums with a rule editor: match all / any / none, nested groups, 26 fields.