Page MenuHomePhabricator
Differential D11312

[native] use short form for prekeys
ClosedPublic
Actions

Authored by varun on Mar 12 2024, 1:00 PM.
Tags
None
Referenced Files
F3364871: D11312.diff
Mon, Nov 25, 5:33 AM
Unknown Object (File)
Thu, Nov 7, 3:31 AM
Unknown Object (File)
Oct 26 2024, 1:40 AM
Unknown Object (File)
Oct 15 2024, 8:36 PM
Unknown Object (File)
Oct 15 2024, 8:36 PM
Unknown Object (File)
Oct 15 2024, 8:35 PM
Unknown Object (File)
Oct 15 2024, 8:35 PM
Unknown Object (File)
Sep 1 2024, 1:45 AM
View All 54 Files
Subscribers
ashoat
tomek

Details

Reviewers
kamil
marcin
Commits
rCOMM3d1844eb73a8: [native] use short form for prekeys
Summary

parse the prekey blob string using folly and then convert the std::string to a rust::String / jsi::String

Test Plan

https://gist.github.com/vdhanan/1b93f4b98a04c1ff0c2c3c7074b4c625

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

varun created this revision.Mar 12 2024, 1:00 PM
Herald added subscribers: tomek, ashoat. · View Herald TranscriptMar 12 2024, 1:00 PM
Harbormaster completed remote builds in B27482: Diff 38031.Mar 12 2024, 1:15 PM
varun requested review of this revision.Mar 12 2024, 1:15 PM
varun added a child revision: D11319: [crypto] make sure short format of prekey is expected when creating an outbound session.Mar 13 2024, 5:22 PM
kamil accepted this revision.Mar 14 2024, 2:48 AM

Can you link the diff where you update parsing prekeys when creating a session and land those two together? Now it's not possible to create a session between two native devices.

This revision is now accepted and ready to land.Mar 14 2024, 2:48 AM
kamil added a comment.Mar 14 2024, 2:57 AM
In D11312#326574, @kamil wrote:

Can you link the diff where you update parsing prekeys when creating a session and land those two together? Now it's not possible to create a session between two native devices.

I see stack now, sorry!

marcin added inline comments.Mar 14 2024, 9:39 AM
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
587 ↗(On Diff #38031)

I would add two checks:

  1. Check if what we have under "curve22519" is folly dynamic object and if values doesn't return empty iterator. I my gists that I shared on Linear I didn't do those checks but those gists weren't code intended to run in production.
  2. Surround it with try{}...catch(const folly::json::parse_error &e){}.

In case anything fails we will have more informative errors.

varun added inline comments.Mar 18 2024, 10:27 PM
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
587 ↗(On Diff #38031)

https://linear.app/comm/issue/ENG-7280/add-checks-to-parseolmprekey

Closed by commit rCOMM3d1844eb73a8: [native] use short form for prekeys (authored by varun). · Explain WhyMar 18 2024, 10:32 PM
This revision was automatically updated to reflect the committed changes.
varun added a commit: rCOMM3d1844eb73a8: [native] use short form for prekeys.
varun mentioned this in D11356: [native] improve error handling in parseOLMPrekeys.Mar 19 2024, 11:53 AM
varun mentioned this in rCOMM528670e7c977: [native] improve error handling in parseOLMPrekeys.Mar 20 2024, 2:33 PM

Revision Contents
Changeset List

PathSize
native/
cpp/
CommonCpp/
NativeModules/
23 lines

Diff 38158

View Options

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp

Loading...