From 663fc5ae486562f0a67d63be0071b03c822c0ed5 Mon Sep 17 00:00:00 2001 From: smattymatty Date: Mon, 20 Jul 2026 18:12:39 +0000 Subject: [PATCH] fix(s3): allow UTF-8 in PostObject form field values (fix #1489) (#1492) PostObject (presigned POST) returns `400 InvalidHeaderValue` when the upload involves a non-ASCII filename. The same key uploads fine vie PUT, as the issue #1489 noted. The problem was that `handle_post_object` stores the form field values in an `http::HeaderMap`. values go in through `HeaderValue::from_str` but are read back with the strict `HeaderValue::to_str()` (ASCII-only) which fails on any UTF-8 value. This is the same problem fixed in eab2b81b for `x-amz-meta-*` headers, and the fix is the same: `std::str::from_utf8(value.as_bytes())` instead of `to_str()`. The `key` field in `post_object.rs` and the standard headers (`content-disposition` etc.) in `extract_metadata_headers`. Added integration tests for PostObject (there were none): UTF-8 key, `${filename}` substitution, and UTF-8 `Content-Disposition` metadata. The substitution test passes even without the fix, proving that the filename path was never broken, only the form-params round-trip. Co-authored-by: Mathew Storm Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/1492 Reviewed-by: trinity-1686a --- src/api/s3/post_object.rs | 10 +- src/api/s3/put.rs | 5 +- src/garage/tests/s3/mod.rs | 1 + src/garage/tests/s3/postobject.rs | 166 ++++++++++++++++++++++++++++++ 4 files changed, 177 insertions(+), 5 deletions(-) create mode 100644 src/garage/tests/s3/postobject.rs diff --git a/src/api/s3/post_object.rs b/src/api/s3/post_object.rs index 711e244b..1910a95a 100644 --- a/src/api/s3/post_object.rs +++ b/src/api/s3/post_object.rs @@ -83,10 +83,12 @@ pub async fn handle_post_object( }; // Current part is file. Do some checks before handling to PutObject code - let key = params - .get("key") - .ok_or_bad_request("No key was provided")? - .to_str()?; + let key = std::str::from_utf8( + params + .get("key") + .ok_or_bad_request("No key was provided")? + .as_bytes(), + )?; let policy = params .get("policy") .ok_or_bad_request("No policy was provided")? diff --git a/src/api/s3/put.rs b/src/api/s3/put.rs index e6f379fa..910f56ad 100644 --- a/src/api/s3/put.rs +++ b/src/api/s3/put.rs @@ -679,7 +679,10 @@ pub(crate) fn extract_metadata_headers( ]; for name in standard_header.iter() { if let Some(value) = headers.get(name) { - ret.push((name.to_string(), value.to_str()?.to_string())); + ret.push(( + name.to_string(), + std::str::from_utf8(value.as_bytes())?.to_string(), + )); } } diff --git a/src/garage/tests/s3/mod.rs b/src/garage/tests/s3/mod.rs index bf217513..34598d15 100644 --- a/src/garage/tests/s3/mod.rs +++ b/src/garage/tests/s3/mod.rs @@ -2,6 +2,7 @@ mod cors; mod list; mod multipart; mod objects; +mod postobject; mod presigned; mod signature_encoding; mod simple; diff --git a/src/garage/tests/s3/postobject.rs b/src/garage/tests/s3/postobject.rs new file mode 100644 index 00000000..e3e838d2 --- /dev/null +++ b/src/garage/tests/s3/postobject.rs @@ -0,0 +1,166 @@ +use base64::prelude::*; +use bytes::Bytes; +use chrono::{Duration, Utc}; +use hmac::Mac; +use http_body_util::Full; +use hyper::body::Incoming; +use hyper::{header, Method, Request, Response, StatusCode}; + +use garage_api_common::signature; + +use crate::common; + +const UTF8_KEY: &str = "uploads/test/копия файла.jpg"; +const UTF8_FILENAME: &str = "café-日本語.txt"; +const UTF8_CONTENT_DISPOSITION: &str = "attachment; filename=\"копия файла.jpg\""; +const REGION: &str = "garage-integ-test"; +const BOUNDARY: &str = "boundary-garage-integ-test"; + +async fn send_post_object( + ctx: &common::Context, + bucket: &str, + key_field: &str, + filename: &str, + extra_fields: &[(&str, &str)], + file_body: &str, +) -> Response { + let now = Utc::now(); + let scope = signature::compute_scope(&now, REGION, "s3"); + let credential = format!("{}/{}", ctx.key.id, scope); + let date = now.format(signature::LONG_DATETIME).to_string(); + let expiration = (now + Duration::hours(1)).to_rfc3339(); + + let mut conditions = vec![ + serde_json::json!({ "bucket": bucket }), + serde_json::json!(["starts-with", "$key", ""]), + serde_json::json!({ "x-amz-algorithm": "AWS4-HMAC-SHA256" }), + serde_json::json!({ "x-amz-credential": &credential }), + serde_json::json!({ "x-amz-date": &date }), + ]; + for (name, value) in extra_fields { + conditions.push(serde_json::json!(["eq", format!("${}", name), value])); + } + let policy = serde_json::json!({ + "expiration": expiration, + "conditions": conditions, + }) + .to_string(); + + let policy_b64 = BASE64_STANDARD.encode(policy.as_bytes()); + + let mut signer = signature::signing_hmac(&now, &ctx.key.secret, REGION, "s3").unwrap(); + signer.update(policy_b64.as_bytes()); + let x_amz_signature = hex::encode(signer.finalize().into_bytes()); + + let mut fields = vec![ + ("key".to_string(), key_field.to_string()), + ("x-amz-algorithm".into(), "AWS4-HMAC-SHA256".into()), + ("x-amz-credential".into(), credential), + ("x-amz-date".into(), date), + ("policy".into(), policy_b64), + ("x-amz-signature".into(), x_amz_signature), + ]; + for (name, value) in extra_fields { + fields.push((name.to_string(), value.to_string())); + } + + let mut body = String::new(); + for (name, value) in &fields { + body.push_str(&format!( + "--{BOUNDARY}\r\nContent-Disposition: form-data; name=\"{name}\"\r\n\r\n{value}\r\n" + )); + } + + body.push_str(&format!( + "--{BOUNDARY}\r\nContent-Disposition: form-data; name=\"file\"; filename=\"{filename}\"\r\nContent-Type: text/plain\r\n\r\n{file_body}\r\n--{BOUNDARY}--\r\n" + )); + + let req = Request::builder() + .method(Method::POST) + .uri(format!("{}{}", ctx.garage.s3_uri(), bucket)) + .header(header::HOST, "s3.garage") + .header( + header::CONTENT_TYPE, + format!("multipart/form-data; boundary={BOUNDARY}"), + ) + .body(Full::new(Bytes::from(body.into_bytes()))) + .unwrap(); + + ctx.custom_request.client().request(req).await.unwrap() +} + +#[tokio::test] +async fn test_post_object_utf8_key() { + let ctx = common::context(); + let bucket = ctx.create_bucket("post-object-utf8-key"); + + let res = send_post_object(&ctx, &bucket, UTF8_KEY, "копия файла.jpg", &[], "hello").await; + assert_eq!(res.status(), StatusCode::NO_CONTENT); + + let obj = ctx + .client + .get_object() + .bucket(&bucket) + .key(UTF8_KEY) + .send() + .await + .unwrap(); + assert_bytes_eq!(obj.body, b"hello"); +} + +#[tokio::test] +async fn test_post_object_utf8_filename_substitution() { + let ctx = common::context(); + let bucket = ctx.create_bucket("post-object-utf8-filename"); + + let res = send_post_object( + &ctx, + &bucket, + "uploads/${filename}", + UTF8_FILENAME, + &[], + "bonjour", + ) + .await; + assert_eq!(res.status(), StatusCode::NO_CONTENT); + + let obj = ctx + .client + .get_object() + .bucket(&bucket) + .key(format!("uploads/{}", UTF8_FILENAME)) + .send() + .await + .unwrap(); + assert_bytes_eq!(obj.body, b"bonjour"); +} + +#[tokio::test] +async fn test_post_object_utf8_content_disposition_metadata() { + let ctx = common::context(); + let bucket = ctx.create_bucket("post-object-utf8-contdisp"); + + let res = send_post_object( + &ctx, + &bucket, + "ascii-key.jpg", + "file.jpg", + &[("content-disposition", UTF8_CONTENT_DISPOSITION)], + "data", + ) + .await; + assert_eq!(res.status(), StatusCode::NO_CONTENT); + + let obj = ctx + .client + .get_object() + .bucket(&bucket) + .key("ascii-key.jpg") + .send() + .await + .unwrap(); + assert_eq!( + obj.content_disposition.as_deref(), + Some(UTF8_CONTENT_DISPOSITION) + ); +}