diff --git a/Cargo.toml b/Cargo.toml index 5e437e6..1d9561f 100644 --- a/Cargo.toml +++ b/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"] } diff --git a/README.md b/README.md index 3b1f2bb..0ec3fa5 100644 --- a/README.md +++ b/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 inet:host:port, or +be in one of the formats inet:host:port or unix:path, for a TCP or UNIX domain socket, respectively. For example, the following invocation starts SpamAssassin Milter on port 3000: diff --git a/spamassassin-milter.8 b/spamassassin-milter.8 index 84f3e30..c3577a0 100644 --- a/spamassassin-milter.8 +++ b/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, diff --git a/src/callbacks.rs b/src/callbacks.rs index 8704b8c..fe0615a 100644 --- a/src/callbacks.rs +++ b/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(conn); + context.data = Some(Connection::new(ip)); Status::Continue } @@ -277,7 +275,7 @@ async fn handle_eom(config: Arc, context: &mut EomContext) - 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 } } diff --git a/src/client.rs b/src/client.rs index a75d3ef..84efd7e 100644 --- a/src/client.rs +++ b/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, output: Option>, @@ -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), - AppendBodyChunk(Vec), - SetErrorReply(String, Option, Vec), + ChangeHeader(CString, i32, Option), + ReplaceBody(Vec), } #[derive(Debug, Default)] - struct MockActionContext { + struct MockEomActions { called: Mutex>, } - 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, ) -> 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, Vec)>, + } + + impl MockSmtpReply { + fn new() -> Self { + Default::default() + } + } + + impl SetErrorReply for MockSmtpReply { fn set_error_reply( &mut self, rcode: &str, @@ -517,21 +527,22 @@ mod tests { I: IntoIterator, 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 = ""; let recipient2 = ""; - let mut client = Client::new(spamc, String::from(sender)); - client.add_recipient(String::from(recipient1)); - client.add_recipient(String::from(recipient2)); + let mut client = Client::new(spamc, sender.into()); + client.add_recipient(recipient1.into()); + 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 status = client.process("id", &mut reply, &actions, &config).await.unwrap(); + let client = Client::new(spamc, "sender".into()); + + 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(), - [ - Action::SetErrorReply( - "550".into(), - Some("5.7.1".into()), - vec![c_str!("Spam message refused").into()], - ), - ] + reply.error_reply, + Some(( + "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::AppendBodyChunk(b"Report".to_vec()), + Action::ChangeHeader(c_str!("x-spam-report").into(), 1, None), + 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()))); } } diff --git a/src/collections.rs b/src/collections.rs index 1a56061..642c099 100644 --- a/src/collections.rs +++ b/src/collections.rs @@ -31,15 +31,22 @@ where } pub fn contains_key>(&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>(&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 { - 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 StrVecMap { /// A vector set containing ASCII-case-insensitive `AsRef` elements. #[derive(Clone, Debug, Default)] -pub struct StrVecSet { - map: StrVecMap, +pub struct StrVecSet { + map: StrVecMap, } -impl StrVecSet +impl StrVecSet where - E: AsRef, + T: AsRef, { 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() } } diff --git a/src/config.rs b/src/config.rs index a3bbd17..bf0264a 100644 --- a/src/config.rs +++ b/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(&mut self, args: I) -> &mut Self + pub fn spamc_args(mut self, args: I) -> Self where I: IntoIterator, S: AsRef, @@ -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_status_code: String::from("5.7.1"), + reply_code: "550".into(), + 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(); - builder.trusted_network("127.0.0.1/8".parse().unwrap()); - let config = builder.build(); + let config = Config::builder() + .trusted_network("127.0.0.1/8".parse().unwrap()) + .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(); - builder.spamc_args(&["-p", "3030"]); - builder.spamc_args(&["-x"]); - let config = builder.build(); + let config = Config::builder() + .spamc_args(&["-p", "3030"]) + .spamc_args(&["-x"]) + .build(); assert_eq!( config.spamc_args(), diff --git a/src/email.rs b/src/email.rs index a188de5..aa5cc85 100644 --- a/src/email.rs +++ b/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>, pub body: &'a [u8], } impl<'a> Email<'a> { - pub fn parse(bytes: &'a [u8]) -> error::Result { + pub fn parse(bytes: &'a [u8]) -> Result { let (header, body) = split_at_eoh(bytes)?; let header = header_lines(header) .into_iter() .map(parse_header_line) - .collect::>>()?; + .collect::>>()?; 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> { +fn parse_header_line(bytes: &[u8]) -> Result> { // 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; -pub type HeaderSet<'e> = StrVecSet<&'e str>; +pub type HeaderSet<'a> = StrVecSet<&'a str>; pub static REWRITE_HEADERS: Lazy> = 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>, { @@ -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,21 +319,21 @@ enum HeaderMod<'a> { } impl HeaderMod<'_> { - async fn execute(&self, actions: &impl ContextActions) -> Result<(), ActionError> { - use HeaderMod::*; - + async fn execute(&self, actions: &impl ContextActions) -> Result<()> { // 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, - Replace { name, value, prepend } => { + Self::Add { name, value, prepend } => add_header(actions, name, value, prepend).await?, + Self::Replace { name, value, prepend } => { delete_header(actions, name).await?; add_header(actions, name, value, prepend).await?; - Ok(()) } - Modify { name, value } => actions.change_header(name, 1, Some(ensure_lf(value))).await, - Delete { name } => delete_header(actions, name).await, + Self::Modify { name, value } => { + actions.change_header(name, 1, Some(ensure_lf(value))).await?; + } + Self::Delete { name } => delete_header(actions, name).await?, } + Ok(()) } } @@ -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> { - actions.change_header(name, 1, None::).await +async fn delete_header(actions: &impl ContextActions, name: &str) -> Result<()> { + actions.change_header(name, 1, None::).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(String::from("subject"), String::from(" original")); - headers.insert(String::from("x-spam-prev-subject"), String::from(" very original")); - headers.insert(String::from("to"), String::from(" recipient@gluet.ch")); + headers.insert("x-spam-level".into(), " ***".into()); + headers.insert("subject".into(), " original".into()); + headers.insert("x-spam-prev-subject".into(), " very original".into()); + headers.insert("to".into(), " recipient@gluet.ch".into()); let config = Default::default(); let mut rewriter = HeaderRewriter::new(headers, &config); diff --git a/src/error.rs b/src/error.rs index c255dc0..d05df43 100644 --- a/src/error.rs +++ b/src/error.rs @@ -7,7 +7,7 @@ use std::{ pub type Result = result::Result; -#[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::Action => write!(f, "failed to execute context action"), + Self::SmtpReply => write!(f, "could not configure SMTP error reply"), + Self::Action => write!(f, "could not execute milter context action"), Self::Io(msg) => msg.fmt(f), } } diff --git a/src/main.rs b/src/main.rs index 5630824..f332083 100644 --- a/src/main.rs +++ b/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::().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<()>) { - while let Some(signal) = signals.next().await { - match signal { - SIGTERM | SIGINT => { - let _ = shutdown_milter.send(()); - break; +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 { + SIGINT | SIGTERM => { + let _ = shutdown_milter.send(()); + break; + } + _ => panic!("unexpected signal"), } - _ => panic!("unexpected signal"), } - } + }) } async fn cleanup(signals_handle: Handle, signals_task: JoinHandle<()>, socket_path: Option<&Path>) { diff --git a/tests/common/mod.rs b/tests/common/mod.rs index fdc6d64..f7d0403 100644 --- a/tests/common/mod.rs +++ b/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 + builder.spamc_args(&["-F", "/dev/null", "--no-safe-fallback", "--log-to-stderr"]) } 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))??; diff --git a/tests/ham_message.rs b/tests/ham_message.rs index 827597c..0940479 100644 --- a/tests/ham_message.rs +++ b/tests/ham_message.rs @@ -5,9 +5,9 @@ use spamassassin_milter::*; #[tokio::test] async fn ham_message() { - let mut builder = configure_spamc(Config::builder()); - builder.spamc_args(&[format!("--port={}", SPAMD_PORT)]); - let config = builder.build(); + let config = configure_spamc(Config::builder()) + .spamc_args(&[format!("--port={}", SPAMD_PORT)]) + .build(); let server = spawn_mock_spamd_server(SPAMD_PORT, |ham| { let mut ham = ham diff --git a/tests/live.rs b/tests/live.rs index 9663f73..70fb40a 100644 --- a/tests/live.rs +++ b/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`). diff --git a/tests/reject_spam.rs b/tests/reject_spam.rs index 19560b5..5a7fb96 100644 --- a/tests/reject_spam.rs +++ b/tests/reject_spam.rs @@ -5,13 +5,12 @@ use spamassassin_milter::*; #[tokio::test] async fn reject_spam() { - let mut builder = configure_spamc(Config::builder()); - builder + let config = configure_spamc(Config::builder()) .reject_spam(true) .reply_code("554".into()) .reply_text("Not allowed!".into()) - .spamc_args(&[format!("--port={}", SPAMD_PORT)]); - let config = builder.build(); + .spamc_args(&[format!("--port={}", SPAMD_PORT)]) + .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)) diff --git a/tests/skip_oversized.rs b/tests/skip_oversized.rs index 009f244..4801ee8 100644 --- a/tests/skip_oversized.rs +++ b/tests/skip_oversized.rs @@ -5,11 +5,10 @@ use spamassassin_milter::*; #[tokio::test] async fn skip_oversized() { - let mut builder = configure_spamc(Config::builder()); - builder + let config = configure_spamc(Config::builder()) .max_message_size(512) - .spamc_args(&[format!("--port={}", SPAMD_PORT)]); - let config = builder.build(); + .spamc_args(&[format!("--port={}", SPAMD_PORT)]) + .build(); let server = spawn_mock_spamd_server(SPAMD_PORT, Ok).await.unwrap(); diff --git a/tests/spam_message.rs b/tests/spam_message.rs index f6ffaa2..b58d2d3 100644 --- a/tests/spam_message.rs +++ b/tests/spam_message.rs @@ -5,9 +5,9 @@ use spamassassin_milter::*; #[tokio::test] async fn spam_message() { - let mut builder = configure_spamc(Config::builder()); - builder.spamc_args(&[format!("--port={}", SPAMD_PORT)]); - let config = builder.build(); + let config = configure_spamc(Config::builder()) + .spamc_args(&[format!("--port={}", SPAMD_PORT)]) + .build(); let server = spawn_mock_spamd_server(SPAMD_PORT, |spam| { let mut spam = spam diff --git a/tests/spamc_connection_error.rs b/tests/spamc_connection_error.rs index bb35bb3..75be941 100644 --- a/tests/spamc_connection_error.rs +++ b/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()); - builder.spamc_args(&[format!("--port={}", SPAMD_PORT)]); - let config = builder.build(); + let config = configure_spamc(Config::builder()) + .spamc_args(&[format!("--port={}", SPAMD_PORT)]) + .build(); let milter = SpamAssassinMilter::spawn(LOCALHOST, config).await.unwrap(); diff --git a/tests/trusted_network_connection.rs b/tests/trusted_network_connection.rs index 5ff2716..c1e575d 100644 --- a/tests/trusted_network_connection.rs +++ b/tests/trusted_network_connection.rs @@ -5,9 +5,9 @@ use spamassassin_milter::*; #[tokio::test] async fn trusted_network_connection() { - let mut builder = Config::builder(); - builder.trusted_network("123.120.0.0/14".parse().unwrap()); - let config = builder.build(); + let config = Config::builder() + .trusted_network("123.120.0.0/14".parse().unwrap()) + .build(); let milter = SpamAssassinMilter::spawn(LOCALHOST, config).await.unwrap();