From cabee0f36eec4faa34ebe3780046516d4a97a41b Mon Sep 17 00:00:00 2001
From: Simon Sawert <simon@sawert.se>
Date: Tue, 29 Oct 2024 12:55:45 +0100
Subject: [PATCH] fix: Don't accept invalid step for `Period` (#122)

* fix: Don't accept invalid intervals for periods

If the specifier is a period specifier, ensure that the interval is
larger than 0 (0 would be every 0th) and less than the max value for the
unit.

* fix: Move step check to appropriate function
---
 src/parsing.rs       | 30 ++++++++++++++++++++++++++++++
 src/time_unit/mod.rs | 10 ++++++++++
 2 files changed, 40 insertions(+)

diff --git a/src/parsing.rs b/src/parsing.rs
index 0491db4..85deedf 100644
--- a/src/parsing.rs
+++ b/src/parsing.rs
@@ -661,4 +661,34 @@ mod test {
         let expression = "* * * ? * ?";
         Schedule::from_str(expression).unwrap();
     }
+
+    /// Issue #59
+    #[test]
+    fn test_reject_invalid_interval() {
+        for invalid_expression in [
+            "1-5/61 * * * * *",
+            "*/61 2 3 4 5 6",
+            "* */61 * * * *",
+            "* * */25 * * *",
+            "* * * */32 * *",
+            "* * * * */13 *",
+            "1,2,3/60 * * * * *",
+            "0 0 0 1 1 ? 2020-2040/2200",
+        ] {
+            assert!(schedule(invalid_expression).is_err());
+        }
+
+        for valid_expression in [
+            "1-5/59 * * * * *",
+            "*/10 2 3 4 5 6",
+            "* */30 * * * *",
+            "* * */23 * * *",
+            "* * * */30 * *",
+            "* * * * */10 *",
+            "1,2,3/5 * * * * *",
+            "0 0 0 1 1 ? 2020-2040/10",
+        ] {
+            assert!(schedule(valid_expression).is_ok());
+        }
+    }
 }
diff --git a/src/time_unit/mod.rs b/src/time_unit/mod.rs
index e468f08..242c3c4 100644
--- a/src/time_unit/mod.rs
+++ b/src/time_unit/mod.rs
@@ -301,6 +301,16 @@ where
                 "range step cannot be zero".to_string(),
             ))?,
             RootSpecifier::Period(start, step) => {
+                if *step < 1 || *step > Self::inclusive_max() {
+                    return Err(ErrorKind::Expression(format!(
+                        "{} must be between 1 and {}. ('{}' specified.)",
+                        Self::name(),
+                        Self::inclusive_max(),
+                        step,
+                    ))
+                    .into());
+                }
+
                 let base_set = match start {
                     // A point prior to a period implies a range whose start is the specified
                     // point and terminating inclusively with the inclusive max