-
Notifications
You must be signed in to change notification settings - Fork 71
BUG: list object v1 doesn't emit next marker properly #544
Copy link
Copy link
Open
Labels
Description
Bug Description
Hi team, IIRC, I think the current implementation doesn't emit next marker, when is_truncated is true
Lines 354 to 470 in 456ba82
| #[tracing::instrument] | |
| async fn list_objects(&self, req: S3Request<ListObjectsInput>) -> S3Result<S3Response<ListObjectsOutput>> { | |
| let v2_resp = self.list_objects_v2(req.map_input(Into::into)).await?; | |
| Ok(v2_resp.map_output(|v2| ListObjectsOutput { | |
| contents: v2.contents, | |
| common_prefixes: v2.common_prefixes, | |
| delimiter: v2.delimiter, | |
| encoding_type: v2.encoding_type, | |
| name: v2.name, | |
| prefix: v2.prefix, | |
| max_keys: v2.max_keys, | |
| is_truncated: v2.is_truncated, | |
| ..Default::default() | |
| })) | |
| } | |
| #[tracing::instrument] | |
| async fn list_objects_v2(&self, req: S3Request<ListObjectsV2Input>) -> S3Result<S3Response<ListObjectsV2Output>> { | |
| let input = req.input; | |
| let path = self.get_bucket_path(&input.bucket)?; | |
| if path.exists().not() { | |
| return Err(s3_error!(NoSuchBucket)); | |
| } | |
| let delimiter = input.delimiter.as_deref(); | |
| let prefix = input.prefix.as_deref().unwrap_or("").trim_start_matches('/'); | |
| let max_keys = input.max_keys.unwrap_or(1000); | |
| // Collect all matching objects and common prefixes | |
| let mut objects: Vec<Object> = default(); | |
| let mut common_prefixes = std::collections::BTreeSet::new(); | |
| if let Some(delimiter) = delimiter { | |
| self.list_objects_with_delimiter(&path, prefix, delimiter, &mut objects, &mut common_prefixes) | |
| .await?; | |
| } else { | |
| self.list_objects_recursive(&path, prefix, &mut objects).await?; | |
| } | |
| // Sort before filtering and limiting | |
| objects.sort_by(|lhs, rhs| { | |
| let lhs_key = lhs.key.as_deref().unwrap_or(""); | |
| let rhs_key = rhs.key.as_deref().unwrap_or(""); | |
| lhs_key.cmp(rhs_key) | |
| }); | |
| // Apply start_after filter if provided | |
| if let Some(marker) = &input.start_after { | |
| objects.retain(|obj| obj.key.as_deref().unwrap_or("") > marker.as_str()); | |
| } | |
| // Convert common_prefixes to sorted list | |
| let common_prefixes_list: Vec<CommonPrefix> = common_prefixes | |
| .into_iter() | |
| .map(|prefix| CommonPrefix { prefix: Some(prefix) }) | |
| .collect(); | |
| // Limit results to max_keys by interleaving objects and common_prefixes | |
| let mut result_objects = Vec::new(); | |
| let mut result_prefixes = Vec::new(); | |
| let mut total_count = 0; | |
| let max_keys_usize = usize::try_from(max_keys).unwrap_or(1000); | |
| let mut obj_idx = 0; | |
| let mut prefix_idx = 0; | |
| while total_count < max_keys_usize { | |
| let obj_key = objects.get(obj_idx).and_then(|o| o.key.as_deref()); | |
| let prefix_key = common_prefixes_list.get(prefix_idx).and_then(|p| p.prefix.as_deref()); | |
| match (obj_key, prefix_key) { | |
| (Some(ok), Some(pk)) => { | |
| if ok < pk { | |
| result_objects.push(objects[obj_idx].clone()); | |
| obj_idx += 1; | |
| } else { | |
| result_prefixes.push(common_prefixes_list[prefix_idx].clone()); | |
| prefix_idx += 1; | |
| } | |
| total_count += 1; | |
| } | |
| (Some(_), None) => { | |
| result_objects.push(objects[obj_idx].clone()); | |
| obj_idx += 1; | |
| total_count += 1; | |
| } | |
| (None, Some(_)) => { | |
| result_prefixes.push(common_prefixes_list[prefix_idx].clone()); | |
| prefix_idx += 1; | |
| total_count += 1; | |
| } | |
| (None, None) => break, | |
| } | |
| } | |
| let is_truncated = obj_idx < objects.len() || prefix_idx < common_prefixes_list.len(); | |
| let key_count = try_!(i32::try_from(total_count)); | |
| let contents = result_objects.is_empty().not().then_some(result_objects); | |
| let common_prefixes = result_prefixes.is_empty().not().then_some(result_prefixes); | |
| let output = ListObjectsV2Output { | |
| key_count: Some(key_count), | |
| max_keys: Some(max_keys), | |
| is_truncated: Some(is_truncated), | |
| contents, | |
| common_prefixes, | |
| delimiter: input.delimiter, | |
| encoding_type: input.encoding_type, | |
| name: Some(input.bucket), | |
| prefix: input.prefix, | |
| ..Default::default() | |
| }; | |
| Ok(S3Response::new(output)) | |
| } |
Steps to Reproduce
No response
Expected Behavior
#[tokio::test]
#[tracing::instrument]
async fn test_list_objects_v1_next_marker_with_delimiter() -> Result<()> {
let _guard = serial().await;
let c = Client::new(config());
let bucket = format!("test-v1-next-marker-{}", Uuid::new_v4());
let bucket = bucket.as_str();
create_bucket(&c, bucket).await?;
// Objects: a.txt, dir/1.txt (→ common prefix "dir/"), z.txt
// With delimiter="/", max_keys=2 → page 1 = [a.txt, dir/], truncated
let keys = ["a.txt", "dir/1.txt", "z.txt"];
for key in &keys {
c.put_object()
.bucket(bucket)
.key(*key)
.body(ByteStream::from_static(b"x"))
.send()
.await?;
}
let page = c
.list_objects()
.bucket(bucket)
.delimiter("/")
.max_keys(2)
.send()
.await?;
assert_eq!(page.is_truncated(), Some(true));
// Per S3 spec: when delimiter is set and is_truncated is true,
// next_marker must be returned so the client can paginate.
assert!(
page.next_marker().is_some(),
"is_truncated is true with delimiter but next_marker is missing"
);
// Cleanup
for key in &keys {
delete_object(&c, bucket, key).await?;
}
delete_bucket(&c, bucket).await?;
Ok(())
}Error message
failures:
---- test_list_objects_v1_next_marker_with_delimiter stdout ----
thread 'test_list_objects_v1_next_marker_with_delimiter' (17687239) panicked at crates/s3s-fs/tests/it_aws.rs:1438:5:
is_truncated is true with delimiter but next_marker is missing
note: run with RUST_BACKTRACE=1 environment variable to display a backtraceEnvironment
No response
Reactions are currently unavailable