Skip to content

Commit

Permalink
MonitorSchedule constructor that validates crontab syntax (#625)
Browse files Browse the repository at this point in the history
* MonitorSchedule constructor the validates crontab syntax

* Added tests, and made improvemnts

* Applied linter fixes

* Deleted commented out code

* from_crontab now returns Result

* Implement error for CrontabParseError

* Removed `is_ok_and` call
  • Loading branch information
szokeasaurusrex authored Nov 10, 2023
1 parent 80e5b13 commit da4ff66
Show file tree
Hide file tree
Showing 5 changed files with 278 additions and 0 deletions.
48 changes: 48 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions sentry-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,6 @@ thiserror = "1.0.15"
time = { version = "0.3.5", features = ["formatting", "parsing"] }
url = { version = "2.1.1", features = ["serde"] }
uuid = { version = "1.0.0", features = ["serde"] }

[dev-dependencies]
rstest = "0.18.2"
149 changes: 149 additions & 0 deletions sentry-types/src/crontab_validator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
use std::ops::RangeInclusive;

struct SegmentAllowedValues<'a> {
/// Range of permitted numeric values
numeric_range: RangeInclusive<u64>,

/// Allowed alphabetic single values
single_values: &'a [&'a str],
}

const MONTHS: &[&str] = &[
"jan", "feb", "mar", "apr", "may", "jun", "jul", "aug", "sep", "oct", "nov", "dec",
];

const DAYS: &[&str] = &["sun", "mon", "tue", "wed", "thu", "fri", "sat"];

const ALLOWED_VALUES: &[&SegmentAllowedValues] = &[
&SegmentAllowedValues {
numeric_range: 0..=59,
single_values: &[],
},
&SegmentAllowedValues {
numeric_range: 0..=23,
single_values: &[],
},
&SegmentAllowedValues {
numeric_range: 1..=31,
single_values: &[],
},
&SegmentAllowedValues {
numeric_range: 1..=12,
single_values: MONTHS,
},
&SegmentAllowedValues {
numeric_range: 0..=6,
single_values: DAYS,
},
];

fn validate_range(range: &str, allowed_values: &SegmentAllowedValues) -> bool {
if range == "*" {
return true;
}

let range_limits: Vec<_> = range.split('-').map(str::parse::<u64>).collect();

range_limits.len() == 2
&& range_limits.iter().all(|limit| match limit {
Ok(limit) => allowed_values.numeric_range.contains(limit),
Err(_) => false,
})
&& range_limits[0].as_ref().unwrap() <= range_limits[1].as_ref().unwrap()
}

fn validate_step(step: &str) -> bool {
match step.parse::<u64>() {
Ok(value) => value > 0,
Err(_) => false,
}
}

fn validate_steprange(steprange: &str, allowed_values: &SegmentAllowedValues) -> bool {
let mut steprange_split = steprange.splitn(2, '/');
let range_is_valid = match steprange_split.next() {
Some(range) => validate_range(range, allowed_values),
None => false,
};

range_is_valid
&& match steprange_split.next() {
Some(step) => validate_step(step),
None => true,
}
}

fn validate_listitem(listitem: &str, allowed_values: &SegmentAllowedValues) -> bool {
match listitem.parse::<u64>() {
Ok(value) => allowed_values.numeric_range.contains(&value),
Err(_) => validate_steprange(listitem, allowed_values),
}
}

fn validate_list(list: &str, allowed_values: &SegmentAllowedValues) -> bool {
list.split(',')
.all(|listitem| validate_listitem(listitem, allowed_values))
}

fn validate_segment(segment: &str, allowed_values: &SegmentAllowedValues) -> bool {
allowed_values
.single_values
.contains(&segment.to_lowercase().as_ref())
|| validate_list(segment, allowed_values)
}

pub fn validate(crontab: &str) -> bool {
let lists: Vec<_> = crontab.split_whitespace().collect();
if lists.len() != 5 {
return false;
}

lists
.iter()
.zip(ALLOWED_VALUES)
.all(|(segment, allowed_values)| validate_segment(segment, allowed_values))
}

#[cfg(test)]
mod tests {
use super::*;
use rstest::rstest;

#[rstest]
#[case("* * * * *", true)]
#[case(" * * * * * ", true)]
#[case("invalid", false)]
#[case("", false)]
#[case("* * * *", false)]
#[case("* * * * * *", false)]
#[case("0 0 1 1 0", true)]
#[case("0 0 0 1 0", false)]
#[case("0 0 1 0 0", false)]
#[case("59 23 31 12 6", true)]
#[case("0 0 1 may sun", true)]
#[case("0 0 1 may sat,sun", false)]
#[case("0 0 1 may,jun sat", false)]
#[case("0 0 1 fri sun", false)]
#[case("0 0 1 JAN WED", true)]
#[case("0,24 5,23,6 1,2,3,31 1,2 5,6", true)]
#[case("0-20 * * * *", true)]
#[case("20-0 * * * *", false)]
#[case("0-20/3 * * * *", true)]
#[case("20/3 * * * *", false)]
#[case("*/3 * * * *", true)]
#[case("*/3,2 * * * *", true)]
#[case("*/foo * * * *", false)]
#[case("1-foo * * * *", false)]
#[case("foo-34 * * * *", false)]
fn test_parse(#[case] crontab: &str, #[case] expected: bool) {
assert_eq!(
validate(crontab),
expected,
"\"{crontab}\" is {}a valid crontab",
match expected {
true => "",
false => "not ",
},
);
}
}
1 change: 1 addition & 0 deletions sentry-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
mod macros;

mod auth;
mod crontab_validator;
mod dsn;
mod project_id;
pub mod protocol;
Expand Down
77 changes: 77 additions & 0 deletions sentry-types/src/protocol/monitor.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,50 @@
use std::{error::Error, fmt::Display};

use serde::{Deserialize, Serialize, Serializer};
use uuid::Uuid;

use crate::crontab_validator;

/// Error type for errors with parsing a crontab schedule
#[derive(Debug)]
pub struct CrontabParseError {
invalid_crontab: String,
}

impl Display for CrontabParseError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(
f,
"\"{}\" is not a valid crontab schedule.\n\t \
For help determining why this schedule is invalid, you can use this site: \
https://crontab.guru/#{}",
self.invalid_crontab,
self.invalid_crontab
.split_whitespace()
.collect::<Vec<_>>()
.join("_"),
)
}
}

impl Error for CrontabParseError {}

impl CrontabParseError {
/// Constructs a new CrontabParseError from a given invalid crontab string
///
/// ## Example
/// ```
/// use sentry_types::protocol::v7::CrontabParseError;
///
/// let error = CrontabParseError::new("* * * *");
/// ```
pub fn new(invalid_crontab: &str) -> Self {
Self {
invalid_crontab: String::from(invalid_crontab),
}
}
}

/// Represents the status of the monitor check-in
#[derive(Clone, Copy, Debug, PartialEq, Deserialize, Serialize)]
#[serde(rename_all = "snake_case")]
Expand Down Expand Up @@ -39,6 +83,39 @@ pub enum MonitorSchedule {
},
}

impl MonitorSchedule {
/// Attempts to create a MonitorSchedule from a provided crontab_str. If the crontab_str is a
/// valid crontab schedule, we return a Result containing the MonitorSchedule; otherwise, we
/// return a Result containing a CrontabParseError.
///
/// ## Example with valid crontab
/// ```
/// use sentry_types::protocol::v7::MonitorSchedule;
///
/// // Create a crontab that runs every other day of the month at midnight.
/// let result = MonitorSchedule::from_crontab("0 0 */2 * *");
/// assert!(result.is_ok())
/// ```
///
/// ## Example with an invalid crontab
/// ```
/// use sentry_types::protocol::v7::MonitorSchedule;
///
/// // Invalid crontab.
/// let result = MonitorSchedule::from_crontab("invalid");
/// assert!(result.is_err());
/// ```
pub fn from_crontab(crontab_str: &str) -> Result<Self, CrontabParseError> {
if crontab_validator::validate(crontab_str) {
Ok(Self::Crontab {
value: String::from(crontab_str),
})
} else {
Err(CrontabParseError::new(crontab_str))
}
}
}

/// The unit for the interval schedule type
#[derive(Clone, Copy, Debug, PartialEq, Deserialize, Serialize)]
#[serde(rename_all = "snake_case")]
Expand Down

0 comments on commit da4ff66

Please sign in to comment.