fix(api): descriptive InvalidArgument reason for Windows-unsupported object keys (#4947)

fix(api): return descriptive InvalidArgument reason for Windows-unsupported object keys

On Windows hosts object keys containing NTFS-reserved characters or
Win32-unaddressable path segments are rejected up front, but the client
only saw a bare "Invalid argument" (issue #3299). Attach an explicit
reason to these rejections and surface non-empty InvalidArgument reasons
through the S3 error message.
This commit is contained in:
Zhengchao An
2026-07-17 19:00:52 +08:00
committed by GitHub
parent e279a4f48a
commit e1fc4b12ea
2 changed files with 66 additions and 4 deletions
+39 -4
View File
@@ -183,6 +183,16 @@ pub fn is_valid_object_name(object: &str) -> bool {
is_valid_object_prefix(object)
}
/// Client-facing reason attached to rejections of object keys that Win32/NTFS
/// cannot represent as file paths (issue #3299). Deployments on Linux/macOS
/// accept the full S3 key character set.
pub const WINDOWS_RESERVED_CHARACTERS_REASON: &str =
"object key contains characters unsupported on Windows hosts (one of ':', '*', '?', '\"', '|', '<', '>')";
/// Client-facing reason for path segments Windows can store but not address
/// afterwards (issue #3449): trailing dot/space or reserved DOS device names.
pub const WINDOWS_RESERVED_SEGMENT_REASON: &str = "object key contains a path segment unsupported on Windows hosts (trailing dot or space, or a reserved device name such as NUL/CON/COM1)";
/// Reserved DOS device names that shadow regular files on Windows, even when
/// an extension is appended (e.g. `NUL.txt` resolves to the `NUL` device).
const WINDOWS_RESERVED_NAMES: &[&str] = &[
@@ -226,13 +236,21 @@ pub fn check_object_name_for_length_and_slash(bucket: &str, object: &str) -> Res
|| object.contains('>')
// || object.contains('\\')
{
return Err(StorageError::ObjectNameInvalid(bucket.to_owned(), object.to_owned()));
return Err(StorageError::InvalidArgument(
bucket.to_owned(),
object.to_owned(),
WINDOWS_RESERVED_CHARACTERS_REASON.to_owned(),
));
}
// Reject names that NTFS would happily create but the Win32 path
// layer cannot read back (os error 3), e.g. `baddir.` or `NUL.txt`.
if object_name_has_windows_incompatible_segment(object) {
return Err(StorageError::ObjectNameInvalid(bucket.to_owned(), object.to_owned()));
return Err(StorageError::InvalidArgument(
bucket.to_owned(),
object.to_owned(),
WINDOWS_RESERVED_SEGMENT_REASON.to_owned(),
));
}
}
@@ -537,8 +555,8 @@ mod tests {
let result = check_object_name_for_length_and_slash("test-bucket", object);
if cfg!(target_os = "windows") {
assert!(
matches!(result, Err(StorageError::ObjectNameInvalid(..))),
"object name must be rejected on Windows: {object:?}"
matches!(&result, Err(StorageError::InvalidArgument(_, _, reason)) if reason == WINDOWS_RESERVED_SEGMENT_REASON),
"object name must be rejected on Windows with a descriptive reason: {object:?}, got {result:?}"
);
} else {
assert!(result.is_ok(), "object name must remain valid on non-Windows: {object:?}");
@@ -554,6 +572,23 @@ mod tests {
}
}
#[test]
fn test_check_object_name_windows_reserved_characters() {
// Keys containing Win32-reserved characters are rejected on Windows
// with a descriptive reason (issue #3299); valid elsewhere.
for object in ["path/*sUt*mykey", "a:b", "what?", "pipe|name", "quote\"d", "lt<gt>"] {
let result = check_object_name_for_length_and_slash("test-bucket", object);
if cfg!(target_os = "windows") {
assert!(
matches!(&result, Err(StorageError::InvalidArgument(_, _, reason)) if reason == WINDOWS_RESERVED_CHARACTERS_REASON),
"object name must be rejected on Windows with a descriptive reason: {object:?}, got {result:?}"
);
} else {
assert!(result.is_ok(), "object name must remain valid on non-Windows: {object:?}");
}
}
}
#[test]
fn test_check_bucket_and_object_names() {
// Valid names
+27
View File
@@ -276,6 +276,13 @@ impl From<StorageError> for ApiError {
let message = if code == S3ErrorCode::InternalError {
err.to_string()
} else if let StorageError::InvalidArgument(_, _, reason) = &err
&& !reason.is_empty()
{
// Argument-validation rejections carry a client-actionable reason
// (e.g. object keys unsupported on Windows hosts, issue #3299);
// a bare "Invalid argument" gives the caller nothing to fix.
format!("Invalid argument: {reason}")
} else {
ApiError::error_code_to_message(&code)
};
@@ -397,6 +404,26 @@ mod tests {
}
}
#[test]
fn test_api_error_surfaces_invalid_argument_reason() {
let err = StorageError::InvalidArgument(
"bucket".into(),
"a:b*c".into(),
"object key contains characters unsupported on Windows hosts".into(),
);
let api_error: ApiError = err.into();
assert_eq!(api_error.code, S3ErrorCode::InvalidArgument);
assert_eq!(
api_error.message,
"Invalid argument: object key contains characters unsupported on Windows hosts"
);
// An empty reason falls back to the generic message.
let err = StorageError::InvalidArgument("bucket".into(), "object".into(), String::new());
let api_error: ApiError = err.into();
assert_eq!(api_error.message, "Invalid argument");
}
#[test]
fn test_api_error_other_function() {
let custom_error = "Custom API error";