Merge pull request 'Upgrade quick-xml crate to 0.39' (#1319) from gwenlg/garage:quick_xml_upgrade into main-v2

Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/1319
Reviewed-by: Alex <lx@deuxfleurs.fr>

Cargo.lock

@@ -3737,9 +3737,9 @@ checksum = "106dd99e98437432fed6519dedecfade6a06a73bb7b2a1e019fdd2bee5778d94"
 
 [[package]]
 name = "quick-xml"
-version = "0.26.0"
+version = "0.39.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7f50b1c63b38611e7d4d7f68b82d3ad0cc71a2ad2e7f61fc10f1328d917c93cd"
+checksum = "f2e3bf4aa9d243beeb01a7b3bc30b77cfe2c44e24ec02d751a7104a53c2c49a1"
 dependencies = [
  "memchr",
  "serde",

Cargo.toml

@@ -95,7 +95,7 @@ fjall = "2.11"
 async-compression = { version = "0.4", features = ["tokio", "zstd"] }
 zstd = { version = "0.13", default-features = false }
 
-quick-xml = { version = "0.26", features = ["serialize"] }
+quick-xml = { version = "0.39", features = ["serialize"] }
 rmp-serde = "1.3"
 serde = { version = "1.0", default-features = false, features = ["derive", "rc"] }
 serde_bytes = "0.11"

src/api/s3/bucket.rs

@@ -330,8 +330,8 @@ fn parse_create_bucket_xml(xml_bytes: &[u8]) -> Option<Option<String>> {
 	// Returns Some(None) if no location constraint is given
 	// Returns Some(Some("xxxx")) where xxxx is the given location constraint
 
-	let xml_str = std::str::from_utf8(xml_bytes).ok()?;
-	if xml_str.trim_matches(char::is_whitespace).is_empty() {
+	let xml_str = std::str::from_utf8(xml_bytes).ok()?.trim();
+	if xml_str.is_empty() {
 		return Some(None);
 	}
 
@@ -373,6 +373,7 @@ mod tests {
 	#[test]
 	fn create_bucket() {
 		assert_eq!(parse_create_bucket_xml(br#""#), Some(None));
+		assert_eq!(parse_create_bucket_xml(br#"		"#), Some(None));
 		assert_eq!(
 			parse_create_bucket_xml(
 				br#"

src/api/s3/copy.rs

@@ -853,7 +853,7 @@ pub struct CopyObjectResult {
 
 #[derive(Debug, Serialize, PartialEq, Eq)]
 pub struct CopyPartResult {
-	#[serde(serialize_with = "xmlns_tag")]
+	#[serde(rename = "@xmlns", serialize_with = "xmlns_tag")]
 	pub xmlns: (),
 	#[serde(rename = "LastModified")]
 	pub last_modified: s3_xml::Value,

src/api/s3/cors.rs

@@ -86,7 +86,7 @@ pub async fn handle_put_cors(
 #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
 #[serde(rename = "CORSConfiguration")]
 pub struct CorsConfiguration {
-	#[serde(serialize_with = "xmlns_tag", skip_deserializing)]
+	#[serde(rename = "@xmlns", serialize_with = "xmlns_tag", skip_deserializing)]
 	pub xmlns: (),
 	#[serde(rename = "CORSRule")]
 	pub cors_rules: Vec<CorsRule>,
@@ -94,9 +94,9 @@ pub struct CorsConfiguration {
 
 #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
 pub struct CorsRule {
-	#[serde(rename = "ID")]
+	#[serde(rename = "ID", skip_serializing_if = "Option::is_none")]
 	pub id: Option<Value>,
-	#[serde(rename = "MaxAgeSeconds")]
+	#[serde(rename = "MaxAgeSeconds", skip_serializing_if = "Option::is_none")]
 	pub max_age_seconds: Option<IntValue>,
 	#[serde(rename = "AllowedOrigin")]
 	pub allowed_origins: Vec<Value>,
@@ -196,6 +196,8 @@ impl CorsRule {
 
 #[cfg(test)]
 mod tests {
+	use crate::unprettify_xml;
+
 	use super::*;
 
 	use quick_xml::de::from_str;
@@ -228,7 +230,8 @@ mod tests {
    <ExposeHeader>*</ExposeHeader>
  </CORSRule>
 </CORSConfiguration>"#;
-		let conf: CorsConfiguration = from_str(message).unwrap();
+		let conf: CorsConfiguration =
+			from_str(message).expect("failed to deserialize xml into `CorsConfiguration` struct");
 		let ref_value = CorsConfiguration {
 			xmlns: (),
 			cors_rules: vec![
@@ -265,8 +268,7 @@ mod tests {
 
 		let message2 = to_xml_with_header(&ref_value)?;
 
-		let cleanup = |c: &str| c.replace(char::is_whitespace, "");
-		assert_eq!(cleanup(message), cleanup(&message2));
+		assert_eq!(unprettify_xml(message), unprettify_xml(&message2));
 
 		Ok(())
 	}

src/api/s3/error.rs

@@ -69,8 +69,16 @@ pub enum Error {
 	InvalidUtf8String(#[from] std::string::FromUtf8Error),
 
 	/// The client sent invalid XML data
-	#[error("Invalid XML: {0}")]
-	InvalidXml(String),
+	#[error("failed to deserialize XML")]
+	InvalidXml(#[from] roxmltree::Error),
+
+	/// The client sent invalid XML data
+	#[error("XML deserialization failed")]
+	InvalidXmlDe(#[from] quick_xml::de::DeError),
+
+	/// The server failed to serialize data into XML
+	#[error("failed to serialize XML")]
+	InvalidXmlSe(#[from] quick_xml::se::SeError),
 
 	/// The client sent a range header with invalid value
 	#[error("Invalid HTTP range: {0:?}")]
@@ -105,18 +113,6 @@ impl From<(http_range::HttpRangeParseError, u64)> for Error {
 	}
 }
 
-impl From<roxmltree::Error> for Error {
-	fn from(err: roxmltree::Error) -> Self {
-		Self::InvalidXml(format!("{}", err))
-	}
-}
-
-impl From<quick_xml::de::DeError> for Error {
-	fn from(err: quick_xml::de::DeError) -> Self {
-		Self::InvalidXml(format!("{}", err))
-	}
-}
-
 impl From<SignatureError> for Error {
 	fn from(err: SignatureError) -> Self {
 		match err {
@@ -149,6 +145,8 @@ impl Error {
 			Error::AuthorizationHeaderMalformed(_) => "AuthorizationHeaderMalformed",
 			Error::NotImplemented(_) => "NotImplemented",
 			Error::InvalidXml(_) => "MalformedXML",
+			Error::InvalidXmlDe(_) => "MalformedXML",
+			Error::InvalidXmlSe(_) => "InternalError",
 			Error::InvalidRange(_) => "InvalidRange",
 			Error::InvalidDigest(_) => "InvalidDigest",
 			Error::InvalidUtf8Str(_) | Error::InvalidUtf8String(_) => "InvalidRequest",
@@ -166,6 +164,7 @@ impl ApiError for Error {
 			Error::PreconditionFailed => StatusCode::PRECONDITION_FAILED,
 			Error::InvalidRange(_) => StatusCode::RANGE_NOT_SATISFIABLE,
 			Error::NotImplemented(_) => StatusCode::NOT_IMPLEMENTED,
+			Error::InvalidXmlSe(_) => StatusCode::INTERNAL_SERVER_ERROR,
 			Error::AuthorizationHeaderMalformed(_)
 			| Error::InvalidPart
 			| Error::InvalidPartOrder
@@ -173,6 +172,7 @@ impl ApiError for Error {
 			| Error::InvalidDigest(_)
 			| Error::InvalidEncryptionAlgorithm(_)
 			| Error::InvalidXml(_)
+			| Error::InvalidXmlDe(_)
 			| Error::InvalidUtf8Str(_)
 			| Error::InvalidUtf8String(_) => StatusCode::BAD_REQUEST,
 		}

src/api/s3/lib.rs

@@ -19,3 +19,11 @@ pub mod website;
 mod encryption;
 mod router;
 pub mod xml;
+
+#[cfg(test)]
+pub(crate) fn unprettify_xml(xml_in: &str) -> String {
+	xml_in.trim().lines().fold(String::new(), |mut val, line| {
+		val.push_str(line.trim());
+		val
+	})
+}

src/api/s3/lifecycle.rs

@@ -83,7 +83,7 @@ pub async fn handle_put_lifecycle(
 
 #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
 pub struct LifecycleConfiguration {
-	#[serde(serialize_with = "xmlns_tag", skip_deserializing)]
+	#[serde(rename = "@xmlns", serialize_with = "xmlns_tag", skip_deserializing)]
 	pub xmlns: (),
 	#[serde(rename = "Rule")]
 	pub lifecycle_rules: Vec<LifecycleRule>,
@@ -91,35 +91,46 @@ pub struct LifecycleConfiguration {
 
 #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
 pub struct LifecycleRule {
-	#[serde(rename = "ID")]
+	#[serde(rename = "ID", skip_serializing_if = "Option::is_none")]
 	pub id: Option<Value>,
 	#[serde(rename = "Status")]
 	pub status: Value,
-	#[serde(rename = "Filter", default)]
+	#[serde(rename = "Filter", default, skip_serializing_if = "Option::is_none")]
 	pub filter: Option<Filter>,
-	#[serde(rename = "Expiration", default)]
+	#[serde(
+		rename = "Expiration",
+		default,
+		skip_serializing_if = "Option::is_none"
+	)]
 	pub expiration: Option<Expiration>,
-	#[serde(rename = "AbortIncompleteMultipartUpload", default)]
+	#[serde(
+		rename = "AbortIncompleteMultipartUpload",
+		default,
+		skip_serializing_if = "Option::is_none"
+	)]
 	pub abort_incomplete_mpu: Option<AbortIncompleteMpu>,
 }
 
 #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Default)]
 pub struct Filter {
-	#[serde(rename = "And")]
+	#[serde(rename = "And", skip_serializing_if = "Option::is_none")]
 	pub and: Option<Box<Filter>>,
-	#[serde(rename = "Prefix")]
+	#[serde(rename = "Prefix", skip_serializing_if = "Option::is_none")]
 	pub prefix: Option<Value>,
-	#[serde(rename = "ObjectSizeGreaterThan")]
+	#[serde(
+		rename = "ObjectSizeGreaterThan",
+		skip_serializing_if = "Option::is_none"
+	)]
 	pub size_gt: Option<IntValue>,
-	#[serde(rename = "ObjectSizeLessThan")]
+	#[serde(rename = "ObjectSizeLessThan", skip_serializing_if = "Option::is_none")]
 	pub size_lt: Option<IntValue>,
 }
 
 #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
 pub struct Expiration {
-	#[serde(rename = "Days")]
+	#[serde(rename = "Days", skip_serializing_if = "Option::is_none")]
 	pub days: Option<IntValue>,
-	#[serde(rename = "Date")]
+	#[serde(rename = "Date", skip_serializing_if = "Option::is_none")]
 	pub at_date: Option<Value>,
 }
 
@@ -285,6 +296,8 @@ impl Expiration {
 
 #[cfg(test)]
 mod tests {
+	use crate::unprettify_xml;
+
 	use super::*;
 
 	use quick_xml::de::from_str;
@@ -357,8 +370,7 @@ mod tests {
 
 		let message2 = to_xml_with_header(&ref_value)?;
 
-		let cleanup = |c: &str| c.replace(char::is_whitespace, "");
-		assert_eq!(cleanup(message), cleanup(&message2));
+		assert_eq!(unprettify_xml(message), unprettify_xml(&message2));
 
 		// Check validation
 		let validated = ref_value
@@ -393,7 +405,7 @@ mod tests {
 		let message3 = to_xml_with_header(&LifecycleConfiguration::from_garage_lifecycle_config(
 			&validated,
 		))?;
-		assert_eq!(cleanup(message), cleanup(&message3));
+		assert_eq!(unprettify_xml(message), unprettify_xml(&message3));
 
 		Ok(())
 	}

src/api/s3/website.rs

@@ -110,7 +110,7 @@ pub async fn handle_put_website(
 
 #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
 pub struct WebsiteConfiguration {
-	#[serde(serialize_with = "xmlns_tag", skip_deserializing)]
+	#[serde(rename = "@xmlns", serialize_with = "xmlns_tag", skip_deserializing)]
 	pub xmlns: (),
 	#[serde(rename = "ErrorDocument")]
 	pub error_document: Option<Key>,
@@ -168,23 +168,29 @@ pub struct Target {
 
 #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
 pub struct Condition {
-	#[serde(rename = "HttpErrorCodeReturnedEquals")]
+	#[serde(
+		rename = "HttpErrorCodeReturnedEquals",
+		skip_serializing_if = "Option::is_none"
+	)]
 	pub http_error_code: Option<IntValue>,
-	#[serde(rename = "KeyPrefixEquals")]
+	#[serde(rename = "KeyPrefixEquals", skip_serializing_if = "Option::is_none")]
 	pub prefix: Option<Value>,
 }
 
 #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
 pub struct Redirect {
-	#[serde(rename = "HostName")]
+	#[serde(rename = "HostName", skip_serializing_if = "Option::is_none")]
 	pub hostname: Option<Value>,
-	#[serde(rename = "Protocol")]
+	#[serde(rename = "Protocol", skip_serializing_if = "Option::is_none")]
 	pub protocol: Option<Value>,
-	#[serde(rename = "HttpRedirectCode")]
+	#[serde(rename = "HttpRedirectCode", skip_serializing_if = "Option::is_none")]
 	pub http_redirect_code: Option<IntValue>,
-	#[serde(rename = "ReplaceKeyPrefixWith")]
+	#[serde(
+		rename = "ReplaceKeyPrefixWith",
+		skip_serializing_if = "Option::is_none"
+	)]
 	pub replace_prefix: Option<Value>,
-	#[serde(rename = "ReplaceKeyWith")]
+	#[serde(rename = "ReplaceKeyWith", skip_serializing_if = "Option::is_none")]
 	pub replace_full: Option<Value>,
 }
 
@@ -373,6 +379,8 @@ impl Redirect {
 
 #[cfg(test)]
 mod tests {
+	use crate::unprettify_xml;
+
 	use super::*;
 
 	use quick_xml::de::from_str;
@@ -416,7 +424,8 @@ mod tests {
       </RoutingRule>
    </RoutingRules>
 </WebsiteConfiguration>"#;
-		let conf: WebsiteConfiguration = from_str(message).unwrap();
+		let conf: WebsiteConfiguration =
+			from_str(message).expect("failed to deserialize xml in `WebsiteConfiguration`");
 		let ref_value = WebsiteConfiguration {
 			xmlns: (),
 			error_document: Some(Key {
@@ -467,8 +476,7 @@ mod tests {
 
 		let message2 = to_xml_with_header(&ref_value)?;
 
-		let cleanup = |c: &str| c.replace(char::is_whitespace, "");
-		assert_eq!(cleanup(message), cleanup(&message2));
+		assert_eq!(unprettify_xml(message), unprettify_xml(&message2));
 
 		Ok(())
 	}

src/api/s3/xml.rs

@@ -1,11 +1,15 @@
-use quick_xml::se::to_string;
+use quick_xml::se::{self, EmptyElementHandling, QuoteLevel};
 use serde::{Deserialize, Serialize, Serializer};
 
 use crate::error::Error as ApiError;
 
 pub fn to_xml_with_header<T: Serialize>(x: &T) -> Result<String, ApiError> {
 	let mut xml = r#"<?xml version="1.0" encoding="UTF-8"?>"#.to_string();
-	xml.push_str(&to_string(x)?);
+
+	let mut ser = se::Serializer::new(&mut xml);
+	ser.set_quote_level(QuoteLevel::Full)
+		.empty_element_handling(EmptyElementHandling::Expanded);
+	let _serialized = x.serialize(ser)?;
 	Ok(xml)
 }
 
@@ -61,7 +65,7 @@ pub struct ListAllMyBucketsResult {
 
 #[derive(Debug, Serialize, PartialEq, Eq)]
 pub struct LocationConstraint {
-	#[serde(serialize_with = "xmlns_tag")]
+	#[serde(rename = "@xmlns", serialize_with = "xmlns_tag")]
 	pub xmlns: (),
 	#[serde(rename = "$value")]
 	pub region: String,
@@ -97,13 +101,13 @@ pub struct DeleteError {
 	pub key: Option<Value>,
 	#[serde(rename = "Message")]
 	pub message: Value,
-	#[serde(rename = "VersionId")]
+	#[serde(rename = "VersionId", skip_serializing_if = "Option::is_none")]
 	pub version_id: Option<Value>,
 }
 
 #[derive(Debug, Serialize, PartialEq, Eq)]
 pub struct DeleteResult {
-	#[serde(serialize_with = "xmlns_tag")]
+	#[serde(rename = "@xmlns", serialize_with = "xmlns_tag")]
 	pub xmlns: (),
 	#[serde(rename = "Deleted")]
 	pub deleted: Vec<Deleted>,
@@ -113,7 +117,7 @@ pub struct DeleteResult {
 
 #[derive(Debug, Serialize, PartialEq, Eq)]
 pub struct InitiateMultipartUploadResult {
-	#[serde(serialize_with = "xmlns_tag")]
+	#[serde(rename = "@xmlns", serialize_with = "xmlns_tag")]
 	pub xmlns: (),
 	#[serde(rename = "Bucket")]
 	pub bucket: Value,
@@ -125,7 +129,7 @@ pub struct InitiateMultipartUploadResult {
 
 #[derive(Debug, Serialize, PartialEq, Eq)]
 pub struct CompleteMultipartUploadResult {
-	#[serde(serialize_with = "xmlns_tag")]
+	#[serde(rename = "@xmlns", serialize_with = "xmlns_tag")]
 	pub xmlns: (),
 	#[serde(rename = "Location")]
 	pub location: Option<Value>,
@@ -135,17 +139,17 @@ pub struct CompleteMultipartUploadResult {
 	pub key: Value,
 	#[serde(rename = "ETag")]
 	pub etag: Value,
-	#[serde(rename = "ChecksumCRC32")]
+	#[serde(rename = "ChecksumCRC32", skip_serializing_if = "Option::is_none")]
 	pub checksum_crc32: Option<Value>,
-	#[serde(rename = "ChecksumCRC32C")]
+	#[serde(rename = "ChecksumCRC32C", skip_serializing_if = "Option::is_none")]
 	pub checksum_crc32c: Option<Value>,
-	#[serde(rename = "ChecksumCR64NVME")]
+	#[serde(rename = "ChecksumCR64NVME", skip_serializing_if = "Option::is_none")]
 	pub checksum_crc64nvme: Option<Value>,
-	#[serde(rename = "ChecksumSHA1")]
+	#[serde(rename = "ChecksumSHA1", skip_serializing_if = "Option::is_none")]
 	pub checksum_sha1: Option<Value>,
-	#[serde(rename = "ChecksumSHA256")]
+	#[serde(rename = "ChecksumSHA256", skip_serializing_if = "Option::is_none")]
 	pub checksum_sha256: Option<Value>,
-	#[serde(rename = "ChecksumType")]
+	#[serde(rename = "ChecksumType", skip_serializing_if = "Option::is_none")]
 	pub checksum_type: Option<Value>,
 }
 
@@ -175,21 +179,21 @@ pub struct ListMultipartItem {
 
 #[derive(Debug, Serialize, PartialEq, Eq)]
 pub struct ListMultipartUploadsResult {
-	#[serde(serialize_with = "xmlns_tag")]
+	#[serde(rename = "@xmlns", serialize_with = "xmlns_tag")]
 	pub xmlns: (),
 	#[serde(rename = "Bucket")]
 	pub bucket: Value,
-	#[serde(rename = "KeyMarker")]
+	#[serde(rename = "KeyMarker", skip_serializing_if = "Option::is_none")]
 	pub key_marker: Option<Value>,
-	#[serde(rename = "UploadIdMarker")]
+	#[serde(rename = "UploadIdMarker", skip_serializing_if = "Option::is_none")]
 	pub upload_id_marker: Option<Value>,
-	#[serde(rename = "NextKeyMarker")]
+	#[serde(rename = "NextKeyMarker", skip_serializing_if = "Option::is_none")]
 	pub next_key_marker: Option<Value>,
-	#[serde(rename = "NextUploadIdMarker")]
+	#[serde(rename = "NextUploadIdMarker", skip_serializing_if = "Option::is_none")]
 	pub next_upload_id_marker: Option<Value>,
 	#[serde(rename = "Prefix")]
 	pub prefix: Value,
-	#[serde(rename = "Delimiter")]
+	#[serde(rename = "Delimiter", skip_serializing_if = "Option::is_none")]
 	pub delimiter: Option<Value>,
 	#[serde(rename = "MaxUploads")]
 	pub max_uploads: IntValue,
@@ -199,7 +203,7 @@ pub struct ListMultipartUploadsResult {
 	pub upload: Vec<ListMultipartItem>,
 	#[serde(rename = "CommonPrefixes")]
 	pub common_prefixes: Vec<CommonPrefix>,
-	#[serde(rename = "EncodingType")]
+	#[serde(rename = "EncodingType", skip_serializing_if = "Option::is_none")]
 	pub encoding_type: Option<Value>,
 }
 
@@ -213,21 +217,21 @@ pub struct PartItem {
 	pub part_number: IntValue,
 	#[serde(rename = "Size")]
 	pub size: IntValue,
-	#[serde(rename = "ChecksumCRC32")]
+	#[serde(rename = "ChecksumCRC32", skip_serializing_if = "Option::is_none")]
 	pub checksum_crc32: Option<Value>,
-	#[serde(rename = "ChecksumCRC32C")]
+	#[serde(rename = "ChecksumCRC32C", skip_serializing_if = "Option::is_none")]
 	pub checksum_crc32c: Option<Value>,
-	#[serde(rename = "ChecksumCRC64NVME")]
+	#[serde(rename = "ChecksumCRC64NVME", skip_serializing_if = "Option::is_none")]
 	pub checksum_crc64nvme: Option<Value>,
-	#[serde(rename = "ChecksumSHA1")]
+	#[serde(rename = "ChecksumSHA1", skip_serializing_if = "Option::is_none")]
 	pub checksum_sha1: Option<Value>,
-	#[serde(rename = "ChecksumSHA256")]
+	#[serde(rename = "ChecksumSHA256", skip_serializing_if = "Option::is_none")]
 	pub checksum_sha256: Option<Value>,
 }
 
 #[derive(Debug, Serialize, PartialEq, Eq)]
 pub struct ListPartsResult {
-	#[serde(serialize_with = "xmlns_tag")]
+	#[serde(rename = "@xmlns", serialize_with = "xmlns_tag")]
 	pub xmlns: (),
 	#[serde(rename = "Bucket")]
 	pub bucket: Value,
@@ -235,9 +239,12 @@ pub struct ListPartsResult {
 	pub key: Value,
 	#[serde(rename = "UploadId")]
 	pub upload_id: Value,
-	#[serde(rename = "PartNumberMarker")]
+	#[serde(rename = "PartNumberMarker", skip_serializing_if = "Option::is_none")]
 	pub part_number_marker: Option<IntValue>,
-	#[serde(rename = "NextPartNumberMarker")]
+	#[serde(
+		rename = "NextPartNumberMarker",
+		skip_serializing_if = "Option::is_none"
+	)]
 	pub next_part_number_marker: Option<IntValue>,
 	#[serde(rename = "MaxParts")]
 	pub max_parts: IntValue,
@@ -275,29 +282,32 @@ pub struct CommonPrefix {
 
 #[derive(Debug, Serialize, PartialEq, Eq)]
 pub struct ListBucketResult {
-	#[serde(serialize_with = "xmlns_tag")]
+	#[serde(rename = "@xmlns", serialize_with = "xmlns_tag")]
 	pub xmlns: (),
 	#[serde(rename = "Name")]
 	pub name: Value,
 	#[serde(rename = "Prefix")]
 	pub prefix: Value,
-	#[serde(rename = "Marker")]
+	#[serde(rename = "Marker", skip_serializing_if = "Option::is_none")]
 	pub marker: Option<Value>,
-	#[serde(rename = "NextMarker")]
+	#[serde(rename = "NextMarker", skip_serializing_if = "Option::is_none")]
 	pub next_marker: Option<Value>,
-	#[serde(rename = "StartAfter")]
+	#[serde(rename = "StartAfter", skip_serializing_if = "Option::is_none")]
 	pub start_after: Option<Value>,
-	#[serde(rename = "ContinuationToken")]
+	#[serde(rename = "ContinuationToken", skip_serializing_if = "Option::is_none")]
 	pub continuation_token: Option<Value>,
-	#[serde(rename = "NextContinuationToken")]
+	#[serde(
+		rename = "NextContinuationToken",
+		skip_serializing_if = "Option::is_none"
+	)]
 	pub next_continuation_token: Option<Value>,
-	#[serde(rename = "KeyCount")]
+	#[serde(rename = "KeyCount", skip_serializing_if = "Option::is_none")]
 	pub key_count: Option<IntValue>,
 	#[serde(rename = "MaxKeys")]
 	pub max_keys: IntValue,
-	#[serde(rename = "Delimiter")]
+	#[serde(rename = "Delimiter", skip_serializing_if = "Option::is_none")]
 	pub delimiter: Option<Value>,
-	#[serde(rename = "EncodingType")]
+	#[serde(rename = "EncodingType", skip_serializing_if = "Option::is_none")]
 	pub encoding_type: Option<Value>,
 	#[serde(rename = "IsTruncated")]
 	pub is_truncated: Value,
@@ -309,15 +319,15 @@ pub struct ListBucketResult {
 
 #[derive(Debug, Serialize, PartialEq, Eq)]
 pub struct VersioningConfiguration {
-	#[serde(serialize_with = "xmlns_tag")]
+	#[serde(rename = "@xmlns", serialize_with = "xmlns_tag")]
 	pub xmlns: (),
-	#[serde(rename = "Status")]
+	#[serde(rename = "Status", skip_serializing_if = "Option::is_none")]
 	pub status: Option<Value>,
 }
 
 #[derive(Debug, Serialize, PartialEq, Eq)]
 pub struct PostObject {
-	#[serde(serialize_with = "xmlns_tag")]
+	#[serde(rename = "@xmlns", serialize_with = "xmlns_tag")]
 	pub xmlns: (),
 	#[serde(rename = "Location")]
 	pub location: Value,
@@ -331,11 +341,11 @@ pub struct PostObject {
 
 #[derive(Debug, Serialize, PartialEq, Eq)]
 pub struct Grantee {
-	#[serde(rename = "xmlns:xsi", serialize_with = "xmlns_xsi_tag")]
+	#[serde(rename = "@xmlns:xsi", serialize_with = "xmlns_xsi_tag")]
 	pub xmlns_xsi: (),
-	#[serde(rename = "xsi:type")]
+	#[serde(rename = "@xsi:type")]
 	pub typ: String,
-	#[serde(rename = "DisplayName")]
+	#[serde(rename = "DisplayName", skip_serializing_if = "Option::is_none")]
 	pub display_name: Option<Value>,
 	#[serde(rename = "ID")]
 	pub id: Option<Value>,
@@ -357,9 +367,9 @@ pub struct AccessControlList {
 
 #[derive(Debug, Serialize, PartialEq, Eq)]
 pub struct AccessControlPolicy {
-	#[serde(serialize_with = "xmlns_tag")]
+	#[serde(rename = "@xmlns", serialize_with = "xmlns_tag")]
 	pub xmlns: (),
-	#[serde(rename = "Owner")]
+	#[serde(rename = "Owner", skip_serializing_if = "Option::is_none")]
 	pub owner: Option<Owner>,
 	#[serde(rename = "AccessControlList")]
 	pub acl: AccessControlList,
@@ -458,7 +468,7 @@ mod tests {
 		assert_eq!(
 			to_xml_with_header(&get_bucket_versioning)?,
 			"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\
-<VersioningConfiguration xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\"/>"
+<VersioningConfiguration xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\"></VersioningConfiguration>"
 		);
 		let get_bucket_versioning2 = VersioningConfiguration {
 			xmlns: (),

src/garage/tests/s3/multipart.rs

@@ -466,7 +466,7 @@ async fn test_uploadlistpart() {
 			.await
 			.unwrap();
 
-		assert!(r.part_number_marker.is_none());
+		assert_eq!(r.part_number_marker.as_deref(), None);
 		assert_eq!(r.next_part_number_marker.as_deref(), Some("1"));
 		assert_eq!(r.max_parts.unwrap(), 1_i32);
 		assert!(r.is_truncated.unwrap());