Bump MSRV to 1.61.0, replace panicking [e]println!

CHANGELOG.md

@@ -1,5 +1,18 @@
 # SpamAssassin Milter changelog
 
+## 0.4.0 (unreleased)
+
+The minimum supported Rust version is now 1.61.0.
+
+### Changed
+
+* The minimum supported Rust version has been raised to 1.61.0.
+
+### Fixed
+
+* The program no longer panics when it cannot write to standard error during
+  operation.
+
 ## 0.3.2 (2022-08-31)
 
 ### Changed

Cargo.toml

@@ -2,7 +2,7 @@
 name = "spamassassin-milter"
 version = "0.3.2"
 edition = "2021"
-rust-version = "1.56.1"
+rust-version = "1.61.0"
 description = "Milter for spam filtering with SpamAssassin"
 license = "GPL-3.0-or-later"
 categories = ["email"]

README.md

@@ -50,7 +50,7 @@ server. By default, `/usr/bin/spamc` is used as the executable. To override
 this, set the environment variable `SPAMASSASSIN_MILTER_SPAMC` to the desired
 path when building the application.
 
-The minimum supported Rust version is 1.56.1.
+The minimum supported Rust version is 1.61.0.
 
 [Rust]: https://www.rust-lang.org
 
@@ -261,7 +261,7 @@ messages with score 5.0 or above into ‘Junk’.
 
 ## Licence
 
-Copyright © 2020–2022 David Bürgin
+Copyright © 2020–2023 David Bürgin
 
 This program is free software: you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software

src/callbacks.rs

@@ -38,7 +38,7 @@ use std::{
 macro_rules! ok_or_tempfail {
     ($expr:expr) => {
         if let ::std::result::Result::Err(e) = $expr {
-            ::std::eprintln!("failed to communicate with spamc: {}", e);
+            eprintln!("failed to communicate with spamc: {e}");
             return ::indymilter::Status::Tempfail;
         }
     };
@@ -149,7 +149,7 @@ async fn handle_connect(
 
     if config.use_trusted_networks() {
         if config.is_in_trusted_networks(&ip) {
-            verbose!(config, "accepted connection from trusted network address {}", ip);
+            verbose!(config, "accepted connection from trusted network address {ip}");
             return Status::Accept;
         }
     } else if ip.is_loopback() {
@@ -179,7 +179,7 @@ async fn handle_mail(
 ) -> Status {
     if !config.auth_untrusted() {
         if let Some(login) = context.macros.get_string(c_str!("{auth_authen}")) {
-            verbose!(config, "accepted message from sender authenticated as \"{}\"", login);
+            verbose!(config, "accepted message from sender authenticated as \"{login}\"");
             return Status::Accept;
         }
     }
@@ -212,7 +212,7 @@ async fn handle_data(context: &mut Context<Connection>) -> Status {
     let id = context.macros.queue_id();
 
     if let Err(e) = client.connect() {
-        eprintln!("{}: failed to start spamc: {}", id, e);
+        eprintln!("{id}: failed to start spamc: {e}");
         return Status::Tempfail;
     }
 
@@ -274,7 +274,7 @@ async fn handle_body(
     let max = config.max_message_size();
     if client.bytes_written() > max {
         let id = context.macros.queue_id();
-        verbose!(config, "{}: skipping rest of message larger than {} bytes", id, max);
+        verbose!(config, "{id}: skipping rest of message larger than {max} bytes");
         client.skip_body();
         Status::Skip
     } else {
@@ -291,7 +291,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!("{}: failed to process message: {}", id, e);
+            eprintln!("{id}: failed to process message: {e}");
             Status::Tempfail
         }
     }

src/client.rs

@@ -114,7 +114,7 @@ impl Process for Spamc {
                     "spamc terminated by signal {}",
                     status.signal().unwrap()
                 )),
-                Some(code) => Error::Io(format!("spamc exited with status code {}", code)),
+                Some(code) => Error::Io(format!("spamc exited with status code {code}")),
             })
         }
     }
@@ -236,7 +236,7 @@ impl Client {
         // As requested during milter protocol negotiation, the value includes
         // leading whitespace. This lets us pass on whitespace exactly as is.
         let value = email::ensure_crlf(value);
-        let buf = format!("{}:{}\r\n", name, value);
+        let buf = format!("{name}:{value}\r\n");
 
         if email::is_spam_assassin_header(name)
             || email::REWRITE_HEADERS.contains(name)
@@ -277,7 +277,7 @@ impl Client {
         let output = match self.process.finish().await {
             Ok(output) => output,
             Err(e) => {
-                eprintln!("{}: failed to complete spamc communication: {}", id, e);
+                eprintln!("{id}: failed to complete spamc communication: {e}");
                 return Ok(Status::Tempfail);
             }
         };
@@ -285,7 +285,7 @@ impl Client {
         let email = match Email::parse(&output) {
             Ok(email) => email,
             Err(e) => {
-                eprintln!("{}: invalid response from spamc: {}", id, e);
+                eprintln!("{id}: invalid response from spamc: {e}");
                 return Ok(Status::Tempfail);
             }
         };
@@ -319,7 +319,7 @@ impl Client {
 
 fn reject_spam(id: &str, reply: &mut impl SetErrorReply, config: &Config) -> Result<Status> {
     if config.dry_run() {
-        verbose!(config, "{}: rejected message flagged as spam [dry run, not done]", id);
+        verbose!(config, "{id}: rejected message flagged as spam [dry run, not done]");
         Ok(Status::Accept)
     } else {
         reply.set_error_reply(
@@ -328,7 +328,7 @@ fn reject_spam(id: &str, reply: &mut impl SetErrorReply, config: &Config) -> Res
             config.reply_text().lines(),
         )?;
 
-        verbose!(config, "{}: rejected message flagged as spam", id);
+        verbose!(config, "{id}: rejected message flagged as spam");
         Ok(if config.reply_code().starts_with('5') {
             Status::Reject
         } else {
@@ -349,9 +349,8 @@ async fn replace_body(
     // This condition ensures message integrity in such a misconfigured setup.
     if skipped {
         eprintln!(
-            "{}: not replacing possibly truncated message body; \
+            "{id}: not replacing possibly truncated message body; \
             please review max message size setting ({})",
-            id,
             config.max_message_size()
         );
     } else {
@@ -589,10 +588,9 @@ mod tests {
         assert_eq!(
             as_mock_spamc(client.process.as_ref()).buf,
             format!(
-                "X-Envelope-From: {}\r\n\
+                "X-Envelope-From: {sender}\r\n\
-                 X-Envelope-To: {},\r\n\
+                 X-Envelope-To: {recipient1},\r\n\
-                 \t{}\r\n",
+                 \t{recipient2}\r\n",
-                sender, recipient1, recipient2
             )
             .as_bytes()
         );

src/email.rs

@@ -300,9 +300,9 @@ where
 {
     for m in mods {
         if config.dry_run() {
-            verbose!(config, "{}: rewriting header: {} [dry run, not done]", id, m);
+            verbose!(config, "{id}: rewriting header: {m} [dry run, not done]");
         } else {
-            verbose!(config, "{}: rewriting header: {}", id, m);
+            verbose!(config, "{id}: rewriting header: {m}");
             m.execute(actions).await?;
         }
     }
@@ -316,9 +316,9 @@ pub async fn replace_body(
     config: &Config,
 ) -> Result<()> {
     if config.dry_run() {
-        verbose!(config, "{}: replacing message body [dry run, not done]", id);
+        verbose!(config, "{id}: replacing message body [dry run, not done]");
     } else {
-        verbose!(config, "{}: replacing message body", id);
+        verbose!(config, "{id}: replacing message body");
         actions.replace_body(body).await?;
     }
     Ok(())
@@ -377,11 +377,11 @@ impl Display for HeaderMod<'_> {
         use HeaderMod::*;
 
         match self {
-            Add { name, .. } => write!(f, "add header \"{}\"", name),
+            Add { name, .. } => write!(f, "add header \"{name}\""),
             Replace { name, .. } | Modify { name, .. } => {
-                write!(f, "replace header \"{}\"", name)
+                write!(f, "replace header \"{name}\"")
             }
-            Delete { name } => write!(f, "delete header \"{}\"", name),
+            Delete { name } => write!(f, "delete header \"{name}\""),
         }
     }
 }

src/lib.rs

@@ -23,10 +23,23 @@
 //!
 //! [SpamAssassin Milter]: https://crates.io/crates/spamassassin-milter
 
+// The standard `eprintln` macro is replaced throughout with a best-effort,
+// non-panicking version.
+macro_rules! eprintln {
+    ($($arg:tt)*) => {
+        {
+            use ::std::io::Write;
+            let _ = ::std::writeln!(::std::io::stderr(), $($arg)*);
+        }
+    };
+}
+
 macro_rules! verbose {
     ($config:ident, $($arg:tt)*) => {
         if $config.verbose() {
-            ::std::eprintln!($($arg)*);
+            // Note: not qualifying `eprintln!` here makes it use textual scope
+            // and thus refer to above definition.
+            eprintln!($($arg)*);
         }
     };
 }
@@ -40,7 +53,10 @@ mod error;
 
 pub use crate::config::{Config, ConfigBuilder};
 use indymilter::IntoListener;
-use std::{future::Future, io};
+use std::{
+    future::Future,
+    io::{self, stderr, Write},
+};
 
 /// The name of the SpamAssassin Milter application.
 pub const MILTER_NAME: &str = "SpamAssassin Milter";
@@ -67,7 +83,7 @@ pub const VERSION: &str = env!("CARGO_PKG_VERSION");
 /// let shutdown = signal::ctrl_c();
 ///
 /// if let Err(e) = spamassassin_milter::run(listener, config, shutdown).await {
-///     eprintln!("failed to run spamassassin-milter: {}", e);
+///     eprintln!("failed to run spamassassin-milter: {e}");
 ///     process::exit(1);
 /// }
 /// # Ok(())
@@ -81,5 +97,15 @@ pub async fn run(
     let callbacks = callbacks::make_callbacks(config);
     let config = Default::default();
 
-    indymilter::run(listener, callbacks, config, shutdown).await
+    // Propagate any I/O error here: don’t start if stderr cannot be written to.
+    writeln!(stderr(), "{MILTER_NAME} {VERSION} starting")?;
+
+    let result = indymilter::run(listener, callbacks, config, shutdown).await;
+
+    match &result {
+        Ok(()) => eprintln!("{MILTER_NAME} {VERSION} shut down"),
+        Err(e) => eprintln!("{MILTER_NAME} {VERSION} terminated with error: {e}"),
+    }
+
+    result
 }

src/main.rs

@@ -20,7 +20,14 @@ use signal_hook::consts::{SIGINT, SIGTERM};
 use signal_hook_tokio::{Handle, Signals};
 use spamassassin_milter::{Config, MILTER_NAME, VERSION};
 use std::{
-    env, error::Error, net::IpAddr, os::unix::fs::FileTypeExt, path::Path, process, str::FromStr,
+    env,
+    error::Error,
+    io::{stderr, stdout, Write},
+    net::IpAddr,
+    os::unix::fs::FileTypeExt,
+    path::Path,
+    process,
+    str::FromStr,
 };
 use tokio::{
     fs,
@@ -29,12 +36,14 @@ use tokio::{
     task::JoinHandle,
 };
 
+const PROGRAM_NAME: &str = env!("CARGO_BIN_NAME");
+
 #[tokio::main]
 async fn main() {
     let (socket, config) = match parse_args() {
         Ok(config) => config,
         Err(e) => {
-            eprintln!("error: {}", e);
+            let _ = writeln!(stderr(), "{PROGRAM_NAME}: {e}");
             process::exit(1);
         }
     };
@@ -52,7 +61,7 @@ async fn main() {
             let listener = match TcpListener::bind(socket).await {
                 Ok(listener) => listener,
                 Err(e) => {
-                    eprintln!("error: could not bind TCP socket: {}", e);
+                    let _ = writeln!(stderr(), "{PROGRAM_NAME}: could not bind TCP socket: {e}");
                     process::exit(1);
                 }
             };
@@ -68,7 +77,7 @@ async fn main() {
             let listener = match UnixListener::bind(socket) {
                 Ok(listener) => listener,
                 Err(e) => {
-                    eprintln!("error: could not create UNIX domain socket: {}", e);
+                    let _ = writeln!(stderr(), "{PROGRAM_NAME}: could not create UNIX domain socket: {e}");
                     process::exit(1);
                 }
             };
@@ -81,20 +90,12 @@ async fn main() {
         }
     };
 
-    eprintln!("{} {} starting", MILTER_NAME, VERSION);
-
     let result = spamassassin_milter::run(listener, config, shutdown).await;
 
     cleanup(signals_handle, signals_task, socket_path).await;
 
-    match result {
+    if result.is_err() {
-        Ok(()) => {
+        process::exit(1);
-            eprintln!("{} {} shut down", MILTER_NAME, VERSION);
-        }
-        Err(e) => {
-            eprintln!("{} {} terminated with error: {}", MILTER_NAME, VERSION, e);
-            process::exit(1);
-        }
     }
 }
 
@@ -112,14 +113,13 @@ impl FromStr for Socket {
         } else if let Some(s) = s.strip_prefix("unix:") {
             Ok(Self::Unix(s.into()))
         } else {
-            Err(format!("invalid value for socket: \"{}\"", s))
+            Err(format!("invalid value for socket: \"{s}\""))
         }
     }
 }
 
 const USAGE_TEXT: &str = "\
-Usage:
+[OPTIONS] <SOCKET> [--] [<SPAMC_ARGS>...]
-  spamassassin-milter [OPTIONS] <SOCKET> [--] [<SPAMC_ARGS>...]
 
 Arguments:
   <SOCKET>                          Listening socket of the milter
@@ -154,17 +154,15 @@ fn parse_args() -> Result<(Socket, Config), Box<dyn Error>> {
     let socket = loop {
         let arg = args.next().ok_or("required argument <SOCKET> missing")??;
 
-        let missing_value = || format!("missing value for option {}", arg);
+        let missing_value = || format!("missing value for option {arg}");
 
         match arg.as_str() {
             "-h" | "--help" => {
-                println!("{} {}", MILTER_NAME, VERSION);
+                write!(stdout(), "Usage: {PROGRAM_NAME} {USAGE_TEXT}")?;
-                println!();
-                print!("{}", USAGE_TEXT);
                 process::exit(0);
             }
             "-V" | "--version" => {
-                println!("{} {}", MILTER_NAME, VERSION);
+                writeln!(stdout(), "{MILTER_NAME} {VERSION}")?;
                 process::exit(0);
             }
             "-a" | "--auth-untrusted" => {
@@ -188,7 +186,7 @@ fn parse_args() -> Result<(Socket, Config), Box<dyn Error>> {
             "-s" | "--max-message-size" => {
                 let arg = args.next().ok_or_else(missing_value)??;
                 let bytes = arg.parse()
-                    .map_err(|_| format!("invalid value for max message size: \"{}\"", arg))?;
+                    .map_err(|_| format!("invalid value for max message size: \"{arg}\""))?;
 
                 config = config.max_message_size(bytes);
             }
@@ -217,7 +215,7 @@ fn parse_args() -> Result<(Socket, Config), Box<dyn Error>> {
                     // supported.
                     let net = net.parse()
                         .or_else(|_| net.parse::<IpAddr>().map(From::from))
-                        .map_err(|_| format!("invalid value for trusted network address: \"{}\"", net))?;
+                        .map_err(|_| format!("invalid value for trusted network address: \"{net}\""))?;
 
                     config = config.trusted_network(net);
                 }
@@ -229,7 +227,7 @@ fn parse_args() -> Result<(Socket, Config), Box<dyn Error>> {
                 if arg.starts_with('-')
                     && arg.chars().all(|c| c.is_ascii_alphanumeric() || c == '-')
                 {
-                    return Err(format!("unrecognized option: \"{}\"", arg).into());
+                    return Err(format!("unrecognized option: \"{arg}\"").into());
                 }
 
                 break arg.parse()?;
@@ -270,16 +268,15 @@ fn validate_reply_codes(
             if !((c1.starts_with('4') || c1.starts_with('5')) && c2.starts_with(&c1[..1])) =>
         {
             Err(format!(
-                "invalid or incompatible values for reply code and status code: \"{}\", \"{}\"",
+                "invalid or incompatible values for reply code and status code: \"{c1}\", \"{c2}\""
-                c1, c2
             )
             .into())
         }
         (Some(c), None) if !c.starts_with('5') => {
-            Err(format!("invalid value for reply code (5XX): \"{}\"", c).into())
+            Err(format!("invalid value for reply code (5XX): \"{c}\"").into())
         }
         (None, Some(c)) if !c.starts_with('5') => {
-            Err(format!("invalid value for reply status code (5.X.X): \"{}\"", c).into())
+            Err(format!("invalid value for reply status code (5.X.X): \"{c}\"").into())
         }
         _ => Ok(()),
     }

tests/ham_message.rs

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

tests/reject_spam.rs

@@ -9,7 +9,7 @@ async fn reject_spam() {
         .reject_spam(true)
         .reply_code("554".into())
         .reply_text("Not allowed!".into())
-        .spamc_args(&[format!("--port={}", SPAMD_PORT)])
+        .spamc_args([format!("--port={SPAMD_PORT}")])
         .build();
 
     let server = spawn_mock_spamd_server(SPAMD_PORT, |spam| {

tests/skip_oversized.rs

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

tests/spam_message.rs

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

tests/spamc_connection_error.rs

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