Details

Reviewers

tomek
bartek
kamil

Commits

rCOMM382293dcee61: Implement method to deserialize olm account from a file

Summary

This differential implements a method in NotificationsCryptoModule that can deserialize flat file containing JSON-fied pickled olm account into in-memory CryptoModule onject that is capable to perform end2end encrypted messaging sessions.

Test Plan

Temporarily make this method public. Partially modify this method so that it does not read from a file but takes JSONI-ified pickled olm account directly and use it to create CryptoModule instance in CommCoreModule where we read pickled public olm account from the database. Use XCode debugger or logging to examine object
content. JSON-ified pickled olm account can be created in CommCoreModule.

Diff Detail

Repository

rCOMM Comm

Branch

marcin/eng-2901

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

marcin created this revision.Feb 20 2023, 4:18 AM

Herald added a subscriber: ashoat. · View Herald TranscriptFeb 20 2023, 4:18 AM

marcin added a parent revision: D6774: Start NotificationsCryptoModule API definition and implementation.Feb 20 2023, 4:19 AM

marcin removed a reviewer: atul.Feb 20 2023, 4:39 AM

marcin added a child revision: D6776: Implement method in NotificationsCryptoModule to serialize CryptoModule instance and atomically store it in a file..

marcin edited the test plan for this revision. (Show Details)

marcin published this revision for review.Feb 20 2023, 4:43 AM

Harbormaster failed remote builds in B16605: Diff 22711!Feb 20 2023, 7:29 AM

Rebase

Harbormaster failed remote builds in B16724: Diff 22879!Feb 21 2023, 10:55 AM

Harbormaster completed remote builds in B16724: Diff 22879.Feb 21 2023, 11:03 AM

bartek added inline comments.Feb 22 2023, 12:25 AM

native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.cpp
26 ↗	(On Diff #22879)	I think `ifstream::in` is the default, but can be left for clarity.
46 ↗	(On Diff #22879)	What if JSON is malformed and `account` doesn't exist?

marcin added inline comments.Feb 22 2023, 2:30 AM

native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.cpp
46 ↗	(On Diff #22879)	The counterpart method to this one - `serializeAndFlushCryptoModule` - introduced in child differential ensures that either entire JSON (containing "account" key) is written or the method fails. Both this and `serializeAndFlushCryptoModule` are private and will be called in public methods only with olm notifications account path. That said if `serializeAndFlushCryptoModule` fails on the very first write (first after last deletion of this file) to olm notifications account path, we will fail on the fist `if` in `deserializeCryptoModule` (the file does not exist). If it failed on n-th write, we will read correct content after last successful write to this file. So I think we can add this if statement and throw, but my understanding is that it is not necessary.

bartek accepted this revision.Feb 23 2023, 12:28 AM

bartek added inline comments.

native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.cpp
46 ↗	(On Diff #22879)	Yes, I understand that, my concern was the same as in D6777. But you are right, if this file is only written by `serializeAndFlushCryptoModule` - this statement is unnecessary.