diff --git a/crates/ecstore/src/bucket/metadata.rs b/crates/ecstore/src/bucket/metadata.rs index ff9d16890..740c52d26 100644 --- a/crates/ecstore/src/bucket/metadata.rs +++ b/crates/ecstore/src/bucket/metadata.rs @@ -1210,6 +1210,7 @@ impl BucketMetadata { "Failed to parse bucket metadata config" ); } + self.notification_config = None; if !self.notification_config_xml.is_empty() && let Err(e) = deserialize::(&self.notification_config_xml) .map(|c| self.notification_config = Some(c)) @@ -1224,6 +1225,7 @@ impl BucketMetadata { "Failed to parse bucket metadata config" ); } + self.lifecycle_config = None; if !self.lifecycle_config_xml.is_empty() && let Err(e) = deserialize::(&self.lifecycle_config_xml).map(|c| self.lifecycle_config = Some(c)) @@ -1238,6 +1240,7 @@ impl BucketMetadata { "Failed to parse bucket metadata config" ); } + self.object_lock_config = None; if !self.object_lock_config_xml.is_empty() && let Err(e) = deserialize::(&self.object_lock_config_xml).map(|c| self.object_lock_config = Some(c)) @@ -1267,6 +1270,7 @@ impl BucketMetadata { "Failed to parse bucket metadata config" ); } + self.sse_config = None; if !self.encryption_config_xml.is_empty() && let Err(e) = deserialize::(&self.encryption_config_xml).map(|c| self.sse_config = Some(c)) @@ -1281,6 +1285,7 @@ impl BucketMetadata { "Failed to parse bucket metadata config" ); } + self.tagging_config = None; if !self.tagging_config_xml.is_empty() && let Err(e) = deserialize::(&self.tagging_config_xml).map(|c| self.tagging_config = Some(c)) { @@ -1346,6 +1351,7 @@ impl BucketMetadata { } else { self.bucket_target_config = Some(BucketTargets::default()); } + self.cors_config = None; if !self.cors_config_xml.is_empty() && let Err(e) = deserialize::(&self.cors_config_xml).map(|c| self.cors_config = Some(c)) { @@ -1359,6 +1365,7 @@ impl BucketMetadata { "Failed to parse bucket metadata config" ); } + self.logging_config = None; if !self.logging_config_xml.is_empty() && let Err(e) = deserialize::(&self.logging_config_xml).map(|c| self.logging_config = Some(c)) { @@ -1372,6 +1379,7 @@ impl BucketMetadata { "Failed to parse bucket metadata config" ); } + self.website_config = None; if !self.website_config_xml.is_empty() && let Err(e) = deserialize::(&self.website_config_xml).map(|c| self.website_config = Some(c)) { @@ -1385,6 +1393,7 @@ impl BucketMetadata { "Failed to parse bucket metadata config" ); } + self.accelerate_config = None; if !self.accelerate_config_xml.is_empty() && let Err(e) = deserialize::(&self.accelerate_config_xml).map(|c| self.accelerate_config = Some(c)) @@ -1399,6 +1408,7 @@ impl BucketMetadata { "Failed to parse bucket metadata config" ); } + self.request_payment_config = None; if !self.request_payment_config_xml.is_empty() && let Err(e) = deserialize::(&self.request_payment_config_xml) .map(|c| self.request_payment_config = Some(c)) @@ -1413,6 +1423,7 @@ impl BucketMetadata { "Failed to parse bucket metadata config" ); } + self.public_access_block_config = None; if !self.public_access_block_config_xml.is_empty() && let Err(e) = deserialize::(&self.public_access_block_config_xml) .map(|c| self.public_access_block_config = Some(c)) @@ -1973,6 +1984,156 @@ mod test { assert_eq!(bm.lifecycle_config_xml, malformed_xml, "raw bytes are retained"); } + const REPARSE_XML_FIXTURES: [(&str, &[u8]); 13] = [ + (BUCKET_NOTIFICATION_CONFIG, br#""#), + ( + BUCKET_LIFECYCLE_CONFIG, + br#"keepEnabled30"#, + ), + ( + OBJECT_LOCK_CONFIG, + br#"EnabledGOVERNANCE7"#, + ), + (BUCKET_VERSIONING_CONFIG, br#"Enabled"#), + ( + BUCKET_SSECONFIG, + br#"AES256"#, + ), + (BUCKET_TAGGING_CONFIG, br#"teamblue"#), + ( + BUCKET_REPLICATION_CONFIG, + br#"arn:aws:iam::123456789012:role/replication-rolecopyEnableddocuments/arn:aws:s3:::destination-bucket"#, + ), + ( + BUCKET_CORS_CONFIG, + br#"https://example.testGET"#, + ), + (BUCKET_LOGGING_CONFIG, br#""#), + (BUCKET_WEBSITE_CONFIG, br#"index.html"#), + (BUCKET_ACCELERATE_CONFIG, br#"Enabled"#), + (BUCKET_REQUEST_PAYMENT_CONFIG, br#"BucketOwner"#), + ( + BUCKET_PUBLIC_ACCESS_BLOCK_CONFIG, + br#"truetruetruetrue"#, + ), + ]; + + fn reparse_xml_state(bm: &BucketMetadata, config: &str) -> &'static str { + fn state(raw: &[u8], parsed: &Option) -> &'static str { + match ConfigState::of(raw, parsed) { + ConfigState::Absent => "absent", + ConfigState::Valid(_) => "valid", + ConfigState::Unreadable { .. } => "unreadable", + } + } + match config { + BUCKET_NOTIFICATION_CONFIG => state(&bm.notification_config_xml, &bm.notification_config), + BUCKET_LIFECYCLE_CONFIG => state(&bm.lifecycle_config_xml, &bm.lifecycle_config), + OBJECT_LOCK_CONFIG => state(&bm.object_lock_config_xml, &bm.object_lock_config), + BUCKET_VERSIONING_CONFIG => state(&bm.versioning_config_xml, &bm.versioning_config), + BUCKET_SSECONFIG => state(&bm.encryption_config_xml, &bm.sse_config), + BUCKET_TAGGING_CONFIG => state(&bm.tagging_config_xml, &bm.tagging_config), + BUCKET_REPLICATION_CONFIG => state(&bm.replication_config_xml, &bm.replication_config), + BUCKET_CORS_CONFIG => state(&bm.cors_config_xml, &bm.cors_config), + BUCKET_LOGGING_CONFIG => state(&bm.logging_config_xml, &bm.logging_config), + BUCKET_WEBSITE_CONFIG => state(&bm.website_config_xml, &bm.website_config), + BUCKET_ACCELERATE_CONFIG => state(&bm.accelerate_config_xml, &bm.accelerate_config), + BUCKET_REQUEST_PAYMENT_CONFIG => state(&bm.request_payment_config_xml, &bm.request_payment_config), + BUCKET_PUBLIC_ACCESS_BLOCK_CONFIG => state(&bm.public_access_block_config_xml, &bm.public_access_block_config), + _ => panic!("unknown XML config: {config}"), + } + } + + // A persisted payload can change without going through the request decoder. + fn replace_xml_raw(bm: &mut BucketMetadata, config: &str, raw: Vec) { + match config { + BUCKET_NOTIFICATION_CONFIG => bm.notification_config_xml = raw, + BUCKET_LIFECYCLE_CONFIG => bm.lifecycle_config_xml = raw, + OBJECT_LOCK_CONFIG => bm.object_lock_config_xml = raw, + BUCKET_VERSIONING_CONFIG => bm.versioning_config_xml = raw, + BUCKET_SSECONFIG => bm.encryption_config_xml = raw, + BUCKET_TAGGING_CONFIG => bm.tagging_config_xml = raw, + BUCKET_REPLICATION_CONFIG => bm.replication_config_xml = raw, + BUCKET_CORS_CONFIG => bm.cors_config_xml = raw, + BUCKET_LOGGING_CONFIG => bm.logging_config_xml = raw, + BUCKET_WEBSITE_CONFIG => bm.website_config_xml = raw, + BUCKET_ACCELERATE_CONFIG => bm.accelerate_config_xml = raw, + BUCKET_REQUEST_PAYMENT_CONFIG => bm.request_payment_config_xml = raw, + BUCKET_PUBLIC_ACCESS_BLOCK_CONFIG => bm.public_access_block_config_xml = raw, + _ => panic!("unknown XML config: {config}"), + } + } + + #[test] + fn reparse_xml_rejects_stale_views_after_corruption() { + let corrupt = b"AES256"#; + let corrupt = b"AES256"#; + persist_bucket_with_raw_config(&sys, &dirs, bucket, BUCKET_SSECONFIG, valid).await; + sys.get_sse_config(bucket).await.expect("valid encryption must be readable"); + + let updated = sys + .update_config_with(bucket, BUCKET_SSECONFIG, |bm| { + assert!(bm.sse_config.is_some(), "rewrite must start with parsed encryption"); + Ok(Vec::new()) + }) + .await + .expect("removing encryption must persist"); + + assert_eq!( + sys.get_sse_config(bucket) + .await + .expect_err("old cached encryption must be gone"), + Error::ConfigNotFound + ); + let (cached, _) = sys.get_config(bucket).await.expect("bucket remains readable"); + assert!(cached.encryption_config_xml.is_empty()); + assert_eq!(cached.encryption_config_updated_at, updated); + + sys.metadata_map.write().await.clear(); + assert_eq!( + sys.get_sse_config(bucket) + .await + .expect_err("removed encryption must stay absent after reload"), + Error::ConfigNotFound + ); + let disk = sys + .get_config_from_disk(bucket) + .await + .expect("read removed encryption from disk"); + assert!(disk.encryption_config_xml.is_empty()); + assert_eq!(disk.encryption_config_updated_at, updated); + } + /// rustfs/backlog#1734: a read-modify-write of a stored config that cannot /// be parsed must be refused before `mutate` runs. Otherwise `mutate` sees /// the unreadable config as absent, rebuilds it from nothing, and the