App update: one download at a time, each in its own file

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 2.2.2
+
+- **Phone app updates.** Tapping **Update** more than once (or on both the home screen and in Settings) started downloads that overwrote each other, and the update then failed with "The download is corrupt (checksum mismatch)". Only one download runs now, its button shows it's downloading, each download goes to a file of its own, and a damaged download is retried once. If your phone shows that error on the way to this version, tap **Update** once and wait, or install `PowerDeck.apk` from the release page.
+
 ## 2.2.1
 
 - **PC window over Remote Desktop.** An administrator of the PC can now open the PowerDeck window over Remote Desktop without running it as administrator. Standard accounts in another session are still refused, and the message now says what to do.

android/app/src/main/java/net/odysseylabs/powerdeck/data/Updater.kt

@@ -16,6 +16,7 @@ import okhttp3.OkHttpClient
 import okhttp3.Request
 import org.json.JSONObject
 import java.io.File
+import java.security.DigestInputStream
 import java.security.MessageDigest
 import java.util.concurrent.TimeUnit
 
@@ -70,15 +71,39 @@ object Updater {
 
     /** Downloads, verifies and hands the APK to the system installer (which asks the user to confirm). */
     suspend fun install(ctx: Context, release: Release) = withContext(Dispatchers.IO) {
-        val file = File(ctx.cacheDir, "update.apk")
-        http.newCall(Request.Builder().url(release.apkUrl).build()).execute().use { r ->
-            check(r.isSuccessful) { "Download failed (HTTP ${r.code})" }
-            file.outputStream().use { out -> r.body.byteStream().copyTo(out) }
+        val file = download(release, ctx.cacheDir)
+        try {
+            commit(ctx, file)
+        } finally {
+            file.delete()
         }
-        release.sha256?.let { want ->
-            val got = MessageDigest.getInstance("SHA-256").digest(file.readBytes()).joinToString("") { "%02x".format(it) }
-            check(got.equals(want, ignoreCase = true)) { "The download is corrupt (checksum mismatch)" }
+    }
+
+    /**
+     * Downloads the APK into a file of its own in [dir], so two downloads can't overwrite each other,
+     * checking the published SHA-256 as it arrives. A download that doesn't match is tried once more.
+     */
+    internal fun download(release: Release, dir: File, client: OkHttpClient = http): File {
+        repeat(2) {
+            val file = File.createTempFile("update-", ".apk", dir)
+            var ok = false
+            try {
+                val md = MessageDigest.getInstance("SHA-256")
+                client.newCall(Request.Builder().url(release.apkUrl).build()).execute().use { r ->
+                    check(r.isSuccessful) { "Download failed (HTTP ${r.code})" }
+                    DigestInputStream(r.body.byteStream(), md).use { input -> file.outputStream().use { input.copyTo(it) } }
+                }
+                val got = md.digest().joinToString("") { "%02x".format(it) }
+                ok = release.sha256 == null || got.equals(release.sha256, ignoreCase = true)
+                if (ok) return file
+            } finally {
+                if (!ok) file.delete()
+            }
         }
+        error("The download was damaged twice (checksum mismatch). Tap Update to try again.")
+    }
+
+    private fun commit(ctx: Context, file: File) {
         val installer = ctx.packageManager.packageInstaller
         val params = PackageInstaller.SessionParams(PackageInstaller.SessionParams.MODE_FULL_INSTALL).apply {
             setAppPackageName(ctx.packageName)
@@ -95,7 +120,6 @@ object Updater {
             )
             s.commit(pi.intentSender)
         }
-        file.delete()
     }
 }
 

android/app/src/main/java/net/odysseylabs/powerdeck/ui/HomeScreen.kt

@@ -104,7 +104,12 @@ fun HomeScreen(state: UiState, a: HomeActions, snackbar: SnackbarHostState, show
                 }
             }
             state.update?.let { r ->
-                item { Banner(Icons.Rounded.SystemUpdate, "Update available", "PowerDeck ${r.version} is ready to install.", "Update", a.update) }
+                val downloading = state.appCheck == AppCheck.Downloading
+                item {
+                    Banner(Icons.Rounded.SystemUpdate, "Update available",
+                        if (downloading) "Downloading PowerDeck ${r.version}…" else "PowerDeck ${r.version} is ready to install.",
+                        if (downloading) null else "Update", a.update)
+                }
             }
             items(state.devices, key = { it.id }) { d ->
                 DeviceCard(d, state.states[d.id], d.id in state.busy, state.tailnet, state.settings.confirmShutdown, now, a)

android/app/src/main/java/net/odysseylabs/powerdeck/ui/MainViewModel.kt

@@ -46,7 +46,7 @@ data class UiState(
     val settings: Settings = Settings(),
     val tiles: List<String?> = emptyList(),
     val update: Release? = null,
-    /** The phone app's update check from Settings: null before any, then checking, then done. */
+    /** The phone app's update: null before any check, then checking, done, or downloading. */
     val appCheck: AppCheck? = null,
     val add: AddStep = AddStep.Idle,
     val found: List<Peer>? = null,
@@ -64,7 +64,7 @@ sealed interface AddStep {
     data class Failed(val message: String) : AddStep
 }
 
-enum class AppCheck { Checking, UpToDate, Failed }
+enum class AppCheck { Checking, UpToDate, Failed, Downloading }
 
 /** Someone is using the device: ask before forcing. */
 data class ForcePrompt(val device: Device, val restart: Boolean, val ask: Ask)
@@ -548,7 +548,7 @@ class MainViewModel(app: Application) : AndroidViewModel(app) {
     }
 
     fun checkUpdate() {
-        if (_state.value.appCheck == AppCheck.Checking) return
+        if (_state.value.appCheck == AppCheck.Checking || _state.value.appCheck == AppCheck.Downloading) return
         _state.update { it.copy(appCheck = AppCheck.Checking) }
         viewModelScope.launch {
             val r = runCatching { Updater.available() }.getOrElse {
@@ -563,15 +563,17 @@ class MainViewModel(app: Application) : AndroidViewModel(app) {
 
     fun installUpdate() {
         val r = _state.value.update ?: return
+        if (_state.value.appCheck == AppCheck.Downloading) return // one download at a time
         val ctx = getApplication<Application>()
         if (!Updater.canInstall(ctx)) {
             ctx.startActivity(Updater.allowInstallsIntent(ctx))
             say("Allow PowerDeck to install updates, then tap Update again.")
             return
         }
+        _state.update { it.copy(appCheck = AppCheck.Downloading) }
         viewModelScope.launch {
-            _messages.send("Downloading PowerDeck ${r.version}…")
             runCatching { Updater.install(ctx, r) }.onFailure { _messages.send(it.message ?: "Update failed.") }
+            _state.update { it.copy(appCheck = null) }
         }
     }
 }

android/app/src/main/java/net/odysseylabs/powerdeck/ui/SettingsScreen.kt

@@ -168,6 +168,7 @@ fun SettingsScreen(
             SettingRow(
                 "PowerDeck ${BuildConfig.VERSION_NAME} on this phone",
                 when {
+                    update != null && appCheck == AppCheck.Downloading -> "Downloading PowerDeck ${update.version}…"
                     update != null -> "PowerDeck ${update.version} is ready. Tap to install"
                     appCheck == AppCheck.Checking -> "Checking for an update…"
                     appCheck == AppCheck.UpToDate -> "Up to date. Tap to check again"
@@ -176,7 +177,7 @@ fun SettingsScreen(
                 },
                 onClick = if (update != null) a.installUpdate else a.checkUpdate,
                 trailing = when {
-                    appCheck == AppCheck.Checking -> { { CircularProgressIndicator(Modifier.size(20.dp), strokeWidth = 2.dp) } }
+                    appCheck == AppCheck.Checking || appCheck == AppCheck.Downloading -> { { CircularProgressIndicator(Modifier.size(20.dp), strokeWidth = 2.dp) } }
                     update != null -> { { FilledTonalButton(onClick = a.installUpdate) { Text("Update") } } }
                     else -> null
                 },

android/app/src/test/java/net/odysseylabs/powerdeck/NetworkTest.kt

@@ -1,26 +1,32 @@
 package net.odysseylabs.powerdeck
 
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.async
+import kotlinx.coroutines.awaitAll
 import kotlinx.coroutines.runBlocking
 import mockwebserver3.MockResponse
 import mockwebserver3.MockWebServer
-import net.odysseylabs.powerdeck.data.KvmAddress
-import net.odysseylabs.powerdeck.data.KvmException
-import net.odysseylabs.powerdeck.data.NanoKvmClient
-import net.odysseylabs.powerdeck.data.NanoKvmCrypto
 import net.odysseylabs.powerdeck.data.AddLink
 import net.odysseylabs.powerdeck.data.HelperClient
 import net.odysseylabs.powerdeck.data.HelperException
+import net.odysseylabs.powerdeck.data.KvmAddress
+import net.odysseylabs.powerdeck.data.KvmException
 import net.odysseylabs.powerdeck.data.Lan
+import net.odysseylabs.powerdeck.data.NanoKvmClient
+import net.odysseylabs.powerdeck.data.NanoKvmCrypto
 import net.odysseylabs.powerdeck.data.Presence
+import net.odysseylabs.powerdeck.data.Release
 import net.odysseylabs.powerdeck.data.Updater
 import net.odysseylabs.powerdeck.ui.clockNote
 import okhttp3.OkHttpClient
 import okhttp3.tls.HandshakeCertificates
 import okhttp3.tls.HeldCertificate
 import org.json.JSONObject
 import org.junit.After
+import org.junit.Assert.assertArrayEquals
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNotEquals
 import org.junit.Assert.assertNotNull
 import org.junit.Assert.assertNull
 import org.junit.Assert.assertThrows
@@ -30,6 +36,8 @@ import java.net.URLDecoder
 import java.security.MessageDigest
 import java.security.SecureRandom
 import java.util.Base64
+import java.util.concurrent.TimeUnit
+import kotlin.io.path.createTempDirectory
 
 class NanoKvmCryptoTest {
     private class FixedSalt(private val salt: ByteArray) : SecureRandom() {
@@ -335,6 +343,51 @@ class UpdaterTest {
         assertNotNull(Updater.APK)
     }
 
+    private val apk = ByteArray(512 * 1024).also { SecureRandom().nextBytes(it) }
+    private val apkSha = MessageDigest.getInstance("SHA-256").digest(apk).joinToString("") { "%02x".format(it) }
+    private fun apkResponse(body: ByteArray = apk) =
+        MockResponse.Builder().body(okio.Buffer().write(body)).throttleBody(16 * 1024, 5, TimeUnit.MILLISECONDS).build()
+    private fun release(server: MockWebServer) = Release("9.9.9", server.url("/PowerDeck.apk").toString(), apkSha, "")
+    private fun tempDir() = createTempDirectory("updates").toFile()
+
+    @Test fun downloadIsChecked() = MockWebServer().use { server ->
+        server.enqueue(apkResponse())
+        server.start()
+        val dir = tempDir()
+        val f = Updater.download(release(server), dir)
+        assertArrayEquals(apk, f.readBytes())
+        assertEquals(listOf(f), dir.listFiles()!!.toList())
+    }
+
+    @Test fun damagedDownloadIsTriedAgain() = MockWebServer().use { server ->
+        server.enqueue(apkResponse(apk.copyOf().also { it[1000] = (it[1000] + 1).toByte() }))
+        server.enqueue(apkResponse())
+        server.start()
+        val dir = tempDir()
+        assertArrayEquals(apk, Updater.download(release(server), dir).readBytes())
+        assertEquals(2, server.requestCount)
+        assertEquals("the damaged copy is gone", 1, dir.listFiles()!!.size)
+    }
+
+    @Test fun damagedTwiceFails() = MockWebServer().use { server ->
+        repeat(2) { server.enqueue(apkResponse(apk.copyOf(apk.size - 1))) }
+        server.start()
+        val dir = tempDir()
+        val e = assertThrows(IllegalStateException::class.java) { Updater.download(release(server), dir) }
+        assertTrue(e.message!!.contains("checksum"))
+        assertEquals(0, dir.listFiles()!!.size)
+    }
+
+    // Two taps on Update used to download into the same file at once and fail the checksum.
+    @Test fun downloadsAtOnceDontMix() = MockWebServer().use { server ->
+        repeat(2) { server.enqueue(apkResponse()) }
+        server.start()
+        val dir = tempDir()
+        val files = runBlocking { List(2) { async(Dispatchers.IO) { Updater.download(release(server), dir) } }.awaitAll() }
+        assertNotEquals(files[0], files[1])
+        files.forEach { assertArrayEquals(apk, it.readBytes()) }
+    }
+
     @Test
     fun scheduleClockNote() {
         assertEquals(null, clockNote("Server", 60, phoneOffsetMinutes = 60))