From 5f695ab29347a8098777ce6f766ecc05c21b1b06 Mon Sep 17 00:00:00 2001 From: w0od Date: Sat, 23 May 2026 16:48:01 +0800 Subject: [PATCH] feat(path): add option to normalize forward slash of object name Signed-off-by: w0od --- crates/s3s/src/config.rs | 23 +++++++ crates/s3s/src/ops/mod.rs | 9 ++- crates/s3s/src/path.rs | 137 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 161 insertions(+), 8 deletions(-) diff --git a/crates/s3s/src/config.rs b/crates/s3s/src/config.rs index b178287a..2645df10 100644 --- a/crates/s3s/src/config.rs +++ b/crates/s3s/src/config.rs @@ -133,6 +133,27 @@ pub struct S3Config { /// /// Default: 900 (15 minutes) pub presigned_url_max_skew_time_secs: u32, + + /// Whether to normalize forward slashes in object keys. + /// + /// If enabled, multiple consecutive slashes will be treated as a single slash: + /// + /// - If the path does not start with `/`, no normalization is performed. + /// - If the path starts with `/`, normalization: + /// - removes all leading slashes (unless the entire path is one or more `/`) + /// - replaces consecutive internal slashes with a single slash + /// - reduces one or more trailing slashes to a single trailing slash + /// - Examples: + /// - "/" -> "/" + /// - "/keyname" -> "keyname" + /// - "//keyname" -> "keyname" + /// - "//keyname/" -> "keyname/" + /// - "//dir////keyname" -> "dir/keyname" + /// - "dir///sub//file" -> "dir///sub//file" + /// - "/dir///sub//file" -> "dir/sub/file" + /// + /// Default: false + pub normalize_forward_slash_path: bool, } impl Default for S3Config { @@ -144,6 +165,7 @@ impl Default for S3Config { form_max_fields_size: 20 * 1024 * 1024, // 20 MB form_max_parts: 1000, presigned_url_max_skew_time_secs: 900, // 15 minutes + normalize_forward_slash_path: false, } } } @@ -349,6 +371,7 @@ mod tests { form_max_fields_size: 5 * 1024 * 1024, form_max_parts: 500, presigned_url_max_skew_time_secs: 600, + normalize_forward_slash_path: false, }; let json = serde_json::to_string(&config).expect("serialize failed"); diff --git a/crates/s3s/src/ops/mod.rs b/crates/s3s/src/ops/mod.rs index 43488d26..4aaca843 100644 --- a/crates/s3s/src/ops/mod.rs +++ b/crates/s3s/src/ops/mod.rs @@ -325,17 +325,22 @@ async fn prepare(req: &mut Request, ccx: &CallContext<'_>) -> S3Result vh_bucket = vh.bucket(); vh_region = vh.region().map(str::to_owned); - break 'parse crate::path::parse_virtual_hosted_style_with_validation( + break 'parse crate::path::parse_virtual_hosted_style_with_validation_and_normalization( vh_bucket, &decoded_uri_path, validation, + ccx.config.snapshot().normalize_forward_slash_path, ); } debug!(?decoded_uri_path, "parsing path-style request"); vh_bucket = None; vh_region = None; - crate::path::parse_path_style_with_validation(&decoded_uri_path, validation) + crate::path::parse_path_style_with_validation_and_normalization( + &decoded_uri_path, + validation, + ccx.config.snapshot().normalize_forward_slash_path, + ) }; req.s3ext.s3_path = Some(result.map_err(|err| convert_parse_s3_path_error(&err))?); diff --git a/crates/s3s/src/path.rs b/crates/s3s/src/path.rs index edb90f7a..c0831db2 100644 --- a/crates/s3s/src/path.rs +++ b/crates/s3s/src/path.rs @@ -6,6 +6,7 @@ //! + [Bucket naming rules](https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html) use crate::validation::{AwsNameValidation, NameValidation}; +use std::borrow::Cow; use std::net::IpAddr; /// A path in the S3 storage @@ -156,17 +157,48 @@ pub const fn check_key(key: &str) -> bool { key.len() <= 1024 } +/// Normalizes forward slash for a path: +fn normalize_forward_slash(path: &str) -> Cow<'_, str> { + if !path.starts_with('/') { + return Cow::Borrowed(path); + } + let end_with_slash = path.ends_with('/'); + let parts: Vec<&str> = path.split('/').filter(|s| !s.is_empty()).collect(); + + if parts.is_empty() { + Cow::Borrowed("/") + } else { + let joined = parts.join("/"); + if end_with_slash { + Cow::Owned(format!("{joined}/")) + } else { + Cow::Owned(joined) + } + } +} + /// Parses a path-style request /// # Errors /// Returns an `Err` if the s3 path is invalid pub fn parse_path_style(uri_path: &str) -> Result { - parse_path_style_with_validation(uri_path, &AwsNameValidation::new()) + parse_path_style_with_validation_and_normalization(uri_path, &AwsNameValidation::new(), false) } /// Parses a path-style request with custom validation /// # Errors /// Returns an `Err` if the s3 path is invalid pub fn parse_path_style_with_validation(uri_path: &str, validation: &dyn NameValidation) -> Result { + parse_path_style_with_validation_and_normalization(uri_path, validation, false) +} + +/// Parses a path-style request with custom validation and optional path normalization +/// # Errors +/// Returns an `Err` if the s3 path is invalid +pub fn parse_path_style_with_validation_and_normalization( + uri_path: &str, + validation: &dyn NameValidation, + normalize_path: bool, +) -> Result { let Some(path) = uri_path.strip_prefix('/') else { return Err(ParseS3PathError::InvalidPath) }; if path.is_empty() { @@ -185,18 +217,24 @@ pub fn parse_path_style_with_validation(uri_path: &str, validation: &dyn NameVal let Some(key) = key else { return Ok(S3Path::bucket(bucket)) }; - if !check_key(key) { + let key = if normalize_path { + normalize_forward_slash(key) + } else { + Cow::Borrowed(key) + }; + + if !check_key(&key) { return Err(ParseS3PathError::KeyTooLong); } - Ok(S3Path::object(bucket, key)) + Ok(S3Path::object(bucket, &key)) } /// Parses a virtual-hosted-style request /// # Errors /// Returns an `Err` if the s3 path is invalid pub fn parse_virtual_hosted_style(vh_bucket: Option<&str>, uri_path: &str) -> Result { - parse_virtual_hosted_style_with_validation(vh_bucket, uri_path, &AwsNameValidation::new()) + parse_virtual_hosted_style_with_validation_and_normalization(vh_bucket, uri_path, &AwsNameValidation::new(), false) } /// Parses a virtual-hosted-style request with custom validation @@ -207,7 +245,21 @@ pub fn parse_virtual_hosted_style_with_validation( uri_path: &str, validation: &dyn NameValidation, ) -> Result { - let Some(bucket) = vh_bucket else { return parse_path_style_with_validation(uri_path, validation) }; + parse_virtual_hosted_style_with_validation_and_normalization(vh_bucket, uri_path, validation, false) +} + +/// Parses a virtual-hosted-style request with custom validation and optional path normalization +/// # Errors +/// Returns an `Err` if the s3 path is invalid +pub fn parse_virtual_hosted_style_with_validation_and_normalization( + vh_bucket: Option<&str>, + uri_path: &str, + validation: &dyn NameValidation, + normalize_path: bool, +) -> Result { + let Some(bucket) = vh_bucket else { + return parse_path_style_with_validation(uri_path, validation); + }; let Some(key) = uri_path.strip_prefix('/') else { return Err(ParseS3PathError::InvalidPath) }; @@ -219,7 +271,13 @@ pub fn parse_virtual_hosted_style_with_validation( return Ok(S3Path::Bucket { bucket: bucket.into() }); } - if !check_key(key) { + let key = if normalize_path { + normalize_forward_slash(key) + } else { + Cow::Borrowed(key) + }; + + if !check_key(&key) { return Err(ParseS3PathError::KeyTooLong); } @@ -393,6 +451,52 @@ mod tests { assert_eq!(result1.is_err(), result2.is_err()); } + #[test] + fn test_path_style_normalize_forward_slash() { + let cases = [ + ("/bucket-name//", Ok(S3Path::object("bucket-name", "/"))), + ("/bucket-name/object-name", Ok(S3Path::object("bucket-name", "object-name"))), + ("/bucket-name//object-name", Ok(S3Path::object("bucket-name", "object-name"))), + ("/bucket-name///object-name/", Ok(S3Path::object("bucket-name", "object-name/"))), + ("/bucket-name/dir/object-name", Ok(S3Path::object("bucket-name", "dir/object-name"))), + ("/bucket-name/dir//object-name", Ok(S3Path::object("bucket-name", "dir//object-name"))), + ("/bucket-name//dir/object-name/", Ok(S3Path::object("bucket-name", "dir/object-name/"))), + ]; + + for (uri_path, expected) in cases { + assert_eq!( + parse_path_style_with_validation_and_normalization(uri_path, &AwsNameValidation::new(), true), + expected + ); + } + } + + #[test] + fn test_virtual_hosted_style_normalize_forward_slash() { + let cases = [ + ("/", Ok(S3Path::bucket("bucket-name"))), + ("//", Ok(S3Path::object("bucket-name", "/"))), + ("///", Ok(S3Path::object("bucket-name", "/"))), + ("/object-name", Ok(S3Path::object("bucket-name", "object-name"))), + ("//object-name", Ok(S3Path::object("bucket-name", "object-name"))), + ("///object-name/", Ok(S3Path::object("bucket-name", "object-name/"))), + ("/dir//object-name/", Ok(S3Path::object("bucket-name", "dir//object-name/"))), + ("//dir//object-name/", Ok(S3Path::object("bucket-name", "dir/object-name/"))), + ]; + + for (uri_path, expected) in cases { + assert_eq!( + parse_virtual_hosted_style_with_validation_and_normalization( + Some("bucket-name"), + uri_path, + &AwsNameValidation::new(), + true + ), + expected + ); + } + } + // --- S3Path accessor coverage --- #[test] @@ -481,4 +585,25 @@ mod tests { let err = ParseS3PathError::KeyTooLong; assert!(!format!("{err}").is_empty()); } + + #[test] + fn key_name_normalize_forward_slash() { + let cases = [ + ("/", "/"), + ("/////", "/"), + ("object-name", "object-name"), + ("object-name/", "object-name/"), + ("object-name///", "object-name///"), + ("/object-name", "object-name"), + ("///object-name", "object-name"), + ("///object-name/", "object-name/"), + ("/dir/object-name", "dir/object-name"), + ("///dir/object-name", "dir/object-name"), + ("/dir////object-name", "dir/object-name"), + ("///dir1////dir2/object-name////////", "dir1/dir2/object-name/"), + ]; + for (input, expected) in &cases { + assert_eq!(normalize_forward_slash(input).as_ref(), *expected); + } + } }