Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions crates/s3s/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
}
}
}
Expand Down Expand Up @@ -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");
Expand Down
9 changes: 7 additions & 2 deletions crates/s3s/src/ops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,17 +325,22 @@ async fn prepare(req: &mut Request, ccx: &CallContext<'_>) -> S3Result<Prepare>

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))?);
Expand Down
137 changes: 131 additions & 6 deletions crates/s3s/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<S3Path, ParseS3PathError> {
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<S3Path, ParseS3PathError> {
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<S3Path, ParseS3PathError> {
let Some(path) = uri_path.strip_prefix('/') else { return Err(ParseS3PathError::InvalidPath) };

if path.is_empty() {
Expand All @@ -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<S3Path, ParseS3PathError> {
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
Expand All @@ -207,7 +245,21 @@ pub fn parse_virtual_hosted_style_with_validation(
uri_path: &str,
validation: &dyn NameValidation,
) -> Result<S3Path, ParseS3PathError> {
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<S3Path, ParseS3PathError> {
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) };

Expand All @@ -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);
}

Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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);
}
}
}