diff --git a/rustfs/src/app/multipart_usecase.rs b/rustfs/src/app/multipart_usecase.rs index 7de72bd2a..4c868bad9 100644 --- a/rustfs/src/app/multipart_usecase.rs +++ b/rustfs/src/app/multipart_usecase.rs @@ -25,7 +25,10 @@ use crate::storage::options::{ copy_src_opts, extract_metadata, get_complete_multipart_upload_opts, get_content_sha256_with_query, get_opts, parse_copy_source_range, put_opts, validate_archive_content_encoding, }; -use crate::storage::s3_api::multipart::{build_list_parts_output, parse_list_parts_params}; +use crate::storage::s3_api::multipart::{ + ListMultipartUploadsParams, build_list_multipart_uploads_output, build_list_parts_output, + parse_list_multipart_uploads_params, parse_list_parts_params, +}; use crate::storage::*; use bytes::Bytes; use futures::StreamExt; @@ -42,7 +45,7 @@ use rustfs_ecstore::client::object_api_utils::to_s3s_etag; use rustfs_ecstore::compress::is_compressible; use rustfs_ecstore::error::{StorageError, is_err_object_not_found, is_err_version_not_found}; use rustfs_ecstore::new_object_layer_fn; -use rustfs_ecstore::set_disk::{MAX_PARTS_COUNT, is_valid_storage_class}; +use rustfs_ecstore::set_disk::is_valid_storage_class; use rustfs_ecstore::store_api::{ ChunkNativePutData, CompletePart, HTTPRangeSpec, MultipartUploadResult, ObjectIO, ObjectOptions, }; @@ -917,55 +920,22 @@ impl DefaultMultipartUsecase { .. } = req.input; + let ListMultipartUploadsParams { + prefix, + key_marker, + max_uploads, + } = parse_list_multipart_uploads_params(prefix, key_marker, max_uploads)?; + let Some(store) = new_object_layer_fn() else { return Err(S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string())); }; - let prefix = prefix.unwrap_or_default(); - let max_uploads = max_uploads.map(|x| x as usize).unwrap_or(MAX_PARTS_COUNT); - - if let Some(key_marker) = &key_marker - && !key_marker.starts_with(prefix.as_str()) - { - return Err(s3_error!(NotImplemented, "Invalid key marker")); - } - let result = store .list_multipart_uploads(&bucket, &prefix, delimiter, key_marker, upload_id_marker, max_uploads) .await .map_err(ApiError::from)?; - let output = ListMultipartUploadsOutput { - bucket: Some(bucket), - prefix: Some(prefix), - delimiter: result.delimiter, - key_marker: result.key_marker, - upload_id_marker: result.upload_id_marker, - max_uploads: Some(result.max_uploads as i32), - is_truncated: Some(result.is_truncated), - uploads: Some( - result - .uploads - .into_iter() - .map(|u| MultipartUpload { - key: Some(u.object), - upload_id: Some(u.upload_id), - initiated: u.initiated.map(Timestamp::from), - ..Default::default() - }) - .collect(), - ), - common_prefixes: Some( - result - .common_prefixes - .into_iter() - .map(|c| CommonPrefix { prefix: Some(c) }) - .collect(), - ), - ..Default::default() - }; - - Ok(S3Response::new(output)) + Ok(S3Response::new(build_list_multipart_uploads_output(bucket, prefix, result))) } pub async fn execute_list_parts(&self, req: S3Request) -> S3Result> { @@ -1456,6 +1426,36 @@ mod tests { assert_eq!(err.code(), &S3ErrorCode::InternalError); } + #[tokio::test] + async fn execute_list_multipart_uploads_rejects_invalid_key_marker_before_store_lookup() { + let input = ListMultipartUploadsInput::builder() + .bucket("bucket".to_string()) + .prefix(Some("prefix/".to_string())) + .key_marker(Some("other/key".to_string())) + .build() + .unwrap(); + let req = build_request(input, Method::GET); + + let err = make_usecase().execute_list_multipart_uploads(req).await.unwrap_err(); + assert_eq!(err.code(), &S3ErrorCode::NotImplemented); + assert_eq!(err.message(), Some("Invalid key marker")); + } + + #[tokio::test] + async fn execute_list_multipart_uploads_rejects_invalid_max_uploads_before_store_lookup() { + let input = ListMultipartUploadsInput::builder() + .bucket("bucket".to_string()) + .max_uploads(Some(0)) + .build() + .unwrap(); + let req = build_request(input, Method::GET); + let expected = "max-uploads must be between 1 and 1000"; + + let err = make_usecase().execute_list_multipart_uploads(req).await.unwrap_err(); + assert_eq!(err.code(), &S3ErrorCode::InvalidArgument); + assert_eq!(err.message(), Some(expected)); + } + #[tokio::test] async fn execute_list_parts_returns_internal_error_when_store_uninitialized() { let input = ListPartsInput::builder() diff --git a/rustfs/src/storage/s3_api/multipart.rs b/rustfs/src/storage/s3_api/multipart.rs index b97a53fc4..ea7ea3f06 100644 --- a/rustfs/src/storage/s3_api/multipart.rs +++ b/rustfs/src/storage/s3_api/multipart.rs @@ -14,11 +14,12 @@ use crate::storage::s3_api::common::{rustfs_initiator, rustfs_owner}; use rustfs_ecstore::client::object_api_utils::to_s3s_etag; -use rustfs_ecstore::set_disk::MAX_PARTS_COUNT; use rustfs_ecstore::store_api::{ListMultipartsInfo, ListPartsInfo}; use s3s::dto::{CommonPrefix, ListMultipartUploadsOutput, ListPartsOutput, MultipartUpload, Part, Timestamp}; use s3s::{S3Error, S3ErrorCode}; +const MAX_MULTIPART_UPLOADS_LIST: i32 = 1000; + #[derive(Debug, PartialEq, Eq)] pub(crate) struct ListPartsParams { pub part_number_marker: Option, @@ -110,23 +111,16 @@ pub(crate) fn parse_list_multipart_uploads_params( let prefix = prefix.unwrap_or_default(); let max_uploads = match max_uploads { Some(value) => { - let value = usize::try_from(value).map_err(|_| { - S3Error::with_message( - S3ErrorCode::InvalidArgument, - format!("max-uploads must be between 1 and {}", MAX_PARTS_COUNT), - ) - })?; - - if value == 0 || value > MAX_PARTS_COUNT { + if !(1..=MAX_MULTIPART_UPLOADS_LIST).contains(&value) { return Err(S3Error::with_message( S3ErrorCode::InvalidArgument, - format!("max-uploads must be between 1 and {}", MAX_PARTS_COUNT), + format!("max-uploads must be between 1 and {}", MAX_MULTIPART_UPLOADS_LIST), )); } - value + value as usize } - None => MAX_PARTS_COUNT, + None => MAX_MULTIPART_UPLOADS_LIST as usize, }; if let Some(key_marker) = &key_marker @@ -181,12 +175,11 @@ pub(crate) fn build_list_multipart_uploads_output( #[cfg(test)] mod tests { use super::{ - build_list_multipart_uploads_output, build_list_parts_output, parse_list_multipart_uploads_params, - parse_list_parts_params, + MAX_MULTIPART_UPLOADS_LIST, build_list_multipart_uploads_output, build_list_parts_output, + parse_list_multipart_uploads_params, parse_list_parts_params, }; use crate::storage::s3_api::common::{rustfs_initiator, rustfs_owner}; use rustfs_ecstore::client::object_api_utils::to_s3s_etag; - use rustfs_ecstore::set_disk::MAX_PARTS_COUNT; use rustfs_ecstore::store_api::{ListMultipartsInfo, ListPartsInfo, MultipartInfo, PartInfo}; use s3s::S3ErrorCode; use s3s::dto::Timestamp; @@ -338,7 +331,7 @@ mod tests { let parsed = parse_list_multipart_uploads_params(None, None, None).expect("expected default params"); assert_eq!(parsed.prefix, ""); assert_eq!(parsed.key_marker, None); - assert_eq!(parsed.max_uploads, MAX_PARTS_COUNT); + assert_eq!(parsed.max_uploads, MAX_MULTIPART_UPLOADS_LIST as usize); } #[test] @@ -360,7 +353,7 @@ mod tests { .expect_err("expected invalid max_uploads"); assert_eq!(*err.code(), S3ErrorCode::InvalidArgument); - let err = parse_list_multipart_uploads_params(Some("prefix/".to_string()), None, Some((MAX_PARTS_COUNT + 1) as i32)) + let err = parse_list_multipart_uploads_params(Some("prefix/".to_string()), None, Some(MAX_MULTIPART_UPLOADS_LIST + 1)) .expect_err("expected invalid max_uploads"); assert_eq!(*err.code(), S3ErrorCode::InvalidArgument); }