From fc55e922991a8d8ce4ca00f6a2cdc100abbd8910 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 13:18:09 +0000 Subject: [PATCH 1/7] perf(blob-cache,db): release index mutex before disk I/O; skip per-acquire DB ping CachedBlobBackend held its single tokio::Mutex across filesystem syscalls, serializing every concurrent cache operation behind one lock: - get_blob_stream / get_blob_range_stream: held across File::open()/seek() - delete_blob: held across remove_file() - initialize: held across the full cache-dir walk - eviction (insert + fetch paths): held across remove_file() loops Now the lock only guards the in-memory LRU. Presence checks bump recency and release the guard before touching the filesystem (a vanished file falls through to the existing fetch-and-cache path, covering the race), and eviction selects victims under the lock then unlinks them after releasing it. The duplicated eviction loop is extracted into CachedRef::collect_evictions. db: set test_before_acquire(false). With warm min_connections and a bounded max_lifetime, the liveness ping sqlx issues on every acquire() costs more than the rare dead connection it catches; stale sockets surface as a query error and the pool recycles them either way. https://claude.ai/code/session_01UtfkS3nZF1vrF5jNAps6wV --- src/infrastructure/db.rs | 6 + .../services/cached_blob_backend.rs | 153 ++++++++++-------- 2 files changed, 92 insertions(+), 67 deletions(-) diff --git a/src/infrastructure/db.rs b/src/infrastructure/db.rs index ad60c39e..628f2cb9 100644 --- a/src/infrastructure/db.rs +++ b/src/infrastructure/db.rs @@ -114,6 +114,12 @@ async fn create_pool_with_retries( .acquire_timeout(Duration::from_secs(connect_timeout_secs)) .idle_timeout(Duration::from_secs(idle_timeout_secs)) .max_lifetime(Duration::from_secs(max_lifetime_secs)) + // Skip the liveness ping sqlx issues on every acquire() (on by + // default): with warm min_connections and a bounded max_lifetime, + // that extra round-trip per checkout costs more than the rare dead + // connection it catches. A stale socket surfaces as a query error + // and the pool recycles it either way. + .test_before_acquire(false) .connect(connection_string) .await { diff --git a/src/infrastructure/services/cached_blob_backend.rs b/src/infrastructure/services/cached_blob_backend.rs index 502edd4a..2cb3288f 100644 --- a/src/infrastructure/services/cached_blob_backend.rs +++ b/src/infrastructure/services/cached_blob_backend.rs @@ -96,9 +96,11 @@ impl BlobStorageBackend for CachedBlobBackend { DomainError::internal_error("BlobCache", format!("mkdir cache_dir: {e}")) })?; - // Scan existing cache to rebuild index + // Scan existing cache to rebuild index. Collect entries WITHOUT + // holding the index lock — a large cache directory walk must not + // serialize concurrent blob operations behind the mutex. let mut total_bytes = 0u64; - let mut idx = index.lock().await; + let mut entries: Vec<(String, u64)> = Vec::new(); if let Ok(mut read_dir) = fs::read_dir(&cache_dir).await { while let Ok(Some(prefix_entry)) = read_dir.next_entry().await { if !prefix_entry.path().is_dir() { @@ -111,14 +113,20 @@ impl BlobStorageBackend for CachedBlobBackend { && let Some(stem) = path.file_stem().and_then(|s| s.to_str()) { let size = fs::metadata(&path).await.map(|m| m.len()).unwrap_or(0); - idx.put(stem.to_string(), CacheEntry { size }); + entries.push((stem.to_string(), size)); total_bytes += size; } } } } } - drop(idx); + // Bulk-insert the rebuilt index under a single brief lock. + { + let mut idx = index.lock().await; + for (stem, size) in entries { + idx.put(stem, CacheEntry { size }); + } + } current_size.store(total_bytes, Ordering::Relaxed); tracing::info!( "Blob cache initialized: {} bytes in cache at {}", @@ -196,19 +204,18 @@ impl BlobStorageBackend for CachedBlobBackend { let max_cache_bytes = self.max_cache_bytes; let current_size = self.current_size.clone(); Box::pin(async move { - // Check cache - { - let mut idx = index.lock().await; - if idx.get(&hash).is_some() { - if let Ok(file) = fs::File::open(&cached).await { - let stream: BlobStream = - Box::pin(ReaderStream::with_capacity(file, STREAM_CHUNK_SIZE)); - return Ok(stream); - } - // Cache entry stale — remove - if let Some(entry) = idx.pop(&hash) { - current_size.fetch_sub(entry.size, Ordering::Relaxed); - } + // Check cache presence (and bump LRU recency) under a brief lock, + // then release it BEFORE touching the filesystem so concurrent + // readers don't serialize behind a single open() syscall. + if index.lock().await.get(&hash).is_some() { + if let Ok(file) = fs::File::open(&cached).await { + let stream: BlobStream = + Box::pin(ReaderStream::with_capacity(file, STREAM_CHUNK_SIZE)); + return Ok(stream); + } + // Cache entry stale (file vanished) — drop it from the index. + if let Some(entry) = index.lock().await.pop(&hash) { + current_size.fetch_sub(entry.size, Ordering::Relaxed); } } @@ -243,25 +250,24 @@ impl BlobStorageBackend for CachedBlobBackend { let max_cache_bytes = self.max_cache_bytes; let current_size = self.current_size.clone(); Box::pin(async move { - // Try cache first - { - let mut idx = index.lock().await; - if idx.get(&hash).is_some() { - if let Ok(mut file) = fs::File::open(&cached).await { - file.seek(std::io::SeekFrom::Start(start)) - .await - .map_err(|e| { - DomainError::internal_error("BlobCache", format!("seek: {e}")) - })?; - let take_len = end.map(|e| e - start + 1).unwrap_or(u64::MAX); - let limited = file.take(take_len); - let stream: BlobStream = - Box::pin(ReaderStream::with_capacity(limited, STREAM_CHUNK_SIZE)); - return Ok(stream); - } - if let Some(entry) = idx.pop(&hash) { - current_size.fetch_sub(entry.size, Ordering::Relaxed); - } + // Check cache presence (and bump LRU recency) under a brief lock, + // then release it BEFORE the open()/seek() syscalls so concurrent + // range readers don't serialize behind the index mutex. + if index.lock().await.get(&hash).is_some() { + if let Ok(mut file) = fs::File::open(&cached).await { + file.seek(std::io::SeekFrom::Start(start)) + .await + .map_err(|e| { + DomainError::internal_error("BlobCache", format!("seek: {e}")) + })?; + let take_len = end.map(|e| e - start + 1).unwrap_or(u64::MAX); + let limited = file.take(take_len); + let stream: BlobStream = + Box::pin(ReaderStream::with_capacity(limited, STREAM_CHUNK_SIZE)); + return Ok(stream); + } + if let Some(entry) = index.lock().await.pop(&hash) { + current_size.fetch_sub(entry.size, Ordering::Relaxed); } } @@ -298,9 +304,9 @@ impl BlobStorageBackend for CachedBlobBackend { let current_size = self.current_size.clone(); Box::pin(async move { inner.delete_blob(&hash).await?; - // Remove from cache - let mut idx = index.lock().await; - if let Some(entry) = idx.pop(&hash) { + // Remove from cache — drop the index lock before the unlink() + // syscall so deletes don't serialize concurrent cache lookups. + if let Some(entry) = index.lock().await.pop(&hash) { current_size.fetch_sub(entry.size, Ordering::Relaxed); } let _ = fs::remove_file(&cached).await; @@ -402,6 +408,26 @@ impl CachedRef { self.cache_dir.join(prefix).join(format!("{hash}.blob")) } + /// Pop LRU entries until the cache is back within its byte budget, + /// returning the on-disk paths of the evicted blobs. + /// + /// Only the in-memory index is touched here (atomic counter + LRU map); + /// the caller MUST unlink the returned paths AFTER releasing the index + /// lock so the `remove_file` syscalls never run while the mutex is held. + fn collect_evictions(&self, idx: &mut LruCache) -> Vec { + let mut victims = Vec::new(); + while self.current_size.load(Ordering::Relaxed) > self.max_cache_bytes { + if let Some((evicted_hash, evicted_entry)) = idx.pop_lru() { + self.current_size + .fetch_sub(evicted_entry.size, Ordering::Relaxed); + victims.push(self.cached_path(&evicted_hash)); + } else { + break; + } + } + victims + } + async fn insert_into_cache_static( &self, hash: &str, @@ -423,21 +449,19 @@ impl CachedRef { DomainError::internal_error("BlobCache", format!("cache copy failed: {e}")) })?; - let mut idx = self.index.lock().await; - if let Some(old) = idx.put(hash.to_string(), CacheEntry { size }) { - self.current_size.fetch_sub(old.size, Ordering::Relaxed); - } - self.current_size.fetch_add(size, Ordering::Relaxed); - - while self.current_size.load(Ordering::Relaxed) > self.max_cache_bytes { - if let Some((evicted_hash, evicted_entry)) = idx.pop_lru() { - self.current_size - .fetch_sub(evicted_entry.size, Ordering::Relaxed); - let evicted_path = self.cached_path(&evicted_hash); - let _ = fs::remove_file(&evicted_path).await; - } else { - break; + // Update the index and pick eviction victims under a single brief + // lock, then unlink the evicted files AFTER releasing it — file + // removal must not run while the index mutex is held. + let to_evict = { + let mut idx = self.index.lock().await; + if let Some(old) = idx.put(hash.to_string(), CacheEntry { size }) { + self.current_size.fetch_sub(old.size, Ordering::Relaxed); } + self.current_size.fetch_add(size, Ordering::Relaxed); + self.collect_evictions(&mut idx) + }; + for path in to_evict { + let _ = fs::remove_file(&path).await; } Ok(()) } @@ -482,21 +506,16 @@ impl CachedRef { .await .map_err(|e| DomainError::internal_error("BlobCache", format!("rename: {e}")))?; - let mut idx = self.index.lock().await; - if let Some(old) = idx.put(hash.to_string(), CacheEntry { size: total }) { - self.current_size.fetch_sub(old.size, Ordering::Relaxed); - } - self.current_size.fetch_add(total, Ordering::Relaxed); - - while self.current_size.load(Ordering::Relaxed) > self.max_cache_bytes { - if let Some((evicted_hash, evicted_entry)) = idx.pop_lru() { - self.current_size - .fetch_sub(evicted_entry.size, Ordering::Relaxed); - let evicted_path = self.cached_path(&evicted_hash); - let _ = fs::remove_file(&evicted_path).await; - } else { - break; + let to_evict = { + let mut idx = self.index.lock().await; + if let Some(old) = idx.put(hash.to_string(), CacheEntry { size: total }) { + self.current_size.fetch_sub(old.size, Ordering::Relaxed); } + self.current_size.fetch_add(total, Ordering::Relaxed); + self.collect_evictions(&mut idx) + }; + for path in to_evict { + let _ = fs::remove_file(&path).await; } Ok(dest) From f82c5ccf4721f034a5f846ffb5a10ad39367f353 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 13:24:22 +0000 Subject: [PATCH 2/7] perf(authz): bound unbounded grant-list queries with a fail-loud row cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit list_incoming_grants and list_grants_on_resource ran fetch_all with no LIMIT, so a pathological number of grants on one resource (or targeting one subject) would be pulled fully into memory. These back the grant-management endpoints ("Manage sharing", "Shared with me"), not the hot require() path. A blind LIMIT is unsafe here: apply_role reads the full grant set to compute an add/remove diff, so a silently truncated list would be acted on as if complete (stale grants never revoked). Instead fetch MAX_GRANT_ROWS + 1 and reject with an audit line (authz.grant_list_rejected / reason=over_row_cap) when the cap is exceeded, bounding worst-case RAM without ever returning a partial set. The check is shared via PgAclEngine::guard_grant_row_cap. Cap is 10_000 — orders of magnitude above any realistic single-resource/subject grant count. https://claude.ai/code/session_01UtfkS3nZF1vrF5jNAps6wV --- src/infrastructure/services/pg_acl_engine.rs | 41 ++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/infrastructure/services/pg_acl_engine.rs b/src/infrastructure/services/pg_acl_engine.rs index b86b6357..b60ba832 100644 --- a/src/infrastructure/services/pg_acl_engine.rs +++ b/src/infrastructure/services/pg_acl_engine.rs @@ -61,6 +61,18 @@ struct QueryCounters { expanded_groups: AtomicU32, } +/// Defensive upper bound on the number of grant rows the *unbounded* list +/// methods (`list_incoming_grants`, `list_grants_on_resource`) will pull into +/// memory. These back management surfaces ("Manage sharing", "Shared with +/// me"), not the hot `require()` path, so a single resource or subject +/// realistically accumulates orders of magnitude fewer grants than this. +/// +/// We fetch `MAX_GRANT_ROWS + 1` and *reject* when the cap is exceeded rather +/// than silently truncating: `apply_role` computes an add/remove diff from the +/// returned set, so a partial list would be acted on as if complete. Hitting +/// the cap signals pathological data and is surfaced to operators via audit. +const MAX_GRANT_ROWS: i64 = 10_000; + pub struct PgAclEngine { pool: Arc, folder_repo: Arc, @@ -406,6 +418,29 @@ impl PgAclEngine { }) } + /// Reject an over-cap grant listing rather than returning a truncated set. + /// The unbounded list methods fetch `MAX_GRANT_ROWS + 1` and pass the row + /// count here; callers diff against the full result, so silently dropping + /// rows would corrupt that diff. Emits an audit line before failing so the + /// pathological resource/subject is visible to operators. + fn guard_grant_row_cap(returned: usize, op: &str) -> Result<(), DomainError> { + if returned as i64 > MAX_GRANT_ROWS { + tracing::info!( + target: "audit", + event = "authz.grant_list_rejected", + reason = "over_row_cap", + op, + cap = MAX_GRANT_ROWS, + "👮🏻‍♂️ grant listing exceeded the row safety cap; refusing to return a partial set", + ); + return Err(DomainError::internal_error( + "PgAcl", + format!("{op}: too many grants (cap {})", MAX_GRANT_ROWS), + )); + } + Ok(()) + } + /// The actual permission decision. Wrapped by `check()` which adds /// per-call instrumentation. async fn check_inner( @@ -525,15 +560,18 @@ impl AuthorizationEngine for PgAclEngine { AND subject_id = ANY($2) AND ($3::text IS NULL OR permission = $3) ORDER BY granted_at DESC + LIMIT $4 "#, ) .bind(&subject_types) .bind(&subject_ids) .bind(perm_str) + .bind(MAX_GRANT_ROWS + 1) .fetch_all(self.pool.as_ref()) .await .map_err(|e| DomainError::internal_error("PgAcl", format!("list incoming: {e}")))?; + Self::guard_grant_row_cap(rows.len(), "list_incoming_grants")?; rows.into_iter().map(Self::row_to_grant).collect() } @@ -855,14 +893,17 @@ impl AuthorizationEngine for PgAclEngine { WHERE resource_type = $1 AND resource_id = $2 ORDER BY granted_at DESC + LIMIT $3 "#, ) .bind(resource.type_str()) .bind(resource.id()) + .bind(MAX_GRANT_ROWS + 1) .fetch_all(self.pool.as_ref()) .await .map_err(|e| DomainError::internal_error("PgAcl", format!("list on resource: {e}")))?; + Self::guard_grant_row_cap(rows.len(), "list_grants_on_resource")?; rows.into_iter().map(Self::row_to_grant).collect() } From ec8ddebc30f52685fd14e535a6a6dcbda569203d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 13:29:05 +0000 Subject: [PATCH 3/7] perf: drop intermediate allocs in WebDAV href encoding; fold group-list COUNT into one query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit webdav encode_uri_path runs on every PROPFIND href and did .map(...).collect::>().join("/"), allocating a String per segment plus a joined Vec. Write each utf8_percent_encode Display adapter straight into a single preallocated String. Behavior is identical (split on '/', encode each segment, join with '/'), including leading/trailing-slash edge cases. subject_group list / list_with_counts each issued a second SELECT COUNT(*) round-trip for the total. Fold it into the page query via COUNT(*) OVER() — the pattern folder_db_repository already uses — halving the round-trips. total_count is read from the first row and is 0 on an empty page, matching folder_db_repository's documented convention. https://claude.ai/code/session_01UtfkS3nZF1vrF5jNAps6wV --- .../pg/subject_group_pg_repository.rs | 109 +++++++----------- src/interfaces/api/handlers/webdav_handler.rs | 21 +++- 2 files changed, 58 insertions(+), 72 deletions(-) diff --git a/src/infrastructure/repositories/pg/subject_group_pg_repository.rs b/src/infrastructure/repositories/pg/subject_group_pg_repository.rs index 39bdc61c..7484a155 100644 --- a/src/infrastructure/repositories/pg/subject_group_pg_repository.rs +++ b/src/infrastructure/repositories/pg/subject_group_pg_repository.rs @@ -126,30 +126,26 @@ impl SubjectGroupRepository for SubjectGroupPgRepository { offset: u32, name_query: Option<&str>, ) -> Result<(Vec, u64), SubjectGroupRepositoryError> { - // Two queries: one for the page, one for the total count. The query - // is small and frequent; a window function would add complexity for - // no measurable win. - let (sql_page, sql_count, pattern) = match name_query { - Some(q) => { - let pat = like_escape(q); - ( - "SELECT id, name, description, is_virtual, created_at, updated_at - FROM auth.subject_groups - WHERE name ILIKE $1 - ORDER BY is_virtual DESC, name - LIMIT $2 OFFSET $3" - .to_string(), - "SELECT COUNT(*) FROM auth.subject_groups WHERE name ILIKE $1".to_string(), - Some(pat), - ) - } + // Single query: the page plus `COUNT(*) OVER()` for the total matching + // count, folding what used to be a separate COUNT round-trip into one. + let (sql_page, pattern) = match name_query { + Some(q) => ( + "SELECT id, name, description, is_virtual, created_at, updated_at, + COUNT(*) OVER() AS total_count + FROM auth.subject_groups + WHERE name ILIKE $1 + ORDER BY is_virtual DESC, name + LIMIT $2 OFFSET $3" + .to_string(), + Some(like_escape(q)), + ), None => ( - "SELECT id, name, description, is_virtual, created_at, updated_at + "SELECT id, name, description, is_virtual, created_at, updated_at, + COUNT(*) OVER() AS total_count FROM auth.subject_groups ORDER BY is_virtual DESC, name LIMIT $1 OFFSET $2" .to_string(), - "SELECT COUNT(*) FROM auth.subject_groups".to_string(), None, ), }; @@ -170,19 +166,9 @@ impl SubjectGroupRepository for SubjectGroupPgRepository { } .map_err(|e| Self::map_sqlx_err("list page", e))?; - let total: i64 = if let Some(ref p) = pattern { - sqlx::query_scalar(&sql_count) - .bind(p) - .fetch_one(self.pool.as_ref()) - .await - } else { - sqlx::query_scalar(&sql_count) - .fetch_one(self.pool.as_ref()) - .await - } - .map_err(|e| Self::map_sqlx_err("list count", e))?; - - Ok((rows.iter().map(Self::row_to_group).collect(), total as u64)) + // total_count is identical in every row; 0 when the page is empty. + let total = rows.first().map_or(0, |r| r.get::("total_count")) as u64; + Ok((rows.iter().map(Self::row_to_group).collect(), total)) } async fn list_with_counts( @@ -191,38 +177,35 @@ impl SubjectGroupRepository for SubjectGroupPgRepository { offset: u32, name_query: Option<&str>, ) -> Result<(Vec<(SubjectGroup, i64)>, u64), SubjectGroupRepositoryError> { - // Single SQL: groups + COUNT of direct members per group, via LEFT JOIN - // on `auth.subject_group_members`. No N+1; one round-trip for the - // page, a second for the unfiltered total (matches `list`). - let (sql_page, sql_count, pattern) = match name_query { - Some(q) => { - let pat = like_escape(q); - ( - "SELECT g.id, g.name, g.description, g.is_virtual, - g.created_at, g.updated_at, - COUNT(m.group_id) AS member_count - FROM auth.subject_groups g - LEFT JOIN auth.subject_group_members m ON m.group_id = g.id - WHERE g.name ILIKE $1 - GROUP BY g.id - ORDER BY g.is_virtual DESC, g.name - LIMIT $2 OFFSET $3" - .to_string(), - "SELECT COUNT(*) FROM auth.subject_groups WHERE name ILIKE $1".to_string(), - Some(pat), - ) - } + // Single SQL: groups + per-group member COUNT via LEFT JOIN on + // `auth.subject_group_members`, plus `COUNT(*) OVER()` for the total + // group count. No N+1 and no separate COUNT round-trip — one query. + let (sql_page, pattern) = match name_query { + Some(q) => ( + "SELECT g.id, g.name, g.description, g.is_virtual, + g.created_at, g.updated_at, + COUNT(m.group_id) AS member_count, + COUNT(*) OVER() AS total_count + FROM auth.subject_groups g + LEFT JOIN auth.subject_group_members m ON m.group_id = g.id + WHERE g.name ILIKE $1 + GROUP BY g.id + ORDER BY g.is_virtual DESC, g.name + LIMIT $2 OFFSET $3" + .to_string(), + Some(like_escape(q)), + ), None => ( "SELECT g.id, g.name, g.description, g.is_virtual, g.created_at, g.updated_at, - COUNT(m.group_id) AS member_count + COUNT(m.group_id) AS member_count, + COUNT(*) OVER() AS total_count FROM auth.subject_groups g LEFT JOIN auth.subject_group_members m ON m.group_id = g.id GROUP BY g.id ORDER BY g.is_virtual DESC, g.name LIMIT $1 OFFSET $2" .to_string(), - "SELECT COUNT(*) FROM auth.subject_groups".to_string(), None, ), }; @@ -243,24 +226,14 @@ impl SubjectGroupRepository for SubjectGroupPgRepository { } .map_err(|e| Self::map_sqlx_err("list_with_counts page", e))?; - let total: i64 = if let Some(ref p) = pattern { - sqlx::query_scalar(&sql_count) - .bind(p) - .fetch_one(self.pool.as_ref()) - .await - } else { - sqlx::query_scalar(&sql_count) - .fetch_one(self.pool.as_ref()) - .await - } - .map_err(|e| Self::map_sqlx_err("list_with_counts total", e))?; - + // total_count is identical in every row; 0 when the page is empty. + let total = rows.first().map_or(0, |r| r.get::("total_count")) as u64; let items = rows .iter() .map(|r| (Self::row_to_group(r), r.get::("member_count"))) .collect(); - Ok((items, total as u64)) + Ok((items, total)) } async fn count_members(&self, id: Uuid) -> Result { diff --git a/src/interfaces/api/handlers/webdav_handler.rs b/src/interfaces/api/handlers/webdav_handler.rs index ef339dff..779efe7b 100644 --- a/src/interfaces/api/handlers/webdav_handler.rs +++ b/src/interfaces/api/handlers/webdav_handler.rs @@ -63,10 +63,23 @@ fn encode_path_segment(segment: &str) -> String { /// Percent-encode a full slash-separated path, encoding each segment individually. pub(crate) fn encode_uri_path(path: &str) -> String { - path.split('/') - .map(encode_path_segment) - .collect::>() - .join("/") + use std::fmt::Write as _; + // `utf8_percent_encode` returns a `Display` adapter, so write each encoded + // segment straight into `out` — avoids a String per segment and the joined + // Vec the previous `.map(...).collect::>().join("/")` allocated on + // every PROPFIND href. + let mut out = String::with_capacity(path.len() + 8); + for (i, segment) in path.split('/').enumerate() { + if i > 0 { + out.push('/'); + } + let _ = write!( + out, + "{}", + utf8_percent_encode(segment, PATH_SEGMENT_ENCODE_SET) + ); + } + out } /// Build the `` value for a non-collection (file) resource. From bf46c1ca102d0312d8a7c503b8b974616f5fe85d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 13:45:16 +0000 Subject: [PATCH 4/7] perf: make StoragePath::join and File builders consume self to avoid clones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit StoragePath::join deep-cloned the whole Vec (every segment String) just to append one element. Take self by value and push in place. All callers pass owned values except PathService::create_file_path, which holds a borrow and now clones explicitly — the same copy the old &self join already made. File::with_name / with_folder / with_size took &self and rebuilt the struct, cloning every carried-over field (id, mime_type, folder_id, blob_hash, ...). Consume self and mutate only the fields that change. Behaviour is identical; the fallible builders now drop the input on Err, which is fine for these rename/move/resize transforms (all current callers replace the file). Impact is small in practice — with_folder/with_size have no callers and with_name is test-only, while the one hot join caller (create_file_path) must copy segments regardless — but the consuming form is the idiomatic one. Verified: cargo fmt + clippy --all-features --all-targets -D warnings clean; domain tests (path_service::, entities::file::) pass — 30 + 6. https://claude.ai/code/session_01UtfkS3nZF1vrF5jNAps6wV --- src/domain/entities/file.rs | 74 +++++++-------------- src/domain/services/path_service.rs | 12 ++-- src/infrastructure/services/path_service.rs | 4 +- 3 files changed, 31 insertions(+), 59 deletions(-) diff --git a/src/domain/entities/file.rs b/src/domain/entities/file.rs index f42147f9..0c379040 100644 --- a/src/domain/entities/file.rs +++ b/src/domain/entities/file.rs @@ -388,45 +388,36 @@ impl File { // Methods to create new versions of the file (immutable) /// Creates a new version of the file with updated name - pub fn with_name(&self, new_name: String) -> FileResult { + pub fn with_name(mut self, new_name: String) -> FileResult { let new_name = normalize_storage_name(&new_name); if let Err(reason) = validate_storage_name(&new_name) { return Err(FileError::InvalidFileName(format!("{new_name}: {reason}"))); } - // Update path based on name - let parent_path = self.storage_path.parent(); - let new_storage_path = match parent_path { + // Recompute the path from the unchanged parent + the new name. + let new_storage_path = match self.storage_path.parent() { Some(parent) => parent.join(&new_name), None => StoragePath::from_string(&new_name), }; - // Update string representation - let new_path_string = new_storage_path.to_string(); - let now = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .unwrap_or_default() .as_secs(); - Ok(Self { - id: self.id.clone(), - name: new_name, - storage_path: new_storage_path, - path_string: new_path_string, - size: self.size, - mime_type: self.mime_type.clone(), - folder_id: self.folder_id.clone(), - created_at: self.created_at, - modified_at: now, - owner_id: self.owner_id, - blob_hash: self.blob_hash.clone(), - }) + // Consume `self` and mutate in place — only the path, name and mtime + // change; id / mime_type / folder_id / blob_hash are carried over + // without the per-field clone the old `&self` builder paid. + self.path_string = new_storage_path.to_string(); + self.storage_path = new_storage_path; + self.name = new_name; + self.modified_at = now; + Ok(self) } /// Creates a new version of the file with updated folder pub fn with_folder( - &self, + mut self, folder_id: Option, folder_path: Option, ) -> FileResult { @@ -436,49 +427,30 @@ impl File { None => StoragePath::from_string(&self.name), // Root }; - // Update string representation - let new_path_string = new_storage_path.to_string(); - let now = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .unwrap_or_default() .as_secs(); - Ok(Self { - id: self.id.clone(), - name: self.name.clone(), - storage_path: new_storage_path, - path_string: new_path_string, - size: self.size, - mime_type: self.mime_type.clone(), - folder_id, - created_at: self.created_at, - modified_at: now, - owner_id: self.owner_id, - blob_hash: self.blob_hash.clone(), - }) + // Consume `self`: only the path, folder_id and mtime change. + self.path_string = new_storage_path.to_string(); + self.storage_path = new_storage_path; + self.folder_id = folder_id; + self.modified_at = now; + Ok(self) } /// Creates a new version of the file with updated size - pub fn with_size(&self, new_size: u64) -> Self { + pub fn with_size(mut self, new_size: u64) -> Self { let now = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .unwrap_or_default() .as_secs(); - Self { - id: self.id.clone(), - name: self.name.clone(), - storage_path: self.storage_path.clone(), - path_string: self.path_string.clone(), - size: new_size, - mime_type: self.mime_type.clone(), - folder_id: self.folder_id.clone(), - created_at: self.created_at, - modified_at: now, - owner_id: self.owner_id, - blob_hash: self.blob_hash.clone(), - } + // Consume `self`: only size and mtime change — no per-field clone. + self.size = new_size; + self.modified_at = now; + self } } diff --git a/src/domain/services/path_service.rs b/src/domain/services/path_service.rs index 70fa4b4c..e1ebbe71 100644 --- a/src/domain/services/path_service.rs +++ b/src/domain/services/path_service.rs @@ -103,18 +103,16 @@ impl StoragePath { Self { segments } } - /// Appends a segment to the path. + /// Appends a segment to the path, consuming `self` so the existing + /// segment buffer is reused instead of deep-cloned. /// /// Traversal segments (`.`, `..`) and segments containing `/` are /// silently ignored to prevent path-traversal attacks. - pub fn join(&self, segment: &str) -> Self { - let mut new_segments = self.segments.clone(); + pub fn join(mut self, segment: &str) -> Self { if Self::is_safe_segment(segment) { - new_segments.push(segment.to_string()); - } - Self { - segments: new_segments, + self.segments.push(segment.to_string()); } + self } /// Gets the file name (last segment) diff --git a/src/infrastructure/services/path_service.rs b/src/infrastructure/services/path_service.rs index 6c9189d2..dc60e341 100644 --- a/src/infrastructure/services/path_service.rs +++ b/src/infrastructure/services/path_service.rs @@ -64,7 +64,9 @@ impl PathService { /// Creates a file path within a folder pub fn create_file_path(&self, folder_path: &StoragePath, file_name: &str) -> StoragePath { - folder_path.join(file_name) + // `join` consumes its receiver to reuse the buffer; we only hold a + // borrow here, so clone first — the same copy the old `&self` join did. + folder_path.clone().join(file_name) } /// Checks if a path is a direct child of another From 19b41af2e4797606bc23f8612a9ba65fe54e5007 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 14:08:27 +0000 Subject: [PATCH 5/7] =?UTF-8?q?perf(http):=20single=20smart=20compression?= =?UTF-8?q?=20layer=20=E2=80=94=20stop=20compressing=20media=20on=20/api?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /api router added its own predicate-less CompressionLayer (routes.rs), nested inside the global predicate-aware one in main.rs. As the inner layer it compressed responses first, so the global predicate that skips already- compressed media was bypassed for every /api response: video/audio/image/zip downloads got Brotli-compressed (CPU + first-byte latency for ~0 bytes saved) and lost their Content-Length (forced to chunked -> no client progress bar). - Remove the redundant /api CompressionLayer; /api now flows through the single global layer in main.rs. - Make that predicate smarter: compress by default so nothing shrinkable is missed, and skip ONLY already-compressed types. It no longer blanket-excludes image/*, so image/svg+xml (text, ~70% shrink) now compresses; raster formats are listed individually. Added the previously-missed already-compressed types: Office (docx/xlsx/pptx), ODF, epub, jar, apk, 7z/rar/bzip2/zstd/xz, woff/woff2 fonts, icons. Net: media downloads keep Content-Length and skip pointless compression, while text/JSON/JS/CSS/SVG/XML/ttf/otf/wasm still compress. fmt + clippy clean. https://claude.ai/code/session_01UtfkS3nZF1vrF5jNAps6wV --- src/interfaces/api/routes.rs | 14 ++++----- src/main.rs | 61 +++++++++++++++++++++++++++++++----- 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/src/interfaces/api/routes.rs b/src/interfaces/api/routes.rs index 4baff7c7..f2ac1662 100644 --- a/src/interfaces/api/routes.rs +++ b/src/interfaces/api/routes.rs @@ -9,7 +9,7 @@ use axum::{ }; use serde_json::json; use std::sync::Arc; -use tower_http::{compression::CompressionLayer, trace::TraceLayer}; +use tower_http::trace::TraceLayer; use utoipa::OpenApi; /// Liveness probe — returns 200 if the process is running, no DB check. @@ -579,10 +579,10 @@ pub fn create_api_routes(app_state: &Arc) -> Router> { .with_state(app_state.clone()); router = router.nest("/users", users_router); - // Transparent compression (gzip + brotli) for all API responses. - // tower-http negotiates via Accept-Encoding and skips already-compressed - // content types automatically. No manual compression in handlers. - router - .layer(CompressionLayer::new().br(true).gzip(true)) - .layer(TraceLayer::new_for_http()) + // Compression is applied once, globally, in `main.rs` with a content-type + // aware predicate that skips already-compressed media. Re-applying it here + // would double-wrap `/api`: this inner layer (no predicate) would compress + // media downloads, burning CPU for ~0 gain and stripping `Content-Length`. + // So this router only adds tracing; compression is the global layer's job. + router.layer(TraceLayer::new_for_http()) } diff --git a/src/main.rs b/src/main.rs index 192b4146..eb558f7c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -541,25 +541,70 @@ async fn main() -> Result<(), Box> { app = app.layer(DefaultBodyLimit::max(BODY_LIMIT)); // ── HTTP compression (gzip + Brotli) ───────────────────────────────── - // Negotiates the best encoding via Accept-Encoding. Skips responses - // that are already compressed or wouldn't benefit (images, video, etc.). - // Compatible with a future reverse proxy — if the proxy sees - // `Content-Encoding` it will pass the response through untouched. + // Negotiates the best encoding via Accept-Encoding. Policy: compress + // everything by default so no shrinkable response is ever missed (text, + // JSON, JS/CSS, XML, SVG, fonts ttf/otf, WASM…), and skip ONLY content + // that is already compressed — where a second pass burns CPU and adds + // latency for ~0 bytes saved. + // + // We deliberately do NOT blanket-exclude `image/*`: `image/svg+xml` is + // plain text and compresses ~70%, so the genuinely-compressed raster + // formats are listed individually instead, leaving SVG compressible. + // + // This is the single, global compression layer (the `/api` router used to + // add its own predicate-less one, which silently compressed media). It is + // reverse-proxy friendly: a proxy that sees `Content-Encoding` passes + // the response through untouched. { use tower_http::compression::CompressionLayer; use tower_http::compression::predicate::{NotForContentType, Predicate, SizeAbove}; let predicate = SizeAbove::new(256) .and(NotForContentType::GRPC) - .and(NotForContentType::IMAGES) .and(NotForContentType::SSE) - .and(NotForContentType::const_new("application/octet-stream")) + // ── already-compressed raster images (SVG intentionally absent) ── + .and(NotForContentType::const_new("image/jpeg")) + .and(NotForContentType::const_new("image/png")) + .and(NotForContentType::const_new("image/gif")) + .and(NotForContentType::const_new("image/webp")) + .and(NotForContentType::const_new("image/avif")) + .and(NotForContentType::const_new("image/heic")) + .and(NotForContentType::const_new("image/heif")) + .and(NotForContentType::const_new("image/jp2")) + .and(NotForContentType::const_new("image/x-icon")) + .and(NotForContentType::const_new("image/vnd.microsoft.icon")) + // ── audio / video families (already compressed) ── + .and(NotForContentType::const_new("video/")) + .and(NotForContentType::const_new("audio/")) + // ── already-compressed web fonts; ttf/otf left compressible ── + .and(NotForContentType::const_new("font/woff")) + .and(NotForContentType::const_new("application/font-woff")) + // ── archives & compressed containers ── .and(NotForContentType::const_new("application/zip")) .and(NotForContentType::const_new("application/gzip")) + .and(NotForContentType::const_new("application/x-gzip")) .and(NotForContentType::const_new("application/x-tar")) + .and(NotForContentType::const_new("application/x-7z-compressed")) + .and(NotForContentType::const_new("application/x-rar-compressed")) + .and(NotForContentType::const_new("application/x-bzip2")) + .and(NotForContentType::const_new("application/zstd")) + .and(NotForContentType::const_new("application/x-xz")) + // ── zip-based document / app bundles (docx/xlsx/pptx, odf, epub…) ── + .and(NotForContentType::const_new( + "application/vnd.openxmlformats-officedocument", + )) + .and(NotForContentType::const_new( + "application/vnd.oasis.opendocument", + )) + .and(NotForContentType::const_new("application/epub+zip")) + .and(NotForContentType::const_new("application/java-archive")) + .and(NotForContentType::const_new( + "application/vnd.android.package-archive", + )) + // ── PDF: streams are usually already deflated; often large ── .and(NotForContentType::const_new("application/pdf")) - .and(NotForContentType::const_new("video/")) - .and(NotForContentType::const_new("audio/")); + // ── opaque binary we couldn't identify ── + .and(NotForContentType::const_new("application/octet-stream")); app = app.layer(CompressionLayer::new().compress_when(predicate)); } From 8dc528f403e6cbc37ea5cacebd0f00abee48422b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 14:19:14 +0000 Subject: [PATCH 6/7] fix(dedup): count chunk ref_count per distinct chunk, not per occurrence store_chunks bumped storage.blobs.ref_count once per chunk *occurrence* (it looped over the full chunk list, duplicates included), but remove_manifest_reference decrements once per *distinct* chunk (WHERE hash = ANY(chunk_hashes) matches each row a single time). For any file that repeats a chunk -- zero-filled regions in disk/VM images, repeated document structures, concatenated archives -- storing added +N while deleting removed -1, so the blob's ref_count never returned to 0 and the chunk was never garbage-collected: a permanent storage leak. Count per distinct chunk on the store side too, matching deletion. This also makes it faster: - existing chunks: one batched `UPDATE ... WHERE hash = ANY($1)` instead of one UPDATE per occurrence; - a brand-new chunk repeated within a file is read, uploaded and INSERTed once instead of once per occurrence. The manifest still stores the full per-occurrence chunk sequence (needed to reassemble the file). Forward fix: blobs already over-counted by the old path stay over-counted (a reconcile/verify pass could recompute them), but the bias is upward (leak), so no data is ever deleted early. CDC tests pass (12); fmt + clippy clean. https://claude.ai/code/session_01UtfkS3nZF1vrF5jNAps6wV --- src/infrastructure/services/dedup_service.rs | 189 +++++++++---------- 1 file changed, 91 insertions(+), 98 deletions(-) diff --git a/src/infrastructure/services/dedup_service.rs b/src/infrastructure/services/dedup_service.rs index de2ee35e..a916d60d 100644 --- a/src/infrastructure/services/dedup_service.rs +++ b/src/infrastructure/services/dedup_service.rs @@ -459,8 +459,12 @@ impl DedupService { /// exist in `storage.blobs`. /// Phase 1: Reads only *new* chunks from the source file (the biggest /// I/O saving for versioned files where most chunks are unchanged). - /// Phase 2: Parallel operations — uploads new chunks, bumps ref_count - /// for existing ones — with up to [`CHUNK_UPLOAD_CONCURRENCY`] in flight. + /// Uploads each new chunk and bumps `ref_count` for chunks that already + /// exist, with up to [`CHUNK_UPLOAD_CONCURRENCY`] uploads in flight. + /// + /// `ref_count` is incremented once per *distinct* chunk (one reference per + /// manifest), staying symmetric with `remove_manifest_reference` so a file + /// that repeats a chunk cannot over-count and leak the blob forever. async fn store_chunks( &self, source_path: &Path, @@ -469,20 +473,22 @@ impl DedupService { let pool = &self.pool; let backend = &self.backend; - // ── Phase 0: Batch-check which chunks already exist ────── - let unique_hashes: Vec = { - let mut seen = std::collections::HashSet::new(); - chunks - .iter() - .filter_map(|c| { - if seen.insert(c.hash.as_str()) { - Some(c.hash.clone()) - } else { - None - } - }) - .collect() - }; + // ── Phase 0: de-duplicate chunk hashes, then batch-check existence ── + // A single file can legitimately repeat the same chunk many times + // (zero-filled regions in disk/VM images, repeated document structures, + // concatenated archives). `ref_count` is tracked per *distinct* chunk — + // one reference per manifest — to stay symmetric with + // `remove_manifest_reference`, which decrements via + // `WHERE hash = ANY(chunk_hashes)` (matching each row once). Counting + // per-occurrence here would over-increment and leak the blob forever. + // Keep the first occurrence of each hash so new chunks know where to + // read their bytes. + let mut seen = std::collections::HashSet::new(); + let unique_chunks: Vec<&ChunkMeta> = chunks + .iter() + .filter(|c| seen.insert(c.hash.as_str())) + .collect(); + let unique_hashes: Vec = unique_chunks.iter().map(|c| c.hash.clone()).collect(); let existing_hashes: std::collections::HashSet = sqlx::query_scalar::<_, String>("SELECT hash FROM storage.blobs WHERE hash = ANY($1)") @@ -498,98 +504,86 @@ impl DedupService { .into_iter() .collect(); - // ── Phase 1+2 (fused): upload NEW chunks just-in-time ──── - // Read each new chunk by positioned I/O immediately before its - // upload, instead of first materializing every new chunk's *data* in - // a Vec. Peak heap for file content is bounded to - // ~CHUNK_UPLOAD_CONCURRENCY × CDC_MAX_CHUNK (≈ 8 MiB) — proportional - // to the chunk size, never the file size, so storing a large - // brand-new file no longer spikes RAM. Existing chunks skip all disk - // I/O and just bump ref_count. - // - // We first collect *owned* per-chunk metadata (hash + offset + length - // + existence flag — no file data) so the stream below does not borrow - // the `chunks` parameter across an `.await` (which would make this - // future non-`Send` and break the upload handlers). - let chunk_ops: Vec<(String, u64, usize, bool)> = chunks + // ── Phase 1: bump ref_count for every existing chunk in ONE query ── + // (was one UPDATE per occurrence — now a single batched round-trip). + let existing: Vec = unique_hashes .iter() - .map(|chunk| { - let exists = existing_hashes.contains(&chunk.hash); - ( - chunk.hash.clone(), - chunk.offset as u64, - chunk.length, - exists, - ) - }) + .filter(|h| existing_hashes.contains(*h)) + .cloned() + .collect(); + if !existing.is_empty() { + sqlx::query("UPDATE storage.blobs SET ref_count = ref_count + 1 WHERE hash = ANY($1)") + .bind(&existing) + .execute(pool.as_ref()) + .await + .map_err(|e| { + DomainError::internal_error("Dedup", format!("Failed to bump ref_count: {}", e)) + })?; + } + + // ── Phase 2: upload each NEW chunk once, concurrently ────────────── + // Read each new chunk by positioned I/O immediately before its upload + // instead of materializing every chunk's *data* up front. Peak heap for + // file content stays bounded to ~CHUNK_UPLOAD_CONCURRENCY × CDC_MAX_CHUNK + // (≈ 8 MiB) — proportional to the chunk size, never the file size. + // + // Owned metadata (hash + offset + length, no file data) so the stream + // below does not borrow `chunks` across an `.await` (which would make + // this future non-`Send` and break the upload handlers). + let new_ops: Vec<(String, u64, usize)> = unique_chunks + .iter() + .filter(|c| !existing_hashes.contains(&c.hash)) + .map(|c| (c.hash.clone(), c.offset as u64, c.length)) .collect(); let source = Arc::new(std::fs::File::open(source_path).map_err(|e| { DomainError::internal_error("Dedup", format!("Failed to open source file: {}", e)) })?); - let results: Vec> = stream::iter(chunk_ops) - .map(|(hash, offset, length, exists)| { + let results: Vec> = stream::iter(new_ops) + .map(|(hash, offset, length)| { let source = source.clone(); let pool = pool.clone(); let backend = backend.clone(); async move { - if exists { - // Existing chunk: bump ref_count, no disk I/O. - sqlx::query( - "UPDATE storage.blobs - SET ref_count = ref_count + 1 - WHERE hash = $1", - ) - .bind(&hash) - .execute(pool.as_ref()) - .await - .map_err(|e| { - DomainError::internal_error( - "Dedup", - format!("Failed to bump ref_count: {}", e), - ) - })?; - } else { - // New chunk: positioned read of just this chunk - // (≤ CDC_MAX_CHUNK) off the async runtime, then upload. - let bytes = tokio::task::spawn_blocking(move || { - use std::os::unix::fs::FileExt; - let mut buf = vec![0u8; length]; - source.read_exact_at(&mut buf, offset)?; - Ok::, std::io::Error>(buf) - }) - .await - .map_err(|e| { - DomainError::internal_error("Dedup", format!("Read task failed: {}", e)) - })? - .map_err(|e| { - DomainError::internal_error( - "Dedup", - format!("Failed to read chunk: {}", e), - ) - })?; + // Positioned read of just this chunk (≤ CDC_MAX_CHUNK) off + // the async runtime, then upload. + let bytes = tokio::task::spawn_blocking(move || { + use std::os::unix::fs::FileExt; + let mut buf = vec![0u8; length]; + source.read_exact_at(&mut buf, offset)?; + Ok::, std::io::Error>(buf) + }) + .await + .map_err(|e| { + DomainError::internal_error("Dedup", format!("Read task failed: {}", e)) + })? + .map_err(|e| { + DomainError::internal_error("Dedup", format!("Failed to read chunk: {}", e)) + })?; - backend - .put_blob_from_bytes(&hash, Bytes::from(bytes)) - .await?; - sqlx::query( - "INSERT INTO storage.blobs (hash, size, ref_count) - VALUES ($1, $2, 1) - ON CONFLICT (hash) DO UPDATE - SET ref_count = storage.blobs.ref_count + 1", + backend + .put_blob_from_bytes(&hash, Bytes::from(bytes)) + .await?; + // ON CONFLICT covers a concurrent uploader inserting the + // same brand-new chunk between the existence check above and + // this INSERT. + sqlx::query( + "INSERT INTO storage.blobs (hash, size, ref_count) + VALUES ($1, $2, 1) + ON CONFLICT (hash) DO UPDATE + SET ref_count = storage.blobs.ref_count + 1", + ) + .bind(&hash) + .bind(length as i64) + .execute(pool.as_ref()) + .await + .map_err(|e| { + DomainError::internal_error( + "Dedup", + format!("Failed to upsert chunk: {}", e), ) - .bind(&hash) - .bind(length as i64) - .execute(pool.as_ref()) - .await - .map_err(|e| { - DomainError::internal_error( - "Dedup", - format!("Failed to upsert chunk: {}", e), - ) - })?; - } + })?; Ok(()) } }) @@ -597,13 +591,12 @@ impl DedupService { .collect() .await; - // All operations must succeed. Order preservation is not needed - // here — chunk_hashes/chunk_sizes are derived from the input - // `chunks` slice which keeps the original CDC order. for result in results { result?; } + // chunk_hashes/chunk_sizes keep the full per-occurrence CDC sequence — + // the manifest needs every chunk, in order, to reassemble the file. let chunk_hashes: Vec = chunks.iter().map(|c| c.hash.clone()).collect(); let chunk_sizes: Vec = chunks.iter().map(|c| c.length as u64).collect(); From 175a908e99ada03150378804167426f5c91318be Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 14:59:01 +0000 Subject: [PATCH 7/7] perf(search): clamp search page size to bound per-request memory The main search endpoint took `limit: params.limit.unwrap_or(100)` with no ceiling, and that value flows straight into the SQL LIMIT of search_files_paginated / search_files_in_subtree. A client passing ?limit= would make Postgres return that many rows into memory and into the result cache. The suggestions endpoint already clamps (.min(20)); search did not. Cap at MAX_SEARCH_LIMIT (500). total_count still reflects the full match set (COUNT(*) OVER()), so deeper results stay reachable via offset. https://claude.ai/code/session_01UtfkS3nZF1vrF5jNAps6wV --- src/interfaces/api/handlers/search_handler.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/interfaces/api/handlers/search_handler.rs b/src/interfaces/api/handlers/search_handler.rs index 16609a1a..5fba8009 100644 --- a/src/interfaces/api/handlers/search_handler.rs +++ b/src/interfaces/api/handlers/search_handler.rs @@ -21,6 +21,14 @@ use std::sync::Arc; * formatting) is performed server-side. These handlers are thin HTTP * adapters that delegate to the SearchUseCase. */ +/// Hard cap on the search page size. The default is 100; without a ceiling a +/// client could pass `?limit=`, which flows straight into the SQL `LIMIT` +/// and would pull that many rows into memory (and into the result cache). 500 +/// is a generous page for a search UI — `total_count` still reflects the full +/// match set, so deeper results stay reachable via `offset`. Mirrors the +/// suggestions endpoint, which already clamps with `.min(20)`. +const MAX_SEARCH_LIMIT: usize = 500; + pub struct SearchHandler; impl SearchHandler { @@ -62,7 +70,7 @@ impl SearchHandler { max_size: params.max_size, folder_id: params.folder_id, recursive: params.recursive.unwrap_or(true), - limit: params.limit.unwrap_or(100), + limit: params.limit.unwrap_or(100).min(MAX_SEARCH_LIMIT), offset: params.offset.unwrap_or(0), sort_by: params.sort_by.unwrap_or_else(|| "relevance".to_string()), };