From 07111fbf7303e3b2963cb8421d3df1dbf0ca4a56 Mon Sep 17 00:00:00 2001 From: Luca Cominardi Date: Thu, 20 Aug 2026 12:12:00 +0200 Subject: [PATCH 1/3] Filter lexical siblings from prefixed listings --- .../quickwit-storage/src/prefix_storage.rs | 62 ++++++++++++++----- 1 file changed, 45 insertions(+), 17 deletions(-) diff --git a/quickwit/quickwit-storage/src/prefix_storage.rs b/quickwit/quickwit-storage/src/prefix_storage.rs index 312998ac91f..30719a9f05d 100644 --- a/quickwit/quickwit-storage/src/prefix_storage.rs +++ b/quickwit/quickwit-storage/src/prefix_storage.rs @@ -23,10 +23,7 @@ use quickwit_common::uri::Uri; use tokio::io::AsyncRead; use crate::storage::SendableAsync; -use crate::{ - BulkDeleteError, ListObjectsStream, ObjectMetadata, OwnedBytes, Storage, StorageErrorKind, - StorageResult, -}; +use crate::{BulkDeleteError, ListObjectsStream, ObjectMetadata, OwnedBytes, Storage}; /// This storage acts as a proxy to another storage that simply modifies each API call /// by preceding each path with a given a prefix. @@ -111,23 +108,22 @@ impl Storage for PrefixStorage { /// Makes listed paths relative to this storage root again, undoing the prefix added by /// [`PrefixStorage::list`]. fn strip_prefix_from_objects( - mut objects: Vec, + objects: Vec, prefix: &Path, - ) -> StorageResult> { + ) -> Vec { if prefix == Path::new("") { - return Ok(objects); + return objects; } - for object in &mut objects { - let relative_path = object.path.strip_prefix(prefix).map_err(|error| { - StorageErrorKind::Internal.with_error(anyhow::anyhow!( - "listed object `{}` is not under storage prefix `{}`: {error}", - object.path.display(), - prefix.display() - )) - })?; + let mut relative_objects = Vec::with_capacity(objects.len()); + for mut object in objects { + let Ok(relative_path) = object.path.strip_prefix(prefix) else { + // Some backends use byte-prefix semantics and may return lexical siblings. + continue; + }; object.path = relative_path.to_path_buf(); + relative_objects.push(object); } - Ok(objects) + relative_objects } let storage_prefix = self.prefix.clone(); @@ -135,7 +131,7 @@ impl Storage for PrefixStorage { .list(&self.prefix.join(prefix)) .map(move |objects_res| { let objects = objects_res?; - strip_prefix_from_objects(objects, &storage_prefix) + Ok(strip_prefix_from_objects(objects, &storage_prefix)) }) .boxed() } @@ -257,6 +253,38 @@ mod tests { assert_eq!(pages[0][0].size, bytesize::ByteSize(11)); } + #[tokio::test] + async fn test_prefix_storage_list_filters_lexical_siblings() { + let mut mock_storage = MockStorage::default(); + mock_storage.expect_list().times(1).returning(|prefix| { + assert_eq!(prefix, Path::new("ram:///indexes")); + let objects = vec![ + ObjectMetadata { + path: PathBuf::from("ram:///indexes/foo.split"), + size: bytesize::ByteSize(11), + last_modified: SystemTime::UNIX_EPOCH, + }, + ObjectMetadata { + path: PathBuf::from("ram:///indexes-old/unrelated.split"), + size: bytesize::ByteSize(13), + last_modified: SystemTime::UNIX_EPOCH, + }, + ]; + stream::once(async move { Ok(objects) }).boxed() + }); + let storage = add_prefix_to_storage( + Arc::new(mock_storage), + PathBuf::from("ram:///indexes"), + Uri::for_test("ram:///indexes"), + ); + let pages: Vec> = + storage.list(Path::new("")).try_collect().await.unwrap(); + + assert_eq!(pages.len(), 1); + assert_eq!(pages[0].len(), 1); + assert_eq!(pages[0][0].path, Path::new("foo.split")); + } + #[test] fn test_strip_prefix_from_error() { { From 45df4c3ba2fb0b3d36572f1869a4e245ebd2dd73 Mon Sep 17 00:00:00 2001 From: Luca Cominardi Date: Thu, 20 Aug 2026 13:15:38 +0200 Subject: [PATCH 2/3] Preserve errors for unrelated listed paths --- .../quickwit-storage/src/prefix_storage.rs | 68 ++++++++++++++++--- 1 file changed, 57 insertions(+), 11 deletions(-) diff --git a/quickwit/quickwit-storage/src/prefix_storage.rs b/quickwit/quickwit-storage/src/prefix_storage.rs index 30719a9f05d..88ed5769afe 100644 --- a/quickwit/quickwit-storage/src/prefix_storage.rs +++ b/quickwit/quickwit-storage/src/prefix_storage.rs @@ -23,7 +23,10 @@ use quickwit_common::uri::Uri; use tokio::io::AsyncRead; use crate::storage::SendableAsync; -use crate::{BulkDeleteError, ListObjectsStream, ObjectMetadata, OwnedBytes, Storage}; +use crate::{ + BulkDeleteError, ListObjectsStream, ObjectMetadata, OwnedBytes, Storage, StorageErrorKind, + StorageResult, +}; /// This storage acts as a proxy to another storage that simply modifies each API call /// by preceding each path with a given a prefix. @@ -110,20 +113,37 @@ impl Storage for PrefixStorage { fn strip_prefix_from_objects( objects: Vec, prefix: &Path, - ) -> Vec { + ) -> StorageResult> { if prefix == Path::new("") { - return objects; + return Ok(objects); } + let prefix_bytes = prefix.as_os_str().as_encoded_bytes(); let mut relative_objects = Vec::with_capacity(objects.len()); for mut object in objects { - let Ok(relative_path) = object.path.strip_prefix(prefix) else { - // Some backends use byte-prefix semantics and may return lexical siblings. - continue; - }; - object.path = relative_path.to_path_buf(); - relative_objects.push(object); + match object.path.strip_prefix(prefix) { + Ok(relative_path) => { + object.path = relative_path.to_path_buf(); + relative_objects.push(object); + } + Err(_) + if object + .path + .as_os_str() + .as_encoded_bytes() + .starts_with(prefix_bytes) => + { + // Byte-prefix backends may return lexical siblings of the storage root. + } + Err(error) => { + return Err(StorageErrorKind::Internal.with_error(anyhow::anyhow!( + "listed object `{}` is not under storage prefix `{}`: {error}", + object.path.display(), + prefix.display() + ))); + } + } } - relative_objects + Ok(relative_objects) } let storage_prefix = self.prefix.clone(); @@ -131,7 +151,7 @@ impl Storage for PrefixStorage { .list(&self.prefix.join(prefix)) .map(move |objects_res| { let objects = objects_res?; - Ok(strip_prefix_from_objects(objects, &storage_prefix)) + strip_prefix_from_objects(objects, &storage_prefix) }) .boxed() } @@ -285,6 +305,32 @@ mod tests { assert_eq!(pages[0][0].path, Path::new("foo.split")); } + #[tokio::test] + async fn test_prefix_storage_list_rejects_unrelated_paths() { + let mut mock_storage = MockStorage::default(); + mock_storage.expect_list().times(1).returning(|prefix| { + assert_eq!(prefix, Path::new("ram:///indexes")); + let objects = vec![ObjectMetadata { + path: PathBuf::from("ram:///unrelated/foo.split"), + size: bytesize::ByteSize(11), + last_modified: SystemTime::UNIX_EPOCH, + }]; + stream::once(async move { Ok(objects) }).boxed() + }); + let storage = add_prefix_to_storage( + Arc::new(mock_storage), + PathBuf::from("ram:///indexes"), + Uri::for_test("ram:///indexes"), + ); + let error = storage + .list(Path::new("")) + .try_collect::>>() + .await + .unwrap_err(); + + assert_eq!(error.kind(), StorageErrorKind::Internal); + } + #[test] fn test_strip_prefix_from_error() { { From 9e3cce32d47ff52a788b6be52c86204307477fe5 Mon Sep 17 00:00:00 2001 From: Luca Cominardi Date: Thu, 20 Aug 2026 13:23:18 +0200 Subject: [PATCH 3/3] Preserve errors for unrelated listed paths Filter byte-prefix lexical siblings while surfacing objects returned outside the storage prefix. --- .../quickwit-storage/src/prefix_storage.rs | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/quickwit/quickwit-storage/src/prefix_storage.rs b/quickwit/quickwit-storage/src/prefix_storage.rs index 88ed5769afe..3242f24ea49 100644 --- a/quickwit/quickwit-storage/src/prefix_storage.rs +++ b/quickwit/quickwit-storage/src/prefix_storage.rs @@ -125,21 +125,19 @@ impl Storage for PrefixStorage { object.path = relative_path.to_path_buf(); relative_objects.push(object); } - Err(_) - if object + Err(error) => { + let is_under_prefix = object .path .as_os_str() .as_encoded_bytes() - .starts_with(prefix_bytes) => - { - // Byte-prefix backends may return lexical siblings of the storage root. - } - Err(error) => { - return Err(StorageErrorKind::Internal.with_error(anyhow::anyhow!( - "listed object `{}` is not under storage prefix `{}`: {error}", - object.path.display(), - prefix.display() - ))); + .starts_with(prefix_bytes); + if !is_under_prefix { + return Err(StorageErrorKind::Internal.with_error(anyhow::anyhow!( + "listed object `{}` is not under storage prefix `{}`: {error}", + object.path.display(), + prefix.display() + ))); + } } } }