From 7d87a14c89a0a7c36b32dc691be28a9553da09fb Mon Sep 17 00:00:00 2001 From: Rahul Patel Date: Sat, 30 May 2026 00:49:04 +0530 Subject: [PATCH] Harden download routine: storage checks, single verification, URL expiry - Check free space before downloading (only the not-yet-fetched bytes) and, on Android O+, use StorageManager.getAllocatableBytes/allocateBytes so the system can evict its cache; fail fast with a clear message instead of dying mid-write. SessionInstaller sets a SessionParams size hint so staging can reserve space too. - Verify each APK at most once: files already verified during the download pass are skipped in the final verification gate. Prefer SHA-256 and log SHA-1 fallback. - An expired download URL (403/410) now clears the stored file lists and retries, re-purchasing fresh URLs instead of repeatedly failing on the dead one. - Cancel promptly mid-file rather than only between files, and cancel copyTo's progress timer in a finally to avoid leaking the timer thread. - Download.canInstall now requires a real .apk on disk; DownloadStatus finished/running are Sets. --- .../java/com/aurora/extensions/InputStream.kt | 23 +++-- .../store/data/installer/SessionInstaller.kt | 10 +- .../aurora/store/data/model/DownloadStatus.kt | 4 +- .../store/data/room/download/Download.kt | 5 +- .../aurora/store/data/work/DownloadWorker.kt | 99 ++++++++++++++++++- app/src/main/res/values/strings.xml | 1 + 6 files changed, 124 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/com/aurora/extensions/InputStream.kt b/app/src/main/java/com/aurora/extensions/InputStream.kt index 4dc969a3d..7a508f2c3 100644 --- a/app/src/main/java/com/aurora/extensions/InputStream.kt +++ b/app/src/main/java/com/aurora/extensions/InputStream.kt @@ -12,7 +12,6 @@ import kotlinx.coroutines.flow.flowOn fun InputStream.copyTo(out: OutputStream, streamSize: Long): Flow = flow { var bytesCopied: Long = 0 val buffer = ByteArray(DEFAULT_BUFFER_SIZE) - var bytes = read(buffer) var lastTotalBytesRead: Long = 0 var speed: Long = 0 @@ -23,14 +22,20 @@ fun InputStream.copyTo(out: OutputStream, streamSize: Long): Flow lastTotalBytesRead = totalBytesRead } - while (bytes >= 0) { - out.write(buffer, 0, bytes) - out.flush() + // Cancel the timer even when the collector aborts mid-stream (e.g. the download is + // stopped), otherwise the timer thread would leak. + try { + var bytes = read(buffer) + while (bytes >= 0) { + out.write(buffer, 0, bytes) + out.flush() - bytesCopied += bytes - // Emit stream progress in percentage - emit(DownloadInfo((bytesCopied * 100 / streamSize).toInt(), bytes.toLong(), speed)) - bytes = read(buffer) + bytesCopied += bytes + // Emit stream progress in percentage + emit(DownloadInfo((bytesCopied * 100 / streamSize).toInt(), bytes.toLong(), speed)) + bytes = read(buffer) + } + } finally { + timer.cancel() } - timer.cancel() }.flowOn(Dispatchers.IO) diff --git a/app/src/main/java/com/aurora/store/data/installer/SessionInstaller.kt b/app/src/main/java/com/aurora/store/data/installer/SessionInstaller.kt index 413ca79e0..997fedebe 100644 --- a/app/src/main/java/com/aurora/store/data/installer/SessionInstaller.kt +++ b/app/src/main/java/com/aurora/store/data/installer/SessionInstaller.kt @@ -199,7 +199,12 @@ class SessionInstaller @Inject constructor( ): Int? { val resolvedPackageName = sharedLibPkgName.ifBlank { packageName } - val sessionParams = buildSessionParams(resolvedPackageName) + // Size hint lets the system reserve space (and evict its cache) for the staged copy. + val totalSize = runCatching { + getFiles(packageName, versionCode, sharedLibPkgName).sumOf { it.length() } + }.getOrDefault(0L) + + val sessionParams = buildSessionParams(resolvedPackageName, totalSize) val sessionId = packageInstaller.createSession(sessionParams) val session = packageInstaller.openSession(sessionId) @@ -226,9 +231,10 @@ class SessionInstaller @Inject constructor( } } - private fun buildSessionParams(packageName: String): SessionParams = + private fun buildSessionParams(packageName: String, totalSize: Long = 0L): SessionParams = SessionParams(SessionParams.MODE_FULL_INSTALL).apply { setAppPackageName(packageName) + if (totalSize > 0) setSize(totalSize) setInstallLocation(PackageInfo.INSTALL_LOCATION_AUTO) if (isNAndAbove) { setOriginatingUid(Process.myUid()) diff --git a/app/src/main/java/com/aurora/store/data/model/DownloadStatus.kt b/app/src/main/java/com/aurora/store/data/model/DownloadStatus.kt index e703d221c..f25ea7d52 100644 --- a/app/src/main/java/com/aurora/store/data/model/DownloadStatus.kt +++ b/app/src/main/java/com/aurora/store/data/model/DownloadStatus.kt @@ -16,8 +16,8 @@ enum class DownloadStatus(@StringRes val localized: Int) { INSTALLED(R.string.status_installed); companion object { - val finished = listOf(FAILED, CANCELLED, COMPLETED, INSTALLED) - val running = listOf(QUEUED, PURCHASING, DOWNLOADING) + val finished = setOf(FAILED, CANCELLED, COMPLETED, INSTALLED) + val running = setOf(QUEUED, PURCHASING, DOWNLOADING) /** * States in which a download worker is actively occupying the (single) download diff --git a/app/src/main/java/com/aurora/store/data/room/download/Download.kt b/app/src/main/java/com/aurora/store/data/room/download/Download.kt index 634cb5b6d..079ae8fac 100644 --- a/app/src/main/java/com/aurora/store/data/room/download/Download.kt +++ b/app/src/main/java/com/aurora/store/data/room/download/Download.kt @@ -115,7 +115,10 @@ data class Download( } fun canInstall(context: Context): Boolean { + if (!isSuccessful) return false val dir = PathUtil.getAppDownloadDir(context, packageName, versionCode) - return isSuccessful && dir.listFiles() != null + // Require at least one actual APK on disk, not just that the directory exists — + // an empty/partially-cleaned directory must not look installable. + return dir.listFiles()?.any { it.name.endsWith(".apk") } == true } } diff --git a/app/src/main/java/com/aurora/store/data/work/DownloadWorker.kt b/app/src/main/java/com/aurora/store/data/work/DownloadWorker.kt index 4bc4135a6..9a972665c 100644 --- a/app/src/main/java/com/aurora/store/data/work/DownloadWorker.kt +++ b/app/src/main/java/com/aurora/store/data/work/DownloadWorker.kt @@ -11,6 +11,7 @@ import android.content.Context import android.content.pm.ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC import android.graphics.Bitmap import android.graphics.BitmapFactory +import android.os.storage.StorageManager import android.util.Log import androidx.core.content.getSystemService import androidx.core.graphics.scale @@ -21,6 +22,7 @@ import androidx.work.WorkInfo.Companion.STOP_REASON_USER import androidx.work.WorkerParameters import com.aurora.extensions.TAG import com.aurora.extensions.copyTo +import com.aurora.extensions.isOAndAbove import com.aurora.extensions.isPAndAbove import com.aurora.extensions.isQAndAbove import com.aurora.extensions.isSAndAbove @@ -48,6 +50,9 @@ import dagger.assisted.Assisted import dagger.assisted.AssistedInject import java.io.File import java.io.FileOutputStream +import java.io.IOException +import java.net.HttpURLConnection.HTTP_FORBIDDEN +import java.net.HttpURLConnection.HTTP_GONE import java.net.HttpURLConnection.HTTP_PARTIAL import java.net.SocketException import java.net.SocketTimeoutException @@ -93,6 +98,10 @@ class DownloadWorker @AssistedInject constructor( private var totalProgress = 0 private var downloadedBytes = 0L + // Absolute paths of files already verified during the download pass, so the final + // verification gate doesn't hash large APKs a second time. + private val verifiedFiles = mutableSetOf() + inner class NoNetworkException : Exception(context.getString(R.string.title_no_network)) inner class NothingToDownloadException : Exception(context.getString(R.string.purchase_no_file)) inner class DownloadFailedException : Exception(context.getString(R.string.download_failed)) @@ -102,6 +111,11 @@ class DownloadWorker @AssistedInject constructor( inner class VerificationFailedException : Exception(context.getString(R.string.verification_failed)) + inner class ExpiredUrlException : Exception(context.getString(R.string.download_failed)) + + inner class InsufficientStorageException : + Exception(context.getString(R.string.download_failed_storage)) + override suspend fun doWork(): Result { super.doWork() @@ -178,6 +192,15 @@ class DownloadWorker @AssistedInject constructor( downloadDao.updateFiles(download.packageName, download.fileList) downloadDao.updateSharedLibs(download.packageName, download.sharedLibs) + // Fail fast (and let the system free its cache) if there isn't room for the download, + // instead of dying mid-write with a partial file. Only the not-yet-downloaded bytes + // need to fit. + try { + ensureStorageAvailable(totalBytes - downloadedBytesOnDisk(files)) + } catch (exception: Exception) { + return onFailure(exception) + } + // Download files try { for (file in files) { @@ -205,10 +228,13 @@ class DownloadWorker @AssistedInject constructor( // retried with the partials intact rather than treated as a hard failure. if (isStopped) return onFailure(DownloadCancelledException()) - // Verify downloaded files + // Verify downloaded files (skipping any already verified during the download pass) try { notifyStatus(DownloadStatus.VERIFYING) - files.forEach { file -> require(verifyFile(file)) } + files.forEach { file -> + val path = PathUtil.getLocalFile(context, file, download).absolutePath + if (path !in verifiedFiles) require(verifyFile(file)) + } } catch (exception: Exception) { Log.e(TAG, "Failed to verify ${download.packageName}", exception) // Drop the corrupt files so the next attempt re-downloads them clean instead @@ -259,7 +285,9 @@ class DownloadWorker @AssistedInject constructor( is NoNetworkException, is SocketException, is SocketTimeoutException, - is UnknownHostException -> true + is UnknownHostException, + // Expired URLs were re-purchased by clearing the file list; retrying re-fetches them. + is ExpiredUrlException -> true else -> isRetryable(throwable.cause) } @@ -355,6 +383,7 @@ class DownloadWorker @AssistedInject constructor( if (file.exists() && verifyFile(gFile)) { Log.i(TAG, "$file is already downloaded!") downloadedBytes += file.length() + verifiedFiles.add(file.absolutePath) return@withContext true } @@ -372,7 +401,20 @@ class DownloadWorker @AssistedInject constructor( val response = okHttpClient.call(gFile.url, headers) if (!response.isSuccessful) { + val code = response.code response.close() + // Play download URLs are short-lived; a 403/410 means ours expired while + // the download sat queued. Drop the stale file lists so the retry + // re-purchases fresh URLs instead of hammering the dead one. + if (code == HTTP_FORBIDDEN || code == HTTP_GONE) { + Log.w(TAG, "Download URL for ${download.packageName} expired (code=$code)") + downloadDao.updateFiles(download.packageName, emptyList()) + downloadDao.updateSharedLibs( + download.packageName, + download.sharedLibs.map { it.copy(fileList = emptyList()) } + ) + throw ExpiredUrlException() + } throw DownloadFailedException() } @@ -392,7 +434,12 @@ class DownloadWorker @AssistedInject constructor( response.body.byteStream().use { input -> FileOutputStream(tmpFile, resuming).use { - input.copyTo(it, gFile.size).collect { info -> onProgress(info) } + input.copyTo(it, gFile.size).collect { info -> + // Abort promptly mid-file when stopped, instead of only checking + // between files (a single split can be hundreds of MB). + if (isStopped) throw CancellationException("Download stopped") + onProgress(info) + } } } @@ -526,6 +573,10 @@ class DownloadWorker @AssistedInject constructor( val algorithm = if (gFile.sha256.isBlank()) Algorithm.SHA1 else Algorithm.SHA256 val expectedSha = if (algorithm == Algorithm.SHA1) gFile.sha1 else gFile.sha256 + if (algorithm == Algorithm.SHA1) { + Log.w(TAG, "No SHA-256 for ${gFile.name}, falling back to SHA-1") + } + if (expectedSha.isBlank()) return false return withContext(Dispatchers.IO) { @@ -563,4 +614,44 @@ class DownloadWorker @AssistedInject constructor( Log.i(TAG, "Deleted Temp: $tmpFile") } } + + /** + * Bytes already present on disk (final or partial .tmp) for [files], so the storage check + * only requires room for what's still left to fetch. + */ + private fun downloadedBytesOnDisk(files: List): Long = files.sumOf { gFile -> + val file = PathUtil.getLocalFile(context, gFile, download) + val tmpFile = File(file.absolutePath + ".tmp") + when { + file.exists() -> file.length() + tmpFile.exists() -> tmpFile.length() + else -> 0L + } + } + + /** + * Ensures there's room for [requiredBytes] before downloading, throwing + * [InsufficientStorageException] otherwise. On Android O+ this also asks the system to + * evict its own cache to make space, per the storage guidelines. + */ + private fun ensureStorageAvailable(requiredBytes: Long) { + if (requiredBytes <= 0) return + + val dir = PathUtil.getDownloadDirectory(context).apply { mkdirs() } + if (isOAndAbove) { + val storageManager = context.getSystemService()!! + try { + val uuid = storageManager.getUuidForPath(dir) + if (storageManager.getAllocatableBytes(uuid) < requiredBytes) { + throw InsufficientStorageException() + } + storageManager.allocateBytes(uuid, requiredBytes) + } catch (exception: IOException) { + Log.e(TAG, "Failed to allocate space for ${download.packageName}", exception) + throw InsufficientStorageException() + } + } else if (dir.usableSpace < requiredBytes) { + throw InsufficientStorageException() + } + } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index c95c36e05..7084817f4 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -149,6 +149,7 @@ %1$dm %2$ds left %1$ds left "Download failed" + Not enough storage space to download this app "Force clear all" "Getting metadata" "No downloads"