diff --git a/crates/e2e_test/src/list_objects_v2_pagination_test.rs b/crates/e2e_test/src/list_objects_v2_pagination_test.rs index 624d21f67..0d287ad32 100644 --- a/crates/e2e_test/src/list_objects_v2_pagination_test.rs +++ b/crates/e2e_test/src/list_objects_v2_pagination_test.rs @@ -612,4 +612,421 @@ mod tests { env.stop_server(); } + + /// Regression test: delimiter listing must not produce false-positive truncation + /// when many raw keys collapse into a small number of CommonPrefixes. + /// + /// Scenario: 30 objects across 3 directories + 2 direct files under prefix. + /// With max_keys=1000, all 5 visible results (3 prefixes + 2 objects) fit in one + /// page, so IsTruncated must be false even though raw entry count is much larger. + #[tokio::test] + #[serial] + async fn test_list_objects_v2_delimiter_collapsed_prefix_no_false_truncation() { + init_logging(); + info!("Starting test: ListObjectsV2 delimiter collapsed-prefix no false truncation"); + + let mut env = RustFSTestEnvironment::new().await.expect("Failed to create test environment"); + env.start_rustfs_server(vec![]).await.expect("Failed to start RustFS"); + + let client = create_s3_client(&env); + let bucket = "test-collapsed-prefix"; + let prefix = "data/"; + + create_bucket(&client, bucket).await.expect("Failed to create bucket"); + + // Create objects under 3 subdirectories (10 each = 30 raw keys) + let dirs = ["alpha/", "beta/", "gamma/"]; + let mut expected_keys = Vec::new(); + for dir in &dirs { + for i in 0..10 { + let key = format!("{prefix}{dir}file{i:02}.txt"); + client + .put_object() + .bucket(bucket) + .key(&key) + .body(ByteStream::from_static(b"x")) + .send() + .await + .expect("Failed to put object"); + expected_keys.push(key); + } + } + // Add 2 direct objects under prefix + for name in ["readme.txt", "config.json"] { + let key = format!("{prefix}{name}"); + client + .put_object() + .bucket(bucket) + .key(&key) + .body(ByteStream::from_static(b"x")) + .send() + .await + .expect("Failed to put object"); + expected_keys.push(key); + } + + // List with delimiter="/", max_keys large enough to cover all visible results + // Visible: 3 common prefixes + 2 direct objects = 5 + let output = client + .list_objects_v2() + .bucket(bucket) + .prefix(prefix) + .delimiter("/") + .max_keys(1000) + .send() + .await + .expect("Failed to list objects"); + + let listed_keys: Vec = output + .contents() + .iter() + .filter_map(|obj| obj.key()) + .map(|k| k.to_string()) + .collect(); + let listed_prefixes: Vec = output + .common_prefixes() + .iter() + .filter_map(|cp| cp.prefix()) + .map(|p| p.to_string()) + .collect(); + + // KEY ASSERTION: IsTruncated must be false because all visible results fit within max_keys + let is_truncated = output.is_truncated().unwrap_or(false); + assert!( + !is_truncated, + "BUG: IsTruncated should be false when all visible results ({} objects + {} prefixes = {}) fit within max_keys (1000)", + listed_keys.len(), + listed_prefixes.len(), + listed_keys.len() + listed_prefixes.len() + ); + + assert!( + output.next_continuation_token().is_none(), + "NextContinuationToken should be None when IsTruncated is false" + ); + + // All 32 objects must be covered (listed_keys + keys under listed_prefixes) + let total_raw_count = expected_keys.len(); + let keys_under_prefixes: usize = listed_prefixes + .iter() + .map(|p| { + let dir = p.trim_end_matches('/'); + expected_keys.iter().filter(|k| k.starts_with(&format!("{dir}/"))).count() + }) + .sum(); + let covered = listed_keys.len() + keys_under_prefixes; + + assert_eq!( + covered, + total_raw_count, + "Collapsed-prefix listing must cover all {} objects, got {} (keys={}, under_prefixes={})", + total_raw_count, + covered, + listed_keys.len(), + keys_under_prefixes + ); + + info!( + "Collapsed-prefix test passed: {} objects covered ({} keys + {} prefixes), IsTruncated=false", + covered, + listed_keys.len(), + listed_prefixes.len() + ); + + env.stop_server(); + } + + /// Regression test: delimiter listing pagination must traverse all keys + /// even when the visible result count per page is much smaller than the + /// raw entry count (due to CommonPrefix collapse). + /// + /// Scenario: 1000 objects across 100 directories, paginated with max_keys=50. + /// Each page returns up to 50 CommonPrefixes. The server must correctly set + /// IsTruncated and provide a valid continuation token across all pages. + #[tokio::test] + #[serial] + async fn test_list_objects_v2_delimiter_small_page_traverses_all() { + init_logging(); + info!("Starting test: ListObjectsV2 delimiter small page traverses all keys"); + + let mut env = RustFSTestEnvironment::new().await.expect("Failed to create test environment"); + env.start_rustfs_server(vec![]).await.expect("Failed to start RustFS"); + + let client = create_s3_client(&env); + let bucket = "test-delimiter-small-page"; + + create_bucket(&client, bucket).await.expect("Failed to create bucket"); + + // Create 1000 objects: 100 directories with 10 files each + let mut all_keys = Vec::new(); + let dirs: Vec = (0..100).map(|i| format!("dir-{:03}/", i)).collect(); + for dir in &dirs { + for i in 0..10 { + let key = format!("{dir}file{:02}.txt", i); + client + .put_object() + .bucket(bucket) + .key(&key) + .body(ByteStream::from_static(b"x")) + .send() + .await + .expect("Failed to put object"); + all_keys.push(key); + } + } + + // Paginate with delimiter="/" and max_keys=50 + // Visible per page: up to 50 CommonPrefixes + let mut listed_keys = Vec::new(); + let mut listed_prefixes = Vec::new(); + let mut continuation_token: Option = None; + let mut page_count = 0; + let mut last_page_is_truncated: bool; + + loop { + let mut request = client.list_objects_v2().bucket(bucket).delimiter("/").max_keys(50); + + if let Some(token) = continuation_token.take() { + request = request.continuation_token(token); + } + + let output = request.send().await.expect("Failed to list objects"); + last_page_is_truncated = output.is_truncated().unwrap_or(false); + + for obj in output.contents() { + if let Some(key) = obj.key() { + listed_keys.push(key.to_string()); + } + } + for cp in output.common_prefixes() { + if let Some(p) = cp.prefix() { + listed_prefixes.push(p.to_string()); + } + } + + page_count += 1; + + if last_page_is_truncated { + continuation_token = output.next_continuation_token().map(|s| s.to_string()); + assert!( + continuation_token.is_some(), + "BUG: NextContinuationToken must be present when IsTruncated is true" + ); + } else { + break; + } + + if page_count > 20 { + panic!("Too many pages, possible infinite loop in delimiter pagination"); + } + } + + // Last page must have IsTruncated=false + assert!( + !last_page_is_truncated, + "BUG: Last page must have IsTruncated=false after all results returned" + ); + + // Verify all objects are covered via listed prefixes + let keys_under_prefixes: HashSet = all_keys + .iter() + .filter(|k| { + listed_prefixes + .iter() + .any(|p| k.starts_with(&format!("{}/", p.trim_end_matches('/')))) + }) + .cloned() + .collect(); + let listed_set: HashSet = listed_keys.iter().cloned().collect(); + let covered: HashSet = listed_set.union(&keys_under_prefixes).cloned().collect(); + let expected_set: HashSet = all_keys.iter().cloned().collect(); + + assert_eq!( + covered, + expected_set, + "Delimiter pagination must cover all {} objects, missing: {:?}", + expected_set.difference(&covered).count(), + expected_set.difference(&covered) + ); + + info!( + "Delimiter small-page test passed: {} objects covered in {} pages", + covered.len(), + page_count + ); + + env.stop_server(); + } + + /// Regression test: raw entries exceed MaxKeys but all collapse into fewer + /// visible CommonPrefixes — IsTruncated must be false because there are no + /// more visible results beyond the collapsed prefixes. + /// + /// Scenario: 10 directories with 200 files each = 2000 raw keys. + /// With MaxKeys=1000, the raw entry count exceeds MaxKeys (disk_has_more=true), + /// but after delimiter collapse only 10 CommonPrefixes are visible (10 < 1000). + /// IsTruncated must be false since there are no additional visible results. + #[tokio::test] + #[serial] + async fn test_list_objects_v2_raw_exceeds_maxkeys_but_visible_below() { + init_logging(); + info!("Starting test: ListObjectsV2 raw > MaxKeys but visible < MaxKeys after collapse"); + + let mut env = RustFSTestEnvironment::new().await.expect("Failed to create test environment"); + env.start_rustfs_server(vec![]).await.expect("Failed to start RustFS"); + + let client = create_s3_client(&env); + let bucket = "test-raw-exceeds-visible-below"; + + create_bucket(&client, bucket).await.expect("Failed to create bucket"); + + // Create 10 directories × 200 files = 2000 raw keys + let dir_count = 10; + let files_per_dir = 200; + let mut all_keys = Vec::new(); + for d in 0..dir_count { + let dir = format!("dir-{:02}/", d); + for f in 0..files_per_dir { + let key = format!("{}file{:03}.txt", dir, f); + client + .put_object() + .bucket(bucket) + .key(&key) + .body(ByteStream::from_static(b"x")) + .send() + .await + .expect("Failed to put object"); + all_keys.push(key); + } + } + + // List with delimiter="/", max_keys=1000 + // Visible: 10 CommonPrefixes (dir-00/ .. dir-09/) = 10 visible << 1000 MaxKeys + // But raw entry count (2000+1001 requested) exceeds MaxKeys + let output = client + .list_objects_v2() + .bucket(bucket) + .delimiter("/") + .max_keys(1000) + .send() + .await + .expect("Failed to list objects"); + + let listed_keys: Vec = output + .contents() + .iter() + .filter_map(|obj| obj.key()) + .map(|k| k.to_string()) + .collect(); + let listed_prefixes: Vec = output + .common_prefixes() + .iter() + .filter_map(|cp| cp.prefix()) + .map(|p| p.to_string()) + .collect(); + + // KEY ASSERTION: IsTruncated must be false because visible results (10) + // are far below MaxKeys (1000), even though raw entries exceeded MaxKeys. + let is_truncated = output.is_truncated().unwrap_or(false); + assert!( + !is_truncated, + "BUG: IsTruncated should be false when visible results ({} objects + {} prefixes = {}) < MaxKeys (1000), even if raw entries exceeded MaxKeys", + listed_keys.len(), + listed_prefixes.len(), + listed_keys.len() + listed_prefixes.len() + ); + + assert!( + output.next_continuation_token().is_none(), + "NextContinuationToken should be None when IsTruncated is false" + ); + + // All objects must be covered by listed prefixes + assert_eq!( + listed_prefixes.len(), + dir_count, + "Expected {} prefixes, got {}", + dir_count, + listed_prefixes.len() + ); + for d in 0..dir_count { + let prefix = format!("dir-{:02}/", d); + assert!(listed_prefixes.contains(&prefix), "Missing prefix: {}", prefix); + } + + info!( + "Raw-exceeds-visible test passed: {} raw keys → {} prefixes, IsTruncated=false", + all_keys.len(), + listed_prefixes.len() + ); + + env.stop_server(); + } + + /// Regression test: pagination with MaxKeys exceeding S3 limit (1000) and delimiter. + /// The server caps MaxKeys to 1000. With delimiter="/", many raw keys collapse into + /// few CommonPrefixes. Visible results (prefixes) < capped MaxKeys → IsTruncated=false. + /// + /// This complements test_list_objects_v2_max_keys_above_limit_returns_token which + /// tests the non-delimiter case. + #[tokio::test] + #[serial] + async fn test_list_objects_v2_maxkeys_above_limit_with_delimiter() { + init_logging(); + info!("Starting test: ListObjectsV2 MaxKeys above limit with delimiter"); + + let mut env = RustFSTestEnvironment::new().await.expect("Failed to create test environment"); + env.start_rustfs_server(vec![]).await.expect("Failed to start RustFS"); + + let client = create_s3_client(&env); + let bucket = "test-maxkeys-limit-delimiter"; + + create_bucket(&client, bucket).await.expect("Failed to create bucket"); + + // 12 dirs × 100 files = 1200 raw keys + let dir_count = 12; + let files_per_dir = 100; + for d in 0..dir_count { + for f in 0..files_per_dir { + let key = format!("dir-{:02}/file{:03}.txt", d, f); + client + .put_object() + .bucket(bucket) + .key(&key) + .body(ByteStream::from_static(b"x")) + .send() + .await + .expect("Failed to put object"); + } + } + + // With delimiter: 12 CommonPrefixes visible, all fit within capped 1000 + let output = client + .list_objects_v2() + .bucket(bucket) + .delimiter("/") + .max_keys(2000) + .send() + .await + .expect("Failed to list objects"); + + assert_eq!( + output.common_prefixes().len(), + dir_count, + "Expected {} prefixes, got {}", + dir_count, + output.common_prefixes().len() + ); + assert_eq!(output.max_keys(), Some(1000)); + // 12 visible < 1000 capped MaxKeys → not truncated + assert!( + !output.is_truncated().unwrap_or(false), + "BUG: IsTruncated should be false when visible ({}) < capped MaxKeys (1000)", + output.common_prefixes().len() + ); + + info!("MaxKeys above limit with delimiter test passed"); + + env.stop_server(); + } } diff --git a/crates/ecstore/src/store_list_objects.rs b/crates/ecstore/src/store_list_objects.rs index 3f683f7fe..dd3d3b38a 100644 --- a/crates/ecstore/src/store_list_objects.rs +++ b/crates/ecstore/src/store_list_objects.rs @@ -340,6 +340,9 @@ impl ECStore { ..Default::default() }); + // err=None means gather_results filled its limit → disk has more data + let disk_has_more = list_result.err.is_none(); + if let Some(err) = list_result.err.take() && err != rustfs_filemeta::Error::Unexpected { @@ -371,7 +374,7 @@ impl ECStore { get_objects.truncate(max_keys as usize); } - let next_marker = { + let mut next_marker = { if is_truncated { get_objects.last().map(|last| last.name.clone()) } else { @@ -398,6 +401,27 @@ impl ECStore { } } + // After delimiter collapse, re-evaluate is_truncated based on visible results. + // No delimiter: reduction is from skipped entries → disk_has_more && non-empty. + // With delimiter: reduction may be from collapse → only when visible >= max_keys. + if !is_truncated && disk_has_more { + let visible_count = objects.len() + prefixes.len(); + let should_truncate = if delimiter.is_none() { + visible_count > 0 + } else { + visible_count >= max_keys as usize + }; + if should_truncate { + is_truncated = true; + // Compute next_marker from visible results since get_objects was consumed. + // Prefer last object name; fall back to last prefix for marker. + next_marker = objects + .last() + .map(|last| last.name.clone()) + .or_else(|| prefixes.last().cloned()); + } + } + Ok(ListObjectsInfo { is_truncated, next_marker, @@ -453,6 +477,9 @@ impl ECStore { ..Default::default() }); + // err=None means gather_results filled its limit → disk has more data + let disk_has_more = list_result.err.is_none(); + if let Some(err) = list_result.err.take() && err != rustfs_filemeta::Error::Unexpected { @@ -483,22 +510,13 @@ impl ECStore { get_objects.truncate(max_keys as usize); } - let (next_marker, next_version_idmarker) = { - if is_truncated { - get_objects - .last() - .map(|last| { - ( - Some(last.name.clone()), - // AWS S3 API returns "null" for non-versioned objects - Some(last.version_id.map(|v| v.to_string()).unwrap_or_else(|| "null".to_string())), - ) - }) - .unwrap_or_default() - } else { - (None, None) - } - }; + let mut next_marker: Option = None; + let mut next_version_idmarker: Option = None; + if is_truncated && let Some(last) = get_objects.last() { + next_marker = Some(last.name.clone()); + // AWS S3 API returns "null" for non-versioned objects + next_version_idmarker = Some(last.version_id.map(|v| v.to_string()).unwrap_or_else(|| "null".to_string())); + } let mut prefixes: Vec = Vec::new(); let mut prefix_set: HashSet = HashSet::new(); @@ -519,6 +537,30 @@ impl ECStore { } } + // After delimiter collapse, re-evaluate is_truncated based on visible results. + // Two distinct scenarios (see list_objects_generic for detailed rationale): + // 1. No delimiter: reduction from skipped entries → disk_has_more && non-empty + // 2. With delimiter: reduction from collapse → only when visible >= max_keys + if !is_truncated && disk_has_more { + let visible_count = objects.len() + prefixes.len(); + let should_truncate = if delimiter.is_none() { + visible_count > 0 + } else { + visible_count >= max_keys as usize + }; + if should_truncate { + is_truncated = true; + // Compute markers from visible results since get_objects was consumed. + if let Some(last) = objects.last() { + next_marker = Some(last.name.clone()); + next_version_idmarker = Some(last.version_id.map(|v| v.to_string()).unwrap_or_else(|| "null".to_string())); + } else if let Some(last_prefix) = prefixes.last().cloned() { + next_marker = Some(last_prefix); + next_version_idmarker = None; + } + } + } + Ok(ListObjectVersionsInfo { is_truncated, next_marker,