From 2debc14e4df0f660e765a8a91f254321b869c73b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=AE=89=E6=AD=A3=E8=B6=85?= Date: Sun, 15 Feb 2026 12:55:07 +0800 Subject: [PATCH] refactor(storage): extract ListObjectsV2 parameter parsing (#1831) --- rustfs/src/storage/ecfs.rs | 66 ++++----------- rustfs/src/storage/s3_api/bucket.rs | 119 +++++++++++++++++++++++++++- 2 files changed, 135 insertions(+), 50 deletions(-) diff --git a/rustfs/src/storage/ecfs.rs b/rustfs/src/storage/ecfs.rs index 4ad210768..4aeaa2401 100644 --- a/rustfs/src/storage/ecfs.rs +++ b/rustfs/src/storage/ecfs.rs @@ -24,7 +24,8 @@ use crate::storage::helper::OperationHelper; use crate::storage::options::{filter_object_metadata, get_content_sha256}; use crate::storage::readers::InMemoryAsyncReader; use crate::storage::s3_api::bucket::{ - build_list_object_versions_output, build_list_objects_output, build_list_objects_v2_output, parse_list_object_versions_params, + build_list_object_versions_output, build_list_objects_output, build_list_objects_v2_output, + parse_list_object_versions_params, parse_list_objects_v2_params, }; use crate::storage::s3_api::common::rustfs_owner; use crate::storage::s3_api::multipart::{ @@ -3646,44 +3647,11 @@ impl S3 for FS { .. } = req.input; - let prefix = prefix.unwrap_or_default(); - - // Log debug info for prefixes with special characters to help diagnose encoding issues - if prefix.contains([' ', '+', '%', '\n', '\r', '\0']) { - debug!("LIST objects with special characters in prefix: {:?}", prefix); - } - - let max_keys = max_keys.unwrap_or(1000); - if max_keys < 0 { - return Err(S3Error::with_message(S3ErrorCode::InvalidArgument, "Invalid max keys".to_string())); - } - - let delimiter = delimiter.filter(|v| !v.is_empty()); - - validate_list_object_unordered_with_delimiter(delimiter.as_ref(), req.uri.query())?; - - // Save original start_after for response (per S3 API spec, must echo back if provided) - let response_start_after = start_after.clone(); - let start_after_for_query = start_after.filter(|v| !v.is_empty()); - - // Save original continuation_token for response (per S3 API spec, must echo back if provided) - // Note: empty string should still be echoed back in the response - let response_continuation_token = continuation_token.clone(); - let continuation_token_for_query = continuation_token.filter(|v| !v.is_empty()); - - // Decode continuation_token from base64 for internal use - let decoded_continuation_token = continuation_token_for_query - .map(|token| { - base64_simd::STANDARD - .decode_to_vec(token.as_bytes()) - .map_err(|_| s3_error!(InvalidArgument, "Invalid continuation token")) - .and_then(|bytes| { - String::from_utf8(bytes).map_err(|_| s3_error!(InvalidArgument, "Invalid continuation token")) - }) - }) - .transpose()?; + let parsed = parse_list_objects_v2_params(prefix, delimiter, max_keys, continuation_token, start_after)?; + validate_list_object_unordered_with_delimiter(parsed.delimiter.as_ref(), req.uri.query())?; let store = get_validated_store(&bucket).await?; + let fetch_owner = fetch_owner.unwrap_or_default(); let incl_deleted = req .headers @@ -3693,12 +3661,12 @@ impl S3 for FS { let object_infos = store .list_objects_v2( &bucket, - &prefix, - decoded_continuation_token, - delimiter.clone(), - max_keys, - fetch_owner.unwrap_or_default(), - start_after_for_query, + &parsed.prefix, + parsed.decoded_continuation_token, + parsed.delimiter.clone(), + parsed.max_keys, + fetch_owner, + parsed.start_after_for_query, incl_deleted, ) .await @@ -3706,14 +3674,14 @@ impl S3 for FS { let output = build_list_objects_v2_output( object_infos, - fetch_owner.unwrap_or_default(), - max_keys, + fetch_owner, + parsed.max_keys, bucket, - prefix, - delimiter, + parsed.prefix, + parsed.delimiter, encoding_type, - response_continuation_token, - response_start_after, + parsed.response_continuation_token, + parsed.response_start_after, ); Ok(s3_response(output)) diff --git a/rustfs/src/storage/s3_api/bucket.rs b/rustfs/src/storage/s3_api/bucket.rs index e9bcf86f5..a825dfbb8 100644 --- a/rustfs/src/storage/s3_api/bucket.rs +++ b/rustfs/src/storage/s3_api/bucket.rs @@ -19,9 +19,20 @@ use s3s::dto::{ ObjectStorageClass, ObjectVersion, ObjectVersionStorageClass, Owner, Timestamp, }; use s3s::{S3Error, S3ErrorCode}; +use tracing::debug; use urlencoding::encode; pub(crate) type ListObjectVersionsParams = (String, Option, Option, Option, i32); +#[derive(Debug)] +pub(crate) struct ListObjectsV2Params { + pub prefix: String, + pub max_keys: i32, + pub delimiter: Option, + pub response_start_after: Option, + pub start_after_for_query: Option, + pub response_continuation_token: Option, + pub decoded_continuation_token: Option, +} pub(crate) fn build_list_objects_output(v2: ListObjectsV2Output, request_marker: Option) -> ListObjectsOutput { let next_marker = calculate_next_marker(&v2); @@ -64,6 +75,61 @@ pub(crate) fn parse_list_object_versions_params( Ok((prefix, delimiter, key_marker, version_id_marker, max_keys)) } +pub(crate) fn parse_list_objects_v2_params( + prefix: Option, + delimiter: Option, + max_keys: Option, + continuation_token: Option, + start_after: Option, +) -> Result { + let prefix = prefix.unwrap_or_default(); + + // Log debug info for prefixes with special characters to help diagnose encoding issues + if prefix.contains([' ', '+', '%', '\n', '\r', '\0']) { + debug!("LIST objects with special characters in prefix: {:?}", prefix); + } + + let max_keys = max_keys.unwrap_or(1000); + if max_keys < 0 { + return Err(S3Error::with_message(S3ErrorCode::InvalidArgument, "Invalid max keys".to_string())); + } + + let delimiter = delimiter.filter(|v| !v.is_empty()); + + // Save original start_after for response (per S3 API spec, must echo back if provided) + let response_start_after = start_after.clone(); + let start_after_for_query = start_after.filter(|v| !v.is_empty()); + + // Save original continuation_token for response (per S3 API spec, must echo back if provided) + // Note: empty string should still be echoed back in the response + let response_continuation_token = continuation_token.clone(); + let continuation_token_for_query = continuation_token.filter(|v| !v.is_empty()); + + // Decode continuation_token from base64 for internal use + let decoded_continuation_token = continuation_token_for_query + .map(|token| { + base64_simd::STANDARD + .decode_to_vec(token.as_bytes()) + .map_err(|_| S3Error::with_message(S3ErrorCode::InvalidArgument, "Invalid continuation token".to_string())) + .and_then(|bytes| { + String::from_utf8(bytes).map_err(|_| { + S3Error::with_message(S3ErrorCode::InvalidArgument, "Invalid continuation token".to_string()) + }) + }) + }) + .transpose()?; + + Ok(ListObjectsV2Params { + prefix, + max_keys, + delimiter, + response_start_after, + start_after_for_query, + response_continuation_token, + decoded_continuation_token, + }) +} + pub(crate) fn build_list_object_versions_output( object_infos: ListObjectVersionsInfo, bucket: String, @@ -254,7 +320,7 @@ fn calculate_next_marker(v2: &ListObjectsV2Output) -> Option { mod tests { use super::{ build_list_object_versions_output, build_list_objects_output, build_list_objects_v2_output, - parse_list_object_versions_params, + parse_list_object_versions_params, parse_list_objects_v2_params, }; use rustfs_ecstore::store_api::{ListObjectVersionsInfo, ListObjectsV2Info, ObjectInfo}; use s3s::S3ErrorCode; @@ -408,6 +474,57 @@ mod tests { assert_eq!(*err.code(), S3ErrorCode::InvalidArgument); } + #[test] + fn test_parse_list_objects_v2_params_defaults_and_echo_behavior() { + let parsed = parse_list_objects_v2_params(None, Some(String::new()), None, Some(String::new()), Some(String::new())) + .expect("parse should succeed"); + + assert_eq!(parsed.prefix, String::new()); + assert_eq!(parsed.max_keys, 1000); + assert_eq!(parsed.delimiter, None); + assert_eq!(parsed.response_start_after, Some(String::new())); + assert_eq!(parsed.start_after_for_query, None); + assert_eq!(parsed.response_continuation_token, Some(String::new())); + assert_eq!(parsed.decoded_continuation_token, None); + } + + #[test] + fn test_parse_list_objects_v2_params_rejects_negative_max_keys() { + let err = + parse_list_objects_v2_params(None, None, Some(-1), None, None).expect_err("negative max_keys should be rejected"); + + assert_eq!(*err.code(), S3ErrorCode::InvalidArgument); + } + + #[test] + fn test_parse_list_objects_v2_params_decodes_continuation_token() { + let raw = "token-123"; + let encoded = base64_simd::STANDARD.encode_to_string(raw.as_bytes()); + + let parsed = + parse_list_objects_v2_params(None, None, Some(1000), Some(encoded.clone()), None).expect("parse should succeed"); + + assert_eq!(parsed.response_continuation_token, Some(encoded)); + assert_eq!(parsed.decoded_continuation_token, Some(raw.to_string())); + } + + #[test] + fn test_parse_list_objects_v2_params_rejects_invalid_continuation_token() { + let err = parse_list_objects_v2_params(None, None, Some(1000), Some("%%%".to_string()), None) + .expect_err("invalid base64 token should be rejected"); + + assert_eq!(*err.code(), S3ErrorCode::InvalidArgument); + } + + #[test] + fn test_parse_list_objects_v2_params_rejects_non_utf8_continuation_token() { + let invalid_utf8 = base64_simd::STANDARD.encode_to_string([0xff, 0xfe, 0xfd]); + let err = parse_list_objects_v2_params(None, None, Some(1000), Some(invalid_utf8), None) + .expect_err("non-utf8 token should be rejected"); + + assert_eq!(*err.code(), S3ErrorCode::InvalidArgument); + } + #[test] fn test_build_list_object_versions_output_maps_versions_delete_markers_and_markers() { let object_infos = ListObjectVersionsInfo {