* [pbs-devel] [PATCH proxmox v2 1/3] time: Add traits to DateTimeValue and TimeSpec
2025-08-13 12:43 [pbs-devel] [PATCH proxmox v2 0/3] fix #6161: time: Split parse_time_spec parser into two Maximiliano Sandoval
@ 2025-08-13 12:43 ` Maximiliano Sandoval
2025-08-13 12:43 ` [pbs-devel] [PATCH proxmox v2 2/3] fix #6161: time: Split parse_time_spec parser into two Maximiliano Sandoval
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Maximiliano Sandoval @ 2025-08-13 12:43 UTC (permalink / raw)
To: pbs-devel
which are useful for tests.
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
proxmox-time/src/calendar_event.rs | 1 +
proxmox-time/src/date_time_value.rs | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/proxmox-time/src/calendar_event.rs b/proxmox-time/src/calendar_event.rs
index 696a11a2..5d5bdf83 100644
--- a/proxmox-time/src/calendar_event.rs
+++ b/proxmox-time/src/calendar_event.rs
@@ -341,6 +341,7 @@ fn parse_calendar_event_incomplete(mut i: &str) -> IResult<&str, CalendarEvent>
Ok((i, event))
}
+#[derive(Debug, PartialEq)]
struct TimeSpec {
hour: Vec<DateTimeValue>,
minute: Vec<DateTimeValue>,
diff --git a/proxmox-time/src/date_time_value.rs b/proxmox-time/src/date_time_value.rs
index 4dd79d36..08692b38 100644
--- a/proxmox-time/src/date_time_value.rs
+++ b/proxmox-time/src/date_time_value.rs
@@ -1,4 +1,4 @@
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq)]
pub(crate) enum DateTimeValue {
Single(u32),
Range(u32, u32),
--
2.47.2
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* [pbs-devel] [PATCH proxmox v2 2/3] fix #6161: time: Split parse_time_spec parser into two
2025-08-13 12:43 [pbs-devel] [PATCH proxmox v2 0/3] fix #6161: time: Split parse_time_spec parser into two Maximiliano Sandoval
2025-08-13 12:43 ` [pbs-devel] [PATCH proxmox v2 1/3] time: Add traits to DateTimeValue and TimeSpec Maximiliano Sandoval
@ 2025-08-13 12:43 ` Maximiliano Sandoval
2025-08-13 12:43 ` [pbs-devel] [PATCH proxmox v2 3/3] time: Add more calendat event tests Maximiliano Sandoval
2025-08-26 21:58 ` [pbs-devel] [PATCH proxmox v2 0/3] fix #6161: time: Split parse_time_spec parser into two Thomas Lamprecht
3 siblings, 0 replies; 7+ messages in thread
From: Maximiliano Sandoval @ 2025-08-13 12:43 UTC (permalink / raw)
To: pbs-devel
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@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
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* [pbs-devel] [PATCH proxmox v2 3/3] time: Add more calendat event tests
2025-08-13 12:43 [pbs-devel] [PATCH proxmox v2 0/3] fix #6161: time: Split parse_time_spec parser into two Maximiliano Sandoval
2025-08-13 12:43 ` [pbs-devel] [PATCH proxmox v2 1/3] time: Add traits to DateTimeValue and TimeSpec Maximiliano Sandoval
2025-08-13 12:43 ` [pbs-devel] [PATCH proxmox v2 2/3] fix #6161: time: Split parse_time_spec parser into two Maximiliano Sandoval
@ 2025-08-13 12:43 ` Maximiliano Sandoval
2025-08-26 21:58 ` [pbs-devel] [PATCH proxmox v2 0/3] fix #6161: time: Split parse_time_spec parser into two Thomas Lamprecht
3 siblings, 0 replies; 7+ messages in thread
From: Maximiliano Sandoval @ 2025-08-13 12:43 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
proxmox-time/src/calendar_event.rs | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/proxmox-time/src/calendar_event.rs b/proxmox-time/src/calendar_event.rs
index 6fe453be..6a158b7c 100644
--- a/proxmox-time/src/calendar_event.rs
+++ b/proxmox-time/src/calendar_event.rs
@@ -477,9 +477,15 @@ mod tests {
use std::str::FromStr;
assert!(CalendarEvent::from_str("15:30").is_ok());
+ assert!(CalendarEvent::from_str("0").is_ok());
+ assert!(CalendarEvent::from_str("59").is_ok());
+ assert!(CalendarEvent::from_str("15:30:59").is_ok());
assert!(CalendarEvent::from_str("24:00").is_err());
assert!(CalendarEvent::from_str("40:00").is_err());
+ assert!(CalendarEvent::from_str("60").is_err());
+ assert!(CalendarEvent::from_str("24:61").is_err());
+ assert!(CalendarEvent::from_str("15:30:60").is_err());
}
#[test]
@@ -495,6 +501,10 @@ mod tests {
}
);
+ assert!(parse_time_spec("1:00:00").is_ok());
+ assert!(parse_time_spec("1:00").is_ok());
+ assert!(parse_time_spec("59").is_ok());
+
// 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();
@@ -507,5 +517,23 @@ mod tests {
second: vec![DateTimeValue::Single(0)]
}
);
+
+ assert!(parse_time_spec("61").is_err());
+ // This should be 1:00 instead.
+ assert!(parse_time_spec("60").is_err());
+
+ // Can only partially parse
+ let (input, _data) = parse_time_spec("24:61").unwrap();
+ assert_eq!(input, ":61");
+
+
+ assert_eq!(
+ data,
+ TimeSpec {
+ hour: vec![],
+ minute: vec![DateTimeValue::Single(40)],
+ second: vec![DateTimeValue::Single(0)]
+ }
+ );
}
}
--
2.47.2
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 7+ messages in thread