From 6e4c117fca955779cf0705ed3407c306568835c2 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 9 Oct 2020 16:21:41 -0300 Subject: [PATCH 1/8] Also complile the instrumentation test sources in CI We already don't run instrumentation tests in CI, so we should at least compile them to catch any breakage there. This is important as they don't even get compliled when building and installing the app with Android Studio. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 9414cabf..1c3ce3ad 100644 --- a/.travis.yml +++ b/.travis.yml @@ -24,7 +24,7 @@ before_cache: - rm -f $HOME/.gradle/caches/modules-2/modules-2.lock - rm -fr $HOME/.gradle/caches/*/plugin-resolution/ -script: ./gradlew check assemble ktlintCheck +script: ./gradlew compileDebugAndroidTestSources check assemble ktlintCheck cache: directories: From 68a6403c4bd10951939ca8e627baeb9f2a0703c0 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 12 Oct 2020 14:51:08 -0300 Subject: [PATCH 2/8] Add a compatibility checker for APK splits that tries to figure out compatibility only based on the name of the split. This is not an exact science and there might be errors, but we hope to correctly identify most cases that matter in practice. --- .../install/ApkSplitCompatibilityChecker.kt | 118 ++++++++++++ .../ApkSplitCompatibilityCheckerTest.kt | 172 ++++++++++++++++++ 2 files changed, 290 insertions(+) create mode 100644 app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkSplitCompatibilityChecker.kt create mode 100644 app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkSplitCompatibilityCheckerTest.kt diff --git a/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkSplitCompatibilityChecker.kt b/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkSplitCompatibilityChecker.kt new file mode 100644 index 00000000..721e7e35 --- /dev/null +++ b/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkSplitCompatibilityChecker.kt @@ -0,0 +1,118 @@ +package com.stevesoltys.seedvault.restore.install + +import android.content.Context +import android.os.Build +import android.util.Log + +private const val TAG = "SplitCompatChecker" + +private const val CONFIG_PREFIX = "config." +private const val CONFIG_LENGTH = CONFIG_PREFIX.length + +private const val LDPI_VALUE = 120 +private const val MDPI_VALUE = 160 +private const val TVDPI_VALUE = 213 +private const val HDPI_VALUE = 240 +private const val XHDPI_VALUE = 320 +private const val XXHDPI_VALUE = 480 +private const val XXXHDPI_VALUE = 640 + +class DeviceInfo(context: Context) { + val densityDpi: Int = context.resources.displayMetrics.densityDpi + val supportedABIs: List = Build.SUPPORTED_ABIS.toList() +} + +/** + * Tries to determine APK split compatibility with a device by examining the list of split names. + * This only looks on the supported ABIs and the screen density. + * Other config splits e.g. based on OpenGL or Vulkan version are also possible, + * but don't seem to be widely used, so we don't consider those for now. + */ +class ApkSplitCompatibilityChecker(private val deviceInfo: DeviceInfo) { + + private val abiMap = mapOf( + "armeabi" to "armeabi", + "armeabi_v7a" to "armeabi-v7a", + "arm64_v8a" to "arm64-v8a", + "x86" to "x86", + "x86_64" to "x86_64", + "mips" to "mips", + "mips64" to "mips64" + ) + private val densityMap = mapOf( + "ldpi" to LDPI_VALUE, + "mdpi" to MDPI_VALUE, + "tvdpi" to TVDPI_VALUE, + "hdpi" to HDPI_VALUE, + "xhdpi" to XHDPI_VALUE, + "xxhdpi" to XXHDPI_VALUE, + "xxxhdpi" to XXXHDPI_VALUE + ) + + /** + * Returns true if the list of splits can be considered compatible with the current device, + * and false otherwise. + */ + fun isCompatible(splitNames: Collection): Boolean = splitNames.all { splitName -> + // all individual splits need to be compatible (which can be hard to judge by name only) + isCompatible(splitName) + } + + private fun isCompatible(splitName: String): Boolean { + val index = splitName.indexOf(CONFIG_PREFIX) + // If this is not a standardized config split, we just assume that it will work, + // as it is most likely a dynamic feature module. + if (index == -1) { + Log.v(TAG, "Not a config split '$splitName'. Assuming it is ok.") + return true + } + + val name = splitName.substring(index + CONFIG_LENGTH) + + // Check if this is a known ABI config + if (abiMap.containsKey(name)) { + // The ABI split must be supported by the current device + return isAbiCompatible(name) + } + + // Check if this is a known screen density config + densityMap[name]?.let { splitDensity -> + // the split's density must not be much lower than the device's. + return isDensityCompatible(splitDensity) + } + + // At this point we don't know what to make of that split, + // so let's just hope that it will work. It might just be a language. + Log.v(TAG, "Unhandled config split '$splitName'. Assuming it is ok.") + return true + } + + private fun isAbiCompatible(name: String): Boolean { + return if (deviceInfo.supportedABIs.contains(abiMap[name])) { + Log.v(TAG, "Config split '$name' is supported ABI (${deviceInfo.supportedABIs})") + true + } else { + Log.w(TAG, "Config split '$name' is not supported ABI (${deviceInfo.supportedABIs})") + false + } + } + + private fun isDensityCompatible(splitDensity: Int): Boolean { + @Suppress("MagicNumber") + val acceptableDiff = deviceInfo.densityDpi / 3 + return if (deviceInfo.densityDpi - splitDensity > acceptableDiff) { + Log.w( + TAG, + "Config split density $splitDensity not compatible with ${deviceInfo.densityDpi}" + ) + false + } else { + Log.v( + TAG, + "Config split density $splitDensity compatible with ${deviceInfo.densityDpi}" + ) + true + } + } + +} diff --git a/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkSplitCompatibilityCheckerTest.kt b/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkSplitCompatibilityCheckerTest.kt new file mode 100644 index 00000000..710768af --- /dev/null +++ b/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkSplitCompatibilityCheckerTest.kt @@ -0,0 +1,172 @@ +package com.stevesoltys.seedvault.restore.install + +import com.stevesoltys.seedvault.getRandomString +import com.stevesoltys.seedvault.transport.TransportTest +import io.mockk.every +import io.mockk.mockk +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.jupiter.api.Test + +class ApkSplitCompatibilityCheckerTest : TransportTest() { + + private val deviceInfo: DeviceInfo = mockk() + + private val checker = ApkSplitCompatibilityChecker(deviceInfo) + + @Test + fun `non-config splits always get accepted`() { + assertTrue( + checker.isCompatible( + listOf( + getRandomString(), + getRandomString(), + getRandomString(), + getRandomString(), + getRandomString(), + getRandomString() + ) + ) + ) + } + + @Test + fun `non-config splits mixed with unknown config splits always get accepted`() { + assertTrue( + checker.isCompatible( + listOf( + "config.de", + "config.en", + "config.gu", + getRandomString(), + getRandomString(), + getRandomString() + ) + ) + ) + } + + @Test + fun `all supported ABIs get accepted, non-supported rejected`() { + every { deviceInfo.supportedABIs } returns listOf("arm64-v8a", "armeabi-v7a", "armeabi") + + assertTrue(checker.isCompatible(listOf("config.arm64_v8a"))) + assertTrue(checker.isCompatible(listOf("${getRandomString()}.config.arm64_v8a"))) + assertTrue(checker.isCompatible(listOf("config.armeabi_v7a"))) + assertTrue(checker.isCompatible(listOf("${getRandomString()}.config.armeabi_v7a"))) + assertTrue(checker.isCompatible(listOf("config.armeabi"))) + assertTrue(checker.isCompatible(listOf("${getRandomString()}.config.armeabi"))) + + assertFalse(checker.isCompatible(listOf("config.x86"))) + assertFalse(checker.isCompatible(listOf("config.x86_64"))) + assertFalse(checker.isCompatible(listOf("config.mips"))) + assertFalse(checker.isCompatible(listOf("config.mips64"))) + } + + @Test + fun `armeabi rejects arm64_v8a and armeabi-v7a`() { + every { deviceInfo.supportedABIs } returns listOf("armeabi") + + assertTrue(checker.isCompatible(listOf("config.armeabi"))) + assertTrue(checker.isCompatible(listOf("${getRandomString()}.config.armeabi"))) + + assertFalse(checker.isCompatible(listOf("config.arm64_v8a"))) + assertFalse(checker.isCompatible(listOf("${getRandomString()}.config.arm64_v8a"))) + assertFalse(checker.isCompatible(listOf("config.armeabi_v7a"))) + assertFalse(checker.isCompatible(listOf("${getRandomString()}.config.armeabi_v7a"))) + } + + @Test + fun `screen density is accepted when not too low`() { + every { deviceInfo.densityDpi } returns 440 // xxhdpi - Pixel 4 + + assertTrue( + checker.isCompatible( + listOf( + "config.de", + "config.xxxhdpi", // higher density is accepted + getRandomString() + ) + ) + ) + assertTrue( + checker.isCompatible( + listOf( + "config.de", + "config.xxhdpi", // same density is accepted + getRandomString() + ) + ) + ) + assertTrue( + checker.isCompatible( + listOf( + "config.de", + "config.xhdpi", // one lower density is accepted + getRandomString() + ) + ) + ) + assertFalse( + checker.isCompatible( + listOf( + "config.de", + "config.hdpi", // two lower density is not accepted + getRandomString() + ) + ) + ) + // even lower densities are also not accepted + assertFalse(checker.isCompatible(listOf("config.tvdpi"))) + assertFalse(checker.isCompatible(listOf("config.mdpi"))) + assertFalse(checker.isCompatible(listOf("config.ldpi"))) + } + + @Test + fun `screen density accepts all higher densities`() { + every { deviceInfo.densityDpi } returns 120 + + assertTrue(checker.isCompatible(listOf("config.xxxhdpi"))) + assertTrue(checker.isCompatible(listOf("config.xxhdpi"))) + assertTrue(checker.isCompatible(listOf("config.xhdpi"))) + assertTrue(checker.isCompatible(listOf("config.hdpi"))) + assertTrue(checker.isCompatible(listOf("config.tvdpi"))) + assertTrue(checker.isCompatible(listOf("config.mdpi"))) + assertTrue(checker.isCompatible(listOf("config.ldpi"))) + } + + @Test + fun `test mix of unknown and all known config splits`() { + every { deviceInfo.supportedABIs } returns listOf("armeabi-v7a", "armeabi") + every { deviceInfo.densityDpi } returns 240 + + assertTrue( + checker.isCompatible( + listOf( + "config.de", + "config.xhdpi", + "config.armeabi", + getRandomString() + ) + ) + ) + // same as above, but feature split with unsupported ABI config gets rejected + assertFalse( + checker.isCompatible( + listOf( + "config.de", + "config.xhdpi", + "config.armeabi", + "${getRandomString()}.config.arm64_v8a", + getRandomString() + ) + ) + ) + + assertTrue(checker.isCompatible(listOf("config.xhdpi", "config.armeabi"))) + assertTrue(checker.isCompatible(listOf("config.hdpi", "config.armeabi_v7a"))) + assertFalse(checker.isCompatible(listOf("foo.config.ldpi", "config.armeabi_v7a"))) + assertFalse(checker.isCompatible(listOf("foo.config.xxxhdpi", "bar.config.arm64_v8a"))) + } + +} From b3db859b40f5cc35a86f7c0da28c7e1460ab1a10 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 12 Oct 2020 16:46:37 -0300 Subject: [PATCH 3/8] Re-install APK splits if they are compatible and have proper hash --- .../com/stevesoltys/seedvault/PluginTest.kt | 10 ++- .../saf/DocumentsProviderRestorePlugin.kt | 10 ++- .../seedvault/restore/install/ApkInstaller.kt | 26 ++++--- .../seedvault/restore/install/ApkRestore.kt | 77 ++++++++++++++++--- .../restore/install/InstallModule.kt | 3 +- .../transport/restore/RestorePlugin.kt | 2 +- .../restore/install/ApkRestoreTest.kt | 16 ++-- 7 files changed, 109 insertions(+), 35 deletions(-) diff --git a/app/src/androidTest/java/com/stevesoltys/seedvault/PluginTest.kt b/app/src/androidTest/java/com/stevesoltys/seedvault/PluginTest.kt index fbb5dded..95836c53 100644 --- a/app/src/androidTest/java/com/stevesoltys/seedvault/PluginTest.kt +++ b/app/src/androidTest/java/com/stevesoltys/seedvault/PluginTest.kt @@ -173,14 +173,18 @@ class PluginTest : KoinComponent { backupPlugin.getApkOutputStream(packageInfo, "").writeAndClose(apk1) // assert that read APK bytes match what was written - assertReadEquals(apk1, restorePlugin.getApkInputStream(token, packageInfo.packageName)) + assertReadEquals(apk1, restorePlugin.getApkInputStream(token, packageInfo.packageName, "")) // write random bytes as another APK + val suffix2 = getRandomBase64(23) val apk2 = getRandomByteArray(23 * 1024 * 1024) - backupPlugin.getApkOutputStream(packageInfo2, "").writeAndClose(apk2) + backupPlugin.getApkOutputStream(packageInfo2, suffix2).writeAndClose(apk2) // assert that read APK bytes match what was written - assertReadEquals(apk2, restorePlugin.getApkInputStream(token, packageInfo2.packageName)) + assertReadEquals( + apk2, + restorePlugin.getApkInputStream(token, packageInfo2.packageName, suffix2) + ) } @Test diff --git a/app/src/main/java/com/stevesoltys/seedvault/plugins/saf/DocumentsProviderRestorePlugin.kt b/app/src/main/java/com/stevesoltys/seedvault/plugins/saf/DocumentsProviderRestorePlugin.kt index 88af5399..b1f38fa8 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/plugins/saf/DocumentsProviderRestorePlugin.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/plugins/saf/DocumentsProviderRestorePlugin.kt @@ -94,10 +94,14 @@ internal class DocumentsProviderRestorePlugin( } @Throws(IOException::class) - override suspend fun getApkInputStream(token: Long, packageName: String): InputStream { + override suspend fun getApkInputStream( + token: Long, + packageName: String, + suffix: String + ): InputStream { val setDir = storage.getSetDir(token) ?: throw IOException() - val file = - setDir.findFileBlocking(context, "$packageName.apk") ?: throw FileNotFoundException() + val file = setDir.findFileBlocking(context, "$packageName$suffix.apk") + ?: throw FileNotFoundException() return storage.getInputStream(file) } diff --git a/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkInstaller.kt b/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkInstaller.kt index 65f433d3..586786c2 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkInstaller.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkInstaller.kt @@ -38,7 +38,7 @@ internal class ApkInstaller(private val context: Context) { @Throws(IOException::class, SecurityException::class) internal suspend fun install( - cachedApk: File, + cachedApks: List, packageName: String, installerPackageName: String?, installResult: MutableInstallResult @@ -47,16 +47,16 @@ internal class ApkInstaller(private val context: Context) { override fun onReceive(context: Context, i: Intent) { if (i.action != BROADCAST_ACTION) return context.unregisterReceiver(this) - cont.resume(onBroadcastReceived(i, packageName, cachedApk, installResult)) + cont.resume(onBroadcastReceived(i, packageName, cachedApks, installResult)) } } context.registerReceiver(broadcastReceiver, IntentFilter(BROADCAST_ACTION)) cont.invokeOnCancellation { context.unregisterReceiver(broadcastReceiver) } - install(cachedApk, installerPackageName) + install(cachedApks, installerPackageName) } - private fun install(cachedApk: File, installerPackageName: String?) { + private fun install(cachedApks: List, installerPackageName: String?) { val sessionParams = SessionParams(MODE_FULL_INSTALL).apply { setInstallerPackageName(installerPackageName) // Setting the INSTALL_ALLOW_TEST flag here does not allow us to install test apps, @@ -65,12 +65,14 @@ internal class ApkInstaller(private val context: Context) { // Don't set more sessionParams intentionally here. // We saw strange permission issues when doing setInstallReason() or setting installFlags. val session = installer.openSession(installer.createSession(sessionParams)) - val sizeBytes = cachedApk.length() session.use { s -> - cachedApk.inputStream().use { inputStream -> - s.openWrite("PackageInstaller", 0, sizeBytes).use { out -> - inputStream.copyTo(out) - s.fsync(out) + cachedApks.forEach { cachedApk -> + val sizeBytes = cachedApk.length() + cachedApk.inputStream().use { inputStream -> + s.openWrite(cachedApk.name, 0, sizeBytes).use { out -> + inputStream.copyTo(out) + s.fsync(out) + } } } s.commit(getIntentSender()) @@ -90,7 +92,7 @@ internal class ApkInstaller(private val context: Context) { private fun onBroadcastReceived( i: Intent, expectedPackageName: String, - cachedApk: File, + cachedApks: List, installResult: MutableInstallResult ): InstallResult { val packageName = i.getStringExtra(EXTRA_PACKAGE_NAME)!! @@ -102,9 +104,9 @@ internal class ApkInstaller(private val context: Context) { } Log.d(TAG, "Received result for $packageName: success=$success $statusMsg") - // delete cached APK file on I/O thread + // delete all cached APK files on I/O thread GlobalScope.launch(Dispatchers.IO) { - cachedApk.delete() + cachedApks.forEach { it.delete() } } // update status and offer result diff --git a/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkRestore.kt b/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkRestore.kt index 9c35b88e..886d41d9 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkRestore.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkRestore.kt @@ -5,6 +5,7 @@ import android.content.pm.PackageManager import android.content.pm.PackageManager.GET_SIGNATURES import android.content.pm.PackageManager.GET_SIGNING_CERTIFICATES import android.util.Log +import com.stevesoltys.seedvault.metadata.ApkSplit import com.stevesoltys.seedvault.metadata.PackageMetadata import com.stevesoltys.seedvault.metadata.PackageMetadataMap import com.stevesoltys.seedvault.restore.install.ApkInstallState.FAILED_SYSTEM_APP @@ -26,6 +27,7 @@ private val TAG = ApkRestore::class.java.simpleName internal class ApkRestore( private val context: Context, private val restorePlugin: RestorePlugin, + private val splitCompatChecker: ApkSplitCompatibilityChecker, private val apkInstaller: ApkInstaller ) { @@ -78,11 +80,8 @@ internal class ApkRestore( metadata: PackageMetadata, installResult: MutableInstallResult ) { - // create a cache file to write the APK into - val cachedApk = File.createTempFile(packageName, ".apk", context.cacheDir) - // copy APK to cache file and calculate SHA-256 hash while we are at it - val inputStream = restorePlugin.getApkInputStream(token, packageName) - val sha256 = copyStreamsAndGetHash(inputStream, cachedApk.outputStream()) + // cache the APK and get its hash + val (cachedApk, sha256) = cacheApk(token, packageName) // check APK's SHA-256 hash if (metadata.sha256 != sha256) throw SecurityException( @@ -137,18 +136,78 @@ internal class ApkRestore( } } - if (metadata.splits != null) { - // do not install APKs that require splits (for now) - Log.w(TAG, "Not installing $packageName because it requires splits.") + // process further APK splits, if available + val cachedApks = cacheSplitsIfNeeded(token, packageName, cachedApk, metadata.splits) + if (cachedApks == null) { + Log.w(TAG, "Not installing $packageName because of incompatible splits.") collector.emit(installResult.fail(packageName)) return } // install APK and emit updates from it - val result = apkInstaller.install(cachedApk, packageName, metadata.installer, installResult) + val result = + apkInstaller.install(cachedApks, packageName, metadata.installer, installResult) collector.emit(result) } + /** + * Retrieves APK splits from [RestorePlugin] and caches them locally. + * + * @throws SecurityException if a split has an unexpected SHA-256 hash. + * @return a list of all APKs that need to be installed + * or null if the splits are incompatible with this restore device. + */ + @Throws(IOException::class, SecurityException::class) + private suspend fun cacheSplitsIfNeeded( + token: Long, + packageName: String, + cachedApk: File, + splits: List? + ): List? { + // if null, there are no splits, so we just have a single base APK to consider + val splitNames = splits?.map { it.name } ?: return listOf(cachedApk) + + // return null when splits are incompatible + if (!splitCompatChecker.isCompatible(splitNames)) return null + + // store coming splits in a list + val cachedApks = ArrayList(splits.size + 1).apply { + add(cachedApk) // don't forget the base APK + } + splits.forEach { apkSplit -> // cache and check all splits + val suffix = "_${apkSplit.sha256}" + val (file, sha256) = cacheApk(token, packageName, suffix) + // check APK split's SHA-256 hash + if (apkSplit.sha256 != sha256) throw SecurityException( + "$packageName:${apkSplit.name} has sha256 '$sha256'," + + " but '${apkSplit.sha256}' expected." + ) + cachedApks.add(file) + } + return cachedApks + } + + /** + * Retrieves an APK from the [RestorePlugin] and caches it locally + * while calculating its SHA-256 hash. + * + * @return a [Pair] of the cached [File] and SHA-256 hash. + */ + @Throws(IOException::class) + @Suppress("BlockingMethodInNonBlockingContext") // flows on Dispatcher.IO + private suspend fun cacheApk( + token: Long, + packageName: String, + suffix: String = "" + ): Pair { + // create a cache file to write the APK into + val cachedApk = File.createTempFile(packageName + suffix, ".apk", context.cacheDir) + // copy APK to cache file and calculate SHA-256 hash while we are at it + val inputStream = restorePlugin.getApkInputStream(token, packageName, suffix) + val sha256 = copyStreamsAndGetHash(inputStream, cachedApk.outputStream()) + return Pair(cachedApk, sha256) + } + /** * Returns null if this system app should get re-installed, * or a new [InstallResult] to be emitted otherwise. diff --git a/app/src/main/java/com/stevesoltys/seedvault/restore/install/InstallModule.kt b/app/src/main/java/com/stevesoltys/seedvault/restore/install/InstallModule.kt index 1e27ea5b..9c484e8f 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/restore/install/InstallModule.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/restore/install/InstallModule.kt @@ -5,5 +5,6 @@ import org.koin.dsl.module val installModule = module { factory { ApkInstaller(androidContext()) } - factory { ApkRestore(androidContext(), get(), get()) } + factory { ApkSplitCompatibilityChecker(DeviceInfo(androidContext())) } + factory { ApkRestore(androidContext(), get(), get(), get()) } } diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestorePlugin.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestorePlugin.kt index 91843998..ceae6ca0 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestorePlugin.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestorePlugin.kt @@ -34,6 +34,6 @@ interface RestorePlugin { * Returns an [InputStream] for the given token, for reading an APK that is to be restored. */ @Throws(IOException::class) - suspend fun getApkInputStream(token: Long, packageName: String): InputStream + suspend fun getApkInputStream(token: Long, packageName: String, suffix: String): InputStream } diff --git a/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt b/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt index fcd65815..033f57a8 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt @@ -46,9 +46,11 @@ internal class ApkRestoreTest : TransportTest() { every { packageManager } returns pm } private val restorePlugin: RestorePlugin = mockk() + private val splitCompatChecker: ApkSplitCompatibilityChecker = mockk() private val apkInstaller: ApkInstaller = mockk() - private val apkRestore: ApkRestore = ApkRestore(strictContext, restorePlugin, apkInstaller) + private val apkRestore: ApkRestore = + ApkRestore(strictContext, restorePlugin, splitCompatChecker, apkInstaller) private val icon: Drawable = mockk() @@ -78,7 +80,7 @@ internal class ApkRestoreTest : TransportTest() { val packageMetadataMap: PackageMetadataMap = hashMapOf(packageName to packageMetadata) every { strictContext.cacheDir } returns File(tmpDir.toString()) - coEvery { restorePlugin.getApkInputStream(token, packageName) } returns apkInputStream + coEvery { restorePlugin.getApkInputStream(token, packageName, "") } returns apkInputStream apkRestore.restore(token, packageMetadataMap).collectIndexed { index, value -> when (index) { @@ -109,7 +111,7 @@ internal class ApkRestoreTest : TransportTest() { packageInfo.packageName = getRandomString() every { strictContext.cacheDir } returns File(tmpDir.toString()) - coEvery { restorePlugin.getApkInputStream(token, packageName) } returns apkInputStream + coEvery { restorePlugin.getApkInputStream(token, packageName, "") } returns apkInputStream every { pm.getPackageArchiveInfo(any(), any()) } returns packageInfo apkRestore.restore(token, packageMetadataMap).collectIndexed { index, value -> @@ -137,7 +139,7 @@ internal class ApkRestoreTest : TransportTest() { @Test fun `test apkInstaller throws exceptions`(@TempDir tmpDir: Path) = runBlocking { every { strictContext.cacheDir } returns File(tmpDir.toString()) - coEvery { restorePlugin.getApkInputStream(token, packageName) } returns apkInputStream + coEvery { restorePlugin.getApkInputStream(token, packageName, "") } returns apkInputStream every { pm.getPackageArchiveInfo(any(), any()) } returns packageInfo every { pm.loadItemIcon( @@ -194,7 +196,7 @@ internal class ApkRestoreTest : TransportTest() { } every { strictContext.cacheDir } returns File(tmpDir.toString()) - coEvery { restorePlugin.getApkInputStream(token, packageName) } returns apkInputStream + coEvery { restorePlugin.getApkInputStream(token, packageName, "") } returns apkInputStream every { pm.getPackageArchiveInfo(any(), any()) } returns packageInfo every { pm.loadItemIcon( @@ -247,7 +249,9 @@ internal class ApkRestoreTest : TransportTest() { val isSystemApp = Random.nextBoolean() every { strictContext.cacheDir } returns File(tmpDir.toString()) - coEvery { restorePlugin.getApkInputStream(token, packageName) } returns apkInputStream + coEvery { + restorePlugin.getApkInputStream(token, packageName, "") + } returns apkInputStream every { pm.getPackageArchiveInfo(any(), any()) } returns packageInfo every { pm.loadItemIcon( From 608e67cb6546086715783ce55835fb0933a32b69 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Tue, 13 Oct 2020 16:01:09 -0300 Subject: [PATCH 4/8] Refactor existing ApkRestore unit tests to make adding new ones easier --- .../restore/install/ApkRestoreTest.kt | 228 ++++++++---------- 1 file changed, 99 insertions(+), 129 deletions(-) diff --git a/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt b/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt index 033f57a8..fa6c29ac 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt @@ -22,7 +22,6 @@ import io.mockk.coEvery import io.mockk.every import io.mockk.mockk import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.collectIndexed import kotlinx.coroutines.runBlocking import org.junit.jupiter.api.Assertions @@ -82,26 +81,8 @@ internal class ApkRestoreTest : TransportTest() { every { strictContext.cacheDir } returns File(tmpDir.toString()) coEvery { restorePlugin.getApkInputStream(token, packageName, "") } returns apkInputStream - apkRestore.restore(token, packageMetadataMap).collectIndexed { index, value -> - when (index) { - 0 -> { - val result = value[packageName] - assertEquals(QUEUED, result.state) - assertEquals(1, result.progress) - assertEquals(1, value.total) - } - 1 -> { - val result = value[packageName] - assertEquals(FAILED, result.state) - assertTrue(value.hasFailed) - assertFalse(value.isFinished) - } - 2 -> { - assertTrue(value.hasFailed) - assertTrue(value.isFinished) - } - else -> fail("more values emitted") - } + apkRestore.restore(token, packageMetadataMap).collectIndexed { i, value -> + assertQueuedFailFinished(i, value) } } @@ -114,72 +95,20 @@ internal class ApkRestoreTest : TransportTest() { coEvery { restorePlugin.getApkInputStream(token, packageName, "") } returns apkInputStream every { pm.getPackageArchiveInfo(any(), any()) } returns packageInfo - apkRestore.restore(token, packageMetadataMap).collectIndexed { index, value -> - when (index) { - 0 -> { - val result = value[packageName] - assertEquals(QUEUED, result.state) - assertEquals(1, result.progress) - assertEquals(1, value.total) - } - 1 -> { - val result = value[packageName] - assertEquals(FAILED, result.state) - assertTrue(value.hasFailed) - } - 2 -> { - assertTrue(value.hasFailed) - assertTrue(value.isFinished) - } - else -> fail("more values emitted") - } + apkRestore.restore(token, packageMetadataMap).collectIndexed { i, value -> + assertQueuedFailFinished(i, value) } } @Test fun `test apkInstaller throws exceptions`(@TempDir tmpDir: Path) = runBlocking { - every { strictContext.cacheDir } returns File(tmpDir.toString()) - coEvery { restorePlugin.getApkInputStream(token, packageName, "") } returns apkInputStream - every { pm.getPackageArchiveInfo(any(), any()) } returns packageInfo - every { - pm.loadItemIcon( - packageInfo.applicationInfo, - packageInfo.applicationInfo - ) - } returns icon - every { pm.getApplicationLabel(packageInfo.applicationInfo) } returns appName + cacheBaseApkAndGetInfo(tmpDir) coEvery { - apkInstaller.install(any(), packageName, installerName, any()) + apkInstaller.install(match { it.size == 1 }, packageName, installerName, any()) } throws SecurityException() - apkRestore.restore(token, packageMetadataMap).collectIndexed { index, value -> - when (index) { - 0 -> { - val result = value[packageName] - assertEquals(QUEUED, result.state) - assertEquals(1, result.progress) - assertEquals(installerName, result.installerPackageName) - assertEquals(1, value.total) - } - 1 -> { - val result = value[packageName] - assertEquals(IN_PROGRESS, result.state) - assertEquals(appName, result.name) - assertEquals(icon, result.icon) - assertFalse(value.hasFailed) - } - 2 -> { - val result = value[packageName] - assertTrue(value.hasFailed) - assertEquals(FAILED, result.state) - assertFalse(value.isFinished) - } - 3 -> { - assertTrue(value.hasFailed, "1") - assertTrue(value.isFinished, "2") - } - else -> fail("more values emitted") - } + apkRestore.restore(token, packageMetadataMap).collectIndexed { i, value -> + assertQueuedProgressFailFinished(i, value) } } @@ -195,46 +124,13 @@ internal class ApkRestoreTest : TransportTest() { ) } - every { strictContext.cacheDir } returns File(tmpDir.toString()) - coEvery { restorePlugin.getApkInputStream(token, packageName, "") } returns apkInputStream - every { pm.getPackageArchiveInfo(any(), any()) } returns packageInfo - every { - pm.loadItemIcon( - packageInfo.applicationInfo, - packageInfo.applicationInfo - ) - } returns icon - every { pm.getApplicationLabel(packageInfo.applicationInfo) } returns appName + cacheBaseApkAndGetInfo(tmpDir) coEvery { - apkInstaller.install(any(), packageName, installerName, any()) + apkInstaller.install(match { it.size == 1 }, packageName, installerName, any()) } returns installResult - var i = 0 - apkRestore.restore(token, packageMetadataMap).collect { value -> - when (i) { - 0 -> { - val result = value[packageName] - assertEquals(QUEUED, result.state) - assertEquals(1, result.progress) - assertEquals(1, value.total) - } - 1 -> { - val result = value[packageName] - assertEquals(IN_PROGRESS, result.state) - assertEquals(appName, result.name) - assertEquals(icon, result.icon) - } - 2 -> { - val result = value[packageName] - assertEquals(SUCCEEDED, result.state) - } - 3 -> { - assertFalse(value.hasFailed) - assertTrue(value.isFinished) - } - else -> fail("more values emitted") - } - i++ + apkRestore.restore(token, packageMetadataMap).collectIndexed { i, value -> + assertQueuedProgressSuccessFinished(i, value) } } @@ -248,19 +144,9 @@ internal class ApkRestoreTest : TransportTest() { val willFail = Random.nextBoolean() val isSystemApp = Random.nextBoolean() - every { strictContext.cacheDir } returns File(tmpDir.toString()) - coEvery { - restorePlugin.getApkInputStream(token, packageName, "") - } returns apkInputStream - every { pm.getPackageArchiveInfo(any(), any()) } returns packageInfo - every { - pm.loadItemIcon( - packageInfo.applicationInfo, - packageInfo.applicationInfo - ) - } returns icon + cacheBaseApkAndGetInfo(tmpDir) every { packageInfo.applicationInfo.loadIcon(pm) } returns icon - every { pm.getApplicationLabel(packageInfo.applicationInfo) } returns appName + if (willFail) { every { pm.getPackageInfo(packageName, 0) @@ -280,7 +166,12 @@ internal class ApkRestoreTest : TransportTest() { ) } coEvery { - apkInstaller.install(any(), packageName, installerName, any()) + apkInstaller.install( + match { it.size == 1 }, + packageName, + installerName, + any() + ) } returns installResult } } @@ -315,6 +206,85 @@ internal class ApkRestoreTest : TransportTest() { } } + private fun cacheBaseApkAndGetInfo(tmpDir: Path) { + every { strictContext.cacheDir } returns File(tmpDir.toString()) + coEvery { restorePlugin.getApkInputStream(token, packageName, "") } returns apkInputStream + every { pm.getPackageArchiveInfo(any(), any()) } returns packageInfo + every { + pm.loadItemIcon( + packageInfo.applicationInfo, + packageInfo.applicationInfo + ) + } returns icon + every { pm.getApplicationLabel(packageInfo.applicationInfo) } returns appName + } + + private fun assertQueuedFailFinished(step: Int, value: InstallResult) = when (step) { + 0 -> assertQueuedProgress(step, value) + 1 -> { + val result = value[packageName] + assertEquals(FAILED, result.state) + assertTrue(value.hasFailed) + assertFalse(value.isFinished) + } + 2 -> { + assertTrue(value.hasFailed) + assertTrue(value.isFinished) + } + else -> fail("more values emitted") + } + + private fun assertQueuedProgressSuccessFinished(step: Int, value: InstallResult) = when (step) { + 0 -> assertQueuedProgress(step, value) + 1 -> assertQueuedProgress(step, value) + 2 -> { + val result = value[packageName] + assertEquals(SUCCEEDED, result.state) + } + 3 -> { + assertFalse(value.hasFailed) + assertTrue(value.isFinished) + } + else -> fail("more values emitted") + } + + private fun assertQueuedProgressFailFinished(step: Int, value: InstallResult) = when (step) { + 0 -> assertQueuedProgress(step, value) + 1 -> assertQueuedProgress(step, value) + 2 -> { + // app install has failed + val result = value[packageName] + assertEquals(FAILED, result.state) + assertTrue(value.hasFailed) + assertFalse(value.isFinished) + } + 3 -> { + assertTrue(value.hasFailed) + assertTrue(value.isFinished) + } + else -> fail("more values emitted") + } + + private fun assertQueuedProgress(step: Int, value: InstallResult) = when (step) { + 0 -> { + // single package gets queued + val result = value[packageName] + assertEquals(QUEUED, result.state) + assertEquals(installerName, result.installerPackageName) + assertEquals(1, result.progress) + assertEquals(1, value.total) + } + 1 -> { + // name and icon are available now + val result = value[packageName] + assertEquals(IN_PROGRESS, result.state) + assertEquals(appName, result.name) + assertEquals(icon, result.icon) + assertFalse(value.hasFailed) + } + else -> fail("more values emitted") + } + } private operator fun InstallResult.get(packageName: String): ApkInstallResult { From 0f0f19822881ab77591d222a3f7488544ad16828 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Tue, 13 Oct 2020 16:02:37 -0300 Subject: [PATCH 5/8] Add unit tests for re-installing apps with APK splits --- .../restore/install/ApkRestoreTest.kt | 120 ++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt b/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt index fa6c29ac..75a14505 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt @@ -8,7 +8,10 @@ import android.content.pm.ApplicationInfo.FLAG_UPDATED_SYSTEM_APP import android.content.pm.PackageInfo import android.content.pm.PackageManager import android.graphics.drawable.Drawable +import com.stevesoltys.seedvault.getRandomBase64 +import com.stevesoltys.seedvault.getRandomByteArray import com.stevesoltys.seedvault.getRandomString +import com.stevesoltys.seedvault.metadata.ApkSplit import com.stevesoltys.seedvault.metadata.PackageMetadata import com.stevesoltys.seedvault.metadata.PackageMetadataMap import com.stevesoltys.seedvault.restore.install.ApkInstallState.FAILED @@ -33,6 +36,7 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.io.TempDir import java.io.ByteArrayInputStream import java.io.File +import java.io.IOException import java.nio.file.Path import kotlin.random.Random @@ -206,6 +210,122 @@ internal class ApkRestoreTest : TransportTest() { } } + @Test + fun `incompatible splits cause FAILED state`(@TempDir tmpDir: Path) = runBlocking { + // add one APK split to metadata + val split1Name = getRandomString() + val split2Name = getRandomString() + packageMetadataMap[packageName] = packageMetadataMap[packageName]!!.copy( + splits = listOf( + ApkSplit(split1Name, getRandomBase64()), + ApkSplit(split2Name, getRandomBase64()) + ) + ) + + // cache APK and get icon as well as app name + cacheBaseApkAndGetInfo(tmpDir) + + // splits are NOT compatible + every { splitCompatChecker.isCompatible(listOf(split1Name, split2Name)) } returns false + + apkRestore.restore(token, packageMetadataMap).collectIndexed { i, value -> + assertQueuedProgressFailFinished(i, value) + } + } + + @Test + fun `split signature mismatch causes FAILED state`(@TempDir tmpDir: Path) = runBlocking { + // add one APK split to metadata + val splitName = getRandomString() + val sha256 = getRandomBase64(23) + packageMetadataMap[packageName] = packageMetadataMap[packageName]!!.copy( + splits = listOf(ApkSplit(splitName, sha256)) + ) + + // cache APK and get icon as well as app name + cacheBaseApkAndGetInfo(tmpDir) + + every { splitCompatChecker.isCompatible(listOf(splitName)) } returns true + coEvery { + restorePlugin.getApkInputStream(token, packageName, "_$sha256") + } returns ByteArrayInputStream(getRandomByteArray()) + + apkRestore.restore(token, packageMetadataMap).collectIndexed { i, value -> + assertQueuedProgressFailFinished(i, value) + } + } + + @Test + fun `exception while getting split data causes FAILED state`(@TempDir tmpDir: Path) = + runBlocking { + // add one APK split to metadata + val splitName = getRandomString() + val sha256 = getRandomBase64(23) + packageMetadataMap[packageName] = packageMetadataMap[packageName]!!.copy( + splits = listOf(ApkSplit(splitName, sha256)) + ) + + // cache APK and get icon as well as app name + cacheBaseApkAndGetInfo(tmpDir) + + every { splitCompatChecker.isCompatible(listOf(splitName)) } returns true + coEvery { + restorePlugin.getApkInputStream(token, packageName, "_$sha256") + } throws IOException() + + apkRestore.restore(token, packageMetadataMap).collectIndexed { i, value -> + assertQueuedProgressFailFinished(i, value) + } + } + + @Test + fun `splits get installed along with base APK`(@TempDir tmpDir: Path) = runBlocking { + // add one APK split to metadata + val split1Name = getRandomString() + val split2Name = getRandomString() + val split1sha256 = "A5BYxvLAy0ksUzsKTRTvd8wPeKvMztUofYShogEc-4E" + val split2sha256 = "ZqZ1cVH47lXbEncWx-Pc4L6AdLZOIO2lQuXB5GypxB4" + packageMetadataMap[packageName] = packageMetadataMap[packageName]!!.copy( + splits = listOf( + ApkSplit(split1Name, split1sha256), + ApkSplit(split2Name, split2sha256) + ) + ) + + // cache APK and get icon as well as app name + cacheBaseApkAndGetInfo(tmpDir) + + every { splitCompatChecker.isCompatible(listOf(split1Name, split2Name)) } returns true + + // define bytes of splits and return them as stream (matches above hashes) + val split1Bytes = byteArrayOf(0x01, 0x02, 0x03) + val split2Bytes = byteArrayOf(0x07, 0x08, 0x09) + val split1InputStream = ByteArrayInputStream(split1Bytes) + val split2InputStream = ByteArrayInputStream(split2Bytes) + coEvery { + restorePlugin.getApkInputStream(token, packageName, "_$split1sha256") + } returns split1InputStream + coEvery { + restorePlugin.getApkInputStream(token, packageName, "_$split2sha256") + } returns split2InputStream + + coEvery { + apkInstaller.install(match { it.size == 3 }, packageName, installerName, any()) + } returns MutableInstallResult(1).apply { + set( + packageName, ApkInstallResult( + packageName, + progress = 1, + state = SUCCEEDED + ) + ) + } + + apkRestore.restore(token, packageMetadataMap).collectIndexed { i, value -> + assertQueuedProgressSuccessFinished(i, value) + } + } + private fun cacheBaseApkAndGetInfo(tmpDir: Path) { every { strictContext.cacheDir } returns File(tmpDir.toString()) coEvery { restorePlugin.getApkInputStream(token, packageName, "") } returns apkInputStream From a833df2165e625c1a4b35c7de88096e21f9480e9 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Thu, 15 Oct 2020 14:08:03 -0300 Subject: [PATCH 6/8] Show dialog explaining auto-restore after a restore failed due to not all apps being installed --- .../seedvault/restore/RestoreProgressFragment.kt | 16 ++++++++++++++++ app/src/main/res/values/strings.xml | 2 ++ 2 files changed, 18 insertions(+) diff --git a/app/src/main/java/com/stevesoltys/seedvault/restore/RestoreProgressFragment.kt b/app/src/main/java/com/stevesoltys/seedvault/restore/RestoreProgressFragment.kt index adec8467..e3b4ce77 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/restore/RestoreProgressFragment.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/restore/RestoreProgressFragment.kt @@ -9,6 +9,7 @@ import android.view.WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON import android.widget.Button import android.widget.ProgressBar import android.widget.TextView +import androidx.appcompat.app.AlertDialog import androidx.core.content.ContextCompat.getColor import androidx.fragment.app.Fragment import androidx.lifecycle.Observer @@ -17,6 +18,7 @@ import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.LinearLayoutManager.VERTICAL import androidx.recyclerview.widget.RecyclerView import com.stevesoltys.seedvault.R +import com.stevesoltys.seedvault.ui.AppBackupState.FAILED_NOT_INSTALLED import org.koin.androidx.viewmodel.ext.android.sharedViewModel class RestoreProgressFragment : Fragment() { @@ -83,11 +85,25 @@ class RestoreProgressFragment : Fragment() { backupNameView.setTextColor(getColor(requireContext(), R.color.red)) } else { backupNameView.text = getString(R.string.restore_finished_success) + onRestoreFinished() } activity?.window?.clearFlags(FLAG_KEEP_SCREEN_ON) }) } + private fun onRestoreFinished() { + // check if any restore failed, because the app is not installed + val failed = viewModel.restoreProgress.value?.any { it.state == FAILED_NOT_INSTALLED } + if (failed != true) return // nothing left to do if there's no failures due to not installed + AlertDialog.Builder(requireContext()) + .setTitle(R.string.restore_restoring_error_title) + .setMessage(R.string.restore_restoring_error_message) + .setPositiveButton(android.R.string.ok) { dialog, _ -> + dialog.dismiss() + } + .show() + } + private fun stayScrolledAtTop(add: () -> Unit) { val position = layoutManager.findFirstVisibleItemPosition() add.invoke() diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 54ff96a0..c45c7a1c 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -118,6 +118,8 @@ Tap to install Next Restoring backup + Unable to restore some apps + You can re-install these apps manually and "Automatic restore" will attempt to restore their data (when enabled). System package manager Restore complete An error occurred while restoring the backup. From e6723093c95dc8d2c7bb7bf4312fe398e550e0e2 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Thu, 15 Oct 2020 14:14:55 -0300 Subject: [PATCH 7/8] Android Studio 4.1 and changes to test files --- .idea/runConfigurations/Instrumentation_Tests.xml | 7 +++++-- .idea/runConfigurations/Unit_Tests.xml | 2 +- app/src/main/AndroidManifest.xml | 4 +++- build.gradle | 2 +- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/.idea/runConfigurations/Instrumentation_Tests.xml b/.idea/runConfigurations/Instrumentation_Tests.xml index 0546643d..60200c14 100644 --- a/.idea/runConfigurations/Instrumentation_Tests.xml +++ b/.idea/runConfigurations/Instrumentation_Tests.xml @@ -1,6 +1,6 @@ - - + +