From 177e714001ef99790dea920b1bd99df6c20d8ef4 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 18 Dec 2019 15:50:34 -0300 Subject: [PATCH] Add error messages for unexpected state to ease debugging --- .../java/com/stevesoltys/seedvault/crypto/Crypto.kt | 4 +++- .../java/com/stevesoltys/seedvault/header/Header.kt | 12 +++++++++--- .../seedvault/restore/RestoreViewModel.kt | 2 +- .../seedvault/settings/SettingsManager.kt | 2 +- .../transport/ConfigurableBackupTransportService.kt | 2 +- .../seedvault/transport/backup/BackupCoordinator.kt | 6 +++--- .../seedvault/transport/restore/FullRestore.kt | 10 +++++----- .../seedvault/transport/restore/KVRestore.kt | 2 +- .../transport/restore/RestoreCoordinator.kt | 8 +++++--- .../seedvault/ui/storage/StorageViewModel.kt | 2 +- 10 files changed, 30 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/com/stevesoltys/seedvault/crypto/Crypto.kt b/app/src/main/java/com/stevesoltys/seedvault/crypto/Crypto.kt index d859e91d..05a1e869 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/crypto/Crypto.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/crypto/Crypto.kt @@ -87,7 +87,9 @@ class CryptoImpl( override fun encryptSegment(outputStream: OutputStream, cleartext: ByteArray) { val cipher = cipherFactory.createEncryptionCipher() - check(cipher.getOutputSize(cleartext.size) <= MAX_SEGMENT_LENGTH) + check(cipher.getOutputSize(cleartext.size) <= MAX_SEGMENT_LENGTH) { + "Cipher's output size ${cipher.getOutputSize(cleartext.size)} is larger than maximum segment length ($MAX_SEGMENT_LENGTH)" + } val encrypted = cipher.doFinal(cleartext) val segmentHeader = SegmentHeader(encrypted.size.toShort(), cipher.iv) diff --git a/app/src/main/java/com/stevesoltys/seedvault/header/Header.kt b/app/src/main/java/com/stevesoltys/seedvault/header/Header.kt index 864acd03..6a71bfe7 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/header/Header.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/header/Header.kt @@ -15,8 +15,12 @@ data class VersionHeader( internal val key: String? = null // ?? bytes ) { init { - check(packageName.length <= MAX_PACKAGE_LENGTH_SIZE) - key?.let { check(key.length <= MAX_KEY_LENGTH_SIZE) } + check(packageName.length <= MAX_PACKAGE_LENGTH_SIZE) { + "Package $packageName has name longer than $MAX_PACKAGE_LENGTH_SIZE" + } + key?.let { + check(key.length <= MAX_KEY_LENGTH_SIZE) { "Key $key is longer than $MAX_KEY_LENGTH_SIZE" } + } } } @@ -34,6 +38,8 @@ class SegmentHeader( internal val nonce: ByteArray // 12 bytes ) { init { - check(nonce.size == IV_SIZE) + check(nonce.size == IV_SIZE) { + "Nonce size of ${nonce.size} is not the expected IV size of $IV_SIZE" + } } } diff --git a/app/src/main/java/com/stevesoltys/seedvault/restore/RestoreViewModel.kt b/app/src/main/java/com/stevesoltys/seedvault/restore/RestoreViewModel.kt index 3549aa63..47fd3ad8 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/restore/RestoreViewModel.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/restore/RestoreViewModel.kt @@ -62,7 +62,7 @@ class RestoreViewModel(app: Application) : RequireProvisioningViewModel(app), Re override fun onRestoreSetClicked(set: RestoreSet) { val session = this.session - check(session != null) + check(session != null) { "Restore set clicked, but no session available" } session.restoreAll(set.token, observer, monitor) mChosenRestoreSet.value = set diff --git a/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsManager.kt b/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsManager.kt index a1f2d7a7..4a654b56 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsManager.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsManager.kt @@ -35,7 +35,7 @@ class SettingsManager(context: Context) { fun getStorage(): Storage? { val uriStr = prefs.getString(PREF_KEY_STORAGE_URI, null) ?: return null val uri = Uri.parse(uriStr) - val name = prefs.getString(PREF_KEY_STORAGE_NAME, null) ?: throw IllegalStateException() + val name = prefs.getString(PREF_KEY_STORAGE_NAME, null) ?: throw IllegalStateException("no storage name") val isUsb = prefs.getBoolean(PREF_KEY_STORAGE_IS_USB, false) return Storage(uri, name, isUsb) } diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/ConfigurableBackupTransportService.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/ConfigurableBackupTransportService.kt index c6b8ec52..9d4ef7a8 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/ConfigurableBackupTransportService.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/ConfigurableBackupTransportService.kt @@ -33,7 +33,7 @@ class ConfigurableBackupTransportService : Service() { } override fun onBind(intent: Intent): IBinder { - val transport = this.transport ?: throw IllegalStateException() + val transport = this.transport ?: throw IllegalStateException("no transport in onBind()") return transport.binder.apply { Log.d(TAG, "Transport bound.") } diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt index 857bf0ee..ab7911aa 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt @@ -178,11 +178,11 @@ class BackupCoordinator( fun finishBackup(): Int = when { kv.hasState() -> { - check(!full.hasState()) + check(!full.hasState()) { "K/V backup has state, but full backup has dangling state as well" } kv.finishBackup() } full.hasState() -> { - check(!kv.hasState()) + check(!kv.hasState()) { "Full backup has state, but K/V backup has dangling state as well" } full.finishBackup() } calledInitialize || calledClearBackupData -> { @@ -190,7 +190,7 @@ class BackupCoordinator( calledClearBackupData = false TRANSPORT_OK } - else -> throw IllegalStateException() + else -> throw IllegalStateException("Unexpected state in finishBackup()") } @Throws(IOException::class) diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/FullRestore.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/FullRestore.kt index 17905fed..ce8d40cc 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/FullRestore.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/FullRestore.kt @@ -77,7 +77,7 @@ internal class FullRestore( */ fun getNextFullRestoreDataChunk(socket: ParcelFileDescriptor): Int { Log.i(TAG, "Get next full restore data chunk.") - val state = this.state ?: throw IllegalStateException() + val state = this.state ?: throw IllegalStateException("no state") val packageName = state.packageInfo.packageName if (state.inputStream == null) { @@ -103,9 +103,9 @@ internal class FullRestore( } private fun readInputStream(socket: ParcelFileDescriptor): Int = socket.use { fileDescriptor -> - val state = this.state ?: throw IllegalStateException() + val state = this.state ?: throw IllegalStateException("no state") val packageName = state.packageInfo.packageName - val inputStream = state.inputStream ?: throw IllegalStateException() + val inputStream = state.inputStream ?: throw IllegalStateException("no stream") val outputStream = outputFactory.getOutputStream(fileDescriptor) try { @@ -144,7 +144,7 @@ internal class FullRestore( * with no further attempts to restore app data. */ fun abortFullRestore(): Int { - val state = this.state ?: throw IllegalStateException() + val state = this.state ?: throw IllegalStateException("no state") Log.i(TAG, "Abort full restore of ${state.packageInfo.packageName}!") resetState() @@ -156,7 +156,7 @@ internal class FullRestore( * freeing any resources and connections used during the restore process. */ fun finishRestore() { - val state = this.state ?: throw IllegalStateException() + val state = this.state ?: throw IllegalStateException("no state") Log.i(TAG, "Finish restore of ${state.packageInfo.packageName}!") resetState() diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/KVRestore.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/KVRestore.kt index 0ea2b611..cd24e02d 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/KVRestore.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/KVRestore.kt @@ -55,7 +55,7 @@ internal class KVRestore( * or [TRANSPORT_ERROR] (an error occurred, the restore should be aborted and rescheduled). */ fun getRestoreData(data: ParcelFileDescriptor): Int { - val state = this.state ?: throw IllegalStateException() + val state = this.state ?: throw IllegalStateException("no state") // The restore set is the concatenation of the individual record blobs, // each of which is a file in the package's directory. diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt index 75ea3a24..ba4f583e 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt @@ -43,7 +43,9 @@ internal class RestoreCoordinator( val restoreSets = ArrayList() for (encryptedMetadata in availableBackups) { if (encryptedMetadata.error) continue - check(encryptedMetadata.inputStream != null) // if there's no error, there must be a stream + check(encryptedMetadata.inputStream != null) { + "No error when getting encrypted metadata, but stream is still missing." + } try { val metadata = metadataReader.readMetadata(encryptedMetadata.inputStream, encryptedMetadata.token) val set = RestoreSet(metadata.deviceName, metadata.deviceName, metadata.token) @@ -93,7 +95,7 @@ internal class RestoreCoordinator( * or [TRANSPORT_ERROR] (an error occurred, the restore should be aborted and rescheduled). */ fun startRestore(token: Long, packages: Array): Int { - check(state == null) + check(state == null) { "Started new restore with existing state" } Log.i(TAG, "Start restore with ${packages.map { info -> info.packageName }}") state = RestoreCoordinatorState(token, packages.iterator()) return TRANSPORT_OK @@ -127,7 +129,7 @@ internal class RestoreCoordinator( */ fun nextRestorePackage(): RestoreDescription? { Log.i(TAG, "Next restore package!") - val state = this.state ?: throw IllegalStateException() + val state = this.state ?: throw IllegalStateException("no state") if (!state.packages.hasNext()) return NO_MORE_PACKAGES val packageInfo = state.packages.next() diff --git a/app/src/main/java/com/stevesoltys/seedvault/ui/storage/StorageViewModel.kt b/app/src/main/java/com/stevesoltys/seedvault/ui/storage/StorageViewModel.kt index 60d38376..3a373649 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/ui/storage/StorageViewModel.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/ui/storage/StorageViewModel.kt @@ -87,7 +87,7 @@ internal abstract class StorageViewModel(private val app: Application) : Android */ protected fun saveStorage(uri: Uri): Boolean { // store backup storage location in settings - val root = storageRoot ?: throw IllegalStateException() + val root = storageRoot ?: throw IllegalStateException("no storage root") val name = if (root.isInternal()) { "${root.title} (${app.getString(R.string.settings_backup_location_internal)})" } else {