fix(s3): encode multipart listing response keys (#8347)

This commit is contained in:
Chris
2026-10-04 05:24:26 -07:00
committed by GitHub
parent 00a549504c
commit 182b2bfe98
3 changed files with 89 additions and 10 deletions
+7 -1
View File
@@ -1523,6 +1523,7 @@ impl DefaultMultipartUsecase {
key_marker,
upload_id_marker,
max_uploads,
encoding_type,
..
} = req.input;
@@ -1575,7 +1576,12 @@ impl DefaultMultipartUsecase {
.map_err(ApiError::from)?,
};
Ok(S3Response::new(build_list_multipart_uploads_output(bucket, prefix, result)))
Ok(S3Response::new(build_list_multipart_uploads_output(
bucket,
prefix,
result,
encoding_type.as_ref(),
)))
}
pub async fn execute_list_parts(&self, req: S3Request<ListPartsInput>) -> S3Result<S3Response<ListPartsOutput>> {
+1 -1
View File
@@ -52,7 +52,7 @@ fn encode_s3_name(name: &str) -> String {
.join("/")
}
fn encode_list_output_value(value: String, encoding_type: Option<&EncodingType>) -> String {
pub(super) fn encode_list_output_value(value: String, encoding_type: Option<&EncodingType>) -> String {
if should_encode_url(encoding_type) {
encode_s3_name(&value)
} else {
+81 -8
View File
@@ -12,13 +12,14 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use crate::storage::s3_api::bucket::encode_list_output_value;
use crate::storage::s3_api::common::{rustfs_initiator, rustfs_owner};
use crate::storage::storage_api::effective_storage_class;
use crate::storage::storage_api::s3_api_consumer::multipart::contract::multipart::{
ListMultipartsInfo, ListPartsInfo, MAX_MULTIPART_PART_NUMBER,
};
use crate::storage::storage_api::s3_api_consumer::multipart::to_s3s_etag;
use s3s::dto::{CommonPrefix, ListMultipartUploadsOutput, ListPartsOutput, MultipartUpload, Part, Timestamp};
use s3s::dto::{CommonPrefix, EncodingType, ListMultipartUploadsOutput, ListPartsOutput, MultipartUpload, Part, Timestamp};
use s3s::{S3Error, S3ErrorCode};
const MAX_MULTIPART_UPLOADS_LIST: i32 = 1000;
@@ -166,17 +167,20 @@ pub(crate) fn build_list_multipart_uploads_output(
bucket: String,
prefix: String,
result: ListMultipartsInfo,
encoding_type: Option<&EncodingType>,
) -> ListMultipartUploadsOutput {
let owner = rustfs_owner();
let initiator = rustfs_initiator();
ListMultipartUploadsOutput {
bucket: Some(bucket),
prefix: Some(prefix),
delimiter: result.delimiter,
key_marker: result.key_marker,
prefix: Some(encode_list_output_value(prefix, encoding_type)),
delimiter: result.delimiter.map(|value| encode_list_output_value(value, encoding_type)),
key_marker: result.key_marker.map(|value| encode_list_output_value(value, encoding_type)),
upload_id_marker: result.upload_id_marker,
next_key_marker: result.next_key_marker,
next_key_marker: result
.next_key_marker
.map(|value| encode_list_output_value(value, encoding_type)),
next_upload_id_marker: result.next_upload_id_marker,
max_uploads: Some(result.max_uploads as i32),
is_truncated: Some(result.is_truncated),
@@ -185,7 +189,7 @@ pub(crate) fn build_list_multipart_uploads_output(
.uploads
.into_iter()
.map(|u| MultipartUpload {
key: Some(u.object),
key: Some(encode_list_output_value(u.object, encoding_type)),
upload_id: Some(u.upload_id),
initiated: u.initiated.map(Timestamp::from),
owner: Some(owner.clone()),
@@ -198,9 +202,12 @@ pub(crate) fn build_list_multipart_uploads_output(
result
.common_prefixes
.into_iter()
.map(|c| CommonPrefix { prefix: Some(c) })
.map(|c| CommonPrefix {
prefix: Some(encode_list_output_value(c, encoding_type)),
})
.collect(),
),
encoding_type: encoding_type.cloned(),
..Default::default()
}
}
@@ -452,6 +459,72 @@ mod tests {
);
}
#[test]
fn test_list_multipart_uploads_output_encoding_type() {
for (encoding_type, expected) in [
(None, "dir a/file+b%é\u{0001}"),
(Some(s3s::dto::EncodingType::from_static("url")), "dir%20a/file%2Bb%25%C3%A9%01"),
] {
let raw = "dir a/file+b%é\u{0001}";
let result = ListMultipartsInfo {
delimiter: Some(raw.to_string()),
key_marker: Some(raw.to_string()),
next_key_marker: Some(raw.to_string()),
upload_id_marker: Some("upload+id%".to_string()),
next_upload_id_marker: Some("next+id%".to_string()),
max_uploads: 1,
is_truncated: true,
uploads: vec![MultipartInfo {
object: raw.to_string(),
upload_id: "upload+id%".to_string(),
..Default::default()
}],
common_prefixes: vec![raw.to_string()],
..Default::default()
};
let output =
build_list_multipart_uploads_output("bucket-a".to_string(), raw.to_string(), result, encoding_type.as_ref());
assert_eq!(output.prefix.as_deref(), Some(expected));
assert_eq!(output.delimiter.as_deref(), Some(expected));
assert_eq!(output.key_marker.as_deref(), Some(expected));
assert_eq!(output.next_key_marker.as_deref(), Some(expected));
assert_eq!(output.uploads.as_ref().unwrap()[0].key.as_deref(), Some(expected));
assert_eq!(output.common_prefixes.as_ref().unwrap()[0].prefix.as_deref(), Some(expected));
if encoding_type.is_some() {
let xml = String::from_utf8(crate::storage::storage_api::serialize(&output).unwrap()).unwrap();
assert!(xml.contains("<EncodingType>url</EncodingType>"));
for tag in ["Prefix", "Delimiter", "KeyMarker", "NextKeyMarker", "Key"] {
assert!(xml.contains(&format!("<{tag}>{expected}</{tag}>")), "{xml}");
}
}
assert_eq!(output.encoding_type, encoding_type);
assert_eq!(output.bucket.as_deref(), Some("bucket-a"));
assert_eq!(output.upload_id_marker.as_deref(), Some("upload+id%"));
assert_eq!(output.next_upload_id_marker.as_deref(), Some("next+id%"));
assert_eq!(output.uploads.as_ref().unwrap()[0].upload_id.as_deref(), Some("upload+id%"));
assert_eq!(output.is_truncated, Some(true));
assert_eq!(output.max_uploads, Some(1));
}
}
#[test]
fn test_list_multipart_uploads_output_encoding_preserves_empty_and_absent_fields() {
let encoding_type = s3s::dto::EncodingType::from_static("url");
let output = build_list_multipart_uploads_output(
"bucket-a".to_string(),
String::new(),
ListMultipartsInfo::default(),
Some(&encoding_type),
);
assert_eq!(output.encoding_type, Some(encoding_type));
assert_eq!(output.prefix.as_deref(), Some(""));
assert_eq!(output.delimiter, None);
assert_eq!(output.key_marker, None);
assert_eq!(output.next_key_marker, None);
assert!(output.uploads.unwrap().is_empty());
assert!(output.common_prefixes.unwrap().is_empty());
}
#[test]
fn test_list_multipart_uploads_output_maps_uploads_and_prefixes() {
let result = ListMultipartsInfo {
@@ -472,7 +545,7 @@ mod tests {
..Default::default()
};
let output = build_list_multipart_uploads_output("bucket-a".to_string(), "root/".to_string(), result);
let output = build_list_multipart_uploads_output("bucket-a".to_string(), "root/".to_string(), result, None);
let uploads = output.uploads.as_ref().expect("uploads should be present");
let common_prefixes = output.common_prefixes.as_ref().expect("common prefixes should be present");