From fda33a27441e2ccb1c4e97c0fc582abf25b1561f Mon Sep 17 00:00:00 2001 From: Amaury <1293565+amaury1729@users.noreply.github.com> Date: Wed, 11 Oct 2023 17:13:45 +0200 Subject: [PATCH] feat(core): Bump to 45s timeout for some domains (#1348) BREAKING CHANGE: `SmtpError::TimeoutError` has been removed in favor of the one async-smtp uses, namely `std::io::Error` with `ErrorKind::TimeoutError` --- core/src/rules.json | 9 ++++++--- core/src/rules.rs | 2 ++ core/src/smtp/connect.rs | 31 +++++++++++++++++++++---------- core/src/smtp/error.rs | 10 ---------- core/src/smtp/mod.rs | 6 +++--- 5 files changed, 32 insertions(+), 26 deletions(-) diff --git a/core/src/rules.json b/core/src/rules.json index d73590cad..cdc6b2cbb 100644 --- a/core/src/rules.json +++ b/core/src/rules.json @@ -9,11 +9,14 @@ }, "by_mx_suffix": { ".antispamcloud.com.": { - "rules": ["SkipCatchAll"], - "_comment": "Some take exactly 30s to respond, so we skip the catch-all one, and bump the timeout." + "rules": ["SmtpTimeout45s", "SkipCatchAll"], + "_comment": "Some take 30s to respond (sometimes only on 2nd attempt, not deterministic), so we skip the catch-all one, and bump the timeout to well over 30s." } }, "rules": { - "SkipCatchAll": { "_comment": "Don't perform catch-all check." } + "SkipCatchAll": { "_comment": "Don't perform catch-all check" }, + "SmtpTimeout45s": { + "_comment": "Set SMTP connection timeout to at least 45s. If the user request set an even higher timeout, take that one. Please note that this timeout is **per SMTP connection**. By default, we try 2 connections per email: if the 1st one failed, then we connect again to avoid potential greylisting, in which case the whole verification takes 1min30s." + } } } diff --git a/core/src/rules.rs b/core/src/rules.rs index 8a521a1a2..50542b312 100644 --- a/core/src/rules.rs +++ b/core/src/rules.rs @@ -28,6 +28,8 @@ use std::collections::HashMap; pub enum Rule { /// Don't perform catch-all check. SkipCatchAll, + /// Set the SMTP timeout to 45s. + SmtpTimeout45s, } #[derive(Debug, Deserialize, Serialize)] diff --git a/core/src/smtp/connect.rs b/core/src/smtp/connect.rs index fd6887f08..3df31d921 100644 --- a/core/src/smtp/connect.rs +++ b/core/src/smtp/connect.rs @@ -20,7 +20,6 @@ use async_smtp::{ smtp::{commands::*, extension::ClientId, ServerAddress, Socks5Config}, ClientTlsParameters, EmailAddress, SmtpClient, SmtpTransport, }; -use async_std::future; use rand::rngs::SmallRng; use rand::{distributions::Alphanumeric, Rng, SeedableRng}; use std::iter; @@ -49,16 +48,32 @@ macro_rules! try_smtp ( /// Attempt to connect to host via SMTP, and return SMTP client on success. async fn connect_to_host( + domain: &str, host: &str, port: u16, input: &CheckEmailInput, ) -> Result { + let smtp_timeout = if let Some(t) = input.smtp_timeout { + if has_rule(domain, host, &Rule::SmtpTimeout45s) { + log::debug!( + target: LOG_TARGET, + "[email={}] Bumping SMTP timeout to at least 45s", + input.to_email, + ); + Some(t.max(Duration::from_secs(45))) + } else { + input.smtp_timeout + } + } else { + None + }; + // hostname verification fails if it ends with '.', for example, using // SOCKS5 proxies we can `io: incomplete` error. let host = host.trim_end_matches('.').to_string(); let security = { - let tls_params = ClientTlsParameters::new( + let tls_params: ClientTlsParameters = ClientTlsParameters::new( host.clone(), TlsConnector::new() .use_sni(true) @@ -77,7 +92,7 @@ async fn connect_to_host( security, ) .hello_name(ClientId::Domain(input.hello_name.clone())) - .timeout(Some(Duration::new(30, 0))); // Set timeout to 30s + .timeout(smtp_timeout); if let Some(proxy) = &input.proxy { let socks5_config = match (&proxy.username, &proxy.password) { @@ -259,7 +274,7 @@ async fn create_smtp_future( ) -> Result<(bool, Deliverability), SmtpError> { // FIXME If the SMTP is not connectable, we should actually return an // Ok(SmtpDetails { can_connect_smtp: false, ... }). - let mut smtp_transport = connect_to_host(host, port, input).await?; + let mut smtp_transport = connect_to_host(domain, host, port, input).await?; let is_catch_all = smtp_is_catch_all(&mut smtp_transport, domain, host, input) .await @@ -288,7 +303,7 @@ async fn create_smtp_future( ); let _ = smtp_transport.close().await; - smtp_transport = connect_to_host(host, port, input).await?; + smtp_transport = connect_to_host(domain, host, port, input).await?; result = email_deliverable(&mut smtp_transport, to_email).await; } } @@ -311,11 +326,7 @@ async fn check_smtp_without_retry( input: &CheckEmailInput, ) -> Result { let fut = create_smtp_future(to_email, host, port, domain, input); - let (is_catch_all, deliverability) = if let Some(smtp_timeout) = input.smtp_timeout { - future::timeout(smtp_timeout, fut).await?? - } else { - fut.await? - }; + let (is_catch_all, deliverability) = fut.await?; Ok(SmtpDetails { can_connect_smtp: true, diff --git a/core/src/smtp/error.rs b/core/src/smtp/error.rs index d042135e9..405b83eae 100644 --- a/core/src/smtp/error.rs +++ b/core/src/smtp/error.rs @@ -22,7 +22,6 @@ use super::parser; use super::yahoo::YahooError; use crate::util::ser_with_display::ser_with_display; use async_smtp::smtp::error::Error as AsyncSmtpError; -use async_std::future; use fast_socks5::SocksError; use serde::Serialize; @@ -36,9 +35,6 @@ pub enum SmtpError { /// Error when communicating with SMTP server. #[serde(serialize_with = "ser_with_display")] SmtpError(AsyncSmtpError), - /// Time-out error. - #[serde(serialize_with = "ser_with_display")] - TimeoutError(future::TimeoutError), /// Error when verifying a Yahoo email via HTTP requests. YahooError(YahooError), /// Error when verifying a Gmail email via a HTTP request. @@ -58,12 +54,6 @@ impl From for SmtpError { } } -impl From for SmtpError { - fn from(e: future::TimeoutError) -> Self { - SmtpError::TimeoutError(e) - } -} - impl From for SmtpError { fn from(e: YahooError) -> Self { SmtpError::YahooError(e) diff --git a/core/src/smtp/mod.rs b/core/src/smtp/mod.rs index 32d3dde12..29abee6ff 100644 --- a/core/src/smtp/mod.rs +++ b/core/src/smtp/mod.rs @@ -110,7 +110,7 @@ pub async fn check_smtp( #[cfg(test)] mod tests { use super::{check_smtp, CheckEmailInput, SmtpError}; - use async_smtp::EmailAddress; + use async_smtp::{smtp::error::Error, EmailAddress}; use std::{str::FromStr, time::Duration}; use tokio::runtime::Runtime; use trust_dns_proto::rr::Name; @@ -120,13 +120,13 @@ mod tests { let runtime = Runtime::new().unwrap(); let to_email = EmailAddress::from_str("foo@gmail.com").unwrap(); - let host = Name::from_str("gmail.com").unwrap(); + let host = Name::from_str("alt4.aspmx.l.google.com.").unwrap(); let mut input = CheckEmailInput::default(); input.set_smtp_timeout(Some(Duration::from_millis(1))); let res = runtime.block_on(check_smtp(&to_email, &host, 25, "gmail.com", &input)); match res { - Err(SmtpError::TimeoutError(_)) => (), + Err(SmtpError::SmtpError(Error::Io(_))) => (), // ErrorKind == Timeout _ => panic!("check_smtp did not time out"), } }