[sse-c] Remove special case for Content-Type headersse-c

5 files changed, 50 insertions, 45 deletions

diff --git a/src/api/s3/get.rs b/src/api/s3/get.rs
index 1bca4671..ec300ab7 100644
--- a/src/api/s3/get.rs
+++ b/src/api/s3/get.rs
@@ -1,4 +1,5 @@
 //! Function related to GET and HEAD requests
+use std::collections::BTreeMap;
 use std::convert::TryInto;
 use std::sync::Arc;
 use std::time::{Duration, UNIX_EPOCH};
@@ -53,7 +54,6 @@ fn object_headers(
 	let date_str = httpdate::fmt_http_date(date);
 
 	let mut resp = Response::builder()
-		.header(CONTENT_TYPE, headers.content_type.to_string())
 		.header(LAST_MODIFIED, date_str)
 		.header(ACCEPT_RANGES, "bytes".to_string());
 
@@ -61,8 +61,23 @@ fn object_headers(
 		resp = resp.header(ETAG, format!("\"{}\"", version_meta.etag));
 	}
 
-	for (k, v) in headers.other.iter() {
-		resp = resp.header(k, v.to_string());
+	// When metadata is retrieved through the REST API, Amazon S3 combines headers that
+	// have the same name (ignoring case) into a comma-delimited list.
+	// See: https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingMetadata.html
+	let mut headers_by_name = BTreeMap::new();
+	for (name, value) in headers.0.iter() {
+		match headers_by_name.get_mut(name) {
+			None => {
+				headers_by_name.insert(name, vec![value.as_str()]);
+			}
+			Some(headers) => {
+				headers.push(value.as_str());
+			}
+		}
+	}
+
+	for (name, values) in headers_by_name {
+		resp = resp.header(name, values.join(","));
 	}
 
 	encryption.add_response_headers(&mut resp);
diff --git a/src/api/s3/list.rs b/src/api/s3/list.rs
index a7eebbb1..1678f1fa 100644
--- a/src/api/s3/list.rs
+++ b/src/api/s3/list.rs
@@ -945,10 +945,7 @@ mod tests {
 			state: ObjectVersionState::Uploading {
 				multipart: true,
 				encryption: ObjectVersionEncryption::Plaintext {
-					headers: ObjectVersionHeaders {
-						content_type: "text/plain".to_string(),
-						other: BTreeMap::<String, String>::new(),
-					},
+					headers: ObjectVersionHeaders(vec![]),
 				},
 			},
 		}
diff --git a/src/api/s3/put.rs b/src/api/s3/put.rs
index 745c2219..941e4122 100644
--- a/src/api/s3/put.rs
+++ b/src/api/s3/put.rs
@@ -1,4 +1,4 @@
-use std::collections::{BTreeMap, HashMap};
+use std::collections::HashMap;
 use std::sync::Arc;
 
 use base64::prelude::*;
@@ -609,57 +609,35 @@ impl Drop for InterruptedCleanup {
 
 // ============ helpers ============
 
-pub(crate) fn get_mime_type(headers: &HeaderMap<HeaderValue>) -> Result<String, Error> {
-	Ok(headers
-		.get(hyper::header::CONTENT_TYPE)
-		.map(|x| x.to_str())
-		.unwrap_or(Ok("blob"))?
-		.to_string())
-}
-
 pub(crate) fn get_headers(headers: &HeaderMap<HeaderValue>) -> Result<ObjectVersionHeaders, Error> {
-	let content_type = get_mime_type(headers)?;
-	let mut other = BTreeMap::new();
+	let mut ret = Vec::new();
 
 	// Preserve standard headers
 	let standard_header = vec![
+		hyper::header::CONTENT_TYPE,
 		hyper::header::CACHE_CONTROL,
 		hyper::header::CONTENT_DISPOSITION,
 		hyper::header::CONTENT_ENCODING,
 		hyper::header::CONTENT_LANGUAGE,
 		hyper::header::EXPIRES,
 	];
-	for h in standard_header.iter() {
-		if let Some(v) = headers.get(h) {
-			match v.to_str() {
-				Ok(v_str) => {
-					other.insert(h.to_string(), v_str.to_string());
-				}
-				Err(e) => {
-					warn!("Discarding header {}, error in .to_str(): {}", h, e);
-				}
-			}
+	for name in standard_header.iter() {
+		if let Some(value) = headers.get(name) {
+			ret.push((name.to_string(), value.to_str()?.to_string()));
 		}
 	}
 
 	// Preserve x-amz-meta- headers
-	for (k, v) in headers.iter() {
-		if k.as_str().starts_with("x-amz-meta-") {
-			match std::str::from_utf8(v.as_bytes()) {
-				Ok(v_str) => {
-					other.insert(k.to_string(), v_str.to_string());
-				}
-				Err(e) => {
-					warn!("Discarding header {}, error in .to_str(): {}", k, e);
-				}
-			}
+	for (name, value) in headers.iter() {
+		if name.as_str().starts_with("x-amz-meta-") {
+			ret.push((
+				name.to_string(),
+				std::str::from_utf8(value.as_bytes())?.to_string(),
+			));
 		}
 	}
 
-	Ok(ObjectVersionHeaders {
-		content_type,
-		other,
-	})
+	Ok(ObjectVersionHeaders(ret))
 }
 
 pub(crate) fn next_timestamp(existing_object: Option<&Object>) -> u64 {
diff --git a/src/model/Cargo.toml b/src/model/Cargo.toml
index 33898e20..776671d0 100644
--- a/src/model/Cargo.toml
+++ b/src/model/Cargo.toml
@@ -27,6 +27,7 @@ blake2.workspace = true
 chrono.workspace = true
 err-derive.workspace = true
 hex.workspace = true
+http.workspace = true
 base64.workspace = true
 tracing.workspace = true
 rand.workspace = true
diff --git a/src/model/s3/object_table.rs b/src/model/s3/object_table.rs
index 7fa4b9e0..f2d21493 100644
--- a/src/model/s3/object_table.rs
+++ b/src/model/s3/object_table.rs
@@ -216,8 +216,6 @@ mod v010 {
 
 	use super::v09;
 
-	pub use v09::ObjectVersionHeaders;
-
 	/// An object
 	#[derive(PartialEq, Eq, Clone, Debug, Serialize, Deserialize)]
 	pub struct Object {
@@ -303,6 +301,10 @@ mod v010 {
 		},
 	}
 
+	/// Vector of headers, as tuples of the format (header name, header value)
+	#[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Debug, Serialize, Deserialize)]
+	pub struct ObjectVersionHeaders(pub Vec<(String, String)>);
+
 	impl garage_util::migrate::Migrate for Object {
 		const VERSION_MARKER: &'static [u8] = b"G010s3ob";
 
@@ -357,7 +359,19 @@ mod v010 {
 	}
 
 	fn migrate_headers(old: v09::ObjectVersionHeaders) -> ObjectVersionEncryption {
-		ObjectVersionEncryption::Plaintext { headers: old }
+		use http::header::CONTENT_TYPE;
+
+		let mut new_headers = Vec::with_capacity(old.other.len() + 1);
+		if old.content_type != "blob" {
+			new_headers.push((CONTENT_TYPE.as_str().to_string(), old.content_type));
+		}
+		for (name, value) in old.other.into_iter() {
+			new_headers.push((name, value));
+		}
+
+		ObjectVersionEncryption::Plaintext {
+			headers: ObjectVersionHeaders(new_headers),
+		}
 	}
 
 	// Since ObjectVersionHeaders can now be serialized independently, for the