Revise config builder, tests, style

Cargo.toml

@@ -21,7 +21,7 @@ ipnet = "2.3.1"
 once_cell = "1.9.0"
 signal-hook = "0.3.13"
 signal-hook-tokio = { version = "0.3.0", features = ["futures-v0_3"] }
-tokio = { version = "1.15.0", features = ["fs", "process", "rt", "rt-multi-thread"] }
+tokio = { version = "1.15.0", features = ["fs", "io-util", "macros", "net", "process", "rt", "rt-multi-thread", "sync"] }
 
 [dev-dependencies]
 tokio = { version = "1.15.0", features = ["signal", "time"] }

README.md

@@ -70,7 +70,7 @@ use an up-to-date version of `miltertest`.)
 Once installed, SpamAssassin Milter can be invoked as `spamassassin-milter`.
 `spamassassin-milter` takes one mandatory argument, namely the listening socket
 of the milter (the socket to which the MTA will connect). The socket spec should
-be in one of the formats <code>inet:<em>host</em>:<em>port</em></code>, or
+be in one of the formats <code>inet:<em>host</em>:<em>port</em></code> or
 <code>unix:<em>path</em></code>, for a TCP or UNIX domain socket, respectively.
 
 For example, the following invocation starts SpamAssassin Milter on port 3000:

spamassassin-milter.8

@@ -35,7 +35,7 @@ argument specifies the listening socket to open.
 can be either an IPv4/IPv6 TCP socket in the form
 .BI inet: HOST : PORT
 (for example,
-.BR inet:localhost:3000 ),
+.BR inet:localhost:3000 )
 or a UNIX domain socket in the form
 .BI unix: PATH
 (for example,

src/callbacks.rs

@@ -22,7 +22,7 @@ use std::{
 macro_rules! ok_or_tempfail {
     ($expr:expr) => {
         if let ::std::result::Result::Err(e) = $expr {
-            ::std::eprintln!("error while communicating with spamc: {}", e);
+            ::std::eprintln!("failed to communicate with spamc: {}", e);
             return ::indymilter::Status::Tempfail;
         }
     };
@@ -141,9 +141,7 @@ async fn handle_connect(
         return Status::Accept;
     }
 
-    let conn = Connection::new(ip);
+    context.data = Some(Connection::new(ip));
-
-    context.data = Some(conn);
 
     Status::Continue
 }
@@ -277,7 +275,7 @@ async fn handle_eom(config: Arc<Config>, context: &mut EomContext<Connection>) -
     match client.process(&id, &mut context.reply, &context.actions, &config).await {
         Ok(status) => status,
         Err(e) => {
-            eprintln!("{}: error while processing message: {}", id, e);
+            eprintln!("{}: failed to process message: {}", id, e);
             Status::Tempfail
         }
     }

src/client.rs

@@ -351,7 +351,7 @@ mod tests {
     use indymilter::{ActionError, IntoCString, SmtpReplyError};
     use std::{ffi::CString, result, sync::Mutex};
 
-    #[derive(Clone, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd)]
+    #[derive(Debug, Default)]
     struct MockSpamc {
         buf: Vec<u8>,
         output: Option<Vec<u8>>,
@@ -397,28 +397,27 @@ mod tests {
         process.as_any().downcast_ref().unwrap()
     }
 
-    #[derive(Clone, Debug, Eq, Hash, PartialEq)]
+    #[derive(Debug, Eq, PartialEq)]
     enum Action {
         AddHeader(CString, CString),
         InsertHeader(i32, CString, CString),
-        ReplaceHeader(CString, i32, Option<CString>),
+        ChangeHeader(CString, i32, Option<CString>),
-        AppendBodyChunk(Vec<u8>),
+        ReplaceBody(Vec<u8>),
-        SetErrorReply(String, Option<String>, Vec<CString>),
     }
 
     #[derive(Debug, Default)]
-    struct MockActionContext {
+    struct MockEomActions {
         called: Mutex<Vec<Action>>,
     }
 
-    impl MockActionContext {
+    impl MockEomActions {
         fn new() -> Self {
             Default::default()
         }
     }
 
     #[async_trait]
-    impl ContextActions for MockActionContext {
+    impl ContextActions for MockEomActions {
         async fn add_header<'cx, 'k, 'v>(
             &'cx self,
             name: impl IntoCString + Send + 'k,
@@ -446,7 +445,7 @@ mod tests {
             index: i32,
             value: Option<impl IntoCString + Send + 'v>,
         ) -> result::Result<(), ActionError> {
-            let action = Action::ReplaceHeader(
+            let action = Action::ChangeHeader(
                 name.into_c_string(),
                 index,
                 value.map(|v| v.into_c_string()),
@@ -459,7 +458,7 @@ mod tests {
             &'cx self,
             chunk: &'a [u8],
         ) -> result::Result<(), ActionError> {
-            let action = Action::AppendBodyChunk(chunk.to_vec());
+            let action = Action::ReplaceBody(chunk.to_vec());
             self.called.lock().unwrap().push(action);
             Ok(())
         }
@@ -506,7 +505,18 @@ mod tests {
         }
     }
 
-    impl SetErrorReply for MockActionContext {
+    #[derive(Debug, Default)]
+    struct MockSmtpReply {
+        error_reply: Option<(String, Option<String>, Vec<CString>)>,
+    }
+
+    impl MockSmtpReply {
+        fn new() -> Self {
+            Default::default()
+        }
+    }
+
+    impl SetErrorReply for MockSmtpReply {
         fn set_error_reply<I, T>(
             &mut self,
             rcode: &str,
@@ -517,21 +527,22 @@ mod tests {
             I: IntoIterator<Item = T>,
             T: IntoCString,
         {
-            let action = Action::SetErrorReply(
+            self.error_reply = Some((
                 rcode.into(),
                 xcode.map(|c| c.into()),
                 message.into_iter().map(|l| l.into_c_string()).collect(),
-            );
+            ));
-            self.called.lock().unwrap().push(action);
             Ok(())
         }
     }
 
+    const ID: &str = "NONE";
+
     #[tokio::test]
     async fn client_send_writes_bytes() {
         let spamc = MockSpamc::new();
 
-        let mut client = Client::new(spamc, String::from("sender"));
+        let mut client = Client::new(spamc, "sender".into());
         client.send_header("name1", " value1").await.unwrap();
         client.send_header("name2", " value2\n\tcontinued").await.unwrap();
         client.send_eoh().await.unwrap();
@@ -552,9 +563,9 @@ mod tests {
         let recipient1 = "<recipient1@gluet.ch>";
         let recipient2 = "<recipient2@gluet.ch>";
 
-        let mut client = Client::new(spamc, String::from(sender));
+        let mut client = Client::new(spamc, sender.into());
-        client.add_recipient(String::from(recipient1));
+        client.add_recipient(recipient1.into());
-        client.add_recipient(String::from(recipient2));
+        client.add_recipient(recipient2.into());
 
         client.send_envelope_sender().await.unwrap();
         client.send_envelope_recipients().await.unwrap();
@@ -574,12 +585,14 @@ mod tests {
     #[tokio::test]
     async fn client_process_invalid_response() {
         let spamc = MockSpamc::with_output(b"invalid message response".to_vec());
-        let actions = MockActionContext::new();
+
-        let mut reply = MockActionContext::new();
+        let client = Client::new(spamc, "sender".into());
+
+        let mut reply = MockSmtpReply::new();
+        let actions = MockEomActions::new();
         let config = Default::default();
 
-        let client = Client::new(spamc, String::from("sender"));
+        let status = client.process(ID, &mut reply, &actions, &config).await.unwrap();
-        let status = client.process("id", &mut reply, &actions, &config).await.unwrap();
 
         assert_eq!(status, Status::Tempfail);
     }
@@ -587,27 +600,23 @@ mod tests {
     #[tokio::test]
     async fn client_process_reject_spam() {
         let spamc = MockSpamc::with_output(b"X-Spam-Flag: YES\r\n\r\n".to_vec());
-        let actions = MockActionContext::new();
-        let mut reply = MockActionContext::new();
-        let mut builder = Config::builder();
-        builder.reject_spam(true);
-        let config = builder.build();
 
-        let client = Client::new(spamc, String::from("sender"));
+        let client = Client::new(spamc, "sender".into());
-        let status = client.process("id", &mut reply, &actions, &config).await.unwrap();
+
+        let mut reply = MockSmtpReply::new();
+        let actions = MockEomActions::new();
+        let config = Config::builder().reject_spam(true).build();
+
+        let status = client.process(ID, &mut reply, &actions, &config).await.unwrap();
 
         assert_eq!(status, Status::Reject);
-
-        let called = reply.called.lock().unwrap();
         assert_eq!(
-            called.as_slice(),
+            reply.error_reply,
-            [
+            Some((
-                Action::SetErrorReply(
                 "550".into(),
                 Some("5.7.1".into()),
                 vec![c_str!("Spam message refused").into()],
-                ),
+            )),
-            ]
         );
     }
 
@@ -616,15 +625,17 @@ mod tests {
         let spamc = MockSpamc::with_output(
             b"X-Spam-Flag: YES\r\nX-Spam-Level: *****\r\n\r\nReport".to_vec(),
         );
-        let actions = MockActionContext::new();
-        let mut reply = MockActionContext::new();
-        let config = Default::default();
 
-        let mut client = Client::new(spamc, String::from("sender"));
+        let mut client = Client::new(spamc, "sender".into());
+
         client.send_header("x-spam-level", " *").await.unwrap();
         client.send_header("x-spam-report", " ...").await.unwrap();
 
-        let status = client.process("id", &mut reply, &actions, &config).await.unwrap();
+        let mut reply = MockSmtpReply::new();
+        let actions = MockEomActions::new();
+        let config = Default::default();
+
+        let status = client.process(ID, &mut reply, &actions, &config).await.unwrap();
 
         assert_eq!(status, Status::Continue);
 
@@ -632,11 +643,11 @@ mod tests {
         assert_eq!(
             called.as_slice(),
             [
-                Action::ReplaceHeader(c_str!("X-Spam-Level").into(), 1, None),
+                Action::ChangeHeader(c_str!("X-Spam-Level").into(), 1, None),
                 Action::InsertHeader(0, c_str!("X-Spam-Level").into(), c_str!(" *****").into()),
                 Action::InsertHeader(0, c_str!("X-Spam-Flag").into(), c_str!(" YES").into()),
-                Action::ReplaceHeader(c_str!("x-spam-report").into(), 1, None),
+                Action::ChangeHeader(c_str!("x-spam-report").into(), 1, None),
-                Action::AppendBodyChunk(b"Report".to_vec()),
+                Action::ReplaceBody(b"Report".to_vec()),
             ]
         );
     }
@@ -646,14 +657,15 @@ mod tests {
         let spamc = MockSpamc::with_output(
             b"X-Spam-Flag: YES\r\nX-Spam-Level: *****\r\n\r\nReport".to_vec(),
         );
-        let actions = MockActionContext::new();
-        let mut reply = MockActionContext::new();
-        let config = Default::default();
 
-        let mut client = Client::new(spamc, String::from("sender"));
+        let mut client = Client::new(spamc, "sender".into());
         client.skip_body();
 
-        let status = client.process("id", &mut reply, &actions, &config).await.unwrap();
+        let mut reply = MockSmtpReply::new();
+        let actions = MockEomActions::new();
+        let config = Default::default();
+
+        let status = client.process(ID, &mut reply, &actions, &config).await.unwrap();
 
         assert_eq!(status, Status::Continue);
 
@@ -663,6 +675,6 @@ mod tests {
             c_str!("X-Spam-Level").into(),
             c_str!(" *****").into()
         )));
-        assert!(!called.contains(&Action::AppendBodyChunk(b"Report".to_vec())));
+        assert!(!called.contains(&Action::ReplaceBody(b"Report".to_vec())));
     }
 }

src/collections.rs

@@ -31,15 +31,22 @@ where
     }
 
     pub fn contains_key<Q: AsRef<str>>(&self, key: Q) -> bool {
-        self.iter().any(|e| e.0.as_ref().eq_ignore_ascii_case(key.as_ref()))
+        self.iter()
+            .any(|e| e.0.as_ref().eq_ignore_ascii_case(key.as_ref()))
     }
 
     pub fn get<Q: AsRef<str>>(&self, key: Q) -> Option<&V> {
-        self.iter().find(|e| e.0.as_ref().eq_ignore_ascii_case(key.as_ref())).map(|e| e.1)
+        self.iter()
+            .find(|e| e.0.as_ref().eq_ignore_ascii_case(key.as_ref()))
+            .map(|e| e.1)
     }
 
     pub fn insert(&mut self, key: K, value: V) -> Option<V> {
-        match self.entries.iter_mut().find(|e| e.0.as_ref().eq_ignore_ascii_case(key.as_ref())) {
+        match self
+            .entries
+            .iter_mut()
+            .find(|e| e.0.as_ref().eq_ignore_ascii_case(key.as_ref()))
+        {
             None => {
                 self.entries.push((key, value));
                 None
@@ -67,13 +74,13 @@ impl<V> StrVecMap<String, V> {
 
 /// A vector set containing ASCII-case-insensitive `AsRef<str>` elements.
 #[derive(Clone, Debug, Default)]
-pub struct StrVecSet<E> {
+pub struct StrVecSet<T> {
-    map: StrVecMap<E, ()>,
+    map: StrVecMap<T, ()>,
 }
 
-impl<E> StrVecSet<E>
+impl<T> StrVecSet<T>
 where
-    E: AsRef<str>,
+    T: AsRef<str>,
 {
     pub fn new() -> Self {
         Self {
@@ -85,7 +92,7 @@ where
         self.map.contains_key(key)
     }
 
-    pub fn insert(&mut self, key: E) -> bool {
+    pub fn insert(&mut self, key: T) -> bool {
         self.map.insert(key, ()).is_none()
     }
 }

src/config.rs

@@ -20,23 +20,23 @@ pub struct ConfigBuilder {
 }
 
 impl ConfigBuilder {
-    pub fn use_trusted_networks(&mut self, value: bool) -> &mut Self {
+    pub fn use_trusted_networks(mut self, value: bool) -> Self {
         self.use_trusted_networks = value;
         self
     }
 
-    pub fn trusted_network(&mut self, net: IpNet) -> &mut Self {
+    pub fn trusted_network(mut self, net: IpNet) -> Self {
         self.use_trusted_networks = true;
         self.trusted_networks.insert(net);
         self
     }
 
-    pub fn auth_untrusted(&mut self, value: bool) -> &mut Self {
+    pub fn auth_untrusted(mut self, value: bool) -> Self {
         self.auth_untrusted = value;
         self
     }
 
-    pub fn spamc_args<I, S>(&mut self, args: I) -> &mut Self
+    pub fn spamc_args<I, S>(mut self, args: I) -> Self
     where
         I: IntoIterator<Item = S>,
         S: AsRef<str>,
@@ -45,47 +45,47 @@ impl ConfigBuilder {
         self
     }
 
-    pub fn max_message_size(&mut self, value: usize) -> &mut Self {
+    pub fn max_message_size(mut self, value: usize) -> Self {
         self.max_message_size = value;
         self
     }
 
-    pub fn dry_run(&mut self, value: bool) -> &mut Self {
+    pub fn dry_run(mut self, value: bool) -> Self {
         self.dry_run = value;
         self
     }
 
-    pub fn reject_spam(&mut self, value: bool) -> &mut Self {
+    pub fn reject_spam(mut self, value: bool) -> Self {
         self.reject_spam = value;
         self
     }
 
-    pub fn reply_code(&mut self, value: String) -> &mut Self {
+    pub fn reply_code(mut self, value: String) -> Self {
         self.reply_code = value;
         self
     }
 
-    pub fn reply_status_code(&mut self, value: String) -> &mut Self {
+    pub fn reply_status_code(mut self, value: String) -> Self {
         self.reply_status_code = value;
         self
     }
 
-    pub fn reply_text(&mut self, value: String) -> &mut Self {
+    pub fn reply_text(mut self, value: String) -> Self {
         self.reply_text = value;
         self
     }
 
-    pub fn preserve_headers(&mut self, value: bool) -> &mut Self {
+    pub fn preserve_headers(mut self, value: bool) -> Self {
         self.preserve_headers = value;
         self
     }
 
-    pub fn preserve_body(&mut self, value: bool) -> &mut Self {
+    pub fn preserve_body(mut self, value: bool) -> Self {
         self.preserve_body = value;
         self
     }
 
-    pub fn verbose(&mut self, value: bool) -> &mut Self {
+    pub fn verbose(mut self, value: bool) -> Self {
         self.verbose = value;
         self
     }
@@ -138,10 +138,10 @@ impl Default for ConfigBuilder {
             reject_spam: Default::default(),
             // This reply code and enhanced status code are the most appropriate
             // choices according to RFCs 5321 and 3463.
-            reply_code: String::from("550"),
+            reply_code: "550".into(),
-            reply_status_code: String::from("5.7.1"),
+            reply_status_code: "5.7.1".into(),
             // Generic reply text that makes no mention of SpamAssassin.
-            reply_text: String::from("Spam message refused"),
+            reply_text: "Spam message refused".into(),
             preserve_headers: Default::default(),
             preserve_body: Default::default(),
             verbose: Default::default(),
@@ -237,9 +237,9 @@ mod tests {
 
     #[test]
     fn trusted_networks_config() {
-        let mut builder = Config::builder();
+        let config = Config::builder()
-        builder.trusted_network("127.0.0.1/8".parse().unwrap());
+            .trusted_network("127.0.0.1/8".parse().unwrap())
-        let config = builder.build();
+            .build();
 
         assert!(config.use_trusted_networks());
         assert!(config.is_in_trusted_networks(&"127.0.0.1".parse().unwrap()));
@@ -248,10 +248,10 @@ mod tests {
 
     #[test]
     fn spamc_args_extends_args() {
-        let mut builder = Config::builder();
+        let config = Config::builder()
-        builder.spamc_args(&["-p", "3030"]);
+            .spamc_args(&["-p", "3030"])
-        builder.spamc_args(&["-x"]);
+            .spamc_args(&["-x"])
-        let config = builder.build();
+            .build();
 
         assert_eq!(
             config.spamc_args(),

src/email.rs

@@ -1,9 +1,9 @@
 use crate::{
     collections::{StrVecMap, StrVecSet},
     config::Config,
-    error::{self, Error},
+    error::{Error, Result},
 };
-use indymilter::{ActionError, ContextActions};
+use indymilter::ContextActions;
 use once_cell::sync::Lazy;
 use std::{
     ffi::CString,
@@ -11,32 +11,32 @@ use std::{
     str,
 };
 
-#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
+#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
 pub struct Header<'a> {
     pub name: &'a str,
     pub value: &'a str,
 }
 
-#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
+#[derive(Clone, Debug, Eq, Hash, PartialEq)]
 pub struct Email<'a> {
     pub header: Vec<Header<'a>>,
     pub body: &'a [u8],
 }
 
 impl<'a> Email<'a> {
-    pub fn parse(bytes: &'a [u8]) -> error::Result<Self> {
+    pub fn parse(bytes: &'a [u8]) -> Result<Self> {
         let (header, body) = split_at_eoh(bytes)?;
 
         let header = header_lines(header)
             .into_iter()
             .map(parse_header_line)
-            .collect::<error::Result<Vec<_>>>()?;
+            .collect::<Result<Vec<_>>>()?;
 
         Ok(Self { header, body })
     }
 }
 
-fn split_at_eoh(bytes: &[u8]) -> error::Result<(&[u8], &[u8])> {
+fn split_at_eoh(bytes: &[u8]) -> Result<(&[u8], &[u8])> {
     bytes
         .windows(4)
         .position(|w| w == b"\r\n\r\n")
@@ -73,7 +73,7 @@ fn header_lines(header: &[u8]) -> Vec<&[u8]> {
     lines
 }
 
-fn parse_header_line(bytes: &[u8]) -> error::Result<Header<'_>> {
+fn parse_header_line(bytes: &[u8]) -> Result<Header<'_>> {
     // This assumes that headers received back from SpamAssassin are valid
     // UTF-8, which should be the case since the client only sent UTF-8 earlier.
     let line = str::from_utf8(bytes).map_err(|_| Error::ParseEmail)?;
@@ -108,7 +108,7 @@ pub fn is_spam_assassin_header(name: &str) -> bool {
 
 // Values use CRLF line breaks and include leading whitespace.
 pub type HeaderMap = StrVecMap<String, String>;
-pub type HeaderSet<'e> = StrVecSet<&'e str>;
+pub type HeaderSet<'a> = StrVecSet<&'a str>;
 
 pub static REWRITE_HEADERS: Lazy<HeaderSet<'static>> = Lazy::new(|| {
     let mut h = HeaderSet::new();
@@ -235,7 +235,7 @@ impl<'a, 'c> HeaderRewriter<'a, 'c> {
         &self,
         id: &str,
         actions: &impl ContextActions,
-    ) -> Result<(), ActionError> {
+    ) -> Result<()> {
         let mods = self.spam_assassin_mods.iter();
         if self.prepend.unwrap_or(false) {
             // Prepend ‘X-Spam-’ headers in reverse order, so that they appear
@@ -260,7 +260,7 @@ impl<'a, 'c> HeaderRewriter<'a, 'c> {
         &self,
         id: &str,
         actions: &impl ContextActions,
-    ) -> Result<(), ActionError> {
+    ) -> Result<()> {
         execute_mods(id, self.rewrite_mods.iter(), actions, self.config).await
     }
 
@@ -268,7 +268,7 @@ impl<'a, 'c> HeaderRewriter<'a, 'c> {
         &self,
         id: &str,
         actions: &impl ContextActions,
-    ) -> Result<(), ActionError> {
+    ) -> Result<()> {
         execute_mods(id, self.report_mods.iter(), actions, self.config).await
     }
 }
@@ -278,7 +278,7 @@ async fn execute_mods<'a, I>(
     mods: I,
     actions: &impl ContextActions,
     config: &Config,
-) -> Result<(), ActionError>
+) -> Result<()>
 where
     I: IntoIterator<Item = &'a HeaderMod<'a>>,
 {
@@ -298,7 +298,7 @@ pub async fn replace_body(
     body: &[u8],
     actions: &impl ContextActions,
     config: &Config,
-) -> Result<(), ActionError> {
+) -> Result<()> {
     if config.dry_run() {
         verbose!(config, "{}: replacing message body [dry run, not done]", id);
     } else {
@@ -310,7 +310,7 @@ pub async fn replace_body(
 
 /// A header rewriting modification operation. These are intended to operate
 /// only on the first instance of headers occurring multiple times.
-#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
+#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
 enum HeaderMod<'a> {
     Add { name: &'a str, value: &'a str, prepend: bool },
     Replace { name: &'a str, value: &'a str, prepend: bool },
@@ -319,22 +319,22 @@ enum HeaderMod<'a> {
 }
 
 impl HeaderMod<'_> {
-    async fn execute(&self, actions: &impl ContextActions) -> Result<(), ActionError> {
+    async fn execute(&self, actions: &impl ContextActions) -> Result<()> {
-        use HeaderMod::*;
-
         // The milter library is smart enough to treat the name in a
         // case-insensitive manner, eg ‘Subject’ may replace ‘sUbject’.
         match *self {
-            Add { name, value, prepend } => add_header(actions, name, value, prepend).await,
+            Self::Add { name, value, prepend } => add_header(actions, name, value, prepend).await?,
-            Replace { name, value, prepend } => {
+            Self::Replace { name, value, prepend } => {
                 delete_header(actions, name).await?;
                 add_header(actions, name, value, prepend).await?;
+            }
+            Self::Modify { name, value } => {
+                actions.change_header(name, 1, Some(ensure_lf(value))).await?;
+            }
+            Self::Delete { name } => delete_header(actions, name).await?,
+        }
         Ok(())
     }
-            Modify { name, value } => actions.change_header(name, 1, Some(ensure_lf(value))).await,
-            Delete { name } => delete_header(actions, name).await,
-        }
-    }
 }
 
 async fn add_header(
@@ -342,16 +342,18 @@ async fn add_header(
     name: &str,
     value: &str,
     prepend: bool,
-) -> Result<(), ActionError> {
+) -> Result<()> {
     if prepend {
-        actions.insert_header(0, name, ensure_lf(value)).await
+        actions.insert_header(0, name, ensure_lf(value)).await?;
     } else {
-        actions.add_header(name, ensure_lf(value)).await
+        actions.add_header(name, ensure_lf(value)).await?;
     }
+    Ok(())
 }
 
-async fn delete_header(actions: &impl ContextActions, name: &str) -> Result<(), ActionError> {
+async fn delete_header(actions: &impl ContextActions, name: &str) -> Result<()> {
-    actions.change_header(name, 1, None::<CString>).await
+    actions.change_header(name, 1, None::<CString>).await?;
+    Ok(())
 }
 
 impl Display for HeaderMod<'_> {
@@ -457,7 +459,7 @@ mod tests {
     #[test]
     fn header_rewriter_flags_spam() {
         let mut headers = HeaderMap::new();
-        headers.insert(String::from("x-spam-flag"), String::from(" no"));
+        headers.insert("x-spam-flag".into(), " no".into());
         let config = Default::default();
 
         let mut rewriter = HeaderRewriter::new(headers, &config);
@@ -490,10 +492,10 @@ mod tests {
     #[test]
     fn header_rewriter_adds_and_replaces_headers() {
         let mut headers = HeaderMap::new();
-        headers.insert(String::from("x-spam-level"), String::from(" ***"));
+        headers.insert("x-spam-level".into(), " ***".into());
-        headers.insert(String::from("subject"), String::from(" original"));
+        headers.insert("subject".into(), " original".into());
-        headers.insert(String::from("x-spam-prev-subject"), String::from(" very original"));
+        headers.insert("x-spam-prev-subject".into(), " very original".into());
-        headers.insert(String::from("to"), String::from(" recipient@gluet.ch"));
+        headers.insert("to".into(), " recipient@gluet.ch".into());
         let config = Default::default();
 
         let mut rewriter = HeaderRewriter::new(headers, &config);

src/error.rs

@@ -7,7 +7,7 @@ use std::{
 
 pub type Result<T> = result::Result<T, Error>;
 
-#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
+#[derive(Clone, Debug, Eq, Hash, PartialEq)]
 pub enum Error {
     ParseEmail,
     SmtpReply,
@@ -21,8 +21,8 @@ impl Display for Error {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         match self {
             Self::ParseEmail => write!(f, "failed to parse email"),
-            Self::SmtpReply => write!(f, "failed to configure SMTP error reply"),
+            Self::SmtpReply => write!(f, "could not configure SMTP error reply"),
-            Self::Action => write!(f, "failed to execute context action"),
+            Self::Action => write!(f, "could not execute milter context action"),
             Self::Io(msg) => msg.fmt(f),
         }
     }

src/main.rs

@@ -99,7 +99,7 @@ async fn main() {
 
     let signals = Signals::new(&[SIGTERM, SIGINT]).expect("failed to install signal handler");
     let signals_handle = signals.handle();
-    let signals_task = tokio::spawn(handle_signals(signals, shutdown_tx));
+    let signals_task = spawn_signals_task(signals, shutdown_tx);
 
     let addr;
     let mut socket_path = None;
@@ -192,7 +192,7 @@ fn build_config(mut command: Command) -> clap::Result<(Socket, Config)> {
     if let Some(bytes) = matches.value_of(ARG_MAX_MESSAGE_SIZE) {
         match bytes.parse() {
             Ok(bytes) => {
-                config.max_message_size(bytes);
+                config = config.max_message_size(bytes);
             }
             Err(_) => {
                 return Err(command.error(
@@ -204,7 +204,7 @@ fn build_config(mut command: Command) -> clap::Result<(Socket, Config)> {
     }
 
     if let Some(nets) = matches.values_of(ARG_TRUSTED_NETWORKS) {
-        config.use_trusted_networks(true);
+        config = config.use_trusted_networks(true);
 
         for net in nets.filter(|n| !n.is_empty()) {
             // Both `ipnet::IpNet` and `std::net::IpAddr` inputs are supported.
@@ -213,7 +213,7 @@ fn build_config(mut command: Command) -> clap::Result<(Socket, Config)> {
                 .or_else(|_| net.parse::<IpAddr>().map(From::from))
             {
                 Ok(net) => {
-                    config.trusted_network(net);
+                    config = config.trusted_network(net);
                 }
                 Err(_) => {
                     return Err(command.error(
@@ -230,34 +230,34 @@ fn build_config(mut command: Command) -> clap::Result<(Socket, Config)> {
     validate_reply_codes(&mut command, reply_code, reply_status_code)?;
 
     if matches.is_present(ARG_AUTH_UNTRUSTED) {
-        config.auth_untrusted(true);
+        config = config.auth_untrusted(true);
     }
     if matches.is_present(ARG_DRY_RUN) {
-        config.dry_run(true);
+        config = config.dry_run(true);
     }
     if matches.is_present(ARG_PRESERVE_BODY) {
-        config.preserve_body(true);
+        config = config.preserve_body(true);
     }
     if matches.is_present(ARG_PRESERVE_HEADERS) {
-        config.preserve_headers(true);
+        config = config.preserve_headers(true);
     }
     if matches.is_present(ARG_REJECT_SPAM) {
-        config.reject_spam(true);
+        config = config.reject_spam(true);
     }
     if matches.is_present(ARG_VERBOSE) {
-        config.verbose(true);
+        config = config.verbose(true);
     }
     if let Some(code) = reply_code {
-        config.reply_code(code.to_owned());
+        config = config.reply_code(code.to_owned());
     }
     if let Some(code) = reply_status_code {
-        config.reply_status_code(code.to_owned());
+        config = config.reply_status_code(code.to_owned());
     }
     if let Some(msg) = matches.value_of(ARG_REPLY_TEXT) {
-        config.reply_text(msg.to_owned());
+        config = config.reply_text(msg.to_owned());
     }
     if let Some(spamc_args) = matches.values_of(ARG_SPAMC_ARGS) {
-        config.spamc_args(spamc_args);
+        config = config.spamc_args(spamc_args);
     };
 
     Ok((socket, config.build()))
@@ -292,16 +292,21 @@ fn validate_reply_codes(
     }
 }
 
-async fn handle_signals(mut signals: Signals, shutdown_milter: oneshot::Sender<()>) {
+fn spawn_signals_task(
+    mut signals: Signals,
+    shutdown_milter: oneshot::Sender<()>,
+) -> JoinHandle<()> {
+    tokio::spawn(async move {
         while let Some(signal) = signals.next().await {
             match signal {
-            SIGTERM | SIGINT => {
+                SIGINT | SIGTERM => {
                     let _ = shutdown_milter.send(());
                     break;
                 }
                 _ => panic!("unexpected signal"),
             }
         }
+    })
 }
 
 async fn cleanup(signals_handle: Handle, signals_task: JoinHandle<()>, socket_path: Option<&Path>) {

tests/common/mod.rs

@@ -13,7 +13,7 @@ use tokio::{
     process::Command,
     sync::oneshot,
     task::JoinHandle,
-    time::timeout,
+    time,
 };
 
 pub const LOCALHOST: (Ipv4Addr, u16) = (Ipv4Addr::LOCALHOST, 0);
@@ -22,12 +22,11 @@ pub const LOCALHOST: (Ipv4Addr, u16) = (Ipv4Addr::LOCALHOST, 0);
 /// importantly, this isolates `spamc` from any configuration file
 /// `/etc/spamassassin/spamc.conf` present on the host, as this configuration is
 /// read by default and may break the integration tests.
-pub fn configure_spamc(mut builder: ConfigBuilder) -> ConfigBuilder {
+pub fn configure_spamc(builder: ConfigBuilder) -> ConfigBuilder {
     // Note: Must use `-F` instead of `--config` due to a bug in `spamc`.
     // `--no-safe-fallback` prevents connection attempts from failing silently,
     // and `--log-to-stderr` avoids polluting syslog with test output.
-    builder.spamc_args(&["-F", "/dev/null", "--no-safe-fallback", "--log-to-stderr"]);
+    builder.spamc_args(&["-F", "/dev/null", "--no-safe-fallback", "--log-to-stderr"])
-    builder
 }
 
 pub const SPAMD_PORT: u16 = 3783;  // mock port
@@ -47,7 +46,7 @@ where
     Ok(tokio::spawn(async move {
         // This server expects and handles only a single connection, so that we
         // can `join` this task in the tests and detect errors and panics.
-        let (mut stream, _) = timeout(Duration::from_secs(10), listener.accept())
+        let (mut stream, _) = time::timeout(Duration::from_secs(10), listener.accept())
             .await
             .map_err(|e| io::Error::new(ErrorKind::Other, e))??;
 

tests/ham_message.rs

@@ -5,9 +5,9 @@ use spamassassin_milter::*;
 
 #[tokio::test]
 async fn ham_message() {
-    let mut builder = configure_spamc(Config::builder());
+    let config = configure_spamc(Config::builder())
-    builder.spamc_args(&[format!("--port={}", SPAMD_PORT)]);
+        .spamc_args(&[format!("--port={}", SPAMD_PORT)])
-    let config = builder.build();
+        .build();
 
     let server = spawn_mock_spamd_server(SPAMD_PORT, |ham| {
         let mut ham = ham

tests/live.rs

@@ -6,8 +6,8 @@ use spamassassin_milter::*;
 /// Runs a ‘live’ test against a real SpamAssassin server instance. This test is
 /// run on demand, as SpamAssassin will actually analyse the input, and do DNS
 /// queries etc.
-#[tokio::test]
 #[ignore]
+#[tokio::test]
 async fn live() {
     // When no port is specified, `spamc` will try to connect to the default
     // `spamd` port 783 (see also `/etc/services`).

tests/reject_spam.rs

@@ -5,13 +5,12 @@ use spamassassin_milter::*;
 
 #[tokio::test]
 async fn reject_spam() {
-    let mut builder = configure_spamc(Config::builder());
+    let config = configure_spamc(Config::builder())
-    builder
         .reject_spam(true)
         .reply_code("554".into())
         .reply_text("Not allowed!".into())
-        .spamc_args(&[format!("--port={}", SPAMD_PORT)]);
+        .spamc_args(&[format!("--port={}", SPAMD_PORT)])
-    let config = builder.build();
+        .build();
 
     let server = spawn_mock_spamd_server(SPAMD_PORT, |spam| {
         Err(spam.replacen("\r\n\r\n", "\r\nX-Spam-Flag: YES\r\n\r\n", 1))

tests/skip_oversized.rs

@@ -5,11 +5,10 @@ use spamassassin_milter::*;
 
 #[tokio::test]
 async fn skip_oversized() {
-    let mut builder = configure_spamc(Config::builder());
+    let config = configure_spamc(Config::builder())
-    builder
         .max_message_size(512)
-        .spamc_args(&[format!("--port={}", SPAMD_PORT)]);
+        .spamc_args(&[format!("--port={}", SPAMD_PORT)])
-    let config = builder.build();
+        .build();
 
     let server = spawn_mock_spamd_server(SPAMD_PORT, Ok).await.unwrap();
 

tests/spam_message.rs

@@ -5,9 +5,9 @@ use spamassassin_milter::*;
 
 #[tokio::test]
 async fn spam_message() {
-    let mut builder = configure_spamc(Config::builder());
+    let config = configure_spamc(Config::builder())
-    builder.spamc_args(&[format!("--port={}", SPAMD_PORT)]);
+        .spamc_args(&[format!("--port={}", SPAMD_PORT)])
-    let config = builder.build();
+        .build();
 
     let server = spawn_mock_spamd_server(SPAMD_PORT, |spam| {
         let mut spam = spam

tests/spamc_connection_error.rs

@@ -5,9 +5,9 @@ use spamassassin_milter::*;
 
 #[tokio::test]
 async fn spamc_connection_error() {
-    let mut builder = configure_spamc(Config::builder());
+    let config = configure_spamc(Config::builder())
-    builder.spamc_args(&[format!("--port={}", SPAMD_PORT)]);
+        .spamc_args(&[format!("--port={}", SPAMD_PORT)])
-    let config = builder.build();
+        .build();
 
     let milter = SpamAssassinMilter::spawn(LOCALHOST, config).await.unwrap();
 

tests/trusted_network_connection.rs

@@ -5,9 +5,9 @@ use spamassassin_milter::*;
 
 #[tokio::test]
 async fn trusted_network_connection() {
-    let mut builder = Config::builder();
+    let config = Config::builder()
-    builder.trusted_network("123.120.0.0/14".parse().unwrap());
+        .trusted_network("123.120.0.0/14".parse().unwrap())
-    let config = builder.build();
+        .build();
 
     let milter = SpamAssassinMilter::spawn(LOCALHOST, config).await.unwrap();