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.

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

UTD Hook | Add support for late decryption grace period for parity with Web #4265

Open
BillCarsonFr opened this issue Nov 14, 2024 · 3 comments

Comments

@BillCarsonFr
Copy link
Member

On web we have a configurable grace period for late key delivery
https://github.com/element-hq/element-web/blob/9a126795a81d13aba2d331b38df75878682d54a1/src/DecryptionFailureTracker.ts#L122

    /** If the event is successfully decrypted in less than 4s, we don't report. */
    public static GRACE_PERIOD_MS = 4000;

There is no similar mecanism with the rust UTD hook, the late decryption are never graced

// Only let the parent hook know about the late decryption if the event is
// a pending UTD. If so, remove the event from the pending list —
// doing so will cause the reporting task to no-op if it runs.
let Some(pending_utd_report) = self.pending_delayed.lock().unwrap().remove(event_id) else {
return;
};
// We can also cancel the reporting task.
pending_utd_report.report_task.abort();
// Now we can report the late decryption.
let info = UnableToDecryptInfo {
event_id: event_id.to_owned(),
time_to_decrypt: Some(pending_utd_report.marked_utd_at.elapsed()),
cause,
};

@bnjbvr
Copy link
Member

bnjbvr commented Nov 14, 2024

For what it's worth: it was simple enough that we implemented it at the FFI layer, in the UTD hook trait impl:

impl UnableToDecryptHook for UtdHook {
fn on_utd(&self, info: SdkUnableToDecryptInfo) {
const IGNORE_UTD_PERIOD: Duration = Duration::from_secs(4);
// UTDs that have been decrypted in the `IGNORE_UTD_PERIOD` are just ignored and
// not considered UTDs.
if let Some(duration) = &info.time_to_decrypt {
if *duration < IGNORE_UTD_PERIOD {
return;
}
}
// Report the UTD to the client.
self.delegate.on_utd(info.into());
}
}

Might be fine to move it over to the SDK, if that's what you implied here, but probably lower priority.

@andybalaam
Copy link
Member

@BillCarsonFr will check over this and close this item.

@BillCarsonFr
Copy link
Member Author

I'd prefer that we moved that down to the SDK and properly test it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants