[PATCH proxmox v3 17/39] notify: matcher: don't allow empty field/severity matchers

[Lukas Wagner <l.wagner@proxmox.com>]

Severity and field matcher in 'exact' mode allow matching on multiple
values, which is equivalent to an 'any-of' of multiple matchers matching
on a single value. Since we don't allow empty 'any-of' groups, it makes
sense to reject empty/severity matchers as well.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    new in v2

 proxmox-notify/src/matcher/calendar.rs   |  8 ++++
 proxmox-notify/src/matcher/expression.rs | 13 ++++++
 proxmox-notify/src/matcher/field.rs      | 16 +++++++
 proxmox-notify/src/matcher/mod.rs        | 55 ++++++++++++++++++++++++
 proxmox-notify/src/matcher/severity.rs   | 14 ++++++
 5 files changed, 106 insertions(+)

diff --git a/proxmox-notify/src/matcher/calendar.rs b/proxmox-notify/src/matcher/calendar.rs
index ceb4db76..7fd18291 100644
--- a/proxmox-notify/src/matcher/calendar.rs
+++ b/proxmox-notify/src/matcher/calendar.rs
@@ -67,6 +67,10 @@ impl MatchDirective for CalendarMatcher {
             .time_match(notification.metadata.timestamp, false)
             .map_err(|err| Error::Generic(format!("could not match timestamp: {err}")))
     }
+
+    fn validate(&self) -> Result<(), Error> {
+        Ok(())
+    }
 }
 
 #[cfg(feature = "legacy-matchers")]
@@ -84,6 +88,10 @@ pub mod inline {
         fn matches(&self, notification: &Notification) -> Result<bool, Error> {
             self.0.matches(notification)
         }
+
+        fn validate(&self) -> Result<(), Error> {
+            self.0.validate()
+        }
     }
 
     impl fmt::Display for InlineCalendarMatcher {
diff --git a/proxmox-notify/src/matcher/expression.rs b/proxmox-notify/src/matcher/expression.rs
index 9bb12f74..55c71cc4 100644
--- a/proxmox-notify/src/matcher/expression.rs
+++ b/proxmox-notify/src/matcher/expression.rs
@@ -32,6 +32,19 @@ impl MatchExpression for NotificationMatcher {
     }
 }
 
+impl NotificationMatcher {
+    /// Validate any additional constraints that cannot be expressed in the type system.
+    pub(crate) fn validate(&self) -> Result<(), Error> {
+        use crate::matcher::MatchDirective;
+
+        match self {
+            NotificationMatcher::Field(field_matcher) => field_matcher.validate(),
+            NotificationMatcher::Calendar(calendar_matcher) => calendar_matcher.validate(),
+            NotificationMatcher::Severity(severity_matcher) => severity_matcher.validate(),
+        }
+    }
+}
+
 #[cfg(test)]
 mod test {
     use proxmox_match_expression::Expression;
diff --git a/proxmox-notify/src/matcher/field.rs b/proxmox-notify/src/matcher/field.rs
index 4a4b2588..cd23ff9c 100644
--- a/proxmox-notify/src/matcher/field.rs
+++ b/proxmox-notify/src/matcher/field.rs
@@ -53,6 +53,18 @@ impl MatchDirective for FieldMatcher {
             }
         })
     }
+
+    fn validate(&self) -> Result<(), Error> {
+        if let FieldMatcher::Exact { values, .. } = self {
+            if values.is_empty() {
+                return Err(Error::Generic(
+                    "field matcher must contain values to match".to_string(),
+                ));
+            }
+        }
+
+        Ok(())
+    }
 }
 
 #[cfg(feature = "legacy-matchers")]
@@ -95,6 +107,10 @@ pub mod inline {
         fn matches(&self, notification: &Notification) -> Result<bool, Error> {
             self.0.matches(notification)
         }
+
+        fn validate(&self) -> Result<(), Error> {
+            self.0.validate()
+        }
     }
 
     impl fmt::Display for InlineFieldMatcher {
diff --git a/proxmox-notify/src/matcher/mod.rs b/proxmox-notify/src/matcher/mod.rs
index 7279ef25..f4c11c37 100644
--- a/proxmox-notify/src/matcher/mod.rs
+++ b/proxmox-notify/src/matcher/mod.rs
@@ -187,6 +187,8 @@ pub struct MatcherConfig {
 
 trait MatchDirective {
     fn matches(&self, notification: &Notification) -> Result<bool, Error>;
+    /// Validate any additional constraints that cannot be expressed in the type system.
+    fn validate(&self) -> Result<(), Error>;
 }
 
 impl MatcherConfig {
@@ -332,6 +334,9 @@ impl MatcherConfig {
                     ));
                 }
             }
+            Expression::Match(matcher) => {
+                matcher.validate()?;
+            }
             _ => {}
         }
 
@@ -452,6 +457,7 @@ pub fn check_matches<'a>(
 
 #[cfg(test)]
 mod tests {
+    use super::*;
 
     #[test]
     #[cfg(feature = "legacy-matchers")]
@@ -473,4 +479,53 @@ mod tests {
             assert!(config.matches(&notification).unwrap().is_some())
         }
     }
+
+    #[test]
+    fn test_expression_constraints() {
+        let empty_any_of = r#"
+            {
+              "any-of": []
+            }"#;
+        let empty_all_of = r#"
+            {
+              "all-of": []
+            }"#;
+        let empty_one_of = r#"
+            {
+              "one-of": []
+            }"#;
+
+        let empty_field_matcher = r#"
+            {
+              "match": {
+                "type": "field",
+                "field": "something",
+                "values": []
+              }
+            }"#;
+
+        let empty_severity_matcher = r#"
+            {
+              "match": {
+                "type": "severity",
+                "severities": []
+              }
+            }"#;
+
+        for expr in [
+            empty_one_of,
+            empty_all_of,
+            empty_any_of,
+            empty_field_matcher,
+            empty_severity_matcher,
+        ] {
+            let config = MatcherConfig {
+                name: "matcher".to_string(),
+                expression: Some(expr.into()),
+                ..Default::default()
+            };
+
+            assert!(config.ensure_valid().is_err());
+        }
+    }
 }
diff --git a/proxmox-notify/src/matcher/severity.rs b/proxmox-notify/src/matcher/severity.rs
index 2088ca74..2360dfca 100644
--- a/proxmox-notify/src/matcher/severity.rs
+++ b/proxmox-notify/src/matcher/severity.rs
@@ -23,6 +23,16 @@ impl MatchDirective for SeverityMatcher {
     fn matches(&self, notification: &Notification) -> Result<bool, Error> {
         Ok(self.severities.contains(&notification.metadata.severity))
     }
+
+    fn validate(&self) -> Result<(), Error> {
+        if self.severities.is_empty() {
+            return Err(Error::Generic(
+                "severity matcher must contain severities to match".to_string(),
+            ));
+        }
+
+        Ok(())
+    }
 }
 
 #[cfg(feature = "legacy-matchers")]
@@ -44,6 +54,10 @@ pub mod inline {
         fn matches(&self, notification: &Notification) -> Result<bool, Error> {
             self.0.matches(notification)
         }
+
+        fn validate(&self) -> Result<(), Error> {
+            self.0.validate()
+        }
     }
 
     impl fmt::Display for InlineSeverityMatcher {
-- 
2.47.3