From aee8a4df516146532bbe433d0abfa6079a21f3e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=AE=89=E6=AD=A3=E8=B6=85?= Date: Thu, 9 Apr 2026 22:59:56 +0800 Subject: [PATCH] refactor(app): reuse list parts parsing (#2446) --- rustfs/src/app/multipart_usecase.rs | 56 ++++++++++++++++++++------ rustfs/src/storage/s3_api/multipart.rs | 7 ++++ 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/rustfs/src/app/multipart_usecase.rs b/rustfs/src/app/multipart_usecase.rs index 8daab532c..7de72bd2a 100644 --- a/rustfs/src/app/multipart_usecase.rs +++ b/rustfs/src/app/multipart_usecase.rs @@ -25,7 +25,7 @@ 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; +use crate::storage::s3_api::multipart::{build_list_parts_output, parse_list_parts_params}; use crate::storage::*; use bytes::Bytes; use futures::StreamExt; @@ -982,23 +982,21 @@ impl DefaultMultipartUsecase { .. } = req.input; + let params = parse_list_parts_params(part_number_marker, max_parts)?; + let Some(store) = new_object_layer_fn() else { return Err(S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string())); }; - let part_number_marker = part_number_marker.map(|x| x as usize); - let max_parts = match max_parts { - Some(parts) => { - if !(1..=1000).contains(&parts) { - return Err(s3_error!(InvalidArgument, "max-parts must be between 1 and 1000")); - } - parts as usize - } - None => 1000, - }; - let res = store - .list_object_parts(&bucket, &key, &upload_id, part_number_marker, max_parts, &ObjectOptions::default()) + .list_object_parts( + &bucket, + &key, + &upload_id, + params.part_number_marker, + params.max_parts, + &ObjectOptions::default(), + ) .await .map_err(ApiError::from)?; @@ -1472,6 +1470,38 @@ mod tests { assert_eq!(err.code(), &S3ErrorCode::InternalError); } + #[tokio::test] + async fn execute_list_parts_rejects_negative_part_number_marker_before_store_lookup() { + let input = ListPartsInput::builder() + .bucket("bucket".to_string()) + .key("object".to_string()) + .upload_id("upload-id".to_string()) + .part_number_marker(Some(-1)) + .build() + .unwrap(); + let req = build_request(input, Method::GET); + + let err = make_usecase().execute_list_parts(req).await.unwrap_err(); + assert_eq!(err.code(), &S3ErrorCode::InvalidArgument); + assert_eq!(err.message(), Some("part-number-marker must be non-negative")); + } + + #[tokio::test] + async fn execute_list_parts_rejects_invalid_max_parts_before_store_lookup() { + let input = ListPartsInput::builder() + .bucket("bucket".to_string()) + .key("object".to_string()) + .upload_id("upload-id".to_string()) + .max_parts(Some(1001)) + .build() + .unwrap(); + let req = build_request(input, Method::GET); + + let err = make_usecase().execute_list_parts(req).await.unwrap_err(); + assert_eq!(err.code(), &S3ErrorCode::InvalidArgument); + assert_eq!(err.message(), Some("max-parts must be between 1 and 1000")); + } + #[tokio::test] async fn execute_upload_part_copy_returns_internal_error_when_store_uninitialized() { let input = UploadPartCopyInput::builder() diff --git a/rustfs/src/storage/s3_api/multipart.rs b/rustfs/src/storage/s3_api/multipart.rs index 4f35de70f..b97a53fc4 100644 --- a/rustfs/src/storage/s3_api/multipart.rs +++ b/rustfs/src/storage/s3_api/multipart.rs @@ -319,6 +319,13 @@ mod tests { assert_eq!(*err.code(), S3ErrorCode::InvalidArgument); } + #[test] + fn test_parse_list_parts_params_rejects_negative_part_number_marker() { + let err = parse_list_parts_params(Some(-1), None).expect_err("expected invalid part_number_marker"); + assert_eq!(*err.code(), S3ErrorCode::InvalidArgument); + assert_eq!(err.message(), Some("part-number-marker must be non-negative")); + } + #[test] fn test_parse_list_multipart_uploads_params_defaults_and_valid_values() { let parsed =