mirror of
https://github.com/chatmail/core.git
synced 2026-10-27 07:18:13 +03:00
refactor(smtp): make it impossible connect without From address and transport_id
Follow-up to 70730490b2
This way we do not need to manage multiple Options and making sure
they are all Some or all None.
This commit is contained in:
+3
-1
@@ -513,6 +513,8 @@ pub(crate) async fn configure(
|
||||
let smtp_password = configured_param.smtp_password.clone();
|
||||
let smtp_addr = configured_param.addr.clone();
|
||||
|
||||
let transport_id = 0;
|
||||
|
||||
let proxy_config2 = proxy_config.clone();
|
||||
let smtp_config_task = task::spawn(async move {
|
||||
let mut smtp = Smtp::new();
|
||||
@@ -522,6 +524,7 @@ pub(crate) async fn configure(
|
||||
&smtp_password,
|
||||
&proxy_config2,
|
||||
&smtp_addr,
|
||||
transport_id,
|
||||
strict_tls,
|
||||
)
|
||||
.await?;
|
||||
@@ -533,7 +536,6 @@ pub(crate) async fn configure(
|
||||
|
||||
// Configure IMAP
|
||||
|
||||
let transport_id = 0;
|
||||
let (_s, r) = async_channel::bounded(1);
|
||||
let mut imap = Imap::new(ctx, transport_id, configured_param.clone(), r).await?;
|
||||
let imap_session = match imap.connect(ctx).await {
|
||||
|
||||
+49
-40
@@ -32,21 +32,26 @@ use crate::transport::{
|
||||
ConfiguredLoginParam, ConfiguredServerLoginParam, prioritize_server_login_params,
|
||||
};
|
||||
|
||||
/// SMTP connection with information about it.
|
||||
#[derive(Debug)]
|
||||
struct Connection {
|
||||
transport: SmtpTransport<Box<dyn SessionBufStream>>,
|
||||
|
||||
/// Email address that should be put into the From field when sending over this connection.
|
||||
from: EmailAddress,
|
||||
|
||||
/// ID of the transport from which SMTP connection configuration comes from.
|
||||
transport_id: u32,
|
||||
|
||||
/// Timestamp of last successful send/receive network interaction
|
||||
/// (eg connect or send succeeded).
|
||||
last_success: tools::Time,
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
pub(crate) struct Smtp {
|
||||
/// SMTP connection.
|
||||
transport: Option<SmtpTransport<Box<dyn SessionBufStream>>>,
|
||||
|
||||
/// Email address we are sending from.
|
||||
from: Option<EmailAddress>,
|
||||
|
||||
/// Transport we are connected to.
|
||||
transport_id: Option<u32>,
|
||||
|
||||
/// Timestamp of last successful send/receive network interaction
|
||||
/// (eg connect or send succeeded). On initialization and disconnect
|
||||
/// it is set to None.
|
||||
last_success: Option<tools::Time>,
|
||||
connection: Option<Connection>,
|
||||
|
||||
pub(crate) connectivity: ConnectivityStore,
|
||||
|
||||
@@ -95,30 +100,25 @@ impl Smtp {
|
||||
|
||||
/// Disconnect the SMTP transport and drop it entirely.
|
||||
pub fn disconnect(&mut self) {
|
||||
if let Some(mut transport) = self.transport.take() {
|
||||
if let Some(mut connection) = self.connection.take() {
|
||||
// Closing connection with a QUIT command may take some time, especially if it's a
|
||||
// stale connection and an attempt to send the command times out. Send a command in a
|
||||
// separate task to avoid waiting for reply or timeout.
|
||||
task::spawn(async move { transport.quit().await });
|
||||
task::spawn(async move { connection.transport.quit().await });
|
||||
}
|
||||
self.transport_id = None;
|
||||
self.from = None;
|
||||
self.last_success = None;
|
||||
}
|
||||
|
||||
/// Return true if smtp was connected but is not known to
|
||||
/// have been successfully used the last 60 seconds
|
||||
pub fn has_maybe_stale_connection(&self) -> bool {
|
||||
if let Some(last_success) = self.last_success {
|
||||
time_elapsed(&last_success).as_secs() > 60
|
||||
} else {
|
||||
false
|
||||
}
|
||||
self.connection
|
||||
.as_ref()
|
||||
.is_some_and(|connection| time_elapsed(&connection.last_success).as_secs() > 60)
|
||||
}
|
||||
|
||||
/// Check whether we are connected.
|
||||
pub fn is_connected(&self) -> bool {
|
||||
self.transport.is_some()
|
||||
self.connection.is_some()
|
||||
}
|
||||
|
||||
/// Connect using configured parameters.
|
||||
@@ -143,14 +143,12 @@ impl Smtp {
|
||||
&lp.smtp_password,
|
||||
&proxy_config,
|
||||
&lp.addr,
|
||||
transport_id,
|
||||
lp.strict_tls(proxy_config.is_some())?,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(()) => {
|
||||
self.transport_id = Some(transport_id);
|
||||
return Ok(());
|
||||
}
|
||||
Ok(()) => return Ok(()),
|
||||
Err(err) => {
|
||||
warn!(
|
||||
context,
|
||||
@@ -163,6 +161,7 @@ impl Smtp {
|
||||
}
|
||||
|
||||
/// Connect using the provided login params.
|
||||
#[expect(clippy::too_many_arguments)]
|
||||
pub async fn connect(
|
||||
&mut self,
|
||||
context: &Context,
|
||||
@@ -170,6 +169,7 @@ impl Smtp {
|
||||
password: &str,
|
||||
proxy_config: &Option<ProxyConfig>,
|
||||
addr: &str,
|
||||
transport_id: u32,
|
||||
strict_tls: bool,
|
||||
) -> Result<()> {
|
||||
if self.is_connected() {
|
||||
@@ -179,7 +179,6 @@ impl Smtp {
|
||||
|
||||
let from = EmailAddress::new(addr.to_string())
|
||||
.with_context(|| format!("Invalid address {addr:?}"))?;
|
||||
self.from = Some(from);
|
||||
|
||||
let login_params =
|
||||
prioritize_server_login_params(&context.sql, login_params, "smtp").await?;
|
||||
@@ -204,8 +203,12 @@ impl Smtp {
|
||||
}
|
||||
};
|
||||
|
||||
self.transport = Some(transport);
|
||||
self.last_success = Some(tools::Time::now());
|
||||
self.connection = Some(Connection {
|
||||
transport,
|
||||
from,
|
||||
transport_id,
|
||||
last_success: tools::Time::now(),
|
||||
});
|
||||
|
||||
context.emit_event(EventType::SmtpConnected(format!(
|
||||
"SMTP-LOGIN as {} ok",
|
||||
@@ -327,7 +330,6 @@ pub(crate) async fn smtp_send(
|
||||
}
|
||||
};
|
||||
|
||||
// this clears last_success info
|
||||
info!(context, "Failed to send message over SMTP, disconnecting.");
|
||||
smtp.disconnect();
|
||||
|
||||
@@ -355,8 +357,8 @@ pub(crate) async fn smtp_send(
|
||||
};
|
||||
|
||||
if matches!(status, SendResult::Success) {
|
||||
debug_assert!(smtp.transport_id.is_some());
|
||||
if let Some(transport_id) = smtp.transport_id
|
||||
debug_assert!(smtp.connection.is_some());
|
||||
if let Some(transport_id) = smtp.connection.as_ref().map(|conn| conn.transport_id)
|
||||
&& let Err(err) = record_success(context, transport_id).await
|
||||
{
|
||||
warn!(
|
||||
@@ -472,9 +474,10 @@ pub(crate) async fn send_msg_to_smtp(
|
||||
let mut recipients = queued_mail.recipients.clone();
|
||||
if queued_mail.bcc_self {
|
||||
let from_addr = smtp
|
||||
.from
|
||||
.connection
|
||||
.as_ref()
|
||||
.context("No From address available, likely not connected")?
|
||||
.context("Not connected")?
|
||||
.from
|
||||
.to_string();
|
||||
add_self_recipients(
|
||||
context,
|
||||
@@ -504,12 +507,17 @@ pub(crate) async fn send_msg_to_smtp(
|
||||
let public_key = key::load_self_public_key(context).await?;
|
||||
let secret_key = key::load_self_secret_key(context).await?;
|
||||
let from_addr = smtp
|
||||
.from
|
||||
.connection
|
||||
.as_ref()
|
||||
.context("No From address available, likely not connected")?
|
||||
.context("Not connected")?
|
||||
.from
|
||||
.to_string();
|
||||
|
||||
let transport_id = smtp.transport_id.context("Not connected")?;
|
||||
let transport_id = smtp
|
||||
.connection
|
||||
.as_ref()
|
||||
.context("Not connected")?
|
||||
.transport_id;
|
||||
let chunk_size = context
|
||||
.get_max_smtp_rcpt_to(transport_id, &from_addr)
|
||||
.await?
|
||||
@@ -760,9 +768,10 @@ async fn send_mdn_rfc724_mid(
|
||||
let public_key = key::load_self_public_key(context).await?;
|
||||
let secret_key = key::load_self_secret_key(context).await?;
|
||||
let from = smtp
|
||||
.from
|
||||
.connection
|
||||
.as_ref()
|
||||
.context("No From address, not connected")?
|
||||
.context("Not connected")?
|
||||
.from
|
||||
.to_string();
|
||||
let rendered_msg =
|
||||
mimefactory::render_queued_mail(queued_mdn, &public_key, &secret_key, from.clone())?;
|
||||
|
||||
+16
-12
@@ -31,6 +31,14 @@ impl Smtp {
|
||||
recipients: &[EmailAddress],
|
||||
message: &[u8],
|
||||
) -> Result<()> {
|
||||
let Some(ref mut connection) = self.connection else {
|
||||
warn!(
|
||||
context,
|
||||
"Failed to send a message because SMTP client is not connected."
|
||||
);
|
||||
return Err(Error::NoTransport);
|
||||
};
|
||||
|
||||
if !context.get_config_bool(Config::Bot).await? {
|
||||
// Notify ratelimiter about sent message regardless of whether quota is exceeded or not.
|
||||
// Checking whether sending is allowed for low-priority messages should be done by the
|
||||
@@ -38,21 +46,17 @@ impl Smtp {
|
||||
context.ratelimit.write().await.send();
|
||||
}
|
||||
|
||||
let envelope =
|
||||
Envelope::new(self.from.clone(), recipients.to_vec()).map_err(Error::Envelope)?;
|
||||
let envelope = Envelope::new(Some(connection.from.clone()), recipients.to_vec())
|
||||
.map_err(Error::Envelope)?;
|
||||
let mail = SendableEmail::new(envelope, message);
|
||||
|
||||
let Some(ref mut transport) = self.transport else {
|
||||
warn!(
|
||||
context,
|
||||
"Failed to send a message because SMTP client has no SmtpTransport."
|
||||
);
|
||||
return Err(Error::NoTransport);
|
||||
};
|
||||
connection
|
||||
.transport
|
||||
.send(mail)
|
||||
.await
|
||||
.map_err(Error::SmtpSend)?;
|
||||
|
||||
transport.send(mail).await.map_err(Error::SmtpSend)?;
|
||||
|
||||
self.last_success = Some(tools::Time::now());
|
||||
connection.last_success = tools::Time::now();
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user