Skip to content

Commit fca0aa2

Browse files
bmartybmarty
committed
KeysBackup: Avoid using !!, should fix #2262 
1 parent 0bcf42d commit fca0aa2

File tree

3 files changed

+82
-85
lines changed

3 files changed

+82
-85
lines changed

‎CHANGES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ Improvements 🙌:
2121
Bugfix 🐛:
2222
- Messages encrypted with no way to decrypt after SDK update from 0.18 to 1.0.0 (#2252)
2323
- Search Result | scroll jumps after pagination (#2238)
24+
- KeysBackup: Avoid using `!!` (#2262)
2425

2526
Translations 🗣:
2627
-

‎matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/keysbackup/KeysBackupTest.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,14 +257,14 @@ class KeysBackupTest : InstrumentedTest {
257257
// - Check encryptGroupSession() returns stg
258258
val keyBackupData = keysBackup.encryptGroupSession(session)
259259
assertNotNull(keyBackupData)
260-
assertNotNull(keyBackupData.sessionData)
260+
assertNotNull(keyBackupData!!.sessionData)
261261

262262
// - Check pkDecryptionFromRecoveryKey() is able to create a OlmPkDecryption
263263
val decryption = keysBackup.pkDecryptionFromRecoveryKey(keyBackupCreationInfo.recoveryKey)
264264
assertNotNull(decryption)
265265
// - Check decryptKeyBackupData() returns stg
266266
val sessionData = keysBackup
267-
.decryptKeyBackupData(keyBackupData,
267+
.decryptKeyBackupData(keyBackupData!!,
268268
session.olmInboundGroupSession!!.sessionIdentifier(),
269269
cryptoTestData.roomId,
270270
decryption!!)

‎matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/keysbackup/DefaultKeysBackupService.kt

Lines changed: 79 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -1212,94 +1212,91 @@ internal class DefaultKeysBackupService @Inject constructor(
12121212

12131213
// Gather data to send to the homeserver
12141214
// roomId -> sessionId -> MXKeyBackupData
1215-
val keysBackupData = KeysBackupData(
1216-
roomIdToRoomKeysBackupData = HashMap()
1217-
)
1215+
val keysBackupData = KeysBackupData()
12181216

1219-
for (olmInboundGroupSessionWrapper in olmInboundGroupSessionWrappers) {
1220-
val keyBackupData = encryptGroupSession(olmInboundGroupSessionWrapper)
1221-
if (keysBackupData.roomIdToRoomKeysBackupData[olmInboundGroupSessionWrapper.roomId] == null) {
1222-
val roomKeysBackupData = RoomKeysBackupData(
1223-
sessionIdToKeyBackupData = HashMap()
1224-
)
1225-
keysBackupData.roomIdToRoomKeysBackupData[olmInboundGroupSessionWrapper.roomId!!] = roomKeysBackupData
1226-
}
1217+
olmInboundGroupSessionWrappers.forEach { olmInboundGroupSessionWrapper ->
1218+
val roomId = olmInboundGroupSessionWrapper.roomId ?: return@forEach
1219+
val olmInboundGroupSession = olmInboundGroupSessionWrapper.olmInboundGroupSession ?: return@forEach
12271220

12281221
try {
1229-
keysBackupData.roomIdToRoomKeysBackupData[olmInboundGroupSessionWrapper.roomId]!!
1230-
.sessionIdToKeyBackupData[olmInboundGroupSessionWrapper.olmInboundGroupSession!!.sessionIdentifier()] = keyBackupData
1222+
encryptGroupSession(olmInboundGroupSessionWrapper)
1223+
?.let {
1224+
keysBackupData.roomIdToRoomKeysBackupData
1225+
.getOrPut(roomId) { RoomKeysBackupData() }
1226+
.sessionIdToKeyBackupData[olmInboundGroupSession.sessionIdentifier()] = it
1227+
}
12311228
} catch (e: OlmException) {
12321229
Timber.e(e, "OlmException")
12331230
}
12341231
}
12351232

12361233
Timber.v("backupKeys: 4 - Sending request")
12371234

1238-
val sendingRequestCallback = object : MatrixCallback<BackupKeysResult> {
1239-
override fun onSuccess(data: BackupKeysResult) {
1240-
uiHandler.post {
1241-
Timber.v("backupKeys: 5a - Request complete")
1235+
// Make the request
1236+
val version = keysBackupVersion?.version ?: return@withContext
12421237

1243-
// Mark keys as backed up
1244-
cryptoStore.markBackupDoneForInboundGroupSessions(olmInboundGroupSessionWrappers)
1238+
storeSessionDataTask
1239+
.configureWith(StoreSessionsDataTask.Params(version, keysBackupData)) {
1240+
this.callback = object : MatrixCallback<BackupKeysResult> {
1241+
override fun onSuccess(data: BackupKeysResult) {
1242+
uiHandler.post {
1243+
Timber.v("backupKeys: 5a - Request complete")
12451244

1246-
if (olmInboundGroupSessionWrappers.size < KEY_BACKUP_SEND_KEYS_MAX_COUNT) {
1247-
Timber.v("backupKeys: All keys have been backed up")
1248-
onServerDataRetrieved(data.count, data.hash)
1245+
// Mark keys as backed up
1246+
cryptoStore.markBackupDoneForInboundGroupSessions(olmInboundGroupSessionWrappers)
12491247

1250-
// Note: Changing state will trigger the call to backupAllGroupSessionsCallback.onSuccess()
1251-
keysBackupStateManager.state = KeysBackupState.ReadyToBackUp
1252-
} else {
1253-
Timber.v("backupKeys: Continue to back up keys")
1254-
keysBackupStateManager.state = KeysBackupState.WillBackUp
1248+
if (olmInboundGroupSessionWrappers.size < KEY_BACKUP_SEND_KEYS_MAX_COUNT) {
1249+
Timber.v("backupKeys: All keys have been backed up")
1250+
onServerDataRetrieved(data.count, data.hash)
12551251

1256-
backupKeys()
1257-
}
1258-
}
1259-
}
1252+
// Note: Changing state will trigger the call to backupAllGroupSessionsCallback.onSuccess()
1253+
keysBackupStateManager.state = KeysBackupState.ReadyToBackUp
1254+
} else {
1255+
Timber.v("backupKeys: Continue to back up keys")
1256+
keysBackupStateManager.state = KeysBackupState.WillBackUp
12601257

1261-
override fun onFailure(failure: Throwable) {
1262-
if (failure is Failure.ServerError) {
1263-
uiHandler.post {
1264-
Timber.e(failure, "backupKeys: backupKeys failed.")
1265-
1266-
when (failure.error.code) {
1267-
MatrixError.M_NOT_FOUND,
1268-
MatrixError.M_WRONG_ROOM_KEYS_VERSION -> {
1269-
// Backup has been deleted on the server, or we are not using the last backup version
1270-
keysBackupStateManager.state = KeysBackupState.WrongBackUpVersion
1271-
backupAllGroupSessionsCallback?.onFailure(failure)
1272-
resetBackupAllGroupSessionsListeners()
1273-
resetKeysBackupData()
1274-
keysBackupVersion = null
1275-
1276-
// Do not stay in KeysBackupState.WrongBackUpVersion but check what is available on the homeserver
1277-
checkAndStartKeysBackup()
1258+
backupKeys()
1259+
}
12781260
}
1279-
else ->
1280-
// Come back to the ready state so that we will retry on the next received key
1281-
keysBackupStateManager.state = KeysBackupState.ReadyToBackUp
12821261
}
1283-
}
1284-
} else {
1285-
uiHandler.post {
1286-
backupAllGroupSessionsCallback?.onFailure(failure)
1287-
resetBackupAllGroupSessionsListeners()
12881262

1289-
Timber.e("backupKeys: backupKeys failed.")
1263+
override fun onFailure(failure: Throwable) {
1264+
if (failure is Failure.ServerError) {
1265+
uiHandler.post {
1266+
Timber.e(failure, "backupKeys: backupKeys failed.")
1267+
1268+
when (failure.error.code) {
1269+
MatrixError.M_NOT_FOUND,
1270+
MatrixError.M_WRONG_ROOM_KEYS_VERSION -> {
1271+
// Backup has been deleted on the server, or we are not using the last backup version
1272+
keysBackupStateManager.state = KeysBackupState.WrongBackUpVersion
1273+
backupAllGroupSessionsCallback?.onFailure(failure)
1274+
resetBackupAllGroupSessionsListeners()
1275+
resetKeysBackupData()
1276+
keysBackupVersion = null
1277+
1278+
// Do not stay in KeysBackupState.WrongBackUpVersion but check what is available on the homeserver
1279+
checkAndStartKeysBackup()
1280+
}
1281+
else ->
1282+
// Come back to the ready state so that we will retry on the next received key
1283+
keysBackupStateManager.state = KeysBackupState.ReadyToBackUp
1284+
}
1285+
}
1286+
} else {
1287+
uiHandler.post {
1288+
backupAllGroupSessionsCallback?.onFailure(failure)
1289+
resetBackupAllGroupSessionsListeners()
12901290

1291-
// Retry a bit later
1292-
keysBackupStateManager.state = KeysBackupState.ReadyToBackUp
1293-
maybeBackupKeys()
1294-
}
1295-
}
1296-
}
1297-
}
1291+
Timber.e("backupKeys: backupKeys failed.")
12981292

1299-
// Make the request
1300-
storeSessionDataTask
1301-
.configureWith(StoreSessionsDataTask.Params(keysBackupVersion!!.version, keysBackupData)) {
1302-
this.callback = sendingRequestCallback
1293+
// Retry a bit later
1294+
keysBackupStateManager.state = KeysBackupState.ReadyToBackUp
1295+
maybeBackupKeys()
1296+
}
1297+
}
1298+
}
1299+
}
13031300
}
13041301
.executeBy(taskExecutor)
13051302
}
@@ -1308,46 +1305,45 @@ internal class DefaultKeysBackupService @Inject constructor(
13081305

13091306
@VisibleForTesting
13101307
@WorkerThread
1311-
fun encryptGroupSession(olmInboundGroupSessionWrapper: OlmInboundGroupSessionWrapper2): KeyBackupData {
1308+
fun encryptGroupSession(olmInboundGroupSessionWrapper: OlmInboundGroupSessionWrapper2): KeyBackupData? {
13121309
// Gather information for each key
1313-
val device = cryptoStore.deviceWithIdentityKey(olmInboundGroupSessionWrapper.senderKey!!)
1310+
val device = olmInboundGroupSessionWrapper.senderKey?.let { cryptoStore.deviceWithIdentityKey(it) }
13141311

13151312
// Build the m.megolm_backup.v1.curve25519-aes-sha2 data as defined at
13161313
// https://github.com/uhoreg/matrix-doc/blob/e2e_backup/proposals/1219-storing-megolm-keys-serverside.md#mmegolm_backupv1curve25519-aes-sha2-key-format
1317-
val sessionData = olmInboundGroupSessionWrapper.exportKeys()
1314+
val sessionData = olmInboundGroupSessionWrapper.exportKeys() ?: return null
13181315
val sessionBackupData = mapOf(
1319-
"algorithm" to sessionData!!.algorithm,
1316+
"algorithm" to sessionData.algorithm,
13201317
"sender_key" to sessionData.senderKey,
13211318
"sender_claimed_keys" to sessionData.senderClaimedKeys,
13221319
"forwarding_curve25519_key_chain" to (sessionData.forwardingCurve25519KeyChain.orEmpty()),
13231320
"session_key" to sessionData.sessionKey)
13241321

1325-
var encryptedSessionBackupData: OlmPkMessage? = null
1326-
1327-
val moshi = MoshiProvider.providesMoshi()
1328-
val adapter = moshi.adapter(Map::class.java)
1329-
1330-
try {
1331-
val json = adapter.toJson(sessionBackupData)
1322+
val json = MoshiProvider.providesMoshi()
1323+
.adapter(Map::class.java)
1324+
.toJson(sessionBackupData)
13321325

1333-
encryptedSessionBackupData = backupOlmPkEncryption?.encrypt(json)
1326+
val encryptedSessionBackupData = try {
1327+
backupOlmPkEncryption?.encrypt(json)
13341328
} catch (e: OlmException) {
13351329
Timber.e(e, "OlmException")
1330+
null
13361331
}
1332+
?: return null
13371333

13381334
// Build backup data for that key
13391335
return KeyBackupData(
13401336
firstMessageIndex = try {
1341-
olmInboundGroupSessionWrapper.olmInboundGroupSession!!.firstKnownIndex
1337+
olmInboundGroupSessionWrapper.olmInboundGroupSession?.firstKnownIndex ?: 0
13421338
} catch (e: OlmException) {
13431339
Timber.e(e, "OlmException")
13441340
0L
13451341
},
1346-
forwardedCount = olmInboundGroupSessionWrapper.forwardingCurve25519KeyChain!!.size,
1342+
forwardedCount = olmInboundGroupSessionWrapper.forwardingCurve25519KeyChain.orEmpty().size,
13471343
isVerified = device?.isVerified == true,
13481344

13491345
sessionData = mapOf(
1350-
"ciphertext" to encryptedSessionBackupData!!.mCipherText,
1346+
"ciphertext" to encryptedSessionBackupData.mCipherText,
13511347
"mac" to encryptedSessionBackupData.mMac,
13521348
"ephemeral" to encryptedSessionBackupData.mEphemeralKey)
13531349
)

0 commit comments

Comments
 (0)