Repository: memorysafety/sudo-rs
Pull request #795 (2023-10-27 14:05:18)

refactor sudoers tests for readability


Patch:
diff --git a/src/common/command.rs b/src/common/command.rs
index d3b09da7..fd3b8a0d 100644
--- a/src/common/command.rs
+++ b/src/common/command.rs
@@ -63,10 +63,7 @@ impl CommandAndArguments {
                 arguments = vec!["-c".to_string(), escaped(arguments)]
             }
         } else {
-            command = arguments
-                .get(0)
-                .map(|s| s.into())
-                .unwrap_or_else(PathBuf::new);
+            command = arguments.get(0).map(|s| s.into()).unwrap_or_default();
             arguments.remove(0);
 
             // remember the original binary name before resolving symlinks; this is not
diff --git a/src/sudoers/ast.rs b/src/sudoers/ast.rs
index 6f23e5d3..502da184 100644
--- a/src/sudoers/ast.rs
+++ b/src/sudoers/ast.rs
@@ -28,6 +28,15 @@ impl<T> Qualified<T> {
             Qualified::Forbid(item) => Qualified::Allow(item),
         }
     }
+
+    #[cfg(test)]
+    pub fn as_allow(&self) -> Option<&T> {
+        if let Self::Allow(v) = self {
+            Some(v)
+        } else {
+            None
+        }
+    }
 }
 
 /// Type aliases; many items can be replaced by ALL, aliases, and negated.
@@ -133,6 +142,51 @@ pub enum Sudo {
     LineComment = HARDENED_ENUM_VALUE_4,
 }
 
+impl Sudo {
+    #[cfg(test)]
+    pub fn is_spec(&self) -> bool {
+        matches!(self, Self::Spec(..))
+    }
+
+    #[cfg(test)]
+    pub fn is_decl(&self) -> bool {
+        matches!(self, Self::Decl(..))
+    }
+
+    #[cfg(test)]
+    pub fn is_line_comment(&self) -> bool {
+        matches!(self, Self::LineComment)
+    }
+
+    #[cfg(test)]
+    pub fn is_include(&self) -> bool {
+        matches!(self, Self::Include(..))
+    }
+
+    #[cfg(test)]
+    pub fn is_include_dir(&self) -> bool {
+        matches!(self, Self::IncludeDir(..))
+    }
+
+    #[cfg(test)]
+    pub fn as_include(&self) -> &str {
+        if let Self::Include(v) = self {
+            v
+        } else {
+            panic!()
+        }
+    }
+
+    #[cfg(test)]
+    pub fn as_spec(&self) -> Option<&PermissionSpec> {
+        if let Self::Spec(v) = self {
+            Some(v)
+        } else {
+            None
+        }
+    }
+}
+
 /// grammar:
 /// ```text
 /// identifier = name
diff --git a/src/sudoers/tokens.rs b/src/sudoers/tokens.rs
index db3800b1..44ff8e29 100644
--- a/src/sudoers/tokens.rs
+++ b/src/sudoers/tokens.rs
@@ -84,6 +84,13 @@ pub enum Meta<T> {
     Alias(String) = HARDENED_ENUM_VALUE_2,
 }
 
+impl<T> Meta<T> {
+    #[cfg(test)]
+    pub fn is_alias(&self) -> bool {
+        matches!(self, Self::Alias(..))
+    }
+}
+
 impl<T: Token> Token for Meta<T> {
     fn construct(raw: String) -> Result<Self, String> {
         // `T` may accept whitespace resulting in `raw` having trailing whitespace which would make


Test patch:
diff --git a/src/sudoers/test/mod.rs b/src/sudoers/test/mod.rs
index 08fe0244..328e1549 100644
--- a/src/sudoers/test/mod.rs
+++ b/src/sudoers/test/mod.rs
@@ -62,15 +62,22 @@ macro_rules! sudoer {
 }
 
 // alternative to parse_eval, but goes through sudoer! directly
+#[must_use]
 fn parse_line(s: &str) -> Sudo {
     sudoer![s].next().unwrap().unwrap()
 }
 
+/// Returns `None` if a syntax error is encountered
+fn try_parse_line(s: &str) -> Option<Sudo> {
+    parse_lines(&mut [s, ""].join("").chars().peekable())
+        .into_iter()
+        .next()?
+        .ok()
+}
+
 #[test]
 fn ambiguous_spec() {
-    let Sudo::Spec(_) = parse_eval::<ast::Sudo>("marc, User_Alias ALL = ALL") else {
-        todo!()
-    };
+    assert!(parse_eval::<ast::Sudo>("marc, User_Alias ALL = ALL").is_spec());
 }
 
 #[test]
@@ -342,61 +349,27 @@ fn directive_test() {
 #[test]
 // the overloading of '#' causes a lot of issues
 fn hashsign_test() {
-    let Sudo::Spec(_) = parse_line("#42 ALL=ALL") else {
-        panic!()
-    };
-    let Sudo::Spec(_) = parse_line("ALL ALL=(#42) ALL") else {
-        panic!()
-    };
-    let Sudo::Spec(_) = parse_line("ALL ALL=(%#42) ALL") else {
-        panic!()
-    };
-    let Sudo::Spec(_) = parse_line("ALL ALL=(:#42) ALL") else {
-        panic!()
-    };
-    let Sudo::Decl(_) = parse_line("User_Alias FOO=#42, %#0, #3") else {
-        panic!()
-    };
-    let Sudo::LineComment = parse_line("") else {
-        panic!()
-    };
-    let Sudo::LineComment = parse_line("#this is a comment") else {
-        panic!()
-    };
-    let Sudo::Include(_) = parse_line("#include foo") else {
-        panic!()
-    };
-    let Sudo::IncludeDir(_) = parse_line("#includedir foo") else {
-        panic!()
-    };
-    let Sudo::Include(x) = parse_line("#include \"foo bar\"") else {
-        panic!()
-    };
-    assert_eq!(x, "foo bar");
+    assert!(parse_line("#42 ALL=ALL").is_spec());
+    assert!(parse_line("ALL ALL=(#42) ALL").is_spec());
+    assert!(parse_line("ALL ALL=(%#42) ALL").is_spec());
+    assert!(parse_line("ALL ALL=(:#42) ALL").is_spec());
+    assert!(parse_line("User_Alias FOO=#42, %#0, #3").is_decl());
+    assert!(parse_line("").is_line_comment());
+    assert!(parse_line("#this is a comment").is_line_comment());
+    assert!(parse_line("#include foo").is_include());
+    assert!(parse_line("#includedir foo").is_include_dir());
+    assert_eq!("foo bar", parse_line("#include \"foo bar\"").as_include());
     // this is fine
-    let Sudo::LineComment = parse_line("#inlcudedir foo") else {
-        panic!()
-    };
-    let Sudo::Include(_) = parse_line("@include foo") else {
-        panic!()
-    };
-    let Sudo::IncludeDir(_) = parse_line("@includedir foo") else {
-        panic!()
-    };
-    let Sudo::Include(x) = parse_line("@include \"foo bar\"") else {
-        panic!()
-    };
-    assert_eq!(x, "foo bar");
+    assert!(parse_line("#inlcudedir foo").is_line_comment());
+    assert!(parse_line("@include foo").is_include());
+    assert!(parse_line("@includedir foo").is_include_dir());
+    assert_eq!("foo bar", parse_line("@include \"foo bar\"").as_include());
 }
 
 #[test]
 fn gh674_at_include_quoted_backslash() {
-    let Sudo::Include(_) = parse_line(r#"@include "/etc/sudo\ers" "#) else {
-        panic!()
-    };
-    let Sudo::IncludeDir(_) = parse_line(r#"@includedir "/etc/sudo\ers.d" "#) else {
-        panic!()
-    };
+    assert!(parse_line(r#"@include "/etc/sudo\ers" "#).is_include());
+    assert!(parse_line(r#"@includedir "/etc/sudo\ers.d" "#).is_include_dir());
 }
 
 #[test]
@@ -413,47 +386,38 @@ fn gh676_percent_h_escape_unsupported() {
 }
 
 #[test]
-#[should_panic]
 fn hashsign_error() {
-    let Sudo::Include(_) = parse_line("#include foo bar") else {
-        todo!()
-    };
+    assert!(parse_line("#include foo bar").is_line_comment());
 }
 
 #[test]
-#[should_panic]
 fn include_regression() {
-    let Sudo::Include(_) = parse_line("#4,#include foo") else {
-        todo!()
-    };
+    assert!(try_parse_line("#4,#include foo").is_none());
 }
 
 #[test]
-#[should_panic]
 fn nullbyte_regression() {
-    if let Sudo::Spec(PermissionSpec { .. }) = parse_line("ferris ALL=(ALL:ferris\0) ALL") {};
+    assert!(try_parse_line("ferris ALL=(ALL:ferris\0) ALL").is_none());
 }
 
 #[test]
-#[should_panic]
 fn alias_all_regression() {
-    parse_line("User_Alias ALL = sudouser");
+    assert!(try_parse_line("User_Alias ALL = sudouser").is_none())
 }
 
 #[test]
-#[should_panic]
 fn defaults_regression() {
-    parse_line("Defaults .mymachine=ALL");
+    assert!(try_parse_line("Defaults .mymachine=ALL").is_none())
 }
 
 #[test]
 fn useralias_underscore_regression() {
-    let Sudo::Spec(x) = parse_line("FOO_BAR ALL=ALL") else {
-        todo!()
-    };
-    let Qualified::Allow(Meta::Alias(_)) = x.users[0] else {
-        panic!()
-    };
+    let sudo = parse_line("FOO_BAR ALL=ALL");
+    let spec = sudo.as_spec().expect("`Sudo::Spec`");
+    assert!(spec.users[0]
+        .as_allow()
+        .expect("`Qualified::Allow`")
+        .is_alias());
 }
 
 fn test_topo_sort(n: usize) {