fix(android): scope SAF read failures to each descriptor

Remove the activity-wide descriptor failure flag and centralize direct-read/fallback handling. A pipe, revoked URI, or malformed file no longer disables direct metadata reads for subsequent files.

Test fallback isolation, descriptor cleanup, and a successful direct read after a failure.
This commit is contained in:
zarzet
2026-09-06 14:04:04 +07:00
parent 4a7908cf4f
commit df8f4f5d7b
5 changed files with 94 additions and 101 deletions
@@ -93,10 +93,6 @@ class MainActivity: FlutterFragmentActivity() {
private val playbackLeases = LinkedHashMap<String, ParcelFileDescriptor>()
@Volatile internal var safScanCancel = false
@Volatile internal var safScanActive = false
/** Tri-state: null = untested, true = works, false = fails (Samsung SELinux). */
@Volatile internal var procSelfFdReadable: Boolean? = null
@Volatile internal var procSelfFdFallbacks: Int = 0
internal val procSelfFdStateLock = Any()
private val safTreeLauncher = registerForActivityResult(
ActivityResultContracts.StartActivityForResult()
) { activityResult ->
@@ -258,100 +258,48 @@ private fun isSeekableSafDescriptor(descriptor: ParcelFileDescriptor): Boolean {
}
}
internal fun MainActivity.readAudioMetadataFromUri(
uri: Uri,
displayNameHint: String? = null,
fallbackExt: String? = null,
coverCacheKey: String = "",
): JSONObject? {
val displayName = buildUriDisplayName(uri, displayNameHint, fallbackExt)
// Skip /proc/self/fd/ attempt when known to fail (e.g. Samsung SELinux).
if (procSelfFdReadable != false) {
try {
contentResolver.openFileDescriptor(uri, "r")?.use { pfd ->
if (!isSeekableSafDescriptor(pfd)) {
synchronized(procSelfFdStateLock) {
procSelfFdReadable = false
procSelfFdFallbacks = 0
}
return@use
}
val directPath = "/proc/self/fd/${pfd.fd}"
val metadataJson = Gobackend.readAudioMetadataWithHintAndCoverCacheKeyJSON(
directPath,
displayName,
coverCacheKey,
)
if (metadataJson.isNotBlank()) {
val obj = JSONObject(metadataJson)
val filenameFallback = obj.optBoolean("metadataFromFilename", false)
if (!obj.has("error") && !filenameFallback) {
synchronized(procSelfFdStateLock) {
procSelfFdReadable = true
procSelfFdFallbacks = 0
}
return obj
}
// One filename fallback should not disable descriptors for all files.
synchronized(procSelfFdStateLock) {
procSelfFdFallbacks++
if (procSelfFdFallbacks >= 3 && procSelfFdReadable == null) {
procSelfFdReadable = false
android.util.Log.d(
"SpotiFLAC",
"Direct /proc/self/fd read not usable for this provider, " +
"using temp-file fallback",
)
}
}
}
}
} catch (e: Exception) {
synchronized(procSelfFdStateLock) {
if (procSelfFdReadable == null) {
procSelfFdReadable = false
android.util.Log.d(
"SpotiFLAC",
"Direct /proc/self/fd read not usable on this device, " +
"using temp-file fallback for remaining files",
)
}
}
/** Read-only metadata uses a seekable descriptor first. Capability belongs to
* this descriptor: a pipe or revoked URI must not disable other providers. */
private fun MainActivity.readMetadataFromUri(
uri: Uri,
displayNameHint: String? = null,
fallbackExt: String? = null,
acceptDirect: (JSONObject) -> Boolean = { true },
read: (String, String) -> JSONObject?,
): JSONObject? {
val displayName = buildUriDisplayName(uri, displayNameHint, fallbackExt)
return readSafMetadataWithFallback(
directRead = {
contentResolver.openFileDescriptor(uri, "r")?.use { descriptor ->
if (!isSeekableSafDescriptor(descriptor)) return@use null
read("/proc/self/fd/${descriptor.fd}", displayName)?.takeIf(acceptDirect)
}
}
},
fallbackRead = {
val tempPath = copyUriToTemp(uri, fallbackExt)
if (tempPath == null) null else try {
read(tempPath, displayName)
} finally {
try { File(tempPath).delete() } catch (_: Exception) {}
}
},
)
}
val tempPath = try {
copyUriToTemp(uri, fallbackExt)
} catch (e: Exception) {
android.util.Log.w(
"SpotiFLAC",
"SAF metadata fallback copy failed for $uri: ${e.message}",
)
null
} ?: return null
try {
val metadataJson = Gobackend.readAudioMetadataWithHintAndCoverCacheKeyJSON(
tempPath,
displayName,
coverCacheKey,
)
if (metadataJson.isBlank()) return null
val obj = JSONObject(metadataJson)
return if (obj.has("error")) null else obj
} catch (e: Exception) {
android.util.Log.w(
"SpotiFLAC",
"SAF metadata temp read failed for $uri: ${e.message}",
)
return null
} finally {
try {
File(tempPath).delete()
} catch (_: Exception) {}
}
}
internal fun MainActivity.readAudioMetadataFromUri(
uri: Uri,
displayNameHint: String? = null,
fallbackExt: String? = null,
coverCacheKey: String = "",
): JSONObject? = readMetadataFromUri(
uri, displayNameHint, fallbackExt,
acceptDirect = { !it.optBoolean("metadataFromFilename", false) },
) { path, name ->
val obj = JSONObject(Gobackend.readAudioMetadataWithHintAndCoverCacheKeyJSON(
path, name, coverCacheKey,
))
obj.takeUnless { it.has("error") }
}
internal fun MainActivity.writeUriFromPath(uri: Uri, srcPath: String): Boolean {
val srcFile = File(srcPath)
@@ -57,11 +57,6 @@ internal fun MainActivity.resetSafScanProgress() {
synchronized(safScanLock) {
safScanProgress = MainActivity.SafScanProgress()
}
// Allow re-probing /proc/self/fd readability on every new scan session.
synchronized(procSelfFdStateLock) {
procSelfFdReadable = null
procSelfFdFallbacks = 0
}
}
internal fun MainActivity.updateSafScanProgress(block: (MainActivity.SafScanProgress) -> Unit) {
@@ -0,0 +1,12 @@
package com.zarz.spotiflac
/** Failure belongs to this read, never to a different document or provider.
* The direct callback owns and closes its descriptor before fallback starts. */
internal fun <T> readSafMetadataWithFallback(
directRead: () -> T?,
fallbackRead: () -> T?,
): T? {
val direct = try { directRead() } catch (_: Exception) { null }
if (direct != null) return direct
return try { fallbackRead() } catch (_: Exception) { null }
}
@@ -0,0 +1,42 @@
package com.zarz.spotiflac
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Assert.assertNull
import org.junit.Test
class SafMetadataReadPolicyTest {
@Test fun fallbackFailureIsIsolatedToOneFile() {
assertNull(readSafMetadataWithFallback<String>({ null }, { throw IllegalArgumentException("malformed metadata") }))
assertEquals("next file", readSafMetadataWithFallback({ "next file" }, { error("copy") }))
}
@Test fun seekableReadAvoidsTempCopy() {
val metadata = mapOf("lyrics" to "words", "replaygain_track_gain" to "-6 dB")
assertEquals(metadata, readSafMetadataWithFallback({ metadata }, { error("Unexpected copy") }))
}
@Test fun pipeAndRevokedUriDoNotDisableNextDescriptor() {
var directReads = 0
for (fails in listOf(true, false)) {
var closed = false
val value = readSafMetadataWithFallback(
directRead = {
directReads++
try {
if (fails) throw SecurityException("revoked URI")
"descriptor metadata"
} finally { closed = true }
},
fallbackRead = {
assertTrue(closed)
"temporary metadata"
},
)
assertEquals(if (fails) "temporary metadata" else "descriptor metadata", value)
}
assertEquals(2, directReads)
assertEquals("pipe fallback", readSafMetadataWithFallback({ null }, { "pipe fallback" }))
assertEquals("next provider", readSafMetadataWithFallback({ "next provider" }, { error("copy") }))
}
}