From 3e9fc9e75ba0e586cd694b67df3be2842024f181 Mon Sep 17 00:00:00 2001 From: Eliah Rusin Date: Sat, 26 Sep 2026 07:00:06 +0300 Subject: [PATCH] volume: reject invalid -minFreeSpace and duration flags at startup like Go (#11447) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * volume: reject invalid -minFreeSpace and duration flags at startup like Go Symptom: the Rust volume server swallowed invalid values for three startup flags and ran with a limit the operator did not ask for. -minFreeSpace=150 became 150 bytes (Go: fatal, percent must be 0..=100), -minFreeSpace=10GiBx and -minFreeSpace=50B fell through to the 1% default (Go: fatal), and -inflightUploadDataTimeout=30sec, =abc or =30 silently became the 60 s default (Go: flag.Duration usage error, exit 2). Go's byte-size units (1,024MB, 42 mib, 100Ki, 1eb) were also not understood and mapped to 1%. Go reference: weed/util/minfreespace.go:73-106 (MustParseMinFreeSpace / ParseMinFreeSpace), weed/util/bytes.go:212-243 (ParseBytes and bytesSizeTable), weed/command/volume.go:226-227 (flag.Duration, i.e. time.ParseDuration grammar). Fix: the two inflight timeout Cli fields are now std::time::Duration with a clap value_parser that ports time.ParseDuration (units ns/us/µs/ μs/ms/s/m/h, fractions, bare 0, int64-nanosecond overflow), so clap prints a usage error naming the value and exits 2 at the same layer as Go's flag package. parse_min_free_spaces returns Result and is built from Go-parity parse_min_free_space / parse_bytes; resolve_config_with_env aborts with Go's "The value specified in -minFreeSpace not a valid value" message, the fail-fast pattern it already uses for --max, --index and --readMode. No new dependency. Negative durations and NaN percent are rejected explicitly (Go quirks with no sane Rust representation). Tests cover the Go fixtures from minfreespace_test.go and bytes_test.go, the duration grammar, and flag-level rejection through Cli::try_parse_from and resolve_config_with_env; the flag-level tests were confirmed to fail against the previous parser. Co-Authored-By: Claude Fable 5.1 * volume: say that -minFreeSpace commas separate directories, never thousands Review follow-up. parse_bytes keeps Go's ParseBytes thousands-comma support, but the flag is split on commas into per-directory entries before any entry is parsed (Go's MustParseMinFreeSpace does the same), so -minFreeSpace=1,024MB is the two entries 1 and 024MB. The doc comment and the single-entry tests advertised the comma form as if it were reachable from the flag. Document the split and add a flag-level test: two entries from parse_min_free_spaces, and resolve_config aborting on the directory-count mismatch with one -dir. Co-Authored-By: Claude Fable 5.1 * volume: trim comments on the strict flag parsing Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Claude Fable 5.1 Co-authored-by: Chris Lu Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- seaweed-volume/src/config.rs | 609 +++++++++++++++++++++++++++++------ 1 file changed, 506 insertions(+), 103 deletions(-) diff --git a/seaweed-volume/src/config.rs b/seaweed-volume/src/config.rs index 86512e931..b4e4da552 100644 --- a/seaweed-volume/src/config.rs +++ b/seaweed-volume/src/config.rs @@ -178,12 +178,22 @@ pub struct Cli { pub min_free_space: String, /// Inflight upload data wait timeout of volume servers. - #[arg(long = "inflightUploadDataTimeout", default_value = "60s")] - pub inflight_upload_data_timeout: String, + /// Go `time.Duration` syntax: `300ms`, `1.5h`, `2h45m`; a bare number other than `0` is rejected. + #[arg( + long = "inflightUploadDataTimeout", + default_value = "60s", + value_parser = parse_go_duration + )] + pub inflight_upload_data_timeout: std::time::Duration, /// Inflight download data wait timeout of volume servers. - #[arg(long = "inflightDownloadDataTimeout", default_value = "60s")] - pub inflight_download_data_timeout: String, + /// Go `time.Duration` syntax: `300ms`, `1.5h`, `2h45m`; a bare number other than `0` is rejected. + #[arg( + long = "inflightDownloadDataTimeout", + default_value = "60s", + value_parser = parse_go_duration + )] + pub inflight_download_data_timeout: std::time::Duration, /// if true, prevents slow reads from blocking other requests, /// but large file read P99 latency will increase. @@ -471,99 +481,214 @@ fn find_options_arg(args: &[String]) -> String { String::new() } -/// Parse a duration string like "60s", "5m", "1h" into a std::time::Duration. -fn parse_duration(s: &str) -> std::time::Duration { - let s = s.trim(); +/// Parse a duration with Go's `time.ParseDuration` grammar, as Go's +/// `flag.Duration` flags do: `[0-9]*(\.[0-9]*)?` repeated, units +/// `ns`/`us`/`µs`/`μs`/`ms`/`s`/`m`/`h`. A bare `0` is the only unit-less +/// value; a leading sign parses in Go but a negative has no +/// `std::time::Duration`, so it is an error, not a clamp. +fn parse_go_duration(s: &str) -> Result { + let orig = s; + let invalid = || format!("time: invalid duration {:?}", orig); + + let mut s = s; + if let Some(rest) = s.strip_prefix('-') { + if rest.is_empty() { + return Err(invalid()); + } + return Err(format!("negative duration {:?} is not supported", orig)); + } + if let Some(rest) = s.strip_prefix('+') { + s = rest; + } + if s == "0" { + return Ok(std::time::Duration::ZERO); + } if s.is_empty() { - return std::time::Duration::from_secs(60); + return Err(invalid()); } - if let Some(secs) = s.strip_suffix('s') - && let Ok(v) = secs.parse::() - { - return std::time::Duration::from_secs(v); + + // Go bounds the total by 1<<63 (int64 nanoseconds): overflow is an error. + const MAX: u64 = 1 << 63; + let mut total: u64 = 0; + let mut bytes = s.as_bytes(); + while !bytes.is_empty() { + if !(bytes[0] == b'.' || bytes[0].is_ascii_digit()) { + return Err(invalid()); + } + + // Consume [0-9]* + let mut i = 0; + let mut v: u64 = 0; + while i < bytes.len() && bytes[i].is_ascii_digit() { + v = v + .checked_mul(10) + .and_then(|v| v.checked_add(u64::from(bytes[i] - b'0'))) + .filter(|v| *v <= MAX) + .ok_or_else(invalid)?; + i += 1; + } + let pre = i != 0; + bytes = &bytes[i..]; + + // Consume (\.[0-9]*)? + let mut f: u64 = 0; + let mut scale: f64 = 1.0; + let mut post = false; + if let Some((b'.', rest)) = bytes.split_first() { + bytes = rest; + let mut i = 0; + let mut overflow = false; + while i < bytes.len() && bytes[i].is_ascii_digit() { + if !overflow { + // Like Go's leadingFraction, digits past ~19 are dropped. + match f + .checked_mul(10) + .and_then(|f| f.checked_add(u64::from(bytes[i] - b'0'))) + .filter(|f| *f <= MAX) + { + Some(next) => { + f = next; + scale *= 10.0; + } + None => overflow = true, + } + } + i += 1; + } + post = i != 0; + bytes = &bytes[i..]; + } + if !pre && !post { + return Err(invalid()); + } + + // Consume the unit: everything up to the next digit or '.'. + let unit_len = bytes + .iter() + .position(|c| *c == b'.' || c.is_ascii_digit()) + .unwrap_or(bytes.len()); + if unit_len == 0 { + return Err(format!("time: missing unit in duration {:?}", orig)); + } + // The boundary is an ASCII byte or the end, so it is a char boundary. + let unit = std::str::from_utf8(&bytes[..unit_len]).map_err(|_| invalid())?; + bytes = &bytes[unit_len..]; + let unit_nanos: u64 = match unit { + "ns" => 1, + "us" | "\u{00b5}s" | "\u{03bc}s" => 1_000, + "ms" => 1_000_000, + "s" => 1_000_000_000, + "m" => 60 * 1_000_000_000, + "h" => 3600 * 1_000_000_000, + _ => { + return Err(format!( + "time: unknown unit {:?} in duration {:?}", + unit, orig + )); + } + }; + + if v > MAX / unit_nanos { + return Err(invalid()); + } + v *= unit_nanos; + if f > 0 { + // Go's float math, so `1.5h` rounds the same way on both sides. + v += (f as f64 * (unit_nanos as f64 / scale)) as u64; + if v > MAX { + return Err(invalid()); + } + } + total = total + .checked_add(v) + .filter(|t| *t <= MAX) + .ok_or_else(invalid)?; } - if let Some(mins) = s.strip_suffix('m') - && let Ok(v) = mins.parse::() - && let Some(seconds) = v.checked_mul(60) - { - return std::time::Duration::from_secs(seconds); + if total > MAX - 1 { + return Err(invalid()); } - if let Some(hours) = s.strip_suffix('h') - && let Ok(v) = hours.parse::() - && let Some(seconds) = v.checked_mul(3600) - { - return std::time::Duration::from_secs(seconds); - } - // Fallback: try parsing as raw seconds - if let Ok(v) = s.parse::() { - return std::time::Duration::from_secs(v); - } - std::time::Duration::from_secs(60) + Ok(std::time::Duration::from_nanos(total)) } -/// Parse minFreeSpace / minFreeSpacePercent into MinFreeSpace values. -/// Mirrors Go's `util.MustParseMinFreeSpace()`. -fn parse_min_free_spaces(min_free_space: &str, min_free_space_percent: &str) -> Vec { - // If --minFreeSpace is provided, use it (takes precedence). +/// Byte-size suffixes of Go's `util.ParseBytes`, lower-cased: SI (`kb`, `k`) +/// and IEC (`kib`, `ki`) forms, `b`, and no suffix. +fn byte_unit_multiplier(unit: &str) -> Option { + const K: u64 = 1_000; + const KI: u64 = 1 << 10; + Some(match unit { + "" | "b" => 1, + "kb" | "k" => K, + "kib" | "ki" => KI, + "mb" | "m" => K * K, + "mib" | "mi" => KI * KI, + "gb" | "g" => K * K * K, + "gib" | "gi" => KI * KI * KI, + "tb" | "t" => K * K * K * K, + "tib" | "ti" => KI * KI * KI * KI, + "pb" | "p" => K * K * K * K * K, + "pib" | "pi" => KI * KI * KI * KI * KI, + "eb" | "e" => K * K * K * K * K * K, + "eib" | "ei" => KI * KI * KI * KI * KI * KI, + _ => return None, + }) +} + +/// Parse a human-readable byte count with Go's `util.ParseBytes` grammar: +/// decimal number, optional whitespace, then a [`byte_unit_multiplier`] +/// suffix, case-insensitive. `10GiB` → 10737418240, `1.5TB` → 1500000000000. +/// +/// Thousands commas parse as in Go but never arrive from `-minFreeSpace`, +/// which is split on commas into per-directory entries first. +fn parse_bytes(s: &str) -> Result { + let digits_end = s + .find(|c: char| !(c.is_ascii_digit() || c == '.' || c == ',')) + .unwrap_or(s.len()); + let num = s[..digits_end].replace(',', ""); + let value: f64 = num + .parse() + .map_err(|e| format!("invalid byte size {:?}: {}", s, e))?; + let unit = s[digits_end..].trim().to_lowercase(); + let multiplier = + byte_unit_multiplier(&unit).ok_or_else(|| format!("unhandled size name: {}", unit))?; + let bytes = value * multiplier as f64; + if bytes >= u64::MAX as f64 { + return Err(format!("too large: {}", s)); + } + Ok(bytes as u64) +} + +/// Parse one `-minFreeSpace` entry like Go's `util.ParseMinFreeSpace()`: a +/// plain number is a percentage in `0..=100`, anything else a byte size +/// above 100 bytes. `150` errors rather than reading as bytes. +fn parse_min_free_space(s: &str) -> Result { + let s = s.trim(); + if let Ok(percent) = s.parse::() { + // Go's range check lets NaN through; rejecting it is the one + // intentional difference. + if !(0.0..=100.0).contains(&percent) { + return Err(format!("minFreeSpace is invalid: {:?}", s)); + } + return Ok(MinFreeSpace::Percent(percent)); + } + match parse_bytes(s) { + Ok(bytes) if bytes > 100 => Ok(MinFreeSpace::Bytes(bytes)), + _ => Err(format!("minFreeSpace is invalid: {:?}", s)), + } +} + +/// Parse minFreeSpace / minFreeSpacePercent like Go's +/// `util.MustParseMinFreeSpace()`: `--minFreeSpace` wins when set, and any +/// invalid comma-separated entry fails the whole list. +fn parse_min_free_spaces( + min_free_space: &str, + min_free_space_percent: &str, +) -> Result, String> { let source = if !min_free_space.is_empty() { min_free_space } else { min_free_space_percent }; - - source - .split(',') - .map(|s| { - let s = s.trim(); - // Try parsing as a percentage (value <= 100) - if let Ok(v) = s.parse::() { - if v <= 100.0 { - return MinFreeSpace::Percent(v); - } - // Treat as bytes if > 100 - return MinFreeSpace::Bytes(v as u64); - } - // Try parsing human-readable bytes: e.g. "10GiB", "500MiB", "1TiB" - let s_upper = s.to_uppercase(); - if let Some(rest) = s_upper.strip_suffix("TIB") - && let Ok(v) = rest.trim().parse::() - { - return MinFreeSpace::Bytes((v * 1024.0 * 1024.0 * 1024.0 * 1024.0) as u64); - } - if let Some(rest) = s_upper.strip_suffix("GIB") - && let Ok(v) = rest.trim().parse::() - { - return MinFreeSpace::Bytes((v * 1024.0 * 1024.0 * 1024.0) as u64); - } - if let Some(rest) = s_upper.strip_suffix("MIB") - && let Ok(v) = rest.trim().parse::() - { - return MinFreeSpace::Bytes((v * 1024.0 * 1024.0) as u64); - } - if let Some(rest) = s_upper.strip_suffix("KIB") - && let Ok(v) = rest.trim().parse::() - { - return MinFreeSpace::Bytes((v * 1024.0) as u64); - } - if let Some(rest) = s_upper.strip_suffix("TB") - && let Ok(v) = rest.trim().parse::() - { - return MinFreeSpace::Bytes((v * 1_000_000_000_000.0) as u64); - } - if let Some(rest) = s_upper.strip_suffix("GB") - && let Ok(v) = rest.trim().parse::() - { - return MinFreeSpace::Bytes((v * 1_000_000_000.0) as u64); - } - if let Some(rest) = s_upper.strip_suffix("MB") - && let Ok(v) = rest.trim().parse::() - { - return MinFreeSpace::Bytes((v * 1_000_000.0) as u64); - } - // Default: 1% - MinFreeSpace::Percent(1.0) - }) - .collect() + source.split(',').map(parse_min_free_space).collect() } /// Parse comma-separated tag groups like "fast:ssd,archive" into per-folder tag vectors. @@ -657,7 +782,9 @@ fn resolve_config_with_env(cli: Cli, env: EnvLookup<'_>) -> VolumeServerConfig { // Parse min free spaces let mut min_free_spaces = - parse_min_free_spaces(&cli.min_free_space, &cli.min_free_space_percent); + parse_min_free_spaces(&cli.min_free_space, &cli.min_free_space_percent).unwrap_or_else( + |e| panic!("The value specified in -minFreeSpace not a valid value: {e}"), + ); if min_free_spaces.len() == 1 && folder_count > 1 { let v = min_free_spaces[0].clone(); min_free_spaces.resize(folder_count, v); @@ -772,10 +899,6 @@ fn resolve_config_with_env(cli: Cli, env: EnvLookup<'_>) -> VolumeServerConfig { .collect(); white_list.extend(sec.guard_white_list.iter().cloned()); - // Parse durations - let inflight_upload_data_timeout = parse_duration(&cli.inflight_upload_data_timeout); - let inflight_download_data_timeout = parse_duration(&cli.inflight_download_data_timeout); - VolumeServerConfig { port: cli.port, grpc_port, @@ -807,8 +930,8 @@ fn resolve_config_with_env(cli: Cli, env: EnvLookup<'_>) -> VolumeServerConfig { file_size_limit_bytes: cli.file_size_limit_mb as i64 * 1024 * 1024, concurrent_upload_limit: cli.concurrent_upload_limit_mb as i64 * 1024 * 1024, concurrent_download_limit: cli.concurrent_download_limit_mb as i64 * 1024 * 1024, - inflight_upload_data_timeout, - inflight_download_data_timeout, + inflight_upload_data_timeout: cli.inflight_upload_data_timeout, + inflight_download_data_timeout: cli.inflight_download_data_timeout, has_slow_read: cli.has_slow_read, read_buffer_size_mb: cli.read_buffer_size_mb, ldb_timeout: cli.ldb_timeout, @@ -1260,25 +1383,224 @@ mod tests { } #[test] - fn test_parse_duration() { - assert_eq!(parse_duration("60s"), std::time::Duration::from_secs(60)); - assert_eq!(parse_duration("5m"), std::time::Duration::from_secs(300)); - assert_eq!(parse_duration("1h"), std::time::Duration::from_secs(3600)); - assert_eq!(parse_duration("30"), std::time::Duration::from_secs(30)); - assert_eq!(parse_duration(""), std::time::Duration::from_secs(60)); + fn test_parse_go_duration_accepts_go_grammar() { + use std::time::Duration; + let cases = [ + ("60s", Duration::from_secs(60)), + ("5m", Duration::from_secs(300)), + ("1h", Duration::from_secs(3600)), + ("1.5h", Duration::from_secs(5400)), + ("2h45m", Duration::from_secs(2 * 3600 + 45 * 60)), + ("300ms", Duration::from_millis(300)), + ("1.5s", Duration::from_millis(1500)), + ("250us", Duration::from_micros(250)), + ("250\u{00b5}s", Duration::from_micros(250)), + ("250\u{03bc}s", Duration::from_micros(250)), + ("7ns", Duration::from_nanos(7)), + (".5s", Duration::from_millis(500)), + ("5.s", Duration::from_secs(5)), + ("+10s", Duration::from_secs(10)), + ("0", Duration::ZERO), + ("0s", Duration::ZERO), + // Go: fraction digits past the 19th are ignored, not an error. + ("1.00000000000000000001s", Duration::from_secs(1)), + ]; + for (input, expected) in cases { + assert_eq!(parse_go_duration(input), Ok(expected), "input={:?}", input); + } + } + + #[test] + fn test_parse_go_duration_rejects_what_go_rejects() { + // Each of these makes Go's flag.Duration fail flag parsing; the old + // Rust parser turned them all into the 60 s default. + let rejected = [ + "30", // bare number other than 0 + "30sec", // unknown unit + "abc", // no number at all + "", // empty + "1h ", // trailing space is not a unit + "s", // unit without number + ".s", // dot without digits + "1.5", // fraction without unit + "1h-30m", // sign is only allowed at the front + "-", // sign alone + "5124095576030432h", // overflows int64 nanoseconds + "307445734561825861m", // overflows int64 nanoseconds + "9223372036854775808ns", // 1<<63, one past int64 + ]; + for input in rejected { + let result = parse_go_duration(input); + assert!(result.is_err(), "input={:?} parsed as {:?}", input, result); + let message = result.unwrap_err(); + assert!( + message.contains(&format!("{:?}", input)) || input.is_empty(), + "error for {:?} does not name the value: {}", + input, + message + ); + } + // Largest value Go accepts: 1<<63 - 1 nanoseconds. assert_eq!( - parse_duration("307445734561825861m"), + parse_go_duration("9223372036854775807ns"), + Ok(std::time::Duration::from_nanos(i64::MAX as u64)) + ); + // Go accepts negatives; std::time::Duration cannot, so it errors. + let negative = parse_go_duration("-1s").unwrap_err(); + assert!(negative.contains("negative"), "{}", negative); + } + + /// Clap refuses the flag so the process exits with a usage error, exactly + /// where Go's flag package does. + #[test] + fn test_cli_rejects_invalid_duration_flags() { + for value in ["30sec", "abc", "30"] { + let result = Cli::try_parse_from(["bin", "--inflightUploadDataTimeout", value]); + assert!( + result.is_err(), + "--inflightUploadDataTimeout={} was accepted as {:?}", + value, + result.map(|cli| cli.inflight_upload_data_timeout) + ); + let err = Cli::try_parse_from(["bin", "--inflightDownloadDataTimeout", value]) + .err() + .unwrap_or_else(|| panic!("--inflightDownloadDataTimeout={} was accepted", value)); + assert_eq!(err.kind(), clap::error::ErrorKind::ValueValidation); + assert!(err.to_string().contains(value), "{}", err); + } + let cli = Cli::parse_from(["bin", "--inflightUploadDataTimeout", "1.5h"]); + assert_eq!( + cli.inflight_upload_data_timeout, + std::time::Duration::from_secs(5400) + ); + // The default still parses through the same grammar. + let cli = Cli::parse_from(["bin"]); + assert_eq!( + cli.inflight_upload_data_timeout, std::time::Duration::from_secs(60) ); assert_eq!( - parse_duration("5124095576030432h"), + cli.inflight_download_data_timeout, std::time::Duration::from_secs(60) ); } + #[test] + fn test_parse_bytes_matches_go_parse_bytes() { + // Expected values are what Go's util.ParseBytes returns (weed/util/bytes_test.go). + let cases: [(&str, u64); 14] = [ + ("42", 42), + ("42MB", 42_000_000), + ("42 MB", 42_000_000), + ("42 mib", 44_040_192), + ("42M", 42_000_000), + ("100Ki", 100 * 1024), + ("10GiB", 10 * 1024 * 1024 * 1024), + ("10gib", 10 * 1024 * 1024 * 1024), + ("10GB", 10_000_000_000), + ("1.5TB", 1_500_000_000_000), + // Go's ParseBytes accepts thousands commas; the -minFreeSpace flag + // never delivers one (see test_min_free_space_flag_commas_are_list_separators). + ("1,024MB", 1_024_000_000), + ("1,024", 1024), + ("1PiB", 1 << 50), + ("1eb", 1_000_000_000_000_000_000), + ]; + for (input, expected) in cases { + assert_eq!(parse_bytes(input), Ok(expected), "input={:?}", input); + } + for input in [ + "10GiBx", + "GiB", + "", + "-5GB", + "1.2.3GB", + "10 Gi B", + "99999999999999999999EB", + ] { + assert!( + parse_bytes(input).is_err(), + "input={:?} was accepted", + input + ); + } + } + + fn expect_percent(result: Result, expected: f64, input: &str) { + match result { + Ok(MinFreeSpace::Percent(v)) => { + assert!( + (v - expected).abs() < f64::EPSILON, + "input={:?}: {}", + input, + v + ) + } + other => panic!( + "input={:?}: expected Percent({}), got {:?}", + input, expected, other + ), + } + } + + fn expect_bytes(result: Result, expected: u64, input: &str) { + match result { + Ok(MinFreeSpace::Bytes(v)) => assert_eq!(v, expected, "input={:?}", input), + other => panic!( + "input={:?}: expected Bytes({}), got {:?}", + input, expected, other + ), + } + } + + #[test] + fn test_parse_min_free_space_matches_go() { + // Fixtures from weed/util/minfreespace_test.go plus the byte-unit table. + expect_percent(parse_min_free_space("42"), 42.0, "42"); + expect_percent(parse_min_free_space(" 42 "), 42.0, " 42 "); + expect_percent(parse_min_free_space("50"), 50.0, "50"); + expect_percent(parse_min_free_space("100"), 100.0, "100"); + expect_percent(parse_min_free_space("0"), 0.0, "0"); + expect_percent(parse_min_free_space("2.5"), 2.5, "2.5"); + expect_bytes(parse_min_free_space("100Ki"), 100 * 1024, "100Ki"); + expect_bytes(parse_min_free_space("100GiB"), 100 << 30, "100GiB"); + expect_bytes(parse_min_free_space("42M"), 42_000_000, "42M"); + expect_bytes(parse_min_free_space(" 42M "), 42_000_000, " 42M "); + expect_bytes(parse_min_free_space("10GiB"), 10 << 30, "10GiB"); + expect_bytes(parse_min_free_space("10gib"), 10 << 30, "10gib"); + expect_bytes(parse_min_free_space("1.5TB"), 1_500_000_000_000, "1.5TB"); + // Comma inputs never arrive whole from the flag; see + // test_min_free_space_flag_commas_are_list_separators. + expect_bytes(parse_min_free_space("1,024MB"), 1_024_000_000, "1,024MB"); + expect_bytes(parse_min_free_space("1,000"), 1000, "1,000"); + + // Go: percent outside 0..=100, or a byte size of at most 100 bytes. + for input in [ + "150", "101", "-1", "1e3", "inf", "nan", "100B", "50B", "0B", "10GiBx", "", "abc", + "10 Gi B", + ] { + let result = parse_min_free_space(input); + assert!( + result.is_err(), + "input={:?} was accepted as {:?}", + input, + result + ); + assert!( + result + .as_ref() + .unwrap_err() + .contains(&format!("{:?}", input.trim())), + "error for {:?} does not name the value: {:?}", + input, + result + ); + } + } + #[test] fn test_parse_min_free_spaces_percent() { - let result = parse_min_free_spaces("", "1"); + let result = parse_min_free_spaces("", "1").unwrap(); assert_eq!(result.len(), 1); match &result[0] { MinFreeSpace::Percent(v) => assert!((v - 1.0).abs() < f64::EPSILON), @@ -1288,7 +1610,7 @@ mod tests { #[test] fn test_parse_min_free_spaces_bytes() { - let result = parse_min_free_spaces("10GiB", ""); + let result = parse_min_free_spaces("10GiB", "").unwrap(); assert_eq!(result.len(), 1); match &result[0] { MinFreeSpace::Bytes(v) => assert_eq!(*v, 10 * 1024 * 1024 * 1024), @@ -1296,6 +1618,87 @@ mod tests { } } + #[test] + fn test_parse_min_free_spaces_list_and_precedence() { + // --minFreeSpace wins over --minFreeSpacePercent, as in Go's EmptyTo(). + let result = parse_min_free_spaces("5, 10GiB", "1").unwrap(); + assert_eq!(result.len(), 2); + assert!(matches!(result[0], MinFreeSpace::Percent(v) if v == 5.0)); + assert!(matches!(result[1], MinFreeSpace::Bytes(v) if v == 10 << 30)); + // One bad entry fails the whole list and names that entry. + let err = parse_min_free_spaces("5,10GiBx", "1").unwrap_err(); + assert!(err.contains("\"10GiBx\""), "{}", err); + // Both flags empty: Go fatals (ParseMinFreeSpace("") fails) rather than + // assuming 1%. + assert!(parse_min_free_spaces("", "").is_err()); + } + + /// A comma in `-minFreeSpace` separates per-directory entries, as in Go's + /// `MustParseMinFreeSpace`: `1,024MB` is `1` (percent) and `024MB`, and + /// one `-dir` fails the count check. + #[test] + fn test_min_free_space_flag_commas_are_list_separators() { + let result = parse_min_free_spaces("1,024MB", "").unwrap(); + assert_eq!(result.len(), 2, "{result:?}"); + assert!( + matches!(result[0], MinFreeSpace::Percent(v) if v == 1.0), + "{result:?}" + ); + assert!( + matches!(result[1], MinFreeSpace::Bytes(v) if v == 24_000_000), + "{result:?}" + ); + + let _guard = process_state_lock(); + let cli = Cli::parse_from(["bin", "--dir", "/tmp/a", "--minFreeSpace", "1,024MB"]); + let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + resolve_config_with_env(cli, &|_| None).min_free_spaces + })); + assert!( + outcome.is_err(), + "one -dir with two minFreeSpace entries must abort" + ); + } + + /// Negative control at the startup layer: resolving the config with an + /// invalid --minFreeSpace must abort, as Go's MustParseMinFreeSpace does. + #[test] + fn test_resolve_config_rejects_invalid_min_free_space() { + // The security-config search reads the working directory. + let _guard = process_state_lock(); + for (flag, value) in [ + ("--minFreeSpace", "10GiBx"), + ("--minFreeSpace", "150"), + ("--minFreeSpace", "50B"), + ("--minFreeSpacePercent", "150"), + ("--minFreeSpacePercent", "abc"), + ] { + let cli = Cli::parse_from(["bin", flag, value]); + let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + resolve_config_with_env(cli, &|_| None).min_free_spaces + })); + let payload = match outcome { + Ok(spaces) => panic!("{}={} was accepted as {:?}", flag, value, spaces), + Err(payload) => payload, + }; + let message = payload + .downcast_ref::() + .cloned() + .or_else(|| payload.downcast_ref::<&str>().map(|s| s.to_string())) + .unwrap_or_default(); + assert!( + message.contains("-minFreeSpace") && message.contains(value), + "{}={}: unexpected panic message {:?}", + flag, + value, + message + ); + } + let cfg = + resolve_config_with_env(Cli::parse_from(["bin", "--minFreeSpace", "100"]), &|_| None); + assert!(matches!(cfg.min_free_spaces[0], MinFreeSpace::Percent(v) if v == 100.0)); + } + #[test] fn test_parse_volume_tags_single() { let tags = parse_volume_tags("fast:ssd", 3);