diff --git a/android/src/main/java/com/tailscale/ipn/MainActivity.kt b/android/src/main/java/com/tailscale/ipn/MainActivity.kt index d7723e371b..7b1f6e68aa 100644 --- a/android/src/main/java/com/tailscale/ipn/MainActivity.kt +++ b/android/src/main/java/com/tailscale/ipn/MainActivity.kt @@ -215,7 +215,6 @@ class MainActivity : ComponentActivity() { try { TaildropDirectoryStore.saveFileDirectory(uri) permissionsViewModel.refreshCurrentDir() - ShareFileHelper.notifyDirectoryReady() ShareFileHelper.setUri(uri.toString()) } catch (e: Exception) { TSLog.e("MainActivity", "Failed to set Taildrop root: $e") diff --git a/android/src/main/java/com/tailscale/ipn/util/ShareFileHelper.kt b/android/src/main/java/com/tailscale/ipn/util/ShareFileHelper.kt index 4817a797d2..2ec800539f 100644 --- a/android/src/main/java/com/tailscale/ipn/util/ShareFileHelper.kt +++ b/android/src/main/java/com/tailscale/ipn/util/ShareFileHelper.kt @@ -7,7 +7,6 @@ import android.net.Uri import android.os.ParcelFileDescriptor import android.provider.DocumentsContract import androidx.documentfile.provider.DocumentFile -import com.tailscale.ipn.TaildropDirectoryStore import com.tailscale.ipn.ui.notifier.Notifier import com.tailscale.ipn.ui.notifier.TaildropNotifier import com.tailscale.ipn.ui.util.InputStreamAdapter @@ -35,7 +34,9 @@ object ShareFileHelper : libtailscale.ShareFileHelper { private var appContext: Context? = null private var app: libtailscale.Application? = null - private var savedUri: String? = null + // Both the readiness gate and every file op read this, so a transfer can't pass the gate and then + // resolve against a different root. + @Volatile private var savedUri: String? = null private var scope: CoroutineScope? = null @JvmStatic @@ -58,8 +59,7 @@ object ShareFileHelper : libtailscale.ShareFileHelper { @Volatile private var directoryReady: CompletableDeferred? = null fun hasValidTaildropDir(): Boolean { - val uri = TaildropDirectoryStore.loadSavedDir() - if (uri == null) return false + val uri = savedUri?.let(Uri::parse) ?: return false // Only SAF tree URIs are supported if (uri.scheme != "content") { @@ -88,10 +88,6 @@ object ShareFileHelper : libtailscale.ShareFileHelper { } } - fun notifyDirectoryReady() { - directoryReady?.takeIf { !it.isCompleted }?.complete(Unit) - } - // A helper function that opens or creates a SafStream for a given file. private fun openSafFileOutputStream(fileName: String): Pair { val context = appContext ?: return "" to null @@ -324,8 +320,10 @@ object ShareFileHelper : libtailscale.ShareFileHelper { return InputStreamAdapter(inStream) } + // Publishes the root before waking waiters, so a resumed transfer never sees the old one. fun setUri(uri: String) { savedUri = uri + directoryReady?.takeIf { !it.isCompleted }?.complete(Unit) } private class SeekableOutputStream( diff --git a/android/src/test/kotlin/com/tailscale/ipn/util/ShareFileHelperTest.kt b/android/src/test/kotlin/com/tailscale/ipn/util/ShareFileHelperTest.kt new file mode 100644 index 0000000000..548a5dc8ad --- /dev/null +++ b/android/src/test/kotlin/com/tailscale/ipn/util/ShareFileHelperTest.kt @@ -0,0 +1,112 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package com.tailscale.ipn.util + +import android.content.ContentResolver +import android.content.Context +import android.net.Uri +import android.os.ParcelFileDescriptor +import android.util.Log +import androidx.documentfile.provider.DocumentFile +import java.io.File +import java.io.FileOutputStream +import kotlin.concurrent.thread +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import org.mockito.Mockito.mockStatic +import org.mockito.kotlin.any +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.mock + +class ShareFileHelperTest { + private val internalDir = "/data/user/0/com.tailscale.ipn/files" + private val pickedDir = "content://com.android.externalstorage.documents/tree/primary%3ADownload" + + // A transfer that arrives before a folder is picked blocks until setUri. It must resume against + // the picked folder: setUri has to publish the root before waking it, or the transfer can run + // first and write to the filesDir fallback. + @OptIn(ExperimentalCoroutinesApi::class) + @Test(timeout = 10_000) + fun transferWaitingForDirectoryUsesPickedDirectory() { + val file = File.createTempFile("taildrop", ".partial").apply { deleteOnExit() } + val fileUri = fakeUri("$pickedDir/document/a.partial") + val fd = FileOutputStream(file).fd + val pfd = mock { on { fileDescriptor } doReturn fd } + val resolver = mock { on { openFileDescriptor(fileUri, "rw") } doReturn pfd } + val ctx = mock { on { contentResolver } doReturn resolver } + val partial = mock { on { uri } doReturn fileUri } + val dir = + mock { + on { exists() } doReturn true + on { canWrite() } doReturn true + on { createFile(any(), any()) } doReturn partial + } + + val logWrapper = TSLog.libtailscaleWrapper + TSLog.libtailscaleWrapper = mock() + + // State after App.startLibtailscale on a fresh install: no SAF dir, root is filesDir. + setStatic(ShareFileHelper::class.java, "appContext", ctx) + setStatic(ShareFileHelper::class.java, "savedUri", internalDir) + setStatic(ShareFileHelper::class.java, "scope", CoroutineScope(Dispatchers.Default)) + ShareFileHelper.taildropPrompt.resetReplayCache() + + // Static mocks are thread-local, so the transfer runs here and the picker on another thread. + val logs = mockStatic(Log::class.java) + val uris = mockStatic(Uri::class.java) + val docs = mockStatic(DocumentFile::class.java) + try { + uris.`when` { Uri.parse(any()) }.thenAnswer { fakeUri(it.getArgument(0)) } + // Mirrors DocumentsContract.getTreeDocumentId, which rejects non-tree URIs. + docs + .`when` { DocumentFile.fromTreeUri(any(), any()) } + .thenAnswer { + val u = it.getArgument(1).toString() + if (u == pickedDir) dir else throw IllegalArgumentException("Invalid URI: $u") + } + + var rootAtWake: Any? = null + val picker = thread { + runBlocking { withTimeout(5_000) { ShareFileHelper.observeTaildropPrompt().first() } } + // Completion handlers run synchronously inside complete(), so this sees the root exactly + // when the transfer is released. Waking first loses the race on a busy scheduler. + (getStatic(ShareFileHelper::class.java, "directoryReady") as Job).invokeOnCompletion { + rootAtWake = getStatic(ShareFileHelper::class.java, "savedUri") + } + // What MainActivity's directoryPickerLauncher callback does once the user picks a folder. + ShareFileHelper.setUri(pickedDir) + } + + val result = runCatching { ShareFileHelper.openFileWriter("a.partial", 0).close() } + picker.join() + assertEquals(pickedDir, rootAtWake) + assertTrue("openFileWriter: ${result.exceptionOrNull()}", result.isSuccess) + } finally { + docs.close() + uris.close() + logs.close() + TSLog.libtailscaleWrapper = logWrapper + } + } + + private fun fakeUri(s: String): Uri = mock { + on { scheme } doReturn s.substringBefore("://", "") + on { toString() } doReturn s + } + + private fun getStatic(owner: Class<*>, name: String): Any? = + owner.getDeclaredField(name).apply { isAccessible = true }.get(null) + + private fun setStatic(owner: Class<*>, name: String, value: Any?) { + owner.getDeclaredField(name).apply { isAccessible = true }.set(null, value) + } +}