Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only send a SyncMessage when sending to self #338

Merged
merged 10 commits into from
Oct 22, 2024
Merged

Only send a SyncMessage when sending to self #338

Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a5eaa0a
Only send a SyncMessage when sending to self
gferon Oct 21, 2024
453e8da
Fix comment
gferon Oct 21, 2024
076a1bb
Add tracing statement
gferon Oct 21, 2024
335667d
No closure
gferon Oct 21, 2024
b21bd1b
Remove unneeded branch
gferon Oct 21, 2024
261d234
Fix copy-pasta
gferon Oct 21, 2024
6db0923
Add tracing statement to log the span
gferon Oct 22, 2024
d79a78f
Remove unnecessary .iter()
gferon Oct 22, 2024
695e1d6
Clippy
gferon Oct 22, 2024
901c922
Simplify expression
gferon Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 45 additions & 47 deletions src/sender.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use core::time;

Check warning on line 1 in src/sender.rs

View workflow job for this annotation

GitHub Actions / clippy

unused import: `core::time` 

warning: unused import: `core::time`
 --> src/sender.rs:1:5
  |
1 | use core::time;
  |     ^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default
use std::{collections::HashSet, time::SystemTime};

use chrono::prelude::*;
Expand All @@ -6,7 +7,7 @@
ProtocolStore, SenderCertificate, SenderKeyStore, SignalProtocolError,
};
use rand::{CryptoRng, Rng};
use tracing::{error, info, trace};
use tracing::{debug, error, info, trace};
use tracing_futures::Instrument;
use uuid::Uuid;
use zkgroup::GROUP_IDENTIFIER_LEN;
Expand Down Expand Up @@ -347,6 +348,7 @@
online: bool,
) -> SendMessageResult {
let content_body = message.into();
let message_to_self = recipient == &self.local_aci;

use crate::proto::data_message::Flags;

Expand All @@ -357,8 +359,30 @@
_ => false,
};

// don't send anything to self nor session enders to others as sealed sender
if recipient == &self.local_aci || end_session {
// only send a sync message when sending to self and skip the rest of the process
if message_to_self {
debug!("sending note to self");
let sync_message =
Self::create_multi_device_sent_transcript_content(
Some(recipient),
content_body,
timestamp,
None,
);
return self
.try_send_message(
*recipient,
None,
&sync_message,
timestamp,
include_pni_signature,
online,
)
.await;
}

// don't send session enders as sealed sender
if end_session {
unidentified_access.take();
}

Expand All @@ -380,20 +404,11 @@
};

if needs_sync || self.is_multi_device().await {
let data_message = match &content_body {
ContentBody::DataMessage(m) => Some(m.clone()),
_ => None,
};
let edit_message = match &content_body {
ContentBody::EditMessage(m) => Some(m.clone()),
_ => None,
};
tracing::debug!("sending multi-device sync message");
let sync_message = self
.create_multi_device_sent_transcript_content(
debug!("sending multi-device sync message");
let sync_message =
Self::create_multi_device_sent_transcript_content(
Some(recipient),
data_message,
edit_message,
content_body,
timestamp,
Some(&result),
);
Expand Down Expand Up @@ -436,19 +451,10 @@
let content_body: ContentBody = message.into();
let mut results = vec![];

let data_message = match &content_body {
ContentBody::DataMessage(m) => Some(m.clone()),
_ => None,
};
let edit_message = match &content_body {
ContentBody::EditMessage(m) => Some(m.clone()),
_ => None,
};

let mut needs_sync_in_results = false;

for (recipient, unidentified_access, include_pni_signature) in
recipients.as_ref().iter()
recipients.as_ref()
{
let result = self
.try_send_message(
Expand All @@ -471,24 +477,12 @@
results.push(result);
}

if needs_sync_in_results
&& data_message.is_none()
&& edit_message.is_none()
{
// XXX: does this happen?
tracing::warn!(
"Server claims need sync, but not sending data message or edit message"
);
return results;
}

// we only need to send a synchronization message once
if needs_sync_in_results || self.is_multi_device().await {
let sync_message = self
.create_multi_device_sent_transcript_content(
let sync_message =
Self::create_multi_device_sent_transcript_content(
None,
data_message,
edit_message,
content_body,
timestamp,
&results,
);
Expand All @@ -515,7 +509,7 @@
/// Send a message (`content`) to an address (`recipient`).
#[tracing::instrument(
level = "trace",
skip(self, unidentified_access, content_body, recipient),
skip(self, unidentified_access),
fields(unidentified_access = unidentified_access.is_some(), recipient = %recipient),
)]
async fn try_send_message(
Expand All @@ -527,6 +521,8 @@
include_pni_signature: bool,
online: bool,
) -> SendMessageResult {
trace!("trying to send a message");

use prost::Message;

let mut content = content_body.clone().into_proto();
Expand Down Expand Up @@ -1029,14 +1025,17 @@
}

fn create_multi_device_sent_transcript_content<'a>(
&self,
recipient: Option<&ServiceAddress>,
data_message: Option<crate::proto::DataMessage>,
edit_message: Option<crate::proto::EditMessage>,
content_body: ContentBody,
timestamp: u64,
send_message_results: impl IntoIterator<Item = &'a SendMessageResult>,
) -> ContentBody {
use sync_message::sent::UnidentifiedDeliveryStatus;
let (data_message, edit_message) = match content_body {
ContentBody::DataMessage(m) => (Some(m), None),
ContentBody::EditMessage(m) => (None, Some(m)),
_ => (None, None),
};
let unidentified_status: Vec<UnidentifiedDeliveryStatus> =
send_message_results
.into_iter()
Expand Down Expand Up @@ -1066,8 +1065,7 @@
expiration_start_timestamp: data_message
.as_ref()
.and_then(|m| m.expire_timer)
.is_some()
.then_some(timestamp),
.map(|_| timestamp),
message: data_message,
edit_message,
timestamp: Some(timestamp),
Expand Down
Loading