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 <mathew@stormdevelopments.ca>
Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/1492
Reviewed-by: trinity-1686a <trinity-1686a@noreply.localhost>

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
+	let key = std::str::from_utf8(
-		.get("key")
+		params
-		.ok_or_bad_request("No key was provided")?
+			.get("key")
-		.to_str()?;
+			.ok_or_bad_request("No key was provided")?
+			.as_bytes(),
+	)?;
 	let policy = params
 		.get("policy")
 		.ok_or_bad_request("No policy was provided")?

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(),
+			));
 		}
 	}
 

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;

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<Incoming> {
+	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)
+	);
+}