-
Notifications
You must be signed in to change notification settings - Fork 269
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.
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
fix(sliding_sync): Add ignore_verification_requests
method to SlidingSyncBuilder
#4705
base: main
Are you sure you want to change the base?
Changes from all commits
ae8b54c
3a5bce9
f63c523
d53ddc1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -409,6 +409,8 @@ impl BaseClient { | |
limited: bool, | ||
events: Vec<Raw<AnySyncTimelineEvent>>, | ||
ignore_state_events: bool, | ||
#[cfg(feature = "e2e-encryption")] ignore_verification_requests: bool, | ||
#[cfg(not(feature = "e2e-encryption"))] _ignore_verification_requests: bool, | ||
prev_batch: Option<String>, | ||
push_rules: &Ruleset, | ||
user_ids: &mut BTreeSet<OwnedUserId>, | ||
|
@@ -496,13 +498,18 @@ impl BaseClient { | |
SyncMessageLikeEvent::Original(original_event), | ||
) => match &original_event.content.msgtype { | ||
MessageType::VerificationRequest(_) => { | ||
Box::pin(self.handle_verification_event(e, room.room_id())) | ||
.await?; | ||
if !ignore_verification_requests { | ||
Box::pin(self.handle_verification_event(e, room.room_id())) | ||
.await?; | ||
} | ||
Comment on lines
+501
to
+504
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we ignore all verification events? I guess the crypto crate already does so if it doesn't receive an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't that what I'm doing below? I'm not sure I fully understand the underlying logic, but I think this branch takes the verification requests and the one below any other verification event. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, how did I miss that one. You are indeed right. Can we then rename the flag and setting? Since we indeed don't just ignore the requests |
||
} | ||
_ => (), | ||
}, | ||
_ if e.event_type().to_string().starts_with("m.key.verification") => { | ||
Box::pin(self.handle_verification_event(e, room.room_id())).await?; | ||
if !ignore_verification_requests { | ||
Box::pin(self.handle_verification_event(e, room.room_id())) | ||
.await?; | ||
} | ||
} | ||
_ => (), | ||
}, | ||
|
@@ -1039,6 +1046,7 @@ impl BaseClient { | |
new_info.timeline.limited, | ||
new_info.timeline.events, | ||
false, | ||
false, | ||
new_info.timeline.prev_batch, | ||
&push_rules, | ||
&mut user_ids, | ||
|
@@ -1134,6 +1142,7 @@ impl BaseClient { | |
new_info.timeline.limited, | ||
new_info.timeline.events, | ||
false, | ||
false, | ||
new_info.timeline.prev_batch, | ||
&push_rules, | ||
&mut user_ids, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we use this
ignore_verification_requests
inside thehandle_verification_event
method instead of here?