Skip to content

fix: isolate invalid patch bundle import failures #3329

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mostafaNazari702 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: isolate invalid patch bundle import failures #3329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 2 additions & 16 deletions app/src/main/java/app/revanced/manager/domain/repository/PatchBundleRepository.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package app.revanced.manager.domain.repository

import android.app.Application
import android.content.Context
import android.util.Log
import app.revanced.library.mostCommonCompatibleVersions
import app.revanced.manager.R
import app.revanced.manager.data.room.AppDatabase
Expand All @@ -20,9 +19,7 @@ import app.revanced.manager.domain.sources.Source
import app.revanced.manager.patcher.patch.PatchInfo
import app.revanced.manager.patcher.patch.PatchBundle
import app.revanced.manager.patcher.patch.PatchBundleInfo
import app.revanced.manager.util.tag
import kotlinx.collections.immutable.*
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.map
Expand Down Expand Up @@ -158,19 +155,8 @@ class PatchBundleRepository(
(src.loaded ?: return@mapNotNull null) to src
}.toMap()

val metadata = try {
runInterruptible(Dispatchers.Default) {
PatchBundle.Loader.metadata(map.keys)
}
} catch (e: CancellationException) {
throw e
} catch (error: Throwable) {
sources.entries.forEach { entry ->
entry.setValue(entry.value.copy(error = error))
}

Log.e(tag, "Failed to load bundles", error)
emptyMap()
val metadata = runInterruptible(Dispatchers.Default) {
PatchBundle.Loader.metadata(map.keys)
}

val output = buildMap {
Expand Down
86 changes: 77 additions & 9 deletions app/src/main/java/app/revanced/manager/patcher/patch/PatchBundle.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ package app.revanced.manager.patcher.patch
import kotlinx.parcelize.IgnoredOnParcel
import android.os.Parcelable
import app.revanced.patcher.patch.Patch
import app.revanced.patcher.patch.loadPatches
import app.revanced.patcher.patch.loadPatches as loadPatcherPatches
import kotlinx.parcelize.Parcelize
import java.io.File
import java.io.IOException
import java.util.jar.JarFile
import java.util.zip.ZipException
import java.util.zip.ZipFile

@Parcelize
data class PatchBundle(val patchesJar: String) : Parcelable {
Expand Down Expand Up @@ -55,18 +57,78 @@ data class PatchBundle(val patchesJar: String) : Parcelable {

object Loader {
private fun patches(bundles: Iterable<PatchBundle>) = buildMap {
val bundleMap = bundles.associateBy { it.patchesJar }
bundles.forEach { bundle ->
this[bundle] = runCatching { bundle.loadPatchSet() }
}
}

private fun PatchBundle.loadPatchSet(): Set<Patch> {
val file = File(patchesJar)
file.requireDexContainer()

@oSumAtrIX oSumAtrIX May 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need explicit checks like this or should we just raise the original exceptions? @Axelen123

loadPatches(
*bundleMap.keys.map(::File).toTypedArray(),
onFailedToLoad = { file, throwable ->
this[bundleMap[file.absolutePath]!!] = Result.failure(throwable)
var loadFailure: Throwable? = null
val patches = loadPatcherPatches(
file,
onFailedToLoad = { _, throwable ->
loadFailure = throwable
}
).patchesByFile.forEach { (file, patches) ->
putIfAbsent(bundleMap[file.absolutePath]!!, Result.success(patches))
).patchesByFile.entries
.firstOrNull { (patchesFile, _) -> patchesFile.absolutePath == file.absolutePath }
?.value
.orEmpty()

loadFailure?.let { throw it }
return patches
}

private fun File.requireDexContainer() {
if (!isFile) throw InvalidPatchBundleException("Patch bundle file is missing")
if (length() == 0L) throw InvalidPatchBundleException("Patch bundle file is empty")

val header = ByteArray(4)
val bytesRead = inputStream().use { it.read(header) }
if (bytesRead < header.size) throw InvalidPatchBundleException("Patch bundle file is too small")

when {
header.isDexMagic() -> return
header.isZipMagic() -> requireZipWithDexFile()
else -> throw InvalidPatchBundleException(
"Patch bundle is not a DEX or ZIP container (magic: ${header.toHexString(bytesRead)})"
)
}
}

private fun File.requireZipWithDexFile() {
try {
ZipFile(this).use { zip ->
val hasDexFile = zip.entries().asSequence().any { entry ->
!entry.isDirectory && entry.name.substringAfterLast('/').matches(DEX_ENTRY_REGEX)
}

if (!hasDexFile) {
throw InvalidPatchBundleException("Patch bundle ZIP does not contain classes.dex")
}
}
} catch (error: ZipException) {
throw InvalidPatchBundleException("Patch bundle ZIP is corrupt", error)
}
}

private fun ByteArray.isDexMagic() =
this[0] == 'd'.code.toByte() &&
this[1] == 'e'.code.toByte() &&
this[2] == 'x'.code.toByte() &&
this[3] == '\n'.code.toByte()

private fun ByteArray.isZipMagic() =
this[0] == 0x50.toByte() &&
this[1] == 0x4b.toByte() &&
this[2] == 0x03.toByte() &&
this[3] == 0x04.toByte()

private fun ByteArray.toHexString(length: Int) =
take(length).joinToString(" ") { "%02x".format(it.toInt() and 0xff) }

fun metadata(bundles: Iterable<PatchBundle>): Map<PatchBundle, Result<Set<PatchInfo>>> =
patches(bundles).mapValues { (_, result) ->
result.map { patches ->
Expand Down Expand Up @@ -95,4 +157,10 @@ data class PatchBundle(val patchesJar: String) : Parcelable {
}
}
}
}

private class InvalidPatchBundleException(message: String, cause: Throwable? = null) : IOException(message, cause)

private companion object {
private val DEX_ENTRY_REGEX = Regex("classes(\\d*)?\\.dex")
}
}
22 changes: 22 additions & 0 deletions app/src/main/java/app/revanced/manager/ui/viewmodel/DashboardViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import android.app.Application
import android.content.ContentResolver
import android.net.Uri
import android.os.Build
import android.provider.OpenableColumns
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.setValue
Expand All @@ -18,6 +19,7 @@ import app.revanced.manager.domain.repository.ManagerUpdateRepository
import app.revanced.manager.domain.repository.PatchBundleRepository
import app.revanced.manager.network.dto.ReVancedAnnouncement
import app.revanced.manager.util.PM
import app.revanced.manager.util.toast
import app.revanced.manager.util.uiSafe
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.combine
Expand Down Expand Up @@ -126,9 +128,29 @@ class DashboardViewModel(

@SuppressLint("Recycle")
fun createLocalSource(patchBundle: Uri) = viewModelScope.launch {
if (!patchBundle.hasPatchBundleExtension()) {
app.toast(app.getString(R.string.patches_invalid_file_extension))
return@launch
}

patchBundleRepository.createLocal { contentResolver.openInputStream(patchBundle)!! }
}

private fun Uri.hasPatchBundleExtension() = displayName()?.endsWith(".rvp", ignoreCase = true) ?: true

private fun Uri.displayName(): String? {
if (scheme == ContentResolver.SCHEME_CONTENT) {
contentResolver.query(this, arrayOf(OpenableColumns.DISPLAY_NAME), null, null, null)?.use { cursor ->
val displayNameIndex = cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME)
if (displayNameIndex >= 0 && cursor.moveToFirst()) return cursor.getString(displayNameIndex)
}

return null
}

return path?.substringAfterLast('/')
}

fun createRemoteSource(apiUrl: String, autoUpdate: Boolean) = viewModelScope.launch {
patchBundleRepository.createRemote(apiUrl, autoUpdate)
}
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ Second \"item\" text"</string>
<string name="patches_error">Error</string>
<string name="patches_error_description">Couldn’t load patches, tap for details</string>
<string name="patches_not_downloaded">Couldn’t download patches</string>
<string name="patches_invalid_file_extension">Only .rvp patch bundles can be imported.</string>
<string name="patches_name_default">Patches</string>
<string name="source_name_fallback">Unnamed</string>

Expand Down