[pbs-devel] [PATCH proxmox v2 2/3] fix #6161: time: Split parse_time_spec parser into two

[Maximiliano Sandoval]

When using a single parser "40:00" would be parsed as "40 minutes and 00
seconds", but this should error out:

```
$ systemd-analyze calendar 40:00
Failed to parse calendar specification '40:00': Invalid argument
```

We split into two parsers one in which seconds are specified and one in
which they are not and add tests for good measure.

The test added in this commit would not error out before this commit and
was erroneously producing "Every 40 minutes" as a result.

Signed-off-by: Maximiliano Sandoval <m.sandoval at proxmox.com>
---
 proxmox-time/src/calendar_event.rs | 80 +++++++++++++++++++++++++++---
 1 file changed, 73 insertions(+), 7 deletions(-)

diff --git a/proxmox-time/src/calendar_event.rs b/proxmox-time/src/calendar_event.rs
index 5d5bdf83..6fe453be 100644
--- a/proxmox-time/src/calendar_event.rs
+++ b/proxmox-time/src/calendar_event.rs
@@ -1,5 +1,6 @@
 use anyhow::Error;
 use nom::{
+    branch::alt,
     bytes::complete::tag,
     character::complete::space0,
     combinator::opt,
@@ -397,16 +398,13 @@ fn parse_date_time_comp_list(
     }
 }
 
-fn parse_time_spec(i: &str) -> IResult<&str, TimeSpec> {
-    let (i, (opt_hour, minute, opt_second)) = tuple((
-        opt(terminated(parse_date_time_comp_list(0, 24), tag(":"))),
+fn parse_time_spec_with_seconds(i: &str) -> IResult<&str, TimeSpec> {
+    let (i, (hour, minute, second)) = tuple((
+        terminated(parse_date_time_comp_list(0, 24), tag(":")),
         parse_date_time_comp_list(0, 60),
-        opt(preceded(tag(":"), parse_date_time_comp_list(0, 60))),
+        preceded(tag(":"), parse_date_time_comp_list(0, 60)),
     ))(i)?;
 
-    let hour = opt_hour.unwrap_or_default();
-    let second = opt_second.unwrap_or_else(|| vec![DateTimeValue::Single(0)]);
-
     Ok((
         i,
         TimeSpec {
@@ -417,6 +415,32 @@ fn parse_time_spec(i: &str) -> IResult<&str, TimeSpec> {
     ))
 }
 
+fn parse_time_spec_without_seconds(i: &str) -> IResult<&str, TimeSpec> {
+    let (i, (opt_hour, minute)) = tuple((
+        opt(terminated(parse_date_time_comp_list(0, 24), tag(":"))),
+        parse_date_time_comp_list(0, 60),
+    ))(i)?;
+
+    let hour = opt_hour.unwrap_or_default();
+    let second = vec![DateTimeValue::Single(0)];
+
+    Ok((
+        i,
+        TimeSpec {
+            hour,
+            minute,
+            second,
+        },
+    ))
+}
+
+fn parse_time_spec(i: &str) -> IResult<&str, TimeSpec> {
+    alt((
+        parse_time_spec_with_seconds,
+        parse_time_spec_without_seconds,
+    ))(i)
+}
+
 fn parse_date_spec(i: &str) -> IResult<&str, DateSpec> {
     // TODO: implement ~ for days (man systemd.time)
     if let Ok((i, (year, month, day))) = tuple((
@@ -443,3 +467,45 @@ fn parse_date_spec(i: &str) -> IResult<&str, DateSpec> {
         Err(parse_error(i, "invalid date spec"))
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_calendar_event_from_str() {
+        use std::str::FromStr;
+
+        assert!(CalendarEvent::from_str("15:30").is_ok());
+
+        assert!(CalendarEvent::from_str("24:00").is_err());
+        assert!(CalendarEvent::from_str("40:00").is_err());
+    }
+
+    #[test]
+    fn test_parse_time_spec() {
+        let (input, data) = parse_time_spec("15:30").unwrap();
+        assert_eq!(input, "");
+        assert_eq!(
+            data,
+            TimeSpec {
+                hour: vec![DateTimeValue::Single(15)],
+                minute: vec![DateTimeValue::Single(30)],
+                second: vec![DateTimeValue::Single(0)]
+            }
+        );
+
+        // These do not strictly fail, but should fail when parsed with FromStr
+        // since there will be input left to consume.
+        let (input, data) = parse_time_spec("40:00").unwrap();
+        assert_eq!(input, ":00");
+        assert_eq!(
+            data,
+            TimeSpec {
+                hour: vec![],
+                minute: vec![DateTimeValue::Single(40)],
+                second: vec![DateTimeValue::Single(0)]
+            }
+        );
+    }
+}
-- 
2.47.2