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.
This commit is contained in:
@@ -12,7 +12,6 @@ import kotlinx.coroutines.flow.flowOn
|
||||
fun InputStream.copyTo(out: OutputStream, streamSize: Long): Flow<DownloadInfo> = 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<DownloadInfo>
|
||||
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)
|
||||
|
||||
@@ -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())
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<String>()
|
||||
|
||||
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<PlayFile>): 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<StorageManager>()!!
|
||||
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()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -149,6 +149,7 @@
|
||||
<string name="download_eta_min">%1$dm %2$ds left</string>
|
||||
<string name="download_eta_sec">%1$ds left</string>
|
||||
<string name="download_failed">"Download failed"</string>
|
||||
<string name="download_failed_storage">Not enough storage space to download this app</string>
|
||||
<string name="download_force_clear_all">"Force clear all"</string>
|
||||
<string name="download_metadata">"Getting metadata"</string>
|
||||
<string name="download_none">"No downloads"</string>
|
||||
|
||||
Reference in New Issue
Block a user