From 8f02fed2d90764e58c618679291f0c2ce6c24ee0 Mon Sep 17 00:00:00 2001 From: default <216188+jdx@users.noreply.github.com> Date: Wed, 19 Aug 2026 02:13:47 +0000 Subject: [PATCH 1/2] fix(lib): validate every variadic fallback --- lib/src/parse.rs | 77 +++++++++++++++++++++++++++++---------------- lib/src/spec/arg.rs | 27 ++++++++++++++-- 2 files changed, 75 insertions(+), 29 deletions(-) diff --git a/lib/src/parse.rs b/lib/src/parse.rs index 90970e211..df72517ac 100644 --- a/lib/src/parse.rs +++ b/lib/src/parse.rs @@ -418,31 +418,36 @@ impl<'a> Parser<'a> { } if let Some(env_var) = arg.env.as_ref() { if let Some(env_value) = get_env(env_var) { - validate_choice_value( + let values = + split_fallback_values(std::slice::from_ref(&env_value), arg.delimiter); + validate_choice_values( ChoiceTarget::arg(arg), - &env_value, + &values, arg.choices.as_ref(), custom_env, )?; - out.args - .insert(Arc::new(arg.clone()), ParseValue::String(env_value)); + let parsed = if arg.var { + ParseValue::MultiString(values) + } else { + ParseValue::String(values.into_iter().next().unwrap_or_default()) + }; + out.args.insert(Arc::new(arg.clone()), parsed); continue; } } if !arg.default.is_empty() { // Consider var when deciding the type of default return value if arg.var { + let values = split_fallback_values(&arg.default, arg.delimiter); validate_choice_values( ChoiceTarget::arg(arg), - &arg.default, + &values, arg.choices.as_ref(), custom_env, )?; // For var=true, always return a vec (MultiString) - out.args.insert( - Arc::new(arg.clone()), - ParseValue::MultiString(arg.default.clone()), - ); + out.args + .insert(Arc::new(arg.clone()), ParseValue::MultiString(values)); } else { validate_choice_value( ChoiceTarget::arg(arg), @@ -471,14 +476,20 @@ impl<'a> Parser<'a> { if let Some(env_var) = flag.env.as_ref() { if let Some(env_value) = get_env(env_var) { if let Some(arg) = flag.arg.as_ref() { - validate_choice_value( + let values = + split_fallback_values(std::slice::from_ref(&env_value), arg.delimiter); + validate_choice_values( ChoiceTarget::option(flag), - &env_value, + &values, arg.choices.as_ref(), custom_env, )?; - out.flags - .insert(Arc::clone(flag), ParseValue::String(env_value)); + let parsed = if flag.var || arg.var { + ParseValue::MultiString(values) + } else { + ParseValue::String(values.into_iter().next().unwrap_or_default()) + }; + out.flags.insert(Arc::clone(flag), parsed); } else { let is_true = matches!(env_value.as_str(), "1" | "true" | "True" | "TRUE"); out.flags @@ -1779,6 +1790,16 @@ fn fallback_is_true(value: &str) -> bool { matches!(value, "1" | "true" | "True" | "TRUE") } +fn split_fallback_values(values: &[String], delimiter: Option) -> Vec { + match delimiter { + Some(delimiter) => values + .iter() + .flat_map(|value| value.split(delimiter).map(str::to_string)) + .collect(), + None => values.to_vec(), + } +} + /// Bind a fallback the way an unconditional `default` does: one value, or several /// for `var`, and choices checked the same way. fn bind_flag_fallback( @@ -1790,30 +1811,32 @@ fn bind_flag_fallback( if values.is_empty() { return Ok(()); } - if flag.var { - if let Some(arg) = flag.arg.as_ref() { + if let Some(arg) = flag.arg.as_ref() { + let values = split_fallback_values(values, arg.delimiter); + if flag.var || arg.var { validate_choice_values( ChoiceTarget::option(flag), - values, + &values, arg.choices.as_ref(), custom_env, )?; out.flags - .insert(Arc::clone(flag), ParseValue::MultiString(values.to_vec())); + .insert(Arc::clone(flag), ParseValue::MultiString(values)); } else { - let bools: Vec = values.iter().map(|s| fallback_is_true(s)).collect(); + let value = values.into_iter().next().unwrap_or_default(); + validate_choice_value( + ChoiceTarget::option(flag), + &value, + arg.choices.as_ref(), + custom_env, + )?; out.flags - .insert(Arc::clone(flag), ParseValue::MultiBool(bools)); + .insert(Arc::clone(flag), ParseValue::String(value)); } - } else if let Some(arg) = flag.arg.as_ref() { - validate_choice_value( - ChoiceTarget::option(flag), - &values[0], - arg.choices.as_ref(), - custom_env, - )?; + } else if flag.var { + let bools: Vec = values.iter().map(|s| fallback_is_true(s)).collect(); out.flags - .insert(Arc::clone(flag), ParseValue::String(values[0].clone())); + .insert(Arc::clone(flag), ParseValue::MultiBool(bools)); } else { out.flags.insert( Arc::clone(flag), diff --git a/lib/src/spec/arg.rs b/lib/src/spec/arg.rs index 6cc960057..ed71606c8 100644 --- a/lib/src/spec/arg.rs +++ b/lib/src/spec/arg.rs @@ -659,17 +659,40 @@ arg "[port]" env="PORT" validate="int(value) > 0" validate_error="port must be p flag "--mode" default="bad" { arg "" validate="value == 'good'" validate_error="mode must be good" } +arg "[ports]..." env="PORTS" var=#true delimiter="," validate="int(value) > 0" validate_error="all ports must be positive" +flag "--levels" env="LEVELS" { + arg "..." var=#true delimiter="," validate="value == 'good'" validate_error="all levels must be good" +} +flag "--modes" default="good,bad" { + arg "..." var=#true delimiter="," validate="value == 'good'" validate_error="all modes must be good" +} +flag "--conditional" { + default_if "--trigger" "good,bad" + arg "..." var=#true delimiter="," validate="value == 'good'" validate_error="all conditional values must be good" +} +flag "--trigger" "# .parse() .unwrap(); - let env = HashMap::from([("PORT".to_string(), "0".to_string())]); + let env = HashMap::from([ + ("PORT".to_string(), "0".to_string()), + ("PORTS".to_string(), "1,0".to_string()), + ("LEVELS".to_string(), "good,bad".to_string()), + ]); let error = Parser::new(&spec) .with_env(env) - .parse(&["ex".to_string()]) + .parse(&["ex".to_string(), "--trigger".to_string()]) .unwrap_err(); let error = error.to_string(); assert!(error.contains("port must be positive"), "{error}"); assert!(error.contains("mode must be good"), "{error}"); + assert!(error.contains("all ports must be positive"), "{error}"); + assert!(error.contains("all levels must be good"), "{error}"); + assert!(error.contains("all modes must be good"), "{error}"); + assert!( + error.contains("all conditional values must be good"), + "{error}" + ); } } From 44d5cd52d768bd4718d0e5ef61d049d64bfa5951 Mon Sep 17 00:00:00 2001 From: default <216188+jdx@users.noreply.github.com> Date: Wed, 19 Aug 2026 02:28:38 +0000 Subject: [PATCH 2/2] fix(lib): enforce fallback value bounds --- lib/src/parse.rs | 88 +++++++++++++++++++++++++++++++++++++++++++++ lib/src/spec/arg.rs | 22 +++++++++--- 2 files changed, 106 insertions(+), 4 deletions(-) diff --git a/lib/src/parse.rs b/lib/src/parse.rs index df72517ac..0ee7b55dc 100644 --- a/lib/src/parse.rs +++ b/lib/src/parse.rs @@ -427,6 +427,7 @@ impl<'a> Parser<'a> { custom_env, )?; let parsed = if arg.var { + validate_arg_fallback_count(arg, values.len(), &mut out.errors); ParseValue::MultiString(values) } else { ParseValue::String(values.into_iter().next().unwrap_or_default()) @@ -439,6 +440,7 @@ impl<'a> Parser<'a> { // Consider var when deciding the type of default return value if arg.var { let values = split_fallback_values(&arg.default, arg.delimiter); + validate_arg_fallback_count(arg, values.len(), &mut out.errors); validate_choice_values( ChoiceTarget::arg(arg), &values, @@ -485,6 +487,17 @@ impl<'a> Parser<'a> { custom_env, )?; let parsed = if flag.var || arg.var { + if flag.var { + validate_flag_fallback_count(flag, values.len(), &mut out.errors); + } + if arg.var { + validate_flag_arg_fallback_count( + flag, + arg, + values.len(), + &mut out.errors, + ); + } ParseValue::MultiString(values) } else { ParseValue::String(values.into_iter().next().unwrap_or_default()) @@ -1800,6 +1813,74 @@ fn split_fallback_values(values: &[String], delimiter: Option) -> Vec) { + if let Some(min) = arg.var_min { + if count < min { + errors.push(UsageErr::VarArgTooFew { + name: arg.name.clone(), + min, + got: count, + }); + } + } + if let Some(max) = arg.var_max { + if count > max { + errors.push(UsageErr::VarArgTooMany { + name: arg.name.clone(), + max, + got: count, + }); + } + } +} + +fn validate_flag_fallback_count(flag: &SpecFlag, count: usize, errors: &mut Vec) { + if let Some(min) = flag.var_min { + if count < min { + errors.push(UsageErr::VarFlagTooFew { + name: flag.name.clone(), + min, + got: count, + }); + } + } + if let Some(max) = flag.var_max { + if count > max { + errors.push(UsageErr::VarFlagTooMany { + name: flag.name.clone(), + max, + got: count, + }); + } + } +} + +fn validate_flag_arg_fallback_count( + flag: &SpecFlag, + arg: &SpecArg, + count: usize, + errors: &mut Vec, +) { + if let Some(min) = arg.var_min { + if count < min { + errors.push(UsageErr::VarFlagTooFew { + name: flag.name.clone(), + min, + got: count, + }); + } + } + if let Some(max) = arg.var_max { + if count > max { + errors.push(UsageErr::VarFlagTooMany { + name: flag.name.clone(), + max, + got: count, + }); + } + } +} + /// Bind a fallback the way an unconditional `default` does: one value, or several /// for `var`, and choices checked the same way. fn bind_flag_fallback( @@ -1814,6 +1895,12 @@ fn bind_flag_fallback( if let Some(arg) = flag.arg.as_ref() { let values = split_fallback_values(values, arg.delimiter); if flag.var || arg.var { + if flag.var { + validate_flag_fallback_count(flag, values.len(), &mut out.errors); + } + if arg.var { + validate_flag_arg_fallback_count(flag, arg, values.len(), &mut out.errors); + } validate_choice_values( ChoiceTarget::option(flag), &values, @@ -1834,6 +1921,7 @@ fn bind_flag_fallback( .insert(Arc::clone(flag), ParseValue::String(value)); } } else if flag.var { + validate_flag_fallback_count(flag, values.len(), &mut out.errors); let bools: Vec = values.iter().map(|s| fallback_is_true(s)).collect(); out.flags .insert(Arc::clone(flag), ParseValue::MultiBool(bools)); diff --git a/lib/src/spec/arg.rs b/lib/src/spec/arg.rs index ed71606c8..ba61f042e 100644 --- a/lib/src/spec/arg.rs +++ b/lib/src/spec/arg.rs @@ -659,17 +659,18 @@ arg "[port]" env="PORT" validate="int(value) > 0" validate_error="port must be p flag "--mode" default="bad" { arg "" validate="value == 'good'" validate_error="mode must be good" } -arg "[ports]..." env="PORTS" var=#true delimiter="," validate="int(value) > 0" validate_error="all ports must be positive" +arg "[ports]..." env="PORTS" var=#true var_max=1 delimiter="," validate="int(value) > 0" validate_error="all ports must be positive" flag "--levels" env="LEVELS" { - arg "..." var=#true delimiter="," validate="value == 'good'" validate_error="all levels must be good" + arg "..." var=#true var_max=1 delimiter="," validate="value == 'good'" validate_error="all levels must be good" } flag "--modes" default="good,bad" { - arg "..." var=#true delimiter="," validate="value == 'good'" validate_error="all modes must be good" + arg "..." var=#true var_max=1 delimiter="," validate="value == 'good'" validate_error="all modes must be good" } flag "--conditional" { default_if "--trigger" "good,bad" - arg "..." var=#true delimiter="," validate="value == 'good'" validate_error="all conditional values must be good" + arg "..." var=#true var_max=1 delimiter="," validate="value == 'good'" validate_error="all conditional values must be good" } +flag "--repeats " env="REPEATS" var=#true var_max=1 delimiter="," flag "--trigger" "# .parse() @@ -678,6 +679,7 @@ flag "--trigger" ("PORT".to_string(), "0".to_string()), ("PORTS".to_string(), "1,0".to_string()), ("LEVELS".to_string(), "good,bad".to_string()), + ("REPEATS".to_string(), "one,two".to_string()), ]); let error = Parser::new(&spec) .with_env(env) @@ -693,6 +695,18 @@ flag "--trigger" error.contains("all conditional values must be good"), "{error}" ); + assert!( + error.contains("Variadic argument accepts at most 1 value(s), got 2"), + "{error}" + ); + for flag in ["levels", "modes", "conditional", "repeats"] { + assert!( + error.contains(&format!( + "Variadic flag --{flag} accepts at most 1 value(s), got 2" + )), + "{error}" + ); + } } }