diff --git a/Cargo.lock b/Cargo.lock index 952c18d1e..1ea5ef359 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7601,6 +7601,7 @@ dependencies = [ "mime_guess", "moka", "opentelemetry", + "percent-encoding", "pin-project-lite", "pprof-pyroscope-fork", "rand 0.10.0", diff --git a/rustfs/Cargo.toml b/rustfs/Cargo.toml index 17c182df6..61871a403 100644 --- a/rustfs/Cargo.toml +++ b/rustfs/Cargo.toml @@ -127,6 +127,7 @@ matchit = { workspace = true } md5.workspace = true mime_guess = { workspace = true } moka = { workspace = true } +percent-encoding = { workspace = true } pin-project-lite.workspace = true rust-embed = { workspace = true, features = ["interpolate-folder-path"] } s3s.workspace = true diff --git a/rustfs/src/admin/handlers/kms_keys.rs b/rustfs/src/admin/handlers/kms_keys.rs index bee73bc6a..aa154ee6b 100644 --- a/rustfs/src/admin/handlers/kms_keys.rs +++ b/rustfs/src/admin/handlers/kms_keys.rs @@ -302,6 +302,24 @@ mod tests { assert_eq!(extract_key_id(&uri).as_deref(), Some(expected)); } } + + #[test] + fn test_extract_key_id_skips_empty_values_and_uses_next_alias() { + let uri: Uri = "/rustfs/admin/v3/kms/key/status?keyId=&key-id=minio-key&key=fallback-key" + .parse() + .expect("uri should parse"); + + assert_eq!(extract_key_id(&uri).as_deref(), Some("minio-key")); + } + + #[test] + fn test_extract_key_id_prefers_legacy_name_over_aliases() { + let uri: Uri = "/rustfs/admin/v3/kms/key/status?keyId=legacy-key&key-id=minio-key&key=fallback-key" + .parse() + .expect("uri should parse"); + + assert_eq!(extract_key_id(&uri).as_deref(), Some("legacy-key")); + } } /// List KMS keys (legacy endpoint) diff --git a/rustfs/src/admin/handlers/tier.rs b/rustfs/src/admin/handlers/tier.rs index 11b61760f..689b79e85 100644 --- a/rustfs/src/admin/handlers/tier.rs +++ b/rustfs/src/admin/handlers/tier.rs @@ -26,6 +26,7 @@ use http::Uri; use http::{HeaderMap, StatusCode}; use hyper::Method; use matchit::Params; +use percent_encoding::percent_decode_str; use rustfs_common::data_usage::TierStats; use rustfs_config::MAX_ADMIN_REQUEST_BODY_SIZE; use rustfs_ecstore::bucket::lifecycle::bucket_lifecycle_ops::GLOBAL_TransitionState; @@ -83,8 +84,14 @@ pub struct AddTierQuery { pub struct AddTier {} fn resolve_tier_name(uri: &Uri, params: &Params<'_, '_>) -> S3Result { - if let Some(tier) = params.get("tier").map(str::trim).filter(|tier| !tier.is_empty()) { - return Ok(tier.to_string()); + if let Some(tier) = params.get("tier") { + let decoded = percent_decode_str(tier) + .decode_utf8() + .map_err(|_| s3_error!(InvalidArgument, "invalid tier path parameter"))?; + let trimmed = decoded.trim(); + if !trimmed.is_empty() { + return Ok(trimmed.to_string()); + } } let query = if let Some(query) = uri.query() { @@ -712,6 +719,46 @@ mod tests { assert_eq!(tier, "WARM"); } + #[test] + fn resolve_tier_name_falls_back_when_path_parameter_is_blank() { + let uri: Uri = "/rustfs/admin/v3/tier/%20?tier=WARM".parse().expect("uri should parse"); + let mut router = Router::new(); + router + .insert("/rustfs/admin/v3/tier/{tier}", ()) + .expect("route should insert"); + let matched = router.at("/rustfs/admin/v3/tier/%20").expect("route should match"); + + let tier = resolve_tier_name(&uri, &matched.params).expect("query parameter should resolve"); + assert_eq!(tier, "WARM"); + } + + #[test] + fn resolve_tier_name_preserves_plus_in_path_parameter() { + let uri: Uri = "/rustfs/admin/v3/tier/WARM+PLUS".parse().expect("uri should parse"); + let mut router = Router::new(); + router + .insert("/rustfs/admin/v3/tier/{tier}", ()) + .expect("route should insert"); + let matched = router.at("/rustfs/admin/v3/tier/WARM+PLUS").expect("route should match"); + + let tier = resolve_tier_name(&uri, &matched.params).expect("path parameter should resolve"); + assert_eq!(tier, "WARM+PLUS"); + } + + #[test] + fn resolve_tier_name_rejects_blank_path_without_query_fallback() { + let uri: Uri = "/rustfs/admin/v3/tier/%20".parse().expect("uri should parse"); + let mut router = Router::new(); + router + .insert("/rustfs/admin/v3/tier/{tier}", ()) + .expect("route should insert"); + let matched = router.at("/rustfs/admin/v3/tier/%20").expect("route should match"); + + let err = resolve_tier_name(&uri, &matched.params).expect_err("blank path should fail"); + assert_eq!(err.code(), &S3ErrorCode::InvalidArgument); + assert_eq!(err.message(), Some("tier is required")); + } + #[test] fn require_tier_name_rejects_missing_value() { let err = require_tier_name(&AddTierQuery::default()).expect_err("missing tier should return an error");