From 1ce5b2dfcb820e62df92fe88524a05b78acfe641 Mon Sep 17 00:00:00 2001 From: Chris Guidry Date: Fri, 31 Jul 2026 01:41:05 +0000 Subject: [PATCH] Cap dice count, validate mi against die size, clarify two errors Four findings from the dice branch's final review. `4294967295d6` aborted the process trying to allocate billions of dice; parse_group now rejects any count over MAX_DICE (100), the same place the existing zero-count check lives. `d6mi100` parsed and silently produced a total no d6 can roll; parse_group now rejects a minimum greater than the group's die size. The MAX_REROLLS comment cited `d1r1` as an example of an unfailable condition, but d1 was dropped from VALID_SIDES earlier on this branch and is no longer valid notation; reworded to use only the still-valid r>0 example. And `2d6+5000000000` reported "is not a whole number" for a modifier that plainly is one; parse_term now checks the parse error's kind and reports "too large" instead when the value overflowed u32, so a model reading the error has something it can actually fix. --- src/dice.rs | 44 +++++++++++++++++++++++++++++++++++++++----- src/dice_tests.rs | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 file(s) changed, 89 insertion(s)(+), 5 deletion(s)(-) diff --git a/src/dice.rs b/src/dice.rs --- a/src/dice.rs +++ b/src/dice.rs @@ -14,8 +14,8 @@ use rand::RngExt; /// Rerolling until a condition stops matching would loop forever for a -/// condition no roll can ever fail (`d1r1`, or `r>0` on any die). This caps -/// how many times a single die rerolls, so a degenerate condition still +/// condition no roll can ever fail, like `r>0` on any die. This caps how +/// many times a single die rerolls, so a degenerate condition still /// produces a roll instead of hanging. const MAX_REROLLS: u32 = 100; @@ -23,6 +23,12 @@ /// The die sizes a 5e table rolls, including `d3` for the SRD's charge /// tables ("regains 1d3 expended charges daily at dawn"). Anything else /// is not a die this grammar accepts. const VALID_SIDES: [u32; 9] = [2, 3, 4, 6, 8, 10, 12, 20, 100]; + +/// The most dice a single group may roll. Nothing in the SRD calls for +/// more than a few dozen dice at once; this cap keeps a huge count like +/// `4294967295d6` a parse error instead of an attempt to allocate +/// billions of dice. +const MAX_DICE: u32 = 100; /// Notation that failed to parse, with a reason worded so that whatever /// wrote the notation can correct it and try again. @@ -244,9 +250,19 @@ if let Some(d_index) = text.find('d') { return Ok(Term::Group(parse_group(text, d_index, negative, original)?)); } - let value: u32 = text.parse().map_err(|_| DiceError { - notation: original.to_string(), - reason: format!("`{text}` is not a whole number or a dice group like `2d6`"), + let value: u32 = text.parse().map_err(|error: std::num::ParseIntError| { + let reason = if *error.kind() == std::num::IntErrorKind::PosOverflow { + format!( + "`{text}` is too large; the largest modifier this grammar accepts is {}", + u32::MAX + ) + } else { + format!("`{text}` is not a whole number or a dice group like `2d6`") + }; + DiceError { + notation: original.to_string(), + reason, + } })?; let value = i64::from(value); Ok(Term::Modifier(if negative { -value } else { value })) @@ -275,6 +291,14 @@ "a dice count of `0` in `{text}` is invalid; omit it for 1 die, or use a positive number" ), }); } + if count > MAX_DICE { + return Err(DiceError { + notation: original.to_string(), + reason: format!( + "a dice count of `{count}` in `{text}` is too high; the maximum is {MAX_DICE}" + ), + }); + } count }; @@ -303,6 +327,16 @@ }); } let suffixes = parse_suffixes(suffix_text, text, original)?; + if let Some(minimum) = suffixes.minimum + && minimum > sides + { + return Err(DiceError { + notation: original.to_string(), + reason: format!( + "a mi minimum of `{minimum}` in `{text}` is greater than the die's {sides} sides; use {sides} or lower" + ), + }); + } Ok(Group { count, diff --git a/src/dice_tests.rs b/src/dice_tests.rs --- a/src/dice_tests.rs +++ b/src/dice_tests.rs @@ -342,6 +342,27 @@ ), ); } +#[test] +fn a_minimum_equal_to_the_die_size_parses() { + parses_to( + "d6mi6", + notation( + "d6mi6", + vec![Term::Group(Group { + minimum: Some(6), + ..group(1, 6) + })], + ), + ); +} + +// --- Dice count limits ----------------------------------------------------- + +#[test] +fn a_dice_count_at_the_maximum_parses() { + parses_to("100d6", notation("100d6", vec![Term::Group(group(100, 6))])); +} + // --- Suffix combinations and ordering -------------------------------------- #[test] @@ -869,6 +890,35 @@ #[test] fn an_overlong_number_is_an_error() { let error = parse("d6kh99999999999999999999").unwrap_err(); assert!(error.reason.contains("not a valid number")); +} + +#[test] +fn a_dice_count_over_the_maximum_is_an_error() { + let error = parse("101d6").unwrap_err(); + assert!(error.reason.contains("count")); + assert!(error.reason.contains("100")); +} + +#[test] +fn an_absurdly_large_dice_count_is_an_error() { + let error = parse("4294967295d6").unwrap_err(); + assert!(error.reason.contains("count")); + assert!(error.reason.contains("100")); +} + +#[test] +fn a_minimum_greater_than_the_die_size_is_an_error() { + let error = parse("d6mi7").unwrap_err(); + assert!(error.reason.contains("mi")); + assert!(error.reason.contains('6')); + assert!(error.reason.contains('7')); +} + +#[test] +fn an_out_of_range_modifier_is_an_error() { + let error = parse("2d6+5000000000").unwrap_err(); + assert!(error.reason.contains("too large")); + assert!(!error.reason.contains("not a whole number")); } #[test] -- tangled.sh