diff --git a/crates/protocols/src/swift/account.rs b/crates/protocols/src/swift/account.rs index cf3aa538c..8abaa849f 100644 --- a/crates/protocols/src/swift/account.rs +++ b/crates/protocols/src/swift/account.rs @@ -14,11 +14,11 @@ //! Swift account operations and validation +use super::metadata_update::{ACCOUNT_META_TAG_PREFIX, MetadataUpdate}; use super::storage_api::account::{BucketOperations, MakeBucketOptions}; use super::{SwiftError, SwiftResult}; -use super::{get_swift_bucket_metadata, resolve_swift_object_store_handle, update_swift_bucket_tagging, validate_metadata}; +use super::{get_swift_bucket_metadata, resolve_swift_object_store_handle, update_swift_bucket_tagging}; use rustfs_credentials::Credentials; -use s3s::dto::{Tag, Tagging}; use sha2::{Digest, Sha256}; use std::collections::HashMap; @@ -131,9 +131,8 @@ pub async fn get_account_metadata(account: &str, _credentials: &Option, + update: &MetadataUpdate, credentials: &Option, ) -> SwiftResult<()> { let Some(credentials) = credentials.as_ref() else { @@ -171,8 +176,15 @@ pub async fn update_account_metadata( // These tags are persisted into the bucket metadata file, which every // later config write rewrites in full — so unbounded metadata inflates - // the cost of unrelated writes for the life of the account. - validate_metadata(metadata)?; + // the cost of unrelated writes for the life of the account. The item + // count is capped against the merged result, inside the rewrite. + update.validate()?; + + // An update that names no item changes nothing, so there is no reason to + // bring the account's metadata bucket into existence for it. + if update.is_empty() { + return Ok(()); + } let bucket_name = get_account_metadata_bucket_name(account); @@ -190,32 +202,10 @@ pub async fn update_account_metadata( .map_err(|e| SwiftError::InternalServerError(format!("Failed to create account metadata bucket: {}", e)))?; } - // Rewrite the persisted tags: replace swift-account-meta-* tags with the - // new metadata while preserving other tags. An empty result clears the - // tagging config. - update_swift_bucket_tagging(bucket_name, |current| { - let mut tagging = current.cloned().unwrap_or_else(|| Tagging { tag_set: vec![] }); - - tagging.tag_set.retain(|tag| { - if let Some(key) = &tag.key { - !key.starts_with("swift-account-meta-") - } else { - true - } - }); - - for (key, value) in metadata { - tagging.tag_set.push(Tag { - key: Some(format!("swift-account-meta-{}", key)), - value: Some(value.clone()), - }); - } - - tagging - }) - .await?; - - Ok(()) + // Merge into the persisted tags: only the swift-account-meta-* items this + // update names change, and non-Swift tags are left alone. An empty result + // clears the tagging config. + update_swift_bucket_tagging(bucket_name, |current| update.apply_to_tags(current, ACCOUNT_META_TAG_PREFIX)).await } /// Get TempURL key for account diff --git a/crates/protocols/src/swift/container.rs b/crates/protocols/src/swift/container.rs index fb0ad807d..495664525 100644 --- a/crates/protocols/src/swift/container.rs +++ b/crates/protocols/src/swift/container.rs @@ -17,15 +17,13 @@ //! This module implements Swift container CRUD operations and container-bucket translation. use super::account::validate_account_access; +use super::metadata_update::{CONTAINER_META_TAG_PREFIX, MetadataUpdate}; use super::storage_api::container::{ BucketInfo, BucketOperations, BucketOptions, DeleteBucketOptions, ListOperations as _, MakeBucketOptions, }; use super::types::Container; use super::{SwiftError, SwiftResult}; -use super::{ - get_swift_bucket_metadata, get_swift_bucket_usage, resolve_swift_object_store_handle, update_swift_bucket_tagging, - validate_metadata, -}; +use super::{get_swift_bucket_metadata, get_swift_bucket_usage, resolve_swift_object_store_handle, update_swift_bucket_tagging}; use rustfs_credentials::Credentials; use s3s::dto::{Tag, Tagging}; use sha2::{Digest, Sha256}; @@ -62,30 +60,6 @@ fn sanitize_storage_error(operation: &str, error: E) -> Sw SwiftError::InternalServerError(format!("{} operation failed", operation)) } -/// Convert Swift container metadata to S3 tags -/// -/// Swift container metadata uses X-Container-Meta-* headers. -/// We store these as S3 tags with "swift-meta-" prefix to distinguish from regular bucket tags. -/// -/// Example: X-Container-Meta-Color: Blue → S3 Tag: swift-meta-color=Blue -fn swift_metadata_to_s3_tags(metadata: &std::collections::HashMap) -> Option { - let mut tags = Vec::new(); - - for (key, value) in metadata { - // Store with "swift-meta-" prefix to namespace container metadata - tags.push(Tag { - key: Some(format!("swift-meta-{}", key.to_lowercase())), - value: Some(value.clone()), - }); - } - - if tags.is_empty() { - None - } else { - Some(Tagging { tag_set: tags }) - } -} - /// Convert S3 tags back to Swift container metadata /// /// Extracts only tags with "swift-meta-" prefix, which represent Swift container metadata. @@ -94,11 +68,9 @@ fn s3_tags_to_swift_metadata(tagging: &Tagging) -> std::collections::HashMap, + update: MetadataUpdate, ) -> SwiftResult<()> { // Validate account access and extract project_id let project_id = validate_account_access(account, credentials)?; @@ -488,8 +463,9 @@ pub async fn update_container_metadata( // These tags are persisted into the bucket metadata file, which every // later config write rewrites in full — so unbounded metadata inflates - // the cost of unrelated writes for the life of the container. - validate_metadata(&metadata)?; + // the cost of unrelated writes for the life of the container. The item + // count is capped against the merged result, inside the rewrite. + update.validate()?; // Create mapper with default config (tenant prefixing enabled) let mapper = ContainerMapper::default(); @@ -502,7 +478,9 @@ pub async fn update_container_metadata( return Err(SwiftError::InternalServerError("Storage layer not initialized".to_string())); }; - // Verify container exists + // Verify container exists. Checked before the empty-update shortcut below, + // so a POST to a container that does not exist still answers 404 whether + // or not it carried metadata. store .get_bucket_info(&bucket_name, &BucketOptions::default()) .await @@ -514,32 +492,19 @@ pub async fn update_container_metadata( } })?; - // Rewrite the persisted tags: replace swift-meta-* tags with the new - // metadata while preserving non-Swift tags. An empty result clears the - // tagging config. - update_swift_bucket_tagging(bucket_name, |current| { - let mut tagging = current.cloned().unwrap_or_else(|| Tagging { tag_set: vec![] }); + // An update that names no item leaves stored metadata alone, so skip the + // persisted write and the peer reload it triggers rather than rewriting + // the config to its current value. The handler reaches here on every + // container POST, including ACL-only and versioning-only ones. + if update.is_empty() { + return Ok(()); + } - tagging.tag_set.retain(|tag| { - if let Some(key) = &tag.key { - !key.starts_with("swift-meta-") - } else { - true // Keep tags with no key (shouldn't happen, but be safe) - } - }); - - if let Some(mut new_tagging) = swift_metadata_to_s3_tags(&metadata) { - tagging.tag_set.append(&mut new_tagging.tag_set); - } - // If metadata.is_empty() and swift_metadata_to_s3_tags returns None, - // we've already removed swift-meta-* tags above, so only non-Swift - // tags remain - - tagging - }) - .await?; - - Ok(()) + // Merge into the persisted tags: only the swift-meta-* items this update + // names change, so the container's other metadata — and the ACL and + // versioning tags sharing this tag set — survive. An empty result clears + // the tagging config. + update_swift_bucket_tagging(bucket_name, |current| update.apply_to_tags(current, CONTAINER_META_TAG_PREFIX)).await } /// Delete a container @@ -814,7 +779,7 @@ pub async fn enable_versioning( value: Some(archive_container.to_string()), // Store Swift container name, not S3 bucket name }); - tagging + Ok(tagging) }) .await?; @@ -871,7 +836,7 @@ pub async fn disable_versioning(account: &str, container: &str, credentials: &Cr .tag_set .retain(|tag| tag.key.as_deref() != Some("swift-versions-location")); - tagging + Ok(tagging) }) .await?; @@ -1026,7 +991,7 @@ pub async fn set_container_acl( }); } - tagging + Ok(tagging) }) .await?; @@ -1374,64 +1339,6 @@ mod tests { assert!(first_char.is_ascii_lowercase() || first_char.is_ascii_digit()); } - #[test] - fn test_swift_metadata_to_s3_tags() { - let mut metadata = std::collections::HashMap::new(); - metadata.insert("color".to_string(), "blue".to_string()); - metadata.insert("description".to_string(), "test container".to_string()); - - let tagging = swift_metadata_to_s3_tags(&metadata).unwrap(); - assert_eq!(tagging.tag_set.len(), 2); - - // Verify tags have swift-meta- prefix - let color_tag = tagging - .tag_set - .iter() - .find(|t| t.key.as_deref() == Some("swift-meta-color")) - .expect("color tag not found"); - assert_eq!(color_tag.value.as_deref(), Some("blue")); - - let desc_tag = tagging - .tag_set - .iter() - .find(|t| t.key.as_deref() == Some("swift-meta-description")) - .expect("description tag not found"); - assert_eq!(desc_tag.value.as_deref(), Some("test container")); - } - - #[test] - fn test_swift_metadata_to_s3_tags_empty() { - let metadata = std::collections::HashMap::new(); - let tagging = swift_metadata_to_s3_tags(&metadata); - assert!(tagging.is_none()); - } - - #[test] - fn test_swift_metadata_to_s3_tags_case_normalization() { - let mut metadata = std::collections::HashMap::new(); - metadata.insert("Color".to_string(), "Red".to_string()); - metadata.insert("PRIORITY".to_string(), "High".to_string()); - - let tagging = swift_metadata_to_s3_tags(&metadata).unwrap(); - - // Keys should be lowercased - assert!(tagging.tag_set.iter().any(|t| t.key.as_deref() == Some("swift-meta-color"))); - assert!( - tagging - .tag_set - .iter() - .any(|t| t.key.as_deref() == Some("swift-meta-priority")) - ); - - // Values should be preserved as-is - let color_tag = tagging - .tag_set - .iter() - .find(|t| t.key.as_deref() == Some("swift-meta-color")) - .unwrap(); - assert_eq!(color_tag.value.as_deref(), Some("Red")); - } - #[test] fn test_s3_tags_to_swift_metadata() { let tagging = Tagging { @@ -1487,91 +1394,80 @@ mod tests { #[test] fn test_metadata_roundtrip() { - // Test that we can convert metadata -> tags -> metadata without loss - let mut original_metadata = std::collections::HashMap::new(); - original_metadata.insert("color".to_string(), "blue".to_string()); - original_metadata.insert("owner".to_string(), "alice".to_string()); - original_metadata.insert("priority".to_string(), "high".to_string()); + // What a POST writes must be what a HEAD reads back: run the items + // through the tag-merge write path and the tag-parse read path. + let update = MetadataUpdate::default() + .set("color", "blue") + .set("owner", "alice") + .set("priority", "high"); - let tagging = swift_metadata_to_s3_tags(&original_metadata).unwrap(); - let recovered_metadata = s3_tags_to_swift_metadata(&tagging); + let tagging = update + .apply_to_tags(None, CONTAINER_META_TAG_PREFIX) + .expect("merge should be accepted"); + let recovered = s3_tags_to_swift_metadata(&tagging); - assert_eq!(recovered_metadata.len(), original_metadata.len()); - for (key, value) in &original_metadata { - assert_eq!( - recovered_metadata.get(&key.to_lowercase()), - Some(value), - "Metadata key {} not preserved in roundtrip", - key - ); - } + assert_eq!(recovered.len(), 3); + assert_eq!(recovered.get("color").map(String::as_str), Some("blue")); + assert_eq!(recovered.get("owner").map(String::as_str), Some("alice")); + assert_eq!(recovered.get("priority").map(String::as_str), Some("high")); } #[test] fn test_tag_preservation_merge_with_existing() { - // Test merging Swift metadata with existing non-Swift tags - let mut existing_tagging = Tagging { + // A container metadata POST shares its tag set with the container ACL + // and versioning tags, and with whatever S3 tags the bucket carries. + // Only the swift-meta-* items the POST names may change. + let existing = Tagging { tag_set: vec![ Tag { key: Some("swift-meta-color".to_string()), value: Some("blue".to_string()), }, + Tag { + key: Some("swift-acl-read".to_string()), + value: Some(".r:*".to_string()), + }, + Tag { + key: Some("swift-versions-location".to_string()), + value: Some("archive".to_string()), + }, Tag { key: Some("env".to_string()), value: Some("production".to_string()), }, - Tag { - key: Some("team".to_string()), - value: Some("backend".to_string()), - }, ], }; - // Remove old swift-meta-* tags - existing_tagging.tag_set.retain(|tag| { - if let Some(key) = &tag.key { - !key.starts_with("swift-meta-") - } else { - true - } - }); + let merged = MetadataUpdate::default() + .set("description", "test") + .apply_to_tags(Some(&existing), CONTAINER_META_TAG_PREFIX) + .expect("merge should be accepted"); + let tag = |key: &str| { + merged + .tag_set + .iter() + .find(|t| t.key.as_deref() == Some(key)) + .and_then(|t| t.value.as_deref()) + }; - // Add new Swift metadata - let mut new_metadata = std::collections::HashMap::new(); - new_metadata.insert("description".to_string(), "test".to_string()); - let mut new_tagging = swift_metadata_to_s3_tags(&new_metadata).unwrap(); - - // Merge - existing_tagging.tag_set.append(&mut new_tagging.tag_set); - - // Verify: should have env, team, and new swift-meta-description - assert_eq!(existing_tagging.tag_set.len(), 3); - - let has_env = existing_tagging.tag_set.iter().any(|t| t.key.as_deref() == Some("env")); - let has_team = existing_tagging.tag_set.iter().any(|t| t.key.as_deref() == Some("team")); - let has_description = existing_tagging - .tag_set - .iter() - .any(|t| t.key.as_deref() == Some("swift-meta-description")); - - assert!(has_env, "env tag should be preserved"); - assert!(has_team, "team tag should be preserved"); - assert!(has_description, "swift-meta-description should be added"); + assert_eq!(tag("swift-meta-description"), Some("test"), "the new item should be added"); + assert_eq!(tag("swift-meta-color"), Some("blue"), "an unnamed item should be preserved"); + assert_eq!(tag("swift-acl-read"), Some(".r:*"), "the ACL tag should be preserved"); + assert_eq!(tag("swift-versions-location"), Some("archive"), "the versioning tag should be preserved"); + assert_eq!(tag("env"), Some("production"), "non-Swift tags should be preserved"); + assert_eq!(merged.tag_set.len(), 5); } #[test] fn test_tag_preservation_remove_only_swift() { - // Test that clearing Swift metadata preserves non-Swift tags - let mut existing_tagging = Tagging { + // Removing the last container metadata item leaves the other tags — + // and so must not clear the tagging config. + let existing = Tagging { tag_set: vec![ Tag { key: Some("swift-meta-color".to_string()), value: Some("blue".to_string()), }, - Tag { - key: Some("swift-meta-owner".to_string()), - value: Some("alice".to_string()), - }, Tag { key: Some("env".to_string()), value: Some("production".to_string()), @@ -1583,37 +1479,28 @@ mod tests { ], }; - // Remove swift-meta-* tags (simulating empty metadata update) - existing_tagging.tag_set.retain(|tag| { - if let Some(key) = &tag.key { - !key.starts_with("swift-meta-") - } else { - true - } - }); + let merged = MetadataUpdate::default() + .remove("color") + .apply_to_tags(Some(&existing), CONTAINER_META_TAG_PREFIX) + .expect("merge should be accepted"); - // Verify: should only have env and cost-center - assert_eq!(existing_tagging.tag_set.len(), 2); - - let has_env = existing_tagging.tag_set.iter().any(|t| t.key.as_deref() == Some("env")); - let has_cost_center = existing_tagging - .tag_set - .iter() - .any(|t| t.key.as_deref() == Some("cost-center")); - let has_swift_meta = existing_tagging - .tag_set - .iter() - .any(|t| t.key.as_ref().is_some_and(|k| k.starts_with("swift-meta-"))); - - assert!(has_env, "env tag should be preserved"); - assert!(has_cost_center, "cost-center tag should be preserved"); - assert!(!has_swift_meta, "all swift-meta-* tags should be removed"); + assert_eq!(merged.tag_set.len(), 2); + assert!(merged.tag_set.iter().any(|t| t.key.as_deref() == Some("env"))); + assert!(merged.tag_set.iter().any(|t| t.key.as_deref() == Some("cost-center"))); + assert!( + !merged + .tag_set + .iter() + .any(|t| t.key.as_ref().is_some_and(|k| k.starts_with(CONTAINER_META_TAG_PREFIX))), + "the removed item should be gone" + ); } #[test] fn test_tag_preservation_empty_after_swift_removal() { - // Test that if only Swift tags exist, clearing them results in empty tagging - let mut existing_tagging = Tagging { + // Removing every item when nothing else is tagged empties the tag set, + // which is how the caller knows to clear the tagging config. + let existing = Tagging { tag_set: vec![ Tag { key: Some("swift-meta-color".to_string()), @@ -1626,38 +1513,13 @@ mod tests { ], }; - // Remove swift-meta-* tags - existing_tagging.tag_set.retain(|tag| { - if let Some(key) = &tag.key { - !key.starts_with("swift-meta-") - } else { - true - } - }); + let merged = MetadataUpdate::default() + .remove("color") + .remove("owner") + .apply_to_tags(Some(&existing), CONTAINER_META_TAG_PREFIX) + .expect("merge should be accepted"); - // Verify: should be empty - assert!( - existing_tagging.tag_set.is_empty(), - "tagging should be empty after removing all swift-meta-* tags" - ); - } - - #[test] - fn test_tag_preservation_no_existing_tags() { - // Test adding Swift metadata when no tags exist - let existing_tagging = Tagging { tag_set: vec![] }; - - let mut new_metadata = std::collections::HashMap::new(); - new_metadata.insert("color".to_string(), "blue".to_string()); - let mut new_tagging = swift_metadata_to_s3_tags(&new_metadata).unwrap(); - - let mut merged = existing_tagging.clone(); - merged.tag_set.append(&mut new_tagging.tag_set); - - // Verify: should have only the new Swift tag - assert_eq!(merged.tag_set.len(), 1); - assert_eq!(merged.tag_set[0].key.as_deref(), Some("swift-meta-color")); - assert_eq!(merged.tag_set[0].value.as_deref(), Some("blue")); + assert!(merged.tag_set.is_empty(), "tagging should be empty after removing every swift-meta-* tag"); } // Object Versioning Tests diff --git a/crates/protocols/src/swift/handler.rs b/crates/protocols/src/swift/handler.rs index 9961cddf2..0d9cdc455 100644 --- a/crates/protocols/src/swift/handler.rs +++ b/crates/protocols/src/swift/handler.rs @@ -20,6 +20,7 @@ use super::container; use super::dlo; +use super::metadata_update::MetadataUpdate; use super::object; use super::slo; use super::tempurl; @@ -321,32 +322,15 @@ async fn handle_authenticated_request( Err(SwiftError::NotImplemented("Swift Account HEAD operation not yet implemented".to_string())) } Method::POST => { - // Account metadata update - extract headers - let mut metadata = std::collections::HashMap::new(); + // Account metadata update. Additive, per Swift: the request + // names the items to set (X-Account-Meta-*) and the items to + // drop (X-Remove-Account-Meta-*, or an empty value), and + // everything it does not name keeps its stored value. The + // TempURL signing key lives here, so a replacing POST would + // invalidate every outstanding signature for the account. + let update = MetadataUpdate::from_account_headers(&headers); - // Extract X-Account-Meta-* headers - for (key, value) in &headers { - let key_str = key.as_str(); - if let Some(meta_key) = key_str.strip_prefix("x-account-meta-") { - // Strip "x-account-meta-" - if let Ok(value_str) = value.to_str() { - metadata.insert(meta_key.to_string(), value_str.to_string()); - } - } - } - - // Special handling for TempURL key headers - // X-Account-Meta-Temp-URL-Key or X-Account-Meta-Temp-Url-Key - if let Some(tempurl_key) = headers - .get("x-account-meta-temp-url-key") - .or_else(|| headers.get("x-account-meta-temp-Url-key")) - && let Ok(key_str) = tempurl_key.to_str() - { - metadata.insert("temp-url-key".to_string(), key_str.to_string()); - } - - // Update account metadata - super::account::update_account_metadata(&account, &metadata, &credentials_opt).await?; + super::account::update_account_metadata(&account, &update, &credentials_opt).await?; let trans_id = generate_trans_id(); Response::builder() @@ -581,17 +565,15 @@ async fn handle_authenticated_request( container::set_container_acl(&account, &container, new_read, new_write, &credentials).await?; } - // Update container metadata - now we have access to request headers - let mut metadata = std::collections::HashMap::new(); - for (name, value) in headers.iter() { - if let Some(meta_key) = name.as_str().strip_prefix("x-container-meta-") - && let Ok(value_str) = value.to_str() - { - metadata.insert(meta_key.to_string(), value_str.to_string()); - } - } + // Update container metadata. Additive, per Swift: items the + // request does not name keep their stored value, and removal + // is explicit (X-Remove-Container-Meta-*, or an empty value). + // Still called when the request named no item — an ACL-only + // or versioning-only POST — because this is what reports 404 + // for a container that does not exist. + let update = MetadataUpdate::from_container_headers(&headers); - container::update_container_metadata(&account, &container, &credentials, metadata).await?; + container::update_container_metadata(&account, &container, &credentials, update).await?; let trans_id = generate_trans_id(); Response::builder() diff --git a/crates/protocols/src/swift/metadata_update.rs b/crates/protocols/src/swift/metadata_update.rs new file mode 100644 index 000000000..ee7ca2857 --- /dev/null +++ b/crates/protocols/src/swift/metadata_update.rs @@ -0,0 +1,410 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Account and container metadata updates +//! +//! Swift account and container POSTs are *additive*: an item the request does +//! not mention keeps its stored value, and removal is explicit — either +//! `X-Remove-{Account,Container}-Meta-{name}`, or the item sent with an empty +//! value. Object POST is the one that replaces the whole set; `swift::object` +//! handles that separately and must keep doing so. +//! +//! Getting this wrong is not a cosmetic divergence. Account metadata holds the +//! TempURL signing key, so a POST that set an unrelated item while dropping +//! the rest would invalidate every outstanding TempURL and FormPost signature +//! for the account. +//! +//! This module turns a request's headers into the items to write and the items +//! to drop, and applies that to the bucket tag set the metadata is persisted +//! in. + +use super::{MAX_METADATA_COUNT, MAX_METADATA_VALUE_SIZE, SwiftError, SwiftResult}; +use axum::http::HeaderMap; +use s3s::dto::{Tag, Tagging}; +use std::collections::{BTreeMap, BTreeSet}; + +/// Request header prefix carrying an account metadata item. +const ACCOUNT_META_HEADER_PREFIX: &str = "x-account-meta-"; +/// Request header prefix removing an account metadata item. +const ACCOUNT_META_REMOVE_HEADER_PREFIX: &str = "x-remove-account-meta-"; +/// Request header prefix carrying a container metadata item. +const CONTAINER_META_HEADER_PREFIX: &str = "x-container-meta-"; +/// Request header prefix removing a container metadata item. +const CONTAINER_META_REMOVE_HEADER_PREFIX: &str = "x-remove-container-meta-"; + +/// Bucket-tag namespace holding account metadata items. +pub(crate) const ACCOUNT_META_TAG_PREFIX: &str = "swift-account-meta-"; +/// Bucket-tag namespace holding container metadata items. +/// +/// Deliberately narrower than the `swift-` tags around it: the container ACL +/// (`swift-acl-*`) and versioning (`swift-versions-location`) tags share this +/// tag set and must survive a metadata POST. +pub(crate) const CONTAINER_META_TAG_PREFIX: &str = "swift-meta-"; + +/// The metadata changes carried by one account or container POST. +/// +/// Item names are held lowercased. Swift metadata names are case-insensitive, +/// HTTP header names arrive lowercased anyway, and a removal has to match the +/// name a previous POST stored — so normalizing once here is what makes +/// `X-Remove-Container-Meta-Color` find a stored `color`. +/// +/// Ordered rather than hashed so the persisted tag set — and therefore the +/// serialized XML — comes out in a stable order for a given update. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct MetadataUpdate { + /// Items to write, by name. + items: BTreeMap, + /// Names to drop. + removals: BTreeSet, +} + +impl MetadataUpdate { + /// Write `name` = `value`. + pub fn set(mut self, name: &str, value: &str) -> Self { + let name = name.to_lowercase(); + self.removals.remove(&name); + self.items.insert(name, value.to_string()); + self + } + + /// Drop `name`, if it is stored. + pub fn remove(mut self, name: &str) -> Self { + let name = name.to_lowercase(); + self.items.remove(&name); + self.removals.insert(name); + self + } + + /// Parse the metadata headers of an account POST. + pub(crate) fn from_account_headers(headers: &HeaderMap) -> Self { + Self::from_headers(headers, ACCOUNT_META_HEADER_PREFIX, ACCOUNT_META_REMOVE_HEADER_PREFIX) + } + + /// Parse the metadata headers of a container POST. + pub(crate) fn from_container_headers(headers: &HeaderMap) -> Self { + Self::from_headers(headers, CONTAINER_META_HEADER_PREFIX, CONTAINER_META_REMOVE_HEADER_PREFIX) + } + + /// Parse metadata headers under `item_prefix`, and removals under + /// `remove_prefix`. Both prefixes are lowercase, matching how `http` + /// normalizes header names. + /// + /// An item sent with an empty value is a removal — the deletion path + /// Swift clients use when they do not send a dedicated remove header. + /// A name carried by both header forms is removed: the explicit removal + /// wins, as it does in Swift, where a remove header is rewritten into an + /// empty-valued item header. + fn from_headers(headers: &HeaderMap, item_prefix: &str, remove_prefix: &str) -> Self { + let mut update = Self::default(); + + for (name, value) in headers { + let Some(item) = name.as_str().strip_prefix(item_prefix) else { + continue; + }; + // A value that is not valid UTF-8 cannot be stored as a tag. + // Skipping it matches how the rest of the Swift handlers treat + // unreadable header values. + let Ok(value) = value.to_str() else { + continue; + }; + + update = if value.is_empty() { + update.remove(item) + } else { + update.set(item, value) + }; + } + + for name in headers.keys() { + if let Some(item) = name.as_str().strip_prefix(remove_prefix) { + update = update.remove(item); + } + } + + update + } + + /// Whether this update changes anything. + /// + /// An ACL-only or versioning-only container POST produces an empty update: + /// it names no metadata item, so it must leave stored metadata alone. + pub(crate) fn is_empty(&self) -> bool { + self.items.is_empty() && self.removals.is_empty() + } + + /// Reject oversized values before anything is persisted. + /// + /// The item *count* is not checked here — an additive POST has to be + /// measured against the merged result, which only [`Self::apply_to_tags`] + /// can see. + pub(crate) fn validate(&self) -> SwiftResult<()> { + for (name, value) in &self.items { + if value.len() > MAX_METADATA_VALUE_SIZE { + return Err(SwiftError::BadRequest(format!( + "Metadata value for '{}' too large: {} bytes (max: {} bytes)", + name, + value.len(), + MAX_METADATA_VALUE_SIZE + ))); + } + } + + Ok(()) + } + + /// Merge this update into the persisted tag set. + /// + /// `prefix` is the tag namespace holding the items. Tags outside it — the + /// container ACL and versioning tags, plus any S3 tags the bucket carries + /// — are untouched, and so are the namespaced items this update does not + /// name. + /// + /// The item-count cap is checked against the merged result rather than the + /// request: because POSTs are additive, a client could otherwise walk past + /// it one header at a time. This runs inside the bucket metadata write + /// guard, so the count it checks is the one about to be persisted. + pub(crate) fn apply_to_tags(&self, current: Option<&Tagging>, prefix: &str) -> SwiftResult { + let mut tagging = current.cloned().unwrap_or_else(|| Tagging { tag_set: vec![] }); + + tagging.tag_set.retain(|tag| match item_name(tag, prefix) { + Some(name) => !self.items.contains_key(name) && !self.removals.contains(name), + None => true, + }); + + let merged = tagging.tag_set.iter().filter(|tag| item_name(tag, prefix).is_some()).count() + self.items.len(); + if merged > MAX_METADATA_COUNT { + return Err(SwiftError::BadRequest(format!( + "Too many metadata headers: {} (max: {})", + merged, MAX_METADATA_COUNT + ))); + } + + for (name, value) in &self.items { + tagging.tag_set.push(Tag { + key: Some(format!("{}{}", prefix, name)), + value: Some(value.clone()), + }); + } + + Ok(tagging) + } +} + +/// The metadata item name a tag carries, or `None` if the tag does not belong +/// to this namespace. +fn item_name<'a>(tag: &'a Tag, prefix: &str) -> Option<&'a str> { + tag.key.as_deref()?.strip_prefix(prefix) +} + +#[cfg(test)] +mod tests { + use super::*; + use axum::http::{HeaderName, HeaderValue}; + + fn headers(pairs: &[(&str, &str)]) -> HeaderMap { + let mut map = HeaderMap::new(); + for (name, value) in pairs { + map.insert( + HeaderName::from_bytes(name.as_bytes()).expect("test header name should parse"), + HeaderValue::from_str(value).expect("test header value should parse"), + ); + } + map + } + + fn tags(pairs: &[(&str, &str)]) -> Tagging { + Tagging { + tag_set: pairs + .iter() + .map(|(key, value)| Tag { + key: Some((*key).to_string()), + value: Some((*value).to_string()), + }) + .collect(), + } + } + + fn tag_value<'a>(tagging: &'a Tagging, key: &str) -> Option<&'a str> { + tagging + .tag_set + .iter() + .find(|tag| tag.key.as_deref() == Some(key)) + .and_then(|tag| tag.value.as_deref()) + } + + #[test] + fn from_headers_collects_items_and_ignores_unrelated_headers() { + let update = MetadataUpdate::from_container_headers(&headers(&[ + ("x-container-meta-color", "blue"), + ("x-container-read", ".r:*"), + ("content-type", "text/plain"), + ])); + + assert_eq!(update, MetadataUpdate::default().set("color", "blue")); + } + + #[test] + fn from_headers_lowercases_item_names() { + // `http` normalizes header names on the way in, so the mixed case a + // client sends is already gone by the time a handler sees it; the + // lowercasing here is what makes a directly built update match. + let update = MetadataUpdate::from_container_headers(&headers(&[("X-Container-Meta-Color", "Blue")])); + + assert_eq!(update, MetadataUpdate::default().set("COLOR", "Blue")); + } + + #[test] + fn empty_value_is_a_removal() { + let update = MetadataUpdate::from_container_headers(&headers(&[("x-container-meta-color", "")])); + + assert_eq!(update, MetadataUpdate::default().remove("color")); + } + + #[test] + fn remove_header_drops_the_item() { + let update = MetadataUpdate::from_account_headers(&headers(&[("x-remove-account-meta-temp-url-key", "x")])); + + assert_eq!(update, MetadataUpdate::default().remove("temp-url-key")); + } + + #[test] + fn remove_header_wins_over_a_value_for_the_same_item() { + let update = MetadataUpdate::from_container_headers(&headers(&[ + ("x-container-meta-color", "blue"), + ("x-remove-container-meta-color", "x"), + ])); + + assert_eq!(update, MetadataUpdate::default().remove("color")); + } + + #[test] + fn a_removal_header_is_not_mistaken_for_an_item() { + // "x-remove-container-meta-color" must not also parse as the item + // "remove-container-meta-color" or similar. + let update = MetadataUpdate::from_container_headers(&headers(&[("x-remove-container-meta-color", "x")])); + + assert!(update.items.is_empty(), "a removal header must not set an item"); + } + + #[test] + fn a_request_with_no_metadata_headers_is_empty() { + assert!(MetadataUpdate::from_container_headers(&headers(&[("x-container-read", ".r:*")])).is_empty()); + assert!(MetadataUpdate::default().is_empty()); + assert!(!MetadataUpdate::default().remove("color").is_empty()); + } + + #[test] + fn apply_preserves_items_the_update_does_not_name() { + let current = tags(&[ + ("swift-meta-color", "blue"), + ("swift-meta-season", "summer"), + ("swift-acl-read", ".r:*"), + ("swift-versions-location", "archive"), + ("unrelated-s3-tag", "keep"), + ]); + + let merged = MetadataUpdate::default() + .set("mood", "calm") + .apply_to_tags(Some(¤t), CONTAINER_META_TAG_PREFIX) + .expect("merge should be accepted"); + + assert_eq!(tag_value(&merged, "swift-meta-color"), Some("blue")); + assert_eq!(tag_value(&merged, "swift-meta-season"), Some("summer")); + assert_eq!(tag_value(&merged, "swift-meta-mood"), Some("calm")); + assert_eq!(tag_value(&merged, "swift-acl-read"), Some(".r:*")); + assert_eq!(tag_value(&merged, "swift-versions-location"), Some("archive")); + assert_eq!(tag_value(&merged, "unrelated-s3-tag"), Some("keep")); + } + + #[test] + fn apply_overwrites_a_named_item_exactly_once() { + let current = tags(&[("swift-meta-color", "blue")]); + + let merged = MetadataUpdate::default() + .set("color", "red") + .apply_to_tags(Some(¤t), CONTAINER_META_TAG_PREFIX) + .expect("merge should be accepted"); + + assert_eq!(merged.tag_set.len(), 1, "overwriting must not duplicate the tag"); + assert_eq!(tag_value(&merged, "swift-meta-color"), Some("red")); + } + + #[test] + fn apply_drops_only_the_removed_item() { + let current = tags(&[ + ("swift-meta-color", "blue"), + ("swift-meta-season", "summer"), + ("swift-acl-read", ".r:*"), + ]); + + let merged = MetadataUpdate::default() + .remove("color") + .apply_to_tags(Some(¤t), CONTAINER_META_TAG_PREFIX) + .expect("merge should be accepted"); + + assert_eq!(tag_value(&merged, "swift-meta-color"), None); + assert_eq!(tag_value(&merged, "swift-meta-season"), Some("summer")); + assert_eq!(tag_value(&merged, "swift-acl-read"), Some(".r:*")); + } + + #[test] + fn apply_to_an_untagged_bucket_starts_from_nothing() { + let merged = MetadataUpdate::default() + .set("color", "blue") + .apply_to_tags(None, ACCOUNT_META_TAG_PREFIX) + .expect("merge should be accepted"); + + assert_eq!(tag_value(&merged, "swift-account-meta-color"), Some("blue")); + } + + #[test] + fn apply_caps_the_merged_item_count_not_the_request() { + let current = Tagging { + tag_set: (0..MAX_METADATA_COUNT) + .map(|i| Tag { + key: Some(format!("{}item{}", CONTAINER_META_TAG_PREFIX, i)), + value: Some("v".to_string()), + }) + .collect(), + }; + + // One more item than the container already stores: rejected, even + // though the request itself carries a single header. + let err = MetadataUpdate::default() + .set("overflow", "v") + .apply_to_tags(Some(¤t), CONTAINER_META_TAG_PREFIX) + .expect_err("exceeding the item cap must be rejected"); + assert!(matches!(err, SwiftError::BadRequest(_)), "expected BadRequest, got {err:?}"); + + // Overwriting an item already counted stays at the cap. + MetadataUpdate::default() + .set("item0", "v2") + .apply_to_tags(Some(¤t), CONTAINER_META_TAG_PREFIX) + .expect("overwriting an existing item must not trip the cap"); + } + + #[test] + fn validate_rejects_oversized_values() { + let err = MetadataUpdate::default() + .set("color", &"b".repeat(MAX_METADATA_VALUE_SIZE + 1)) + .validate() + .expect_err("an oversized value must be rejected"); + assert!(matches!(err, SwiftError::BadRequest(_)), "expected BadRequest, got {err:?}"); + + MetadataUpdate::default() + .set("color", &"b".repeat(MAX_METADATA_VALUE_SIZE)) + .validate() + .expect("a value at the limit must be accepted"); + } +} diff --git a/crates/protocols/src/swift/mod.rs b/crates/protocols/src/swift/mod.rs index 881b8fd4f..14d07559a 100644 --- a/crates/protocols/src/swift/mod.rs +++ b/crates/protocols/src/swift/mod.rs @@ -44,6 +44,7 @@ pub mod expiration; pub mod expiration_worker; pub mod formpost; pub mod handler; +pub mod metadata_update; pub mod object; pub mod quota; pub mod router; @@ -57,6 +58,7 @@ pub mod types; pub mod versioning; pub use errors::{SwiftError, SwiftResult}; +pub use metadata_update::MetadataUpdate; pub use router::{SwiftRoute, SwiftRouter}; /// Maximum number of metadata headers allowed per resource (Swift standard) @@ -71,9 +73,10 @@ pub(crate) const MAX_METADATA_VALUE_SIZE: usize = 256; /// - Total number of metadata entries doesn't exceed MAX_METADATA_COUNT /// - Individual metadata values don't exceed MAX_METADATA_VALUE_SIZE /// -/// Applies to object, container and account metadata alike: all three are -/// persisted, and container/account metadata additionally lands in the -/// bucket metadata file that every later config write rewrites in full. +/// This is the object-metadata form, where a POST replaces the whole set and +/// the request is therefore the complete result. Account and container POSTs +/// are additive, so their count has to be measured against the merged state +/// instead — see [`MetadataUpdate::apply_to_tags`]. /// /// Returns error if limits are exceeded. pub(crate) fn validate_metadata(metadata: &std::collections::HashMap) -> SwiftResult<()> { diff --git a/crates/protocols/src/swift/storage_api.rs b/crates/protocols/src/swift/storage_api.rs index 70e7b052f..8381b8c64 100644 --- a/crates/protocols/src/swift/storage_api.rs +++ b/crates/protocols/src/swift/storage_api.rs @@ -88,6 +88,10 @@ pub(crate) async fn get_swift_bucket_metadata(bucket: &str) -> SwiftStorageResul /// disk-truth reloads. Peers are then told to reload, matching what the S3 /// handlers do after a config write. /// +/// A rewrite may also refuse the update outright, for limits that can only be +/// judged against the state being merged into. That verdict is the client's +/// answer, so it is returned as-is rather than folded into a storage error. +/// /// Storage failures are logged in full and reported to the client as a /// generic error: these now carry real disk and quorum detail, which does not /// belong in a Swift response body. The one exception is an unreadable @@ -95,8 +99,12 @@ pub(crate) async fn get_swift_bucket_metadata(bucket: &str) -> SwiftStorageResul /// to act on it. pub(crate) async fn update_swift_bucket_tagging(bucket: String, rewrite: F) -> SwiftResult<()> where - F: FnOnce(Option<&Tagging>) -> Tagging + Send, + F: FnOnce(Option<&Tagging>) -> SwiftResult + Send, { + // Carries a rewrite's refusal back out past the storage error the config + // write has to fail with to abort the transaction. + let mut rejected = None; + let result = update_config_with(&bucket, BUCKET_TAGGING_CONFIG, |bm| { // Merging onto an unparseable tag set would silently drop every tag // the bucket has — including the container ACL and versioning tags — @@ -106,7 +114,14 @@ where return Err(SwiftStorageError::other(UNREADABLE_TAGGING_SENTINEL)); } - let tagging = rewrite(bm.tagging_config.as_ref()); + let tagging = match rewrite(bm.tagging_config.as_ref()) { + Ok(tagging) => tagging, + Err(err) => { + rejected = Some(err); + return Err(SwiftStorageError::other("swift: tagging rewrite rejected the update")); + } + }; + if tagging.tag_set.is_empty() { Ok(Vec::new()) } else { @@ -118,6 +133,12 @@ where }) .await; + // Nothing was written, but that is the rewrite's own decision about the + // request rather than a storage fault, so it is not logged as one. + if let Some(err) = rejected { + return Err(err); + } + if let Err(err) = result { let unreadable = err.to_string().contains(UNREADABLE_TAGGING_SENTINEL); tracing::error!( diff --git a/crates/protocols/tests/swift_metadata_persistence.rs b/crates/protocols/tests/swift_metadata_persistence.rs index 25a7e42ad..86f205a45 100644 --- a/crates/protocols/tests/swift_metadata_persistence.rs +++ b/crates/protocols/tests/swift_metadata_persistence.rs @@ -16,15 +16,20 @@ //! metadata file, not just the in-memory cache. The metadata has to survive //! the disk-truth reloads performed by peer LoadBucketMetadata notifications //! and the periodic refresh loop — and, transitively, a process restart. +//! +//! Durability is what makes the *content* of those writes matter, so this file +//! also covers Swift's additive account/container POST semantics: an item the +//! request does not name keeps its stored value, and removal is explicit. The +//! two are tested together because a reload is the only way to tell a real +//! merge from one that happened to look right in the cache. #![cfg(feature = "swift")] use std::collections::HashMap; use rustfs_credentials::Credentials; -use rustfs_protocols::swift::SwiftError; use rustfs_protocols::swift::container::{ContainerMapper, update_container_metadata}; -use rustfs_protocols::swift::{account, container}; +use rustfs_protocols::swift::{MetadataUpdate, SwiftError, account, container}; use rustfs_test_utils::TestECStoreEnv; use serde_json::json; use sha2::{Digest, Sha256}; @@ -97,6 +102,8 @@ async fn swift_metadata_writes_are_durable() { posts_survive_disk_truth_reload(&env).await; tag_writers_preserve_each_others_state(&env).await; + unrelated_account_posts_preserve_the_tempurl_key().await; + acl_only_posts_leave_container_metadata_alone(&env).await; versioning_writes_reject_missing_containers().await; } @@ -109,11 +116,14 @@ async fn posts_survive_disk_truth_reload(env: &TestECStoreEnv) { let bucket = ContainerMapper::default().swift_to_s3_bucket(swift_container, project_id); env.make_bucket(&bucket, false).await; - let mut metadata = HashMap::new(); - metadata.insert("color".to_string(), "blue".to_string()); - update_container_metadata(&swift_account, swift_container, &credentials, metadata) - .await - .expect("container metadata POST should succeed"); + update_container_metadata( + &swift_account, + swift_container, + &credentials, + MetadataUpdate::default().set("color", "blue"), + ) + .await + .expect("container metadata POST should succeed"); reload_bucket_metadata_from_disk(&bucket).await; @@ -124,22 +134,47 @@ async fn posts_survive_disk_truth_reload(env: &TestECStoreEnv) { "container metadata POST must survive a disk-truth metadata reload" ); - // A follow-up POST replaces the Swift metadata and that replacement must - // survive a reload too (the rewrite merges against disk state, so the - // previous value must actually be gone). - let mut metadata = HashMap::new(); - metadata.insert("season".to_string(), "summer".to_string()); - update_container_metadata(&swift_account, swift_container, &credentials, metadata) - .await - .expect("second container metadata POST should succeed"); + // A follow-up POST names a different item. Swift POSTs are additive, so + // the new item lands and the first one stays — and both are still there + // after a reload, which is what proves the merge ran against disk state + // rather than looking right in the cache. + update_container_metadata( + &swift_account, + swift_container, + &credentials, + MetadataUpdate::default().set("season", "summer"), + ) + .await + .expect("second container metadata POST should succeed"); reload_bucket_metadata_from_disk(&bucket).await; let container_meta = persisted_container_metadata(&bucket).await; assert_eq!(container_meta.get("season").map(String::as_str), Some("summer")); + assert_eq!( + container_meta.get("color").map(String::as_str), + Some("blue"), + "a POST that does not name an item must not delete it" + ); + + // X-Remove-Container-Meta-Color is the deletion path, and it has to be + // durable in the other direction: the removed item must not come back + // from the persisted tags on reload. + update_container_metadata(&swift_account, swift_container, &credentials, MetadataUpdate::default().remove("color")) + .await + .expect("container metadata removal should succeed"); + + reload_bucket_metadata_from_disk(&bucket).await; + + let container_meta = persisted_container_metadata(&bucket).await; assert!( !container_meta.contains_key("color"), - "replaced container metadata must not resurrect on reload" + "an explicitly removed item must not resurrect on reload" + ); + assert_eq!( + container_meta.get("season").map(String::as_str), + Some("summer"), + "removing one item must not disturb the others" ); // --- Container versioning POST (X-Versions-Location) --- @@ -163,11 +198,13 @@ async fn posts_survive_disk_truth_reload(env: &TestECStoreEnv) { ); // --- Account metadata POST (TempURL keys etc.) --- - let mut account_meta = HashMap::new(); - account_meta.insert("temp-url-key".to_string(), "s3cr3t".to_string()); - account::update_account_metadata(&swift_account, &account_meta, &Some(credentials.clone())) - .await - .expect("account metadata POST should succeed"); + account::update_account_metadata( + &swift_account, + &MetadataUpdate::default().set("temp-url-key", "s3cr3t"), + &Some(credentials.clone()), + ) + .await + .expect("account metadata POST should succeed"); reload_bucket_metadata_from_disk(&account_metadata_bucket_name(&swift_account)).await; @@ -196,9 +233,7 @@ async fn tag_writers_preserve_each_others_state(env: &TestECStoreEnv) { env.make_bucket(&ContainerMapper::default().swift_to_s3_bucket(archive, project_id), false) .await; - let mut metadata = HashMap::new(); - metadata.insert("color".to_string(), "blue".to_string()); - update_container_metadata(&swift_account, container, &credentials, metadata) + update_container_metadata(&swift_account, container, &credentials, MetadataUpdate::default().set("color", "blue")) .await .expect("container metadata POST should succeed"); container::enable_versioning(&swift_account, container, archive, &credentials) @@ -252,6 +287,111 @@ async fn tag_writers_preserve_each_others_state(env: &TestECStoreEnv) { assert!(!acl.read.is_empty(), "disable_versioning must not disturb the ACL"); } +/// The failure additive POSTs exist to prevent. Account metadata holds the +/// TempURL signing key, so a POST that replaced the whole set — setting a +/// quota, say — would delete that key and permanently invalidate every +/// outstanding TempURL and FormPost signature for the account. Durable +/// storage made that loss permanent instead of a cache blip, so the check +/// runs against reloaded disk state. +/// +/// Relies on the ambient store the caller's `TestECStoreEnv` published; the +/// account metadata bucket is created by the write path itself. +async fn unrelated_account_posts_preserve_the_tempurl_key() { + let project_id = "swiftmergeproj"; + let swift_account = format!("AUTH_{project_id}"); + let credentials = Some(keystone_credentials(project_id)); + let account_bucket = account_metadata_bucket_name(&swift_account); + + account::update_account_metadata(&swift_account, &MetadataUpdate::default().set("temp-url-key", "s3cr3t"), &credentials) + .await + .expect("TempURL key POST should succeed"); + + // A later POST that has nothing to do with the TempURL key. + account::update_account_metadata(&swift_account, &MetadataUpdate::default().set("quota-bytes", "100"), &credentials) + .await + .expect("quota POST should succeed"); + + reload_bucket_metadata_from_disk(&account_bucket).await; + + let loaded = account::get_account_metadata(&swift_account, &None) + .await + .expect("account metadata should load"); + assert_eq!( + loaded.get("temp-url-key").map(String::as_str), + Some("s3cr3t"), + "an unrelated account POST must not delete the TempURL signing key" + ); + assert_eq!(loaded.get("quota-bytes").map(String::as_str), Some("100")); + + // And the lookup that actually breaks when the key is lost still resolves. + assert_eq!( + account::get_tempurl_key(&swift_account, &None) + .await + .expect("TempURL key should load") + .as_deref(), + Some("s3cr3t"), + "TempURL signature validation must still find the key" + ); + + // X-Remove-Account-Meta-Quota-Bytes drops that item and nothing else. + account::update_account_metadata(&swift_account, &MetadataUpdate::default().remove("quota-bytes"), &credentials) + .await + .expect("account metadata removal should succeed"); + + reload_bucket_metadata_from_disk(&account_bucket).await; + + let loaded = account::get_account_metadata(&swift_account, &None) + .await + .expect("account metadata should load"); + assert!( + !loaded.contains_key("quota-bytes"), + "an explicitly removed item must not resurrect on reload" + ); + assert_eq!( + loaded.get("temp-url-key").map(String::as_str), + Some("s3cr3t"), + "removing one item must not disturb the TempURL key" + ); +} + +/// An ACL-only or versioning-only container POST carries no `X-Container-Meta-*` +/// header, but the handler still runs the metadata step on every POST. That +/// step must leave stored metadata alone rather than treating "named nothing" +/// as "wants everything gone". +async fn acl_only_posts_leave_container_metadata_alone(env: &TestECStoreEnv) { + let project_id = "swiftaclonlyproj"; + let swift_account = format!("AUTH_{project_id}"); + let credentials = keystone_credentials(project_id); + let container = "gallery"; + let bucket = ContainerMapper::default().swift_to_s3_bucket(container, project_id); + env.make_bucket(&bucket, false).await; + + update_container_metadata(&swift_account, container, &credentials, MetadataUpdate::default().set("color", "blue")) + .await + .expect("container metadata POST should succeed"); + + // What the handler does for `POST … -H 'X-Container-Read: .r:*'`: set the + // ACL, then run the metadata step with an update that names no item. + container::set_container_acl(&swift_account, container, Some(".r:*"), None, &credentials) + .await + .expect("ACL-only POST should succeed"); + update_container_metadata(&swift_account, container, &credentials, MetadataUpdate::default()) + .await + .expect("the metadata step of an ACL-only POST should succeed"); + + reload_bucket_metadata_from_disk(&bucket).await; + + assert_eq!( + persisted_container_metadata(&bucket).await.get("color").map(String::as_str), + Some("blue"), + "an ACL-only POST must not wipe X-Container-Meta-*" + ); + let acl = container::get_container_acl(&swift_account, container, &credentials) + .await + .expect("container ACL should load"); + assert!(!acl.read.is_empty(), "the ACL that POST set must be stored"); +} + /// A container that does not exist must not get metadata persisted for it: /// the metadata loader turns "nothing on disk" into a fresh default, so an /// unguarded rewrite would create an orphan metadata file and cache a @@ -270,6 +410,16 @@ async fn versioning_writes_reject_missing_containers() { "expected NotFound for a missing container, got {err:?}" ); + // Naming no item skips the persisted write, but must not skip the + // existence check that turns a POST to a missing container into a 404. + let err = update_container_metadata(&swift_account, missing, &credentials, MetadataUpdate::default()) + .await + .expect_err("a metadata POST to a missing container must fail"); + assert!( + matches!(err, SwiftError::NotFound(_)), + "expected NotFound for a missing container, got {err:?}" + ); + let bucket = ContainerMapper::default().swift_to_s3_bucket(missing, project_id); assert!( get_bucket_metadata(&bucket).await.is_err(), @@ -291,8 +441,7 @@ async fn account_metadata_write_rejects_foreign_and_anonymous_callers() { let victim_account = "AUTH_victimproject"; let attacker_credentials = keystone_credentials("attackerproject"); - let mut poisoned = HashMap::new(); - poisoned.insert("temp-url-key".to_string(), "attacker-key".to_string()); + let poisoned = MetadataUpdate::default().set("temp-url-key", "attacker-key"); let err = account::update_account_metadata(victim_account, &poisoned, &Some(attacker_credentials)) .await