From 9dfeffc4c17b44dd26b27914eb226af141389cf1 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Fri, 3 Jul 2026 00:35:37 +0800 Subject: [PATCH] test: prune redundant cases and add high-risk coverage across crates (#4208) Remove or consolidate 57 test cases that cannot catch regressions (literal-constant asserts, construct-then-assert, derived-serde round-trips, near-duplicate env/getter matrices) in common, config, iam, madmin, and object-capacity, keeping all wire-format and error-path guards. Add 13 tests for previously uncovered high-risk behavior: filemeta version-sort determinism and merge resilience to garbage headers, zip extraction path-traversal rejection and exact limit boundaries, JWT tampered-signature rejection, and the bytes variant of dual-key (rustfs/minio) metadata fallback and precedence. Test-only change; no production code touched. Co-authored-by: Claude Fable 5 --- crates/common/src/last_minute.rs | 367 ------------------ crates/config/src/constants/app.rs | 79 ---- crates/config/src/constants/capacity.rs | 16 - crates/config/src/constants/workload.rs | 7 - crates/crypto/src/jwt/tests.rs | 22 ++ crates/filemeta/src/filemeta/version.rs | 70 ++++ crates/iam/src/manager.rs | 27 -- crates/madmin/src/user.rs | 111 ------ .../object-capacity/src/capacity_manager.rs | 191 +++------ crates/utils/src/http/metadata_compat.rs | 35 ++ crates/zip/src/lib.rs | 79 ++++ 11 files changed, 256 insertions(+), 748 deletions(-) diff --git a/crates/common/src/last_minute.rs b/crates/common/src/last_minute.rs index 97a4f376e..52cbae29f 100644 --- a/crates/common/src/last_minute.rs +++ b/crates/common/src/last_minute.rs @@ -200,26 +200,6 @@ mod tests { use super::*; use std::time::Duration; - #[test] - fn test_acc_elem_default() { - let elem = AccElem::default(); - assert_eq!(elem.total, 0); - assert_eq!(elem.size, 0); - assert_eq!(elem.n, 0); - } - - #[test] - fn test_acc_elem_add_single_duration() { - let mut elem = AccElem::default(); - let duration = Duration::from_secs(5); - - elem.add(&duration); - - assert_eq!(elem.total, 5); - assert_eq!(elem.n, 1); - assert_eq!(elem.size, 0); // size is not modified by add - } - #[test] fn test_acc_elem_add_multiple_durations() { let mut elem = AccElem::default(); @@ -233,17 +213,6 @@ mod tests { assert_eq!(elem.size, 0); } - #[test] - fn test_acc_elem_add_zero_duration() { - let mut elem = AccElem::default(); - let duration = Duration::from_secs(0); - - elem.add(&duration); - - assert_eq!(elem.total, 0); - assert_eq!(elem.n, 1); - } - #[test] fn test_acc_elem_add_subsecond_duration() { let mut elem = AccElem::default(); @@ -256,18 +225,6 @@ mod tests { assert_eq!(elem.n, 1); } - #[test] - fn test_acc_elem_merge_empty_elements() { - let mut elem1 = AccElem::default(); - let elem2 = AccElem::default(); - - elem1.merge(&elem2); - - assert_eq!(elem1.total, 0); - assert_eq!(elem1.size, 0); - assert_eq!(elem1.n, 0); - } - #[test] fn test_acc_elem_merge_with_data() { let mut elem1 = AccElem { @@ -288,34 +245,6 @@ mod tests { assert_eq!(elem1.n, 5); } - #[test] - fn test_acc_elem_merge_one_empty() { - let mut elem1 = AccElem { - total: 10, - size: 100, - n: 2, - }; - let elem2 = AccElem::default(); - - elem1.merge(&elem2); - - assert_eq!(elem1.total, 10); - assert_eq!(elem1.size, 100); - assert_eq!(elem1.n, 2); - } - - #[test] - fn test_acc_elem_avg_with_data() { - let elem = AccElem { - total: 15, - size: 0, - n: 3, - }; - - let avg = elem.avg(); - assert_eq!(avg, Duration::from_secs(5)); // 15 / 3 = 5 - } - #[test] fn test_acc_elem_avg_zero_count() { let elem = AccElem { @@ -328,14 +257,6 @@ mod tests { assert_eq!(avg, Duration::from_secs(0)); } - #[test] - fn test_acc_elem_avg_zero_total() { - let elem = AccElem { total: 0, size: 0, n: 5 }; - - let avg = elem.avg(); - assert_eq!(avg, Duration::from_secs(0)); - } - #[test] fn test_acc_elem_avg_rounding() { let elem = AccElem { @@ -348,39 +269,6 @@ mod tests { assert_eq!(avg, Duration::from_secs(3)); // 10 / 3 = 3 (integer division) } - #[test] - fn test_last_minute_latency_default() { - let latency = LastMinuteLatency::default(); - - assert_eq!(latency.totals.len(), 60); - assert_eq!(latency.last_sec, 0); - - // All elements should be default (empty) - for elem in &latency.totals { - assert_eq!(elem.total, 0); - assert_eq!(elem.size, 0); - assert_eq!(elem.n, 0); - } - } - - #[test] - fn test_last_minute_latency_forward_to_same_time() { - let mut latency = LastMinuteLatency { - last_sec: 100, - ..Default::default() - }; - - // Add some data to verify it's not cleared - latency.totals[0].total = 10; - latency.totals[0].n = 1; - - latency.forward_to(100); // Same time - - assert_eq!(latency.last_sec, 100); - assert_eq!(latency.totals[0].total, 10); // Data should remain - assert_eq!(latency.totals[0].n, 1); - } - #[test] fn test_last_minute_latency_forward_to_past_time() { let mut latency = LastMinuteLatency { @@ -442,24 +330,6 @@ mod tests { assert_eq!(latency.totals[42].total, 0); // Cleared during forward } - #[test] - fn test_last_minute_latency_add_all() { - let mut latency = LastMinuteLatency::default(); - let acc_elem = AccElem { - total: 15, - size: 100, - n: 3, - }; - - latency.add_all(1000, &acc_elem); - - assert_eq!(latency.last_sec, 1000); - let idx = 1000 % 60; // Should be 40 - assert_eq!(latency.totals[idx as usize].total, 15); - assert_eq!(latency.totals[idx as usize].size, 100); - assert_eq!(latency.totals[idx as usize].n, 3); - } - #[test] fn test_last_minute_latency_add_all_multiple() { let mut latency = LastMinuteLatency::default(); @@ -484,27 +354,6 @@ mod tests { assert_eq!(latency.totals[idx as usize].n, 6); // 2 + 4 } - #[test] - fn test_last_minute_latency_merge_same_time() { - let mut latency1 = LastMinuteLatency::default(); - let mut latency2 = LastMinuteLatency::default(); - - latency1.last_sec = 1000; - latency2.last_sec = 1000; - - // Add data to both - latency1.totals[0].total = 10; - latency1.totals[0].n = 2; - latency2.totals[0].total = 20; - latency2.totals[0].n = 3; - - let merged = latency1.merge(&latency2); - - assert_eq!(merged.last_sec, 1000); - assert_eq!(merged.totals[0].total, 30); // 10 + 20 - assert_eq!(merged.totals[0].n, 5); // 2 + 3 - } - #[test] fn test_last_minute_latency_merge_different_times() { let mut latency1 = LastMinuteLatency::default(); @@ -523,21 +372,6 @@ mod tests { assert_eq!(merged.totals[0].total, 30); } - #[test] - fn test_last_minute_latency_merge_empty() { - let mut latency1 = LastMinuteLatency::default(); - let latency2 = LastMinuteLatency::default(); - - let merged = latency1.merge(&latency2); - - assert_eq!(merged.last_sec, 0); - for elem in &merged.totals { - assert_eq!(elem.total, 0); - assert_eq!(elem.size, 0); - assert_eq!(elem.n, 0); - } - } - #[test] fn test_last_minute_latency_window_wraparound() { let mut latency = LastMinuteLatency::default(); @@ -557,117 +391,6 @@ mod tests { } } - #[test] - fn test_last_minute_latency_time_progression() { - let mut latency = LastMinuteLatency::default(); - - // Add data at time 1000 - latency.add_all( - 1000, - &AccElem { - total: 10, - size: 0, - n: 1, - }, - ); - - // Forward to time 1030 (30 seconds later) - latency.forward_to(1030); - - // Original data should still be there - let idx_1000 = 1000 % 60; - assert_eq!(latency.totals[idx_1000 as usize].total, 10); - - // Forward to time 1070 (70 seconds from original, > 60 seconds) - latency.forward_to(1070); - - // All data should be cleared due to large gap - for elem in &latency.totals { - assert_eq!(elem.total, 0); - assert_eq!(elem.n, 0); - } - } - - #[test] - fn test_last_minute_latency_realistic_scenario() { - let mut latency = LastMinuteLatency::default(); - let base_time = 1000u64; - - // Add data for exactly 60 seconds to fill the window - for i in 0..60 { - let current_time = base_time + i; - let duration_secs = i % 10 + 1; // Varying durations 1-10 seconds - let acc_elem = AccElem { - total: duration_secs, - size: 1024 * (i % 5 + 1), // Varying sizes - n: 1, - }; - - latency.add_all(current_time, &acc_elem); - } - - // Count non-empty slots after filling the window - let mut non_empty_count = 0; - let mut total_n = 0; - let mut total_sum = 0; - - for elem in &latency.totals { - if elem.n > 0 { - non_empty_count += 1; - total_n += elem.n; - total_sum += elem.total; - } - } - - // We should have exactly 60 non-empty slots (one for each second in the window) - assert_eq!(non_empty_count, 60); - assert_eq!(total_n, 60); // 60 data points total - assert!(total_sum > 0); - - // Test manual total calculation (get_total uses system time which interferes with test) - let mut manual_total = AccElem::default(); - for elem in &latency.totals { - manual_total.merge(elem); - } - assert_eq!(manual_total.n, 60); - assert_eq!(manual_total.total, total_sum); - } - - #[test] - fn test_acc_elem_clone_and_debug() { - let elem = AccElem { - total: 100, - size: 200, - n: 5, - }; - - let cloned = elem; - assert_eq!(elem.total, cloned.total); - assert_eq!(elem.size, cloned.size); - assert_eq!(elem.n, cloned.n); - - // Test Debug trait - let debug_str = format!("{elem:?}"); - assert!(debug_str.contains("100")); - assert!(debug_str.contains("200")); - assert!(debug_str.contains("5")); - } - - #[test] - fn test_last_minute_latency_clone() { - let mut latency = LastMinuteLatency { - last_sec: 1000, - ..Default::default() - }; - latency.totals[0].total = 100; - latency.totals[0].n = 5; - - let cloned = latency.clone(); - assert_eq!(latency.last_sec, cloned.last_sec); - assert_eq!(latency.totals[0].total, cloned.totals[0].total); - assert_eq!(latency.totals[0].n, cloned.totals[0].n); - } - #[test] fn test_edge_case_max_values() { let mut elem = AccElem { @@ -746,96 +469,6 @@ mod tests { assert_eq!(total.size, 600); assert_eq!(total.n, 6); } - - #[test] - fn test_window_index_calculation() { - // Test that window index calculation works correctly - let _latency = LastMinuteLatency::default(); - - let acc_elem = AccElem { total: 1, size: 1, n: 1 }; - - // Test various timestamps - let test_cases = [(0, 0), (1, 1), (59, 59), (60, 0), (61, 1), (119, 59), (120, 0)]; - - for (timestamp, expected_idx) in test_cases { - let mut test_latency = LastMinuteLatency::default(); - test_latency.add_all(timestamp, &acc_elem); - - assert_eq!( - test_latency.totals[expected_idx].n, 1, - "Failed for timestamp {timestamp} (expected index {expected_idx})" - ); - } - } - - #[test] - fn test_concurrent_safety_simulation() { - // Simulate concurrent access patterns - let mut latency = LastMinuteLatency::default(); - - // Use current time to ensure data doesn't get cleared by get_total - let current_time = SystemTime::now() - .duration_since(UNIX_EPOCH) - .expect("Time went backwards") - .as_secs(); - - // Simulate rapid additions within a 60-second window - for i in 0..1000 { - let acc_elem = AccElem { - total: (i % 10) + 1, // Ensure non-zero values - size: (i % 100) + 1, - n: 1, - }; - // Keep all timestamps within the current minute window - latency.add_all(current_time - (i % 60), &acc_elem); - } - - let total = latency.get_total(); - assert!(total.n > 0, "Total count should be greater than 0"); - assert!(total.total > 0, "Total time should be greater than 0"); - } - - #[test] - fn test_acc_elem_debug_format() { - let elem = AccElem { - total: 123, - size: 456, - n: 789, - }; - - let debug_str = format!("{elem:?}"); - assert!(debug_str.contains("123")); - assert!(debug_str.contains("456")); - assert!(debug_str.contains("789")); - } - - #[test] - fn test_large_values() { - let mut elem = AccElem::default(); - - // Test with large duration values - let large_duration = Duration::from_secs(u64::MAX / 2); - elem.add(&large_duration); - - assert_eq!(elem.total, u64::MAX / 2); - assert_eq!(elem.n, 1); - - // Test average calculation with large values - let avg = elem.avg(); - assert_eq!(avg, Duration::from_secs(u64::MAX / 2)); - } - - #[test] - fn test_zero_duration_handling() { - let mut elem = AccElem::default(); - - let zero_duration = Duration::from_secs(0); - elem.add(&zero_duration); - - assert_eq!(elem.total, 0); - assert_eq!(elem.n, 1); - assert_eq!(elem.avg(), Duration::from_secs(0)); - } } const SIZE_LAST_ELEM_MARKER: usize = 10; // Assumed marker size is 10, modify according to actual situation diff --git a/crates/config/src/constants/app.rs b/crates/config/src/constants/app.rs index 20932fc50..2ea351ff5 100644 --- a/crates/config/src/constants/app.rs +++ b/crates/config/src/constants/app.rs @@ -325,85 +325,6 @@ pub const MI_B: usize = 1024 * KI_B; mod tests { use super::*; - #[test] - fn test_app_basic_constants() { - // Test application basic constants - assert_eq!(APP_NAME, "RustFS"); - assert!(!APP_NAME.contains(' '), "App name should not contain spaces"); - - assert_eq!(VERSION, "1.0.0"); - - assert_eq!(SERVICE_VERSION, "1.0.0"); - assert_eq!(VERSION, SERVICE_VERSION, "Version and service version should be consistent"); - } - - #[test] - fn test_logging_constants() { - // Test logging related constants - assert_eq!(DEFAULT_LOG_LEVEL, "error"); - assert!( - ["trace", "debug", "info", "warn", "error"].contains(&DEFAULT_LOG_LEVEL), - "Log level should be a valid tracing level" - ); - - assert_eq!(SAMPLE_RATIO, 1.0); - - assert_eq!(METER_INTERVAL, 30); - } - - #[test] - fn test_environment_constants() { - // Test environment related constants - assert_eq!(ENVIRONMENT, "production"); - assert_eq!(ENV_RUSTFS_LICENSE_PUBLIC_KEY, "RUSTFS_LICENSE_PUBLIC_KEY"); - assert!( - ["development", "staging", "production", "test"].contains(&ENVIRONMENT), - "Environment should be a standard environment name" - ); - } - - #[test] - fn test_file_path_constants() { - assert_eq!(RUSTFS_TLS_KEY, "rustfs_key.pem"); - assert!(RUSTFS_TLS_KEY.ends_with(".pem"), "TLS key should be PEM format"); - - assert_eq!(RUSTFS_TLS_CERT, "rustfs_cert.pem"); - assert!(RUSTFS_TLS_CERT.ends_with(".pem"), "TLS cert should be PEM format"); - } - - #[test] - fn test_port_constants() { - // Test port related constants - assert_eq!(DEFAULT_PORT, 9000); - - assert_eq!(DEFAULT_CONSOLE_PORT, 9001); - - assert_ne!(DEFAULT_PORT, DEFAULT_CONSOLE_PORT, "Main port and console port should be different"); - } - - #[test] - fn test_address_constants() { - // Test address related constants - assert_eq!(DEFAULT_ADDRESS, ":9000"); - assert!(DEFAULT_ADDRESS.starts_with(':'), "Address should start with colon"); - assert!( - DEFAULT_ADDRESS.contains(&DEFAULT_PORT.to_string()), - "Address should contain the default port" - ); - - assert_eq!(DEFAULT_CONSOLE_ADDRESS, ":9001"); - assert!(DEFAULT_CONSOLE_ADDRESS.starts_with(':'), "Console address should start with colon"); - assert!( - DEFAULT_CONSOLE_ADDRESS.contains(&DEFAULT_CONSOLE_PORT.to_string()), - "Console address should contain the console port" - ); - - assert_ne!( - DEFAULT_ADDRESS, DEFAULT_CONSOLE_ADDRESS, - "Main address and console address should be different" - ); - } - #[test] fn test_const_str_concat_functionality() { // Test const_str::concat macro functionality diff --git a/crates/config/src/constants/capacity.rs b/crates/config/src/constants/capacity.rs index 179326cc5..4a8d21397 100644 --- a/crates/config/src/constants/capacity.rs +++ b/crates/config/src/constants/capacity.rs @@ -145,20 +145,4 @@ mod tests { assert_eq!(ENV_CAPACITY_MAX_TIMEOUT, "RUSTFS_CAPACITY_MAX_TIMEOUT"); assert_eq!(ENV_CAPACITY_STALL_TIMEOUT, "RUSTFS_CAPACITY_STALL_TIMEOUT"); } - - #[test] - fn test_default_values() { - assert_eq!(DEFAULT_SCHEDULED_UPDATE_INTERVAL_SECS, 120); - assert_eq!(DEFAULT_WRITE_TRIGGER_DELAY_SECS, 5); - assert_eq!(DEFAULT_WRITE_FREQUENCY_THRESHOLD, 5); - assert_eq!(DEFAULT_FAST_UPDATE_THRESHOLD_SECS, 30); - assert_eq!(DEFAULT_MAX_FILES_THRESHOLD, 200_000); - assert_eq!(DEFAULT_STAT_TIMEOUT_SECS, 3); - assert_eq!(DEFAULT_SAMPLE_RATE, 200); - assert_eq!(DEFAULT_CAPACITY_METRICS_INTERVAL_SECS, 600); - assert_eq!(DEFAULT_CAPACITY_MAX_SYMLINK_DEPTH, 3); - assert_eq!(DEFAULT_CAPACITY_MIN_TIMEOUT_SECS, 2); - assert_eq!(DEFAULT_CAPACITY_MAX_TIMEOUT_SECS, 15); - assert_eq!(DEFAULT_CAPACITY_STALL_TIMEOUT_SECS, 20); - } } diff --git a/crates/config/src/constants/workload.rs b/crates/config/src/constants/workload.rs index 27d618d19..d228a01b3 100644 --- a/crates/config/src/constants/workload.rs +++ b/crates/config/src/constants/workload.rs @@ -45,13 +45,6 @@ mod tests { use super::*; use crate::MI_B; - #[test] - fn test_default_values() { - assert_eq!(DEFAULT_BUFFER_MIN_SIZE, 65536); // 64KB - assert_eq!(DEFAULT_BUFFER_MAX_SIZE, 1048576); // 1MB - assert_eq!(DEFAULT_BUFFER_UNKNOWN_SIZE, 262144); // 256KB - } - #[test] fn test_constants() { assert_eq!(KI_B, 1024); diff --git a/crates/crypto/src/jwt/tests.rs b/crates/crypto/src/jwt/tests.rs index d0adf5c12..48e893f2f 100644 --- a/crates/crypto/src/jwt/tests.rs +++ b/crates/crypto/src/jwt/tests.rs @@ -92,6 +92,28 @@ fn test_jwt_decode_invalid_token_format() { } } +#[test] +fn test_jwt_decode_with_tampered_signature() { + let claims = json!({ + "exp": OffsetDateTime::now_utc().unix_timestamp() + 1000, + "sub": "user123" + }); + + let secret = b"test_secret"; + let jwt_token = encode(secret, &claims).expect("Failed to encode JWT"); + + // Flip one character in the signature segment + let (head, signature) = jwt_token.rsplit_once('.').expect("JWT should have a signature segment"); + let last_char = signature.chars().last().expect("Signature should not be empty"); + let flipped = if last_char == 'A' { 'B' } else { 'A' }; + let mut tampered_signature = signature[..signature.len() - 1].to_string(); + tampered_signature.push(flipped); + let tampered_token = format!("{head}.{tampered_signature}"); + + let result = decode(&tampered_token, secret); + assert!(result.is_err(), "Token with tampered signature should fail to decode"); +} + #[test] fn test_jwt_with_expired_token() { let expired_claims = json!({ diff --git a/crates/filemeta/src/filemeta/version.rs b/crates/filemeta/src/filemeta/version.rs index 3bfe5c092..d33be0d2f 100644 --- a/crates/filemeta/src/filemeta/version.rs +++ b/crates/filemeta/src/filemeta/version.rs @@ -3543,6 +3543,76 @@ mod tests { assert_eq!(fi.transition_version_id, Some(id)); } + #[test] + fn version_header_sorts_before_prefers_object_over_delete_marker_on_equal_mod_time() { + let object = FileMetaVersionHeader { + version_id: Some(Uuid::from_u128(1)), + mod_time: Some(sample_mod_time()), + version_type: VersionType::Object, + ..Default::default() + }; + let delete_marker = FileMetaVersionHeader { + version_id: Some(Uuid::from_u128(2)), + version_type: VersionType::Delete, + ..object + }; + + // With equal mod_time the tie is broken by "prefer lower types": + // Object (1) sorts before Delete (2), regardless of version_id. + assert!(object.sorts_before(&delete_marker)); + assert!(!delete_marker.sorts_before(&object)); + } + + #[test] + fn version_header_sorts_before_equal_mod_time_is_deterministic_and_antisymmetric() { + let a = FileMetaVersionHeader { + version_id: Some(Uuid::from_u128(1)), + mod_time: Some(sample_mod_time()), + version_type: VersionType::Object, + ..Default::default() + }; + let b = FileMetaVersionHeader { + version_id: Some(Uuid::from_u128(2)), + ..a.clone() + }; + + // Equal mod_time and version_type: the higher version_id sorts first, + // and exactly one of the two orderings holds so merge sees a stable + // total order for distinct headers. + assert!(b.sorts_before(&a)); + assert!(!a.sorts_before(&b)); + assert_ne!(a.sorts_before(&b), b.sorts_before(&a)); + + // A header never sorts before an identical header. + assert!(!a.sorts_before(&a.clone())); + } + + #[test] + fn merge_file_meta_versions_survives_garbage_header_stream() { + let valid = FileMetaShallowVersion { + header: sample_header(), + meta: Vec::new(), + }; + let garbage = FileMetaShallowVersion { + header: FileMetaVersionHeader { + version_id: None, + mod_time: None, + signature: [0xde, 0xad, 0xbe, 0xef], + version_type: VersionType::Invalid, + flags: 0xff, + ec_n: 0, + ec_m: 0, + }, + meta: b"not-a-valid-msgpack-version".to_vec(), + }; + + // One disk returns a garbage header alongside two healthy streams: + // merge must not panic and must still return the quorum version. + let merged = merge_file_meta_versions(2, true, 0, &[vec![valid.clone()], vec![valid.clone()], vec![garbage]]); + + assert_eq!(merged, vec![valid]); + } + #[test] fn meta_object_init_free_version_rejects_invalid_tier_free_version_id() { let mut sys = HashMap::new(); diff --git a/crates/iam/src/manager.rs b/crates/iam/src/manager.rs index c620da295..d5200f067 100644 --- a/crates/iam/src/manager.rs +++ b/crates/iam/src/manager.rs @@ -2597,13 +2597,6 @@ mod tests { assert_eq!(sys.sync_failures.load(Ordering::Relaxed), 3); } - #[test] - fn test_iam_format_new_version_1() { - let format = IAMFormat::new_version_1(); - assert_eq!(format.version, IAM_FORMAT_VERSION_1); - assert_eq!(format.version, 1); - } - #[test] fn test_get_iam_format_file_path() { let path = get_iam_format_file_path(); @@ -2717,26 +2710,6 @@ mod tests { assert!(policy.statements.is_empty()); } - #[test] - fn test_constants() { - // Test that constants are properly defined - assert_eq!(IAM_FORMAT_FILE, "format.json"); - assert_eq!(IAM_FORMAT_VERSION_1, 1); - } - - #[test] - fn test_iam_format_serialization() { - let format = IAMFormat::new_version_1(); - - // Test serialization - let serialized = serde_json::to_string(&format).unwrap(); - assert!(serialized.contains("\"version\":1")); - - // Test deserialization - let deserialized: IAMFormat = serde_json::from_str(&serialized).unwrap(); - assert_eq!(deserialized.version, format.version); - } - #[test] fn test_mapped_policy_operations() { let policy_name = "test-policy"; diff --git a/crates/madmin/src/user.rs b/crates/madmin/src/user.rs index 68906e679..d7ef97566 100644 --- a/crates/madmin/src/user.rs +++ b/crates/madmin/src/user.rs @@ -725,24 +725,6 @@ mod tests { use time::OffsetDateTime; use time::macros::datetime; - #[test] - fn test_account_status_default() { - let status = AccountStatus::default(); - assert_eq!(status, AccountStatus::Disabled); - } - - #[test] - fn test_account_status_as_ref() { - assert_eq!(AccountStatus::Enabled.as_ref(), "enabled"); - assert_eq!(AccountStatus::Disabled.as_ref(), "disabled"); - } - - #[test] - fn test_account_status_try_from_valid() { - assert_eq!(AccountStatus::try_from("enabled").unwrap(), AccountStatus::Enabled); - assert_eq!(AccountStatus::try_from("disabled").unwrap(), AccountStatus::Disabled); - } - #[test] fn test_account_status_try_from_invalid() { let result = AccountStatus::try_from("invalid"); @@ -783,32 +765,6 @@ mod tests { assert_eq!(ldap_json, "\"ldap\""); } - #[test] - fn test_user_auth_info_creation() { - let auth_info = UserAuthInfo { - auth_type: UserAuthType::Ldap, - auth_server: Some("ldap.example.com".to_string()), - auth_server_user_id: Some("user123".to_string()), - }; - - assert!(matches!(auth_info.auth_type, UserAuthType::Ldap)); - assert_eq!(auth_info.auth_server.unwrap(), "ldap.example.com"); - assert_eq!(auth_info.auth_server_user_id.unwrap(), "user123"); - } - - #[test] - fn test_user_auth_info_serialization() { - let auth_info = UserAuthInfo { - auth_type: UserAuthType::Builtin, - auth_server: None, - auth_server_user_id: None, - }; - - let json = serde_json::to_string(&auth_info).unwrap(); - assert!(json.contains("builtin")); - assert!(!json.contains("authServer"), "None fields should be skipped"); - } - #[test] fn test_user_info_default() { let user_info = UserInfo::default(); @@ -1020,57 +976,6 @@ mod tests { assert!(json.contains("session123")); } - #[test] - fn test_credentials_without_optional_fields() { - let credentials = Credentials { - access_key: "AKIAIOSFODNN7EXAMPLE", - secret_key: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", - session_token: None, - expiration: None, - }; - - let json = serde_json::to_string(&credentials).unwrap(); - assert!(json.contains("AKIAIOSFODNN7EXAMPLE")); - assert!(!json.contains("sessionToken"), "None fields should be skipped"); - assert!(!json.contains("expiration"), "None fields should be skipped"); - } - - #[test] - fn test_add_service_account_resp_creation() { - let credentials = Credentials { - access_key: "AKIAIOSFODNN7EXAMPLE", - secret_key: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", - session_token: None, - expiration: None, - }; - - let resp = AddServiceAccountResp { credentials }; - - assert_eq!(resp.credentials.access_key, "AKIAIOSFODNN7EXAMPLE"); - assert_eq!(resp.credentials.secret_key, "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"); - } - - #[test] - fn test_info_service_account_resp_creation() { - let now = OffsetDateTime::now_utc(); - let resp = InfoServiceAccountResp { - parent_user: "admin".to_string(), - account_status: "enabled".to_string(), - implied_policy: true, - policy: Some("ReadOnlyAccess".to_string()), - name: Some("test-service".to_string()), - description: Some("Test service account".to_string()), - expiration: Some(now), - }; - - assert_eq!(resp.parent_user, "admin"); - assert_eq!(resp.account_status, "enabled"); - assert!(resp.implied_policy); - assert_eq!(resp.policy.unwrap(), "ReadOnlyAccess"); - assert_eq!(resp.name.unwrap(), "test-service"); - assert!(resp.expiration.is_some()); - } - #[test] fn test_update_service_account_req_validate() { let req = UpdateServiceAccountReq { @@ -1105,22 +1010,6 @@ mod tests { assert_eq!(req.new_policy, None); } - #[test] - fn test_account_info_creation() { - use crate::BackendInfo; - - let account_info = AccountInfo { - account_name: "testuser".to_string(), - server: BackendInfo::default(), - policy: serde_json::json!({"Version": "2012-10-17"}), - buckets: vec![], - }; - - assert_eq!(account_info.account_name, "testuser"); - assert!(account_info.buckets.is_empty()); - assert!(account_info.policy.is_object()); - } - #[test] fn test_bucket_access_info_creation() { let now = OffsetDateTime::now_utc(); diff --git a/crates/object-capacity/src/capacity_manager.rs b/crates/object-capacity/src/capacity_manager.rs index a767a349c..93ffb9fb9 100644 --- a/crates/object-capacity/src/capacity_manager.rs +++ b/crates/object-capacity/src/capacity_manager.rs @@ -1125,155 +1125,64 @@ mod tests { use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; - #[test] - #[serial] - fn test_get_scheduled_update_interval() { - let interval = get_scheduled_update_interval(); - assert_eq!(interval, Duration::from_secs(120)); + /// Table of env-configurable getters: (env var, getter normalized to u64, + /// expected default, override string, expected override value). + /// Durations are normalized to whole seconds. + fn config_getter_cases() -> Vec<(&'static str, fn() -> u64, u64, &'static str, u64)> { + vec![ + ( + ENV_CAPACITY_SCHEDULED_INTERVAL, + || get_scheduled_update_interval().as_secs(), + 120, + "600", + 600, + ), + (ENV_CAPACITY_WRITE_TRIGGER_DELAY, || get_write_trigger_delay().as_secs(), 5, "20", 20), + ( + ENV_CAPACITY_WRITE_FREQUENCY_THRESHOLD, + || get_write_frequency_threshold() as u64, + 5, + "20", + 20, + ), + ( + ENV_CAPACITY_FAST_UPDATE_THRESHOLD, + || get_fast_update_threshold().as_secs(), + 30, + "120", + 120, + ), + ( + ENV_CAPACITY_MAX_FILES_THRESHOLD, + || get_max_files_threshold() as u64, + 200_000, + "2000000", + 2_000_000, + ), + (ENV_CAPACITY_STAT_TIMEOUT, || get_stat_timeout().as_secs(), 3, "10", 10), + (ENV_CAPACITY_SAMPLE_RATE, || get_sample_rate() as u64, 200, "500", 500), + (ENV_CAPACITY_METRICS_INTERVAL, || get_metrics_interval().as_secs(), 600, "90", 90), + ] } #[test] #[serial] - fn test_get_write_trigger_delay() { - let delay = get_write_trigger_delay(); - assert_eq!(delay, Duration::from_secs(5)); + fn test_config_getter_defaults() { + for (env_var, getter, default, _, _) in config_getter_cases() { + temp_env::with_var(env_var, None::<&str>, || { + assert_eq!(getter(), default, "{env_var}: unexpected default value"); + }); + } } #[test] #[serial] - fn test_get_write_frequency_threshold() { - let threshold = get_write_frequency_threshold(); - assert_eq!(threshold, 5); - } - - #[test] - #[serial] - fn test_get_fast_update_threshold() { - let threshold = get_fast_update_threshold(); - assert_eq!(threshold, Duration::from_secs(30)); - } - - #[test] - #[serial] - fn test_get_max_files_threshold() { - let threshold = get_max_files_threshold(); - assert_eq!(threshold, 200_000); - } - - #[test] - #[serial] - fn test_get_stat_timeout() { - let timeout = get_stat_timeout(); - assert_eq!(timeout, Duration::from_secs(3)); - } - - #[test] - #[serial] - fn test_get_sample_rate() { - let rate = get_sample_rate(); - assert_eq!(rate, 200); - } - - #[test] - #[serial] - fn test_get_metrics_interval() { - let interval = get_metrics_interval(); - assert_eq!(interval, Duration::from_secs(600)); - } - - #[test] - #[serial] - fn test_env_var_override_scheduled_interval() { - temp_env::with_var(ENV_CAPACITY_SCHEDULED_INTERVAL, Some("600"), || { - let interval = get_scheduled_update_interval(); - assert_eq!(interval, Duration::from_secs(600)); - }); - } - - #[test] - #[serial] - fn test_env_var_override_write_trigger_delay() { - temp_env::with_var(ENV_CAPACITY_WRITE_TRIGGER_DELAY, Some("20"), || { - let delay = get_write_trigger_delay(); - assert_eq!(delay, Duration::from_secs(20)); - }); - } - - #[test] - #[serial] - fn test_env_var_override_write_frequency_threshold() { - temp_env::with_var(ENV_CAPACITY_WRITE_FREQUENCY_THRESHOLD, Some("20"), || { - let threshold = get_write_frequency_threshold(); - assert_eq!(threshold, 20); - }); - } - - #[test] - #[serial] - fn test_env_var_override_fast_update_threshold() { - temp_env::with_var(ENV_CAPACITY_FAST_UPDATE_THRESHOLD, Some("120"), || { - let threshold = get_fast_update_threshold(); - assert_eq!(threshold, Duration::from_secs(120)); - }); - } - - #[test] - #[serial] - fn test_env_var_override_metrics_interval() { - temp_env::with_var(ENV_CAPACITY_METRICS_INTERVAL, Some("90"), || { - let interval = get_metrics_interval(); - assert_eq!(interval, Duration::from_secs(90)); - }); - } - - #[test] - #[serial] - fn test_env_var_override_max_files_threshold() { - temp_env::with_var(ENV_CAPACITY_MAX_FILES_THRESHOLD, Some("2000000"), || { - let threshold = get_max_files_threshold(); - assert_eq!(threshold, 2_000_000); - }); - } - - #[test] - #[serial] - fn test_env_var_override_stat_timeout() { - temp_env::with_var(ENV_CAPACITY_STAT_TIMEOUT, Some("10"), || { - let timeout = get_stat_timeout(); - assert_eq!(timeout, Duration::from_secs(10)); - }); - } - - #[test] - #[serial] - fn test_env_var_override_sample_rate() { - temp_env::with_var(ENV_CAPACITY_SAMPLE_RATE, Some("200"), || { - let rate = get_sample_rate(); - assert_eq!(rate, 200); - }); - } - - #[tokio::test] - #[serial] - async fn test_capacity_manager_creation() { - let config = HybridStrategyConfig::default(); - let manager = HybridCapacityManager::new(config); - - assert!(manager.get_capacity().await.is_none()); - } - - #[tokio::test] - #[serial] - async fn test_update_capacity() { - let manager = HybridCapacityManager::from_env(); - - manager - .update_capacity(CapacityUpdate::exact(1000, 0), DataSource::RealTime) - .await; - - let cached = manager.get_capacity().await; - assert!(cached.is_some()); - assert_eq!(cached.unwrap().total_used, 1000); + fn test_config_getter_env_overrides() { + for (env_var, getter, _, override_value, expected) in config_getter_cases() { + temp_env::with_var(env_var, Some(override_value), || { + assert_eq!(getter(), expected, "{env_var}: override not applied"); + }); + } } #[tokio::test] diff --git a/crates/utils/src/http/metadata_compat.rs b/crates/utils/src/http/metadata_compat.rs index 1846cf087..16b7625a8 100644 --- a/crates/utils/src/http/metadata_compat.rs +++ b/crates/utils/src/http/metadata_compat.rs @@ -204,4 +204,39 @@ mod tests { assert!(!metadata.contains_key("X-Minio-Internal-compression")); assert!(contains_key_str(&metadata, SUFFIX_ACTUAL_SIZE)); } + + #[test] + fn test_str_prefers_rustfs_key_over_minio() { + let metadata = HashMap::from([ + (internal_key_rustfs(SUFFIX_TRANSITION_TIER), "rustfs-tier".to_string()), + (format!("{MINIO_INTERNAL_PREFIX}{SUFFIX_TRANSITION_TIER}"), "minio-tier".to_string()), + ]); + + assert_eq!(get_str(&metadata, SUFFIX_TRANSITION_TIER).as_deref(), Some("rustfs-tier")); + } + + #[test] + fn test_bytes_lookup_falls_back_to_minio_key() { + let mut meta_sys = + HashMap::from([(format!("{MINIO_INTERNAL_PREFIX}{SUFFIX_TRANSITIONED_VERSION_ID}"), b"version-1".to_vec())]); + + assert!(contains_key_bytes(&meta_sys, SUFFIX_TRANSITIONED_VERSION_ID)); + assert_eq!(get_bytes(&meta_sys, SUFFIX_TRANSITIONED_VERSION_ID), Some(b"version-1".to_vec())); + + remove_bytes(&mut meta_sys, SUFFIX_TRANSITIONED_VERSION_ID); + assert!(!contains_key_bytes(&meta_sys, SUFFIX_TRANSITIONED_VERSION_ID)); + } + + #[test] + fn test_bytes_prefers_rustfs_key_over_minio() { + let meta_sys = HashMap::from([ + (internal_key_rustfs(SUFFIX_TRANSITIONED_VERSION_ID), b"rustfs-version".to_vec()), + ( + format!("{MINIO_INTERNAL_PREFIX}{SUFFIX_TRANSITIONED_VERSION_ID}"), + b"minio-version".to_vec(), + ), + ]); + + assert_eq!(get_bytes(&meta_sys, SUFFIX_TRANSITIONED_VERSION_ID), Some(b"rustfs-version".to_vec())); + } } diff --git a/crates/zip/src/lib.rs b/crates/zip/src/lib.rs index 127e61bb6..77ad30cc8 100644 --- a/crates/zip/src/lib.rs +++ b/crates/zip/src/lib.rs @@ -1532,6 +1532,85 @@ mod tests { assert!(matches!(err, ZipError::UnsafeEntryPath(path) if path == "../escape.txt")); } + #[tokio::test] + async fn test_extract_zip_rejects_parent_traversal_entry_and_writes_nothing_outside_target() { + let temp = tempdir().expect("tempdir should be created"); + let zip_path = temp.path().join("traversal.zip"); + let extract_path = temp.path().join("extract"); + + build_zip_file_with_entries(&zip_path, &[("safe.txt", b"safe"), ("../escape.txt", b"escape")]) + .await + .expect("zip fixture should be created"); + + let err = extract_zip_simple(&zip_path, &extract_path) + .await + .expect_err("zip entry with parent traversal should be rejected"); + + assert!(matches!(err, ZipError::UnsafeEntryPath(path) if path == "../escape.txt")); + assert!( + fs::metadata(temp.path().join("escape.txt")).await.is_err(), + "traversal entry must not be written outside the extraction target" + ); + assert!( + fs::metadata(extract_path.join("escape.txt")).await.is_err(), + "traversal entry must not be written inside the extraction target either" + ); + } + + #[tokio::test] + async fn test_extract_zip_allows_entry_count_exactly_at_limit() { + let temp = tempdir().expect("tempdir should be created"); + let zip_path = temp.path().join("at-limit.zip"); + let extract_path = temp.path().join("extract"); + + build_zip_file_with_entries(&zip_path, &[("one.txt", b"1"), ("two.txt", b"2")]) + .await + .expect("zip fixture should be created"); + + let entries = extract_zip_with_limits( + &zip_path, + &extract_path, + ArchiveLimits { + max_entries: 2, + ..ArchiveLimits::default() + }, + ) + .await + .expect("entry count exactly at the limit should extract successfully"); + + assert_eq!(entries.len(), 2); + } + + #[tokio::test] + async fn test_extract_zip_allows_total_unpacked_size_exactly_at_limit() { + let temp = tempdir().expect("tempdir should be created"); + let zip_path = temp.path().join("total-at-limit.zip"); + let extract_path = temp.path().join("extract"); + + build_zip_file_with_entries(&zip_path, &[("one.txt", b"12345"), ("two.txt", b"67890")]) + .await + .expect("zip fixture should be created"); + + let entries = extract_zip_with_limits( + &zip_path, + &extract_path, + ArchiveLimits { + max_total_unpacked_size: 10, + ..ArchiveLimits::default() + }, + ) + .await + .expect("total unpacked size exactly at the limit should extract successfully"); + + assert_eq!(entries.len(), 2); + assert_eq!( + fs::read(extract_path.join("two.txt")) + .await + .expect("second entry should be extracted"), + b"67890" + ); + } + #[tokio::test] async fn test_extract_zip_rejects_too_many_entries() { let temp = tempdir().expect("tempdir should be created");