From 77fbdd79fc97581e8fa9f1cb2582c47c6a9d6636 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 17 May 2023 10:42:36 -0300 Subject: [PATCH 01/17] First prototype of new state management --- .../org/onionshare/android/ShareManager.kt | 232 +++++++++++------- .../onionshare/android/files/FileManager.kt | 99 +++----- .../onionshare/android/files/FilesState.kt | 23 +- .../android/server/WebserverManager.kt | 9 +- .../org/onionshare/android/tor/TorManager.kt | 102 +++----- .../org/onionshare/android/tor/TorState.kt | 11 +- .../onionshare/android/ui/MainViewModel.kt | 2 + .../onionshare/android/ui/share/FileList.kt | 31 ++- .../android/ui/share/ShareBottomSheet.kt | 16 +- .../onionshare/android/ui/share/ShareUi.kt | 50 ++-- .../android/ui/share/ShareUiSetup.kt | 14 +- .../android/ui/share/ShareUiState.kt | 24 +- 12 files changed, 290 insertions(+), 323 deletions(-) diff --git a/app/src/main/java/org/onionshare/android/ShareManager.kt b/app/src/main/java/org/onionshare/android/ShareManager.kt index 34233793..d9ffcb25 100644 --- a/app/src/main/java/org/onionshare/android/ShareManager.kt +++ b/app/src/main/java/org/onionshare/android/ShareManager.kt @@ -1,25 +1,23 @@ package org.onionshare.android -import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.Job import kotlinx.coroutines.async +import kotlinx.coroutines.cancel import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.coroutineScope -import kotlinx.coroutines.delay import kotlinx.coroutines.ensureActive import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.combineTransform -import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.flow.stateIn +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.transformWhile import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import org.onionshare.android.files.FileManager -import org.onionshare.android.files.FilesState +import org.onionshare.android.files.ZipResult +import org.onionshare.android.files.ZipState import org.onionshare.android.server.WebServerState import org.onionshare.android.server.WebserverManager import org.onionshare.android.tor.TorManager @@ -44,122 +42,176 @@ class ShareManager @Inject constructor( private var startSharingJob: Job? = null private val shouldStop = MutableStateFlow(false) - @OptIn(DelicateCoroutinesApi::class) - val shareState: StateFlow = combineTransform( - flow = fileManager.state, - flow2 = torManager.state, - flow3 = webserverManager.state, - flow4 = shouldStop, - ) { f, t, w, shouldStop -> - if (LOG.isInfoEnabled) { - val s = if (shouldStop) "stop!" else "" - LOG.info("New state from: f-${f::class.simpleName} t-${t::class.simpleName} w-${w::class.simpleName} $s") - } - // initial state: Adding file and services stopped - if (f is FilesState.Added && t is TorState.Stopped && w is WebServerState.Stopped && !w.downloadComplete) { - if (f.files.isEmpty()) emit(ShareUiState.NoFiles) - else emit(ShareUiState.FilesAdded(f.files)) - } // handle error while adding files while Tor is still starting or started - else if (f is FilesState.Error && (t is TorState.Starting || t is TorState.Started)) { - stopSharing() - } // handle error while adding files when Tor has stopped - else if (f is FilesState.Error && t is TorState.Stopped) { - // TODO notify the user when the app is not displayed - emit(ShareUiState.ErrorAddingFile(f.files, f.errorFile)) - // special case handling for error state without file left - if (f.files.isEmpty()) { - delay(1000) - emit(ShareUiState.NoFiles) - } - } // continue with zipping and report state while doing it - else if (f is FilesState.Zipping && t is TorState.Starting) { - val torPercent = (t as? TorState.Starting)?.progress ?: 0 - emit(ShareUiState.Starting(f.files, f.progress, torPercent)) - } // after zipping is complete, and webserver still stopped, start it - else if (f is FilesState.Zipped && !shouldStop && - (t is TorState.Starting || t is TorState.Started) && w is WebServerState.Stopped - ) { - webserverManager.start(f.sendPage) - val torPercent = (t as? TorState.Starting)?.progress ?: 0 - emit(ShareUiState.Starting(f.files, 100, torPercent)) - } // continue to report Tor progress after files are zipped - else if (f is FilesState.Zipped && t is TorState.Starting) { - emit(ShareUiState.Starting(f.files, 100, t.progress)) - } // everything is done, show sharing state with onion address - else if (f is FilesState.Zipped && t is TorState.Started && w is WebServerState.Started) { - val url = "http://${t.onion}.onion" - emit(ShareUiState.Sharing(f.files, url)) - notificationManager.onSharing() - } // if webserver says download is complete, report that back - else if (w is WebServerState.Stopping && w.downloadComplete) { - this@ShareManager.shouldStop.value = true - } // wait with stopping Tor until download has really completed - else if (w is WebServerState.Stopped && w.downloadComplete) { - stopSharing() - emit(ShareUiState.Complete(f.files)) - } // handle stopping state - else if (t is TorState.Stopping) { - emit(ShareUiState.Stopping(f.files)) - } // handle unexpected stopping/stopped only after zipped, because we start webserver only when that happens - else if (!shouldStop && f is FilesState.Zipped && - (t is TorState.Stopping || t is TorState.Stopped || w is WebServerState.Stopped) - ) { - notificationManager.onError() - val torFailed = (t as? TorState.Stopping)?.failedToConnect == true || - (t as? TorState.Stopped)?.failedToConnect == true - LOG.info("Tor failed: $torFailed") - emit(ShareUiState.Error(f.files, torFailed)) - // state hack to ensure the webserver also stops when tor fails, so we add files again - if (webserverManager.state.value !is WebServerState.Stopped) webserverManager.stop() - } else { - LOG.error("Unhandled state: ↑") - } - }.distinctUntilChanged().onEach { - LOG.debug("New state: ${it::class.simpleName}") - }.stateIn(GlobalScope, SharingStarted.Lazily, ShareUiState.NoFiles) + val filesState = fileManager.filesState + private val _shareState = MutableStateFlow(ShareUiState.AddingFiles) + val shareState: StateFlow = _shareState.asStateFlow() + +// @OptIn(DelicateCoroutinesApi::class) +// val shareState: StateFlow = combineTransform( +// flow = fileManager.state, +// flow2 = torManager.state, +// flow3 = webserverManager.state, +// flow4 = shouldStop, +// ) { f, t, w, shouldStop -> +// if (LOG.isInfoEnabled) { +// val s = if (shouldStop) "stop!" else "" +// LOG.info("New state from: f-${f::class.simpleName} t-${t::class.simpleName} w-${w::class.simpleName} $s") +// } +// // initial state: Adding file and services stopped +// if (f is FilesState.Added && t is TorState.Stopped && w is WebServerState.Stopped && !w.downloadComplete) { +// if (f.files.isEmpty()) emit(ShareUiState.NoFiles) +// else emit(ShareUiState.FilesAdded(f.files)) +// } // handle error while adding files while Tor is still starting or started +// else if (f is FilesState.Error && (t is TorState.Starting || t is TorState.Published)) { +// stopSharing() +// } // handle error while adding files when Tor has stopped +// else if (f is FilesState.Error && t is TorState.Stopped) { +// // TODO notify the user when the app is not displayed +// emit(ShareUiState.ErrorAddingFile(f.files, f.errorFile)) +// // special case handling for error state without file left +// if (f.files.isEmpty()) { +// delay(1000) +// emit(ShareUiState.NoFiles) +// } +// } // continue with zipping and report state while doing it +// else if (f is FilesState.Zipping && t is TorState.Starting) { +// val torPercent = (t as? TorState.Starting)?.progress ?: 0 +// emit(ShareUiState.Starting(f.files, f.progress, torPercent)) +// } // after zipping is complete, and webserver still stopped, start it +// else if (f is FilesState.Zipped && !shouldStop && +// (t is TorState.Starting || t is TorState.Published) && w is WebServerState.Stopped +// ) { +// webserverManager.start(f.sendPage) +// val torPercent = (t as? TorState.Starting)?.progress ?: 0 +// emit(ShareUiState.Starting(f.files, 100, torPercent)) +// } // continue to report Tor progress after files are zipped +// else if (f is FilesState.Zipped && t is TorState.Starting) { +// emit(ShareUiState.Starting(f.files, 100, t.progress)) +// } // everything is done, show sharing state with onion address +// else if (f is FilesState.Zipped && t is TorState.Published && w is WebServerState.Started) { +// val url = "http://${t.onion}.onion" +// emit(ShareUiState.Sharing(f.files, url)) +// notificationManager.onSharing() +// } // if webserver says download is complete, report that back +// else if (w is WebServerState.Stopping && w.downloadComplete) { +// this@ShareManager.shouldStop.value = true +// } // wait with stopping Tor until download has really completed +// else if (w is WebServerState.Stopped && w.downloadComplete) { +// stopSharing() +// emit(ShareUiState.Complete(f.files)) +// } // handle stopping state +// else if (t is TorState.Stopping) { +// emit(ShareUiState.Stopping(f.files)) +// } // handle unexpected stopping/stopped only after zipped, because we start webserver only when that happens +// else if (!shouldStop && f is FilesState.Zipped && (t is TorState.Stopped || w is WebServerState.Stopped) +// ) { +// notificationManager.onError() +// val torFailed = (t as? TorState.Stopping)?.failedToConnect == true || +// (t as? TorState.Stopped)?.failedToConnect == true +// LOG.info("Tor failed: $torFailed") +// emit(ShareUiState.Error(f.files, torFailed)) +// // state hack to ensure the webserver also stops when tor fails, so we add files again +// if (webserverManager.state.value !is WebServerState.Stopped) webserverManager.stop() +// } else { +// LOG.error("Unhandled state: ↑") +// } +// }.distinctUntilChanged().onEach { +// LOG.debug("New state: ${it::class.simpleName}") +// }.stateIn(GlobalScope, SharingStarted.Lazily, ShareUiState.NoFiles) suspend fun onStateChangeRequested() = when (shareState.value) { - is ShareUiState.FilesAdded -> startSharing() + is ShareUiState.AddingFiles -> startSharing() is ShareUiState.Starting -> stopSharing() is ShareUiState.Sharing -> stopSharing() is ShareUiState.Complete -> startSharing() is ShareUiState.ErrorAddingFile -> startSharing() is ShareUiState.Error -> startSharing() is ShareUiState.Stopping -> error("Pressing sheet button while stopping should not be possible") - is ShareUiState.NoFiles -> error("Sheet button should not be visible with no files") } - @Suppress("BlockingMethodInNonBlockingContext") private suspend fun startSharing() { if (startSharingJob?.isActive == true) { // TODO check if this always works as expected startSharingJob?.cancelAndJoin() } // the ErrorAddingFile state is transient and needs manual reset to not persist - if (shareState.value is ShareUiState.ErrorAddingFile) fileManager.resetError() + // TODO test +// if (shareState.value is ShareUiState.ErrorAddingFile) fileManager.resetError() shouldStop.value = false // Attention: We'll launch sharing in Global scope, so it survives ViewModel death, // because this gets called implicitly by the ViewModel in ViewModelScope @Suppress("OPT_IN_USAGE") startSharingJob = GlobalScope.launch(Dispatchers.IO) { - coroutineScope { + coroutineScope mainScope@{ // call ensureActive() before any heavy work to ensure we don't continue when cancelled ensureActive() // When the current scope gets cancelled, the async routine gets cancelled as well val fileTask = async { fileManager.zipFiles() } // start tor and onion service - val torTask = async { torManager.start() } - fileTask.await() + val torTask = async { + try { + torManager.start() + } catch (e: Exception) { + LOG.error("Error starting Tor: ", e) + this@mainScope.cancel() + stopSharing() + } + } + val observerTask = async { + fileManager.zipState.combine(torManager.state) { zipState, torState -> + onStarting(zipState, torState) + }.transformWhile { shareUiState -> + emit(shareUiState) + // only continue collecting while we are starting (otherwise would never stop collecting) + shareUiState is ShareUiState.Starting + }.collect { shareUiState -> + _shareState.value = shareUiState + } + } + val zipResult = fileTask.await() + // wait for tor.start() to return, actual startup happens async torTask.await() + if (zipResult is ZipResult.Zipped) { + val port = webserverManager.start(zipResult.sendPage) + torManager.publishOnionService(port) + observerTask.await() + } else if (zipResult is ZipResult.Error) { + this@mainScope.cancel() + stopSharing() + } + } + } + } + + private fun onStarting(zipState: ZipState?, torState: TorState): ShareUiState { + return when (torState) { + is TorState.Starting -> { + // Tor stays in Starting state as long as the HS descriptor wasn't published. + val torPercent = (torState as? TorState.Starting)?.progress ?: 0 + ShareUiState.Starting(zipState?.progress ?: 0, torPercent) + } + + is TorState.Published -> { + // We only create the hidden service after files have been zipped and webserver was started, + // so we are in sharing state once the first HS descriptor has been published. + ShareUiState.Sharing(torState.onion) + } + + TorState.FailedToConnect -> { + ShareUiState.Error(true) + } + + TorState.Stopped -> { + shareState.value } } } private suspend fun stopSharing() = withContext(Dispatchers.IO) { + _shareState.value = ShareUiState.Stopping shouldStop.value = true LOG.info("Stopping sharing...") if (startSharingJob?.isActive == true) { - // TODO check if this always works as expected startSharingJob?.cancelAndJoin() } startSharingJob = null @@ -168,6 +220,8 @@ class ShareManager @Inject constructor( if (webserverManager.state.value !is WebServerState.Stopped) webserverManager.stop() fileManager.stop() notificationManager.onStopped() + + _shareState.value = ShareUiState.AddingFiles } } diff --git a/app/src/main/java/org/onionshare/android/files/FileManager.kt b/app/src/main/java/org/onionshare/android/files/FileManager.kt index bf596ca3..5d9dfa2d 100644 --- a/app/src/main/java/org/onionshare/android/files/FileManager.kt +++ b/app/src/main/java/org/onionshare/android/files/FileManager.kt @@ -11,18 +11,12 @@ import android.util.Base64.encodeToString import androidx.documentfile.provider.DocumentFile import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.Job -import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.currentCoroutineContext import kotlinx.coroutines.ensureActive import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import org.onionshare.android.R -import org.onionshare.android.files.FilesState.Added -import org.onionshare.android.files.FilesState.Zipped -import org.onionshare.android.files.FilesState.Zipping import org.onionshare.android.server.SendFile import org.onionshare.android.server.SendPage import org.slf4j.LoggerFactory.getLogger @@ -45,11 +39,10 @@ class FileManager @Inject constructor( app: Application, ) { private val ctx = app.applicationContext - private val _state = MutableStateFlow(Added(emptyList())) - val state = _state.asStateFlow() - - @Volatile - private var zipJob: Job? = null + private val _filesStateState = MutableStateFlow(FilesState(emptyList())) + val filesState = _filesStateState.asStateFlow() + private val _zipState = MutableStateFlow(null) + val zipState = _zipState.asStateFlow() suspend fun addFiles(uris: List, takePermission: Boolean) = withContext(Dispatchers.IO) { // taking persistable permissions only works with OPEN_DOCUMENT, not GET_CONTENT @@ -72,8 +65,7 @@ class FileManager @Inject constructor( } private fun addFiles(uris: List) { - checkModificationIsAllowed() - val existingFiles = state.value.files + val existingFiles = filesState.value.files val files = uris.mapNotNull { uri -> // continue if we already have that file if (existingFiles.any { it.uri == uri }) return@mapNotNull null @@ -86,55 +78,47 @@ class FileManager @Inject constructor( } SendFile(name, sizeHuman, size, uri, documentFile.type) } - _state.value = Added(existingFiles + files) + _filesStateState.value = FilesState(existingFiles + files) } fun removeFile(file: SendFile) { - checkModificationIsAllowed() - // release persistable Uri permission again file.releaseUriPermission() - val newList = state.value.files.filterNot { it == file } - _state.value = Added(newList) + val newList = filesState.value.files.filterNot { it == file } + _filesStateState.value = FilesState(newList) } fun removeAll() { - checkModificationIsAllowed() - // release persistable Uri permissions again - state.value.files.iterator().forEach { file -> + filesState.value.files.iterator().forEach { file -> file.releaseUriPermission() } - _state.value = Added(emptyList()) + _filesStateState.value = FilesState(emptyList()) } - suspend fun zipFiles() = withContext(Dispatchers.IO) { - if (state.value is Zipped) return@withContext - zipJob?.cancelAndJoin() - zipJob = launch { - try { - zipFilesInternal() - } catch (e: FileErrorException) { - // remove errorFile from list of files, so user can try again - val newFiles = state.value.files.toMutableList().apply { remove(e.file) } - _state.value = FilesState.Error(newFiles, e.file) - } catch (e: IOException) { - _state.value = FilesState.Error(state.value.files) - } + suspend fun zipFiles(): ZipResult = withContext(Dispatchers.IO) { + try { + val sendPage = zipFilesInternal() + ZipResult.Zipped(sendPage) + } catch (e: FileErrorException) { + // remove errorFile from list of files, so user can try again + val newFiles = filesState.value.files.toMutableList().apply { remove(e.file) } + _filesStateState.value = FilesState(newFiles) + ZipResult.Error(e.file) + } catch (e: Exception) { + ZipResult.Error() } } @Throws(IOException::class, FileErrorException::class) - private suspend fun zipFilesInternal() { - checkModificationIsAllowed() - val files = state.value.files + private suspend fun zipFilesInternal(): SendPage { + val files = filesState.value.files val zipFileName = encodeToString(Random.nextBytes(32), NO_PADDING or URL_SAFE).trimEnd() val zipFile = ctx.getFileStreamPath(zipFileName) currentCoroutineContext().ensureActive() - _state.value = Zipping(files, zipFile, 0) + _zipState.value = ZipState(zipFile, 0) try { - @Suppress("BlockingMethodInNonBlockingContext") ctx.openFileOutput(zipFileName, MODE_PRIVATE).use { fileOutputStream -> ZipOutputStream(fileOutputStream).use { zipStream -> files.forEachIndexed { i, file -> @@ -147,7 +131,7 @@ class FileManager @Inject constructor( inputStream.copyTo(zipStream) } currentCoroutineContext().ensureActive() - _state.value = Zipping(files, zipFile, progress) + _zipState.value = ZipState(zipFile, progress) } catch (e: FileNotFoundException) { LOG.warn("Error while opening file: ", e) throw FileErrorException(file) @@ -159,18 +143,16 @@ class FileManager @Inject constructor( } } } catch (e: CancellationException) { + LOG.info("Got cancelled, deleting zip file...") zipFile.delete() throw e } currentCoroutineContext().ensureActive() - _state.value = Zipping(files, zipFile, 100) + _zipState.value = ZipState(zipFile, 100) zipFile.deleteOnExit() // get SendPage for updating to final state - @Suppress("BlockingMethodInNonBlockingContext") - val sendPage = getSendPage(files, zipFile) - currentCoroutineContext().ensureActive() - _state.value = Zipped(files, sendPage) + return getSendPage(files, zipFile) } @Throws(IOException::class) @@ -186,27 +168,10 @@ class FileManager @Inject constructor( } } - suspend fun stop() { - // cancel running zipJob and wait here until cancelled - zipJob?.cancelAndJoin() - val currentState = state.value - if (currentState is Zipping) currentState.zip.delete() - if (currentState is Zipped) currentState.sendPage.zipFile.delete() - _state.value = if (currentState is FilesState.Error) { - FilesState.Error(currentState.files, currentState.errorFile) - } else { - Added(currentState.files) - } - } - - fun resetError() { - check(state.value is FilesState.Error) { "Unexpected state: ${state.value::class.simpleName}" } - _state.value = Added(state.value.files) - } - - private fun checkModificationIsAllowed() { - // initial, completed and error state allow modification of file list - check(state.value !is Zipping) { "Unexpected state: ${state.value::class.simpleName}" } + fun stop() { + _zipState.value?.zip?.delete() + // TODO + // if (currentState is ZipResult.Zipped) currentState.sendPage.zipFile.delete() } private fun Uri.getFallBackName(): String? { diff --git a/app/src/main/java/org/onionshare/android/files/FilesState.kt b/app/src/main/java/org/onionshare/android/files/FilesState.kt index 028a0dd1..39bf96ef 100644 --- a/app/src/main/java/org/onionshare/android/files/FilesState.kt +++ b/app/src/main/java/org/onionshare/android/files/FilesState.kt @@ -4,24 +4,23 @@ import org.onionshare.android.server.SendFile import org.onionshare.android.server.SendPage import java.io.File -sealed class FilesState { - abstract val files: List +data class FilesState(val files: List) { + val totalSize: Long get() = files.totalSize +} - data class Added(override val files: List) : FilesState() +data class ZipState( + val zip: File, + val progress: Int, +) - data class Zipping( - override val files: List, - val zip: File, - val progress: Int, - ) : FilesState() +sealed class ZipResult { data class Zipped( - override val files: List, val sendPage: SendPage, - ) : FilesState() + ) : ZipResult() data class Error( - override val files: List, val errorFile: SendFile? = null, - ) : FilesState() + ) : ZipResult() + } diff --git a/app/src/main/java/org/onionshare/android/server/WebserverManager.kt b/app/src/main/java/org/onionshare/android/server/WebserverManager.kt index a5fdd4b5..d787c188 100644 --- a/app/src/main/java/org/onionshare/android/server/WebserverManager.kt +++ b/app/src/main/java/org/onionshare/android/server/WebserverManager.kt @@ -38,7 +38,6 @@ import javax.inject.Inject import javax.inject.Singleton private val LOG = LoggerFactory.getLogger(WebserverManager::class.java) -internal const val PORT: Int = 17638 sealed class WebServerState { object Starting : WebServerState() @@ -55,15 +54,15 @@ class WebserverManager @Inject constructor() { private val _state = MutableStateFlow(WebServerState.Stopped(false)) val state = _state.asStateFlow() - fun start(sendPage: SendPage) { + suspend fun start(sendPage: SendPage): Int { _state.value = WebServerState.Starting val staticPath = getStaticPath() val staticPathMap = mapOf("static_url_path" to staticPath) TrafficStats.setThreadStatsTag(0x42) - server = embeddedServer( + val server = embeddedServer( factory = Netty, host = "127.0.0.1", - port = PORT, + port = 0, // will be chosen randomly watchPaths = emptyList(), configure = { // disable response timeout @@ -80,6 +79,8 @@ class WebserverManager @Inject constructor() { sendRoutes(sendPage, staticPathMap) } }.also { it.start() } + this.server = server + return server.resolvedConnectors().first().port } fun stop(isFinishingDownloading: Boolean = false) { diff --git a/app/src/main/java/org/onionshare/android/tor/TorManager.kt b/app/src/main/java/org/onionshare/android/tor/TorManager.kt index 5228390c..e45ebbd5 100644 --- a/app/src/main/java/org/onionshare/android/tor/TorManager.kt +++ b/app/src/main/java/org/onionshare/android/tor/TorManager.kt @@ -4,11 +4,8 @@ import android.app.Application import android.content.Context.MODE_PRIVATE import android.content.Intent import android.os.Build.VERSION.SDK_INT -import androidx.annotation.UiThread import androidx.core.content.ContextCompat.startForegroundService -import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow @@ -27,7 +24,6 @@ import org.briarproject.onionwrapper.TorWrapper.TorState.STARTED import org.briarproject.onionwrapper.TorWrapper.TorState.STARTING import org.briarproject.onionwrapper.TorWrapper.TorState.STOPPED import org.briarproject.onionwrapper.TorWrapper.TorState.STOPPING -import org.onionshare.android.server.PORT import org.onionshare.android.ui.settings.SettingsManager import org.slf4j.LoggerFactory.getLogger import java.io.IOException @@ -51,7 +47,7 @@ class TorManager @Inject constructor( private val locationUtils: LocationUtils, ) : TorWrapper.Observer { - private val _state = MutableStateFlow(TorState.Stopped(false)) + private val _state = MutableStateFlow(TorState.Stopped) internal val state = _state.asStateFlow() private var startCheckJob: Job? = null @@ -64,118 +60,92 @@ class TorManager @Inject constructor( * Starts [ShareService] and creates a new onion service. * Suspends until the address of the onion service is available. */ + @Throws(IOException::class, IllegalArgumentException::class, InterruptedException::class) suspend fun start() = withContext(Dispatchers.IO) { - if (state.value !is TorState.Stopped) stop() - LOG.info("Starting...") val now = System.currentTimeMillis() _state.value = TorState.Starting(progress = 0, startTime = now, lastProgressTime = now) - Intent(app, ShareService::class.java).also { intent -> startForegroundService(app, intent) } tor.start() + changeStartingState(5) + if (settingsManager.automaticBridges.value) { + startCheckJob = launch { + LOG.info("Starting check job") + checkStartupProgress() + LOG.info("Check job finished") + } + } else { + val customBridges = settingsManager.customBridges.value + if (customBridges.isNotEmpty()) { + LOG.info("Using ${customBridges.size} custom bridges...") + tor.enableBridges(customBridges.map { "Bridge $it" }) + } + } + tor.enableNetwork(true) } - fun stop(failedToConnect: Boolean = false) { + fun stop() { LOG.info("Stopping...") startCheckJob?.cancel() startCheckJob = null - _state.value = TorState.Stopping(failedToConnect) tor.stop() Intent(app, ShareService::class.java).also { intent -> app.stopService(intent) } } - @OptIn(DelicateCoroutinesApi::class) override fun onState(s: TorWrapper.TorState) { when (s) { NOT_STARTED -> LOG.info("new state: not started") STARTING -> LOG.info("new state: starting") - STARTED -> GlobalScope.launch { - LOG.info("new state: started") - try { - onTorServiceStarted() - } catch (e: Exception) { - LOG.warn("Error while starting Tor: ", e) - stop() - } - } - + STARTED -> LOG.info("new state: started") CONNECTING -> LOG.info("new state: connecting") CONNECTED -> LOG.info("new state: connected") DISABLED -> LOG.info("new state: network disabled") STOPPING -> LOG.info("new state: stopping") - STOPPED -> onStopped() + STOPPED -> { + LOG.info("new state: stopped") + _state.value = TorState.Stopped + } } } - @UiThread - fun onStopped() { - LOG.info("Stopped") - val failedToConnect = (state.value as? TorState.Stopping)?.failedToConnect == true - _state.value = TorState.Stopped(failedToConnect) + fun publishOnionService(port: Int) { + LOG.info("Starting hidden service...") + tor.publishHiddenService(port, 80, null) } override fun onBootstrapPercentage(percentage: Int) { - changeStartingState((percentage * 0.7).roundToInt()) + changeStartingState(5 + (percentage * 0.9).roundToInt()) } override fun onHsDescriptorUpload(onion: String) { - if (state.value !is TorState.Started) changeStartingState(90, onion) - onDescriptorUploaded(onion) + if (state.value !is TorState.Published) { + _state.value = TorState.Published(onion) + startCheckJob?.cancel() + startCheckJob = null + } } override fun onClockSkewDetected(skewSeconds: Long) { // TODO } - @Throws(IOException::class, IllegalArgumentException::class) - private suspend fun onTorServiceStarted() = withContext(Dispatchers.IO) { - changeStartingState(5) - val autoMode = settingsManager.automaticBridges.value - if (!autoMode) { - val customBridges = settingsManager.customBridges.value - if (customBridges.isNotEmpty()) { - LOG.info("Using ${customBridges.size} custom bridges...") - tor.enableBridges(customBridges.map { "Bridge $it" }) - } - } - LOG.info("Starting hidden service...") - val hsResponse = tor.publishHiddenService(PORT, 80, null) - changeStartingState(10, hsResponse.onion) - if (autoMode) { - startCheckJob = launch { - LOG.info("Starting check job") - checkStartupProgress() - LOG.info("Check job finished") - } - } - tor.enableNetwork(true) - } - - private fun changeStartingState(progress: Int, onion: String? = null) { + private fun changeStartingState(progress: Int) { val oldStartingState = state.value as? TorState.Starting if (oldStartingState == null) LOG.warn("Old state was not Starting, but ${state.value}") val now = System.currentTimeMillis() - val newState = if (onion == null) oldStartingState?.copy(progress = progress, lastProgressTime = now) - else oldStartingState?.copy(progress = progress, onion = onion, lastProgressTime = now) + val newState = oldStartingState?.copy(progress = progress, lastProgressTime = now) _state.value = newState ?: TorState.Starting( progress = progress, startTime = now, lastProgressTime = now, - onion = onion, ) } - private fun onDescriptorUploaded(onion: String) { - _state.value = TorState.Started(onion) - startCheckJob?.cancel() - startCheckJob = null - } - private suspend fun checkStartupProgress() { // first we try to start Tor without bridges if (waitForTorToStart()) return @@ -202,8 +172,8 @@ class TorManager @Inject constructor( } useBridges(builtInBridges) if (waitForTorToStart()) return - LOG.info("Could not connect to Tor, stopping...") - stop(failedToConnect = true) + LOG.info("Could not connect to Tor") + _state.value = TorState.FailedToConnect } /** diff --git a/app/src/main/java/org/onionshare/android/tor/TorState.kt b/app/src/main/java/org/onionshare/android/tor/TorState.kt index 82c889d4..eb70eafc 100644 --- a/app/src/main/java/org/onionshare/android/tor/TorState.kt +++ b/app/src/main/java/org/onionshare/android/tor/TorState.kt @@ -2,23 +2,18 @@ package org.onionshare.android.tor sealed class TorState { - data class Stopped( - val failedToConnect: Boolean, - ) : TorState() + object Stopped : TorState() data class Starting( val progress: Int, val startTime: Long, val lastProgressTime: Long, - val onion: String? = null, ) : TorState() - data class Started( + data class Published( val onion: String, ) : TorState() - data class Stopping( - val failedToConnect: Boolean, - ) : TorState() + object FailedToConnect : TorState() } diff --git a/app/src/main/java/org/onionshare/android/ui/MainViewModel.kt b/app/src/main/java/org/onionshare/android/ui/MainViewModel.kt index 4c9f882c..be47f995 100644 --- a/app/src/main/java/org/onionshare/android/ui/MainViewModel.kt +++ b/app/src/main/java/org/onionshare/android/ui/MainViewModel.kt @@ -10,6 +10,7 @@ import kotlinx.coroutines.launch import org.briarproject.android.dontkillmelib.DozeUtils.needsDozeWhitelisting import org.onionshare.android.ShareManager import org.onionshare.android.files.FileManager +import org.onionshare.android.files.FilesState import org.onionshare.android.server.SendFile import org.onionshare.android.ui.settings.SettingsManager import org.onionshare.android.ui.share.ShareUiState @@ -24,6 +25,7 @@ class MainViewModel @Inject constructor( ) : AndroidViewModel(app) { val shareState: StateFlow = shareManager.shareState + val filesState: StateFlow = shareManager.filesState val needsDozeWhitelisting get() = needsDozeWhitelisting(getApplication()) diff --git a/app/src/main/java/org/onionshare/android/ui/share/FileList.kt b/app/src/main/java/org/onionshare/android/ui/share/FileList.kt index a8b1c822..cb5ceb24 100644 --- a/app/src/main/java/org/onionshare/android/ui/share/FileList.kt +++ b/app/src/main/java/org/onionshare/android/ui/share/FileList.kt @@ -29,7 +29,6 @@ import androidx.compose.material.icons.filled.MusicNote import androidx.compose.material.icons.filled.Slideshow import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider -import androidx.compose.runtime.State import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -43,6 +42,7 @@ import androidx.compose.ui.res.stringResource import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import org.onionshare.android.R +import org.onionshare.android.files.FilesState import org.onionshare.android.server.SendFile import org.onionshare.android.ui.theme.OnionAccent import org.onionshare.android.ui.theme.OnionshareTheme @@ -50,13 +50,14 @@ import org.onionshare.android.ui.theme.OnionshareTheme @Composable fun FileList( modifier: Modifier = Modifier, - state: State, + state: ShareUiState, + filesState: FilesState, onFileRemove: (SendFile) -> Unit, onRemoveAll: () -> Unit, ) { - val files = state.value.files + val files = filesState.files val ctx = LocalContext.current - val totalSize = formatShortFileSize(ctx, state.value.totalSize) + val totalSize = formatShortFileSize(ctx, filesState.totalSize) val res = ctx.resources val text = res.getQuantityString(R.plurals.share_file_list_summary, files.size, files.size, totalSize) @@ -68,10 +69,12 @@ fun FileList( ) { item { Row(verticalAlignment = Alignment.CenterVertically) { - Text(text, modifier = Modifier - .weight(1f) - .padding(16.dp)) - if (state.value.allowsModifyingFiles) { + Text( + text, modifier = Modifier + .weight(1f) + .padding(16.dp) + ) + if (state.allowsModifyingFiles) { TextButton(onClick = onRemoveAll, Modifier.padding(end = 8.dp)) { Text( text = stringResource(R.string.clear_all), @@ -82,7 +85,7 @@ fun FileList( } } items(files) { file -> - FileRow(file, state.value.allowsModifyingFiles, onFileRemove) + FileRow(file, state.allowsModifyingFiles, onFileRemove) } } } @@ -182,10 +185,7 @@ fun FileListPreviewDark() { val files = listOf( SendFile("foo bar file", "1337 KiB", 1, Uri.parse("/foo"), null), ) - val mutableState = remember { - mutableStateOf(ShareUiState.FilesAdded(files)) - } - FileList(Modifier, mutableState, {}) {} + FileList(Modifier, ShareUiState.AddingFiles, FilesState(files), {}) {} } } } @@ -199,9 +199,6 @@ fun FileListPreview() { SendFile("bar", "42 MiB", 2, Uri.parse("/bar"), "video/mp4"), SendFile("foo bar", "23 MiB", 3, Uri.parse("/foo/bar"), null), ) - val mutableState = remember { - mutableStateOf(ShareUiState.FilesAdded(files)) - } - FileList(Modifier, mutableState, {}) {} + FileList(Modifier, ShareUiState.AddingFiles, FilesState(files), {}) {} } } diff --git a/app/src/main/java/org/onionshare/android/ui/share/ShareBottomSheet.kt b/app/src/main/java/org/onionshare/android/ui/share/ShareBottomSheet.kt index d407d938..b3c22539 100644 --- a/app/src/main/java/org/onionshare/android/ui/share/ShareBottomSheet.kt +++ b/app/src/main/java/org/onionshare/android/ui/share/ShareBottomSheet.kt @@ -63,11 +63,12 @@ private data class BottomSheetUi( ) private fun getBottomSheetUi(state: ShareUiState) = when (state) { - is ShareUiState.FilesAdded -> BottomSheetUi( + is ShareUiState.AddingFiles -> BottomSheetUi( indicatorColor = IndicatorReady, stateText = R.string.share_state_ready, buttonText = R.string.share_button_start, ) + is ShareUiState.Starting -> BottomSheetUi( indicatorColor = IndicatorStarting, stateText = R.string.share_state_starting, @@ -99,12 +100,10 @@ private fun getBottomSheetUi(state: ShareUiState) = when (state) { stateText = R.string.share_state_error, buttonText = R.string.share_button_error, ) - is ShareUiState.NoFiles -> error("No bottom sheet in empty state.") } @Composable fun BottomSheet(state: ShareUiState, onSheetButtonClicked: () -> Unit) { - if (state is ShareUiState.NoFiles) return val sheetUi = getBottomSheetUi(state) Column { if (state.collapsableSheet) Image( @@ -226,7 +225,7 @@ fun ShareBottomSheetReadyPreview() { OnionshareTheme { Surface(color = MaterialTheme.colors.background) { BottomSheet( - state = ShareUiState.FilesAdded(emptyList()), + state = ShareUiState.AddingFiles, onSheetButtonClicked = {}, ) } @@ -239,7 +238,7 @@ fun ShareBottomSheetStartingPreview() { OnionshareTheme { Surface(color = MaterialTheme.colors.background) { BottomSheet( - state = ShareUiState.Starting(emptyList(), 25, 50), + state = ShareUiState.Starting(25, 50), onSheetButtonClicked = {}, ) } @@ -253,7 +252,6 @@ fun ShareBottomSheetSharingPreview() { Surface(color = MaterialTheme.colors.background) { BottomSheet( state = ShareUiState.Sharing( - emptyList(), "http://openpravyvc6spbd4flzn4g2iqu4sxzsizbtb5aqec25t76dnoo5w7yd.onion/", ), onSheetButtonClicked = {}, @@ -274,7 +272,7 @@ fun ShareBottomSheetCompletePreview() { OnionshareTheme { Surface(color = MaterialTheme.colors.background) { BottomSheet( - state = ShareUiState.Complete(emptyList()), + state = ShareUiState.Complete, onSheetButtonClicked = {}, ) } @@ -287,7 +285,7 @@ fun ShareBottomSheetStoppingPreview() { OnionshareTheme { Surface(color = MaterialTheme.colors.background) { BottomSheet( - state = ShareUiState.Stopping(emptyList()), + state = ShareUiState.Stopping, onSheetButtonClicked = {}, ) } @@ -300,7 +298,7 @@ fun ShareBottomSheetErrorPreview() { OnionshareTheme { Surface(color = MaterialTheme.colors.background) { BottomSheet( - state = ShareUiState.Error(emptyList(), Random.nextBoolean()), + state = ShareUiState.Error(Random.nextBoolean()), onSheetButtonClicked = {}, ) } diff --git a/app/src/main/java/org/onionshare/android/ui/share/ShareUi.kt b/app/src/main/java/org/onionshare/android/ui/share/ShareUi.kt index 454f9941..1becd62d 100644 --- a/app/src/main/java/org/onionshare/android/ui/share/ShareUi.kt +++ b/app/src/main/java/org/onionshare/android/ui/share/ShareUi.kt @@ -32,7 +32,6 @@ import androidx.compose.material.icons.filled.MoreVert import androidx.compose.material.rememberBottomSheetScaffoldState import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect -import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -55,9 +54,8 @@ import androidx.compose.ui.unit.dp import androidx.navigation.NavHostController import androidx.navigation.compose.rememberNavController import kotlinx.coroutines.delay -import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.StateFlow import org.onionshare.android.R +import org.onionshare.android.files.FilesState import org.onionshare.android.server.SendFile import org.onionshare.android.ui.ROUTE_ABOUT import org.onionshare.android.ui.ROUTE_SETTINGS @@ -71,23 +69,23 @@ private val bottomSheetPeekHeight = 60.dp @OptIn(ExperimentalMaterialApi::class) fun ShareUi( navController: NavHostController, - stateFlow: StateFlow, + shareState: ShareUiState, + filesState: FilesState, onFabClicked: () -> Unit, onFileRemove: (SendFile) -> Unit, onRemoveAll: () -> Unit, onSheetButtonClicked: () -> Unit, ) { - val state = stateFlow.collectAsState() val scaffoldState = rememberBottomSheetScaffoldState() val offset = getOffsetInDp(scaffoldState.bottomSheetState) - if (state.value == ShareUiState.NoFiles) { + if (shareState == ShareUiState.AddingFiles && filesState.files.isEmpty()) { Scaffold( - topBar = { ActionBar(navController, R.string.app_name, state.value.allowsModifyingFiles) }, + topBar = { ActionBar(navController, R.string.app_name, shareState.allowsModifyingFiles) }, floatingActionButton = { Fab(scaffoldState.bottomSheetState, onFabClicked) }, ) { innerPadding -> - MainContent(stateFlow, offset, onFileRemove, onRemoveAll, Modifier.padding(innerPadding)) + MainContent(shareState, filesState, offset, onFileRemove, onRemoveAll, Modifier.padding(innerPadding)) } LaunchedEffect("hideSheet") { // This ensures the FAB color can animate back when we transition to NoFiles state @@ -98,15 +96,15 @@ fun ShareUi( delay(750) scaffoldState.bottomSheetState.expand() } - val uiState = state.value - if (uiState is ShareUiState.ErrorAddingFile) { - val errorFile = uiState.errorFile + if (shareState is ShareUiState.ErrorAddingFile) { + val errorFile = shareState.errorFile val text = if (errorFile != null) { stringResource(R.string.share_error_file_snackbar_text, errorFile.basename) } else { stringResource(R.string.share_error_snackbar_text) } - val action = if (uiState.files.isEmpty()) null else stringResource(R.string.share_error_snackbar_action) + val action = + if (filesState.files.isEmpty()) null else stringResource(R.string.share_error_snackbar_action) LaunchedEffect("showSnackbar") { val snackbarResult = scaffoldState.snackbarHostState.showSnackbar( message = text, @@ -116,25 +114,25 @@ fun ShareUi( if (snackbarResult == SnackbarResult.ActionPerformed) onSheetButtonClicked() } } - if (!uiState.collapsableSheet && scaffoldState.bottomSheetState.isCollapsed) { + if (!shareState.collapsableSheet && scaffoldState.bottomSheetState.isCollapsed) { // ensure the bottom sheet is visible - LaunchedEffect(uiState) { + LaunchedEffect(shareState) { scaffoldState.bottomSheetState.expand() } } BottomSheetScaffold( - topBar = { ActionBar(navController, R.string.app_name, uiState.allowsModifyingFiles) }, - floatingActionButton = if (uiState.allowsModifyingFiles) { + topBar = { ActionBar(navController, R.string.app_name, shareState.allowsModifyingFiles) }, + floatingActionButton = if (shareState.allowsModifyingFiles) { { Fab(scaffoldState.bottomSheetState, onFabClicked) } } else null, - sheetGesturesEnabled = uiState.collapsableSheet, + sheetGesturesEnabled = shareState.collapsableSheet, sheetPeekHeight = bottomSheetPeekHeight, sheetShape = RoundedCornerShape(topStart = 16.dp, topEnd = 16.dp), scaffoldState = scaffoldState, sheetElevation = 16.dp, - sheetContent = { BottomSheet(uiState, onSheetButtonClicked) } + sheetContent = { BottomSheet(shareState, onSheetButtonClicked) } ) { innerPadding -> - MainContent(stateFlow, offset, onFileRemove, onRemoveAll, Modifier.padding(innerPadding)) + MainContent(shareState, filesState, offset, onFileRemove, onRemoveAll, Modifier.padding(innerPadding)) } } } @@ -213,14 +211,14 @@ fun Fab(scaffoldState: BottomSheetState, onFabClicked: () -> Unit) { @Composable fun MainContent( - stateFlow: StateFlow, + shareState: ShareUiState, + filesState: FilesState, offset: Dp, onFileRemove: (SendFile) -> Unit, onRemoveAll: () -> Unit, modifier: Modifier, ) { - val state = stateFlow.collectAsState() - if (state.value is ShareUiState.NoFiles) { + if (shareState is ShareUiState.AddingFiles && filesState.files.isEmpty()) { Box( modifier = modifier .fillMaxWidth() @@ -259,7 +257,7 @@ fun MainContent( } } } else { - FileList(Modifier.padding(bottom = offset), state, onFileRemove, onRemoveAll) + FileList(Modifier.padding(bottom = offset), shareState, filesState, onFileRemove, onRemoveAll) } } @@ -272,7 +270,8 @@ fun DefaultPreview() { OnionshareTheme { ShareUi( navController = rememberNavController(), - stateFlow = MutableStateFlow(ShareUiState.FilesAdded(files)), + shareState = ShareUiState.AddingFiles, + filesState = FilesState(files), onFabClicked = {}, onFileRemove = {}, onRemoveAll = {}, @@ -286,7 +285,8 @@ fun NightModePreview() { OnionshareTheme { ShareUi( navController = rememberNavController(), - stateFlow = MutableStateFlow(ShareUiState.NoFiles), + shareState = ShareUiState.AddingFiles, + filesState = FilesState(emptyList()), onFabClicked = {}, onFileRemove = {}, onRemoveAll = {}, diff --git a/app/src/main/java/org/onionshare/android/ui/share/ShareUiSetup.kt b/app/src/main/java/org/onionshare/android/ui/share/ShareUiSetup.kt index c9b3800b..3ae1729f 100644 --- a/app/src/main/java/org/onionshare/android/ui/share/ShareUiSetup.kt +++ b/app/src/main/java/org/onionshare/android/ui/share/ShareUiSetup.kt @@ -12,6 +12,7 @@ import androidx.activity.result.contract.ActivityResultContracts import androidx.activity.result.contract.ActivityResultContracts.GetMultipleContents import androidx.activity.result.contract.ActivityResultContracts.StartActivityForResult import androidx.compose.runtime.Composable +import androidx.compose.runtime.collectAsState import androidx.compose.ui.platform.LocalContext import androidx.navigation.NavHostController import org.briarproject.android.dontkillmelib.DozeUtils.getDozeWhitelistingIntent @@ -35,14 +36,16 @@ internal fun ShareUiSetup(navController: NavHostController, viewModel: MainViewM // TODO we might want to do user testing here to see if the assumption holds viewModel.onSheetButtonClicked() } + val shareState = viewModel.shareState.collectAsState() + val filesState = viewModel.filesState.collectAsState() ShareUi( navController = navController, - stateFlow = viewModel.shareState, + shareState = shareState.value, + filesState = filesState.value, onFabClicked = { onFabClicked(context, contentLauncher, contentFallbackLauncher) }, onFileRemove = viewModel::removeFile, - onRemoveAll = viewModel::removeAll, - onSheetButtonClicked = { onSheetButtonClicked(context, viewModel, batteryLauncher) } - ) + onRemoveAll = viewModel::removeAll + ) { onSheetButtonClicked(context, viewModel, batteryLauncher) } } private fun onUrisReceived( @@ -80,8 +83,7 @@ private fun onSheetButtonClicked( viewModel: MainViewModel, batteryLauncher: ManagedActivityResultLauncher, ) { - if (viewModel.shareState.value is ShareUiState.FilesAdded && viewModel.needsDozeWhitelisting - ) { + if (viewModel.shareState.value is ShareUiState.AddingFiles && viewModel.needsDozeWhitelisting) { try { batteryLauncher.launch(getDozeWhitelistingIntent(context)) } catch (e: ActivityNotFoundException) { diff --git a/app/src/main/java/org/onionshare/android/ui/share/ShareUiState.kt b/app/src/main/java/org/onionshare/android/ui/share/ShareUiState.kt index 69cf3f44..05b48ac8 100644 --- a/app/src/main/java/org/onionshare/android/ui/share/ShareUiState.kt +++ b/app/src/main/java/org/onionshare/android/ui/share/ShareUiState.kt @@ -1,25 +1,15 @@ package org.onionshare.android.ui.share -import org.onionshare.android.files.totalSize import org.onionshare.android.server.SendFile sealed class ShareUiState { - abstract val files: List open val allowsModifyingFiles = true open val collapsableSheet = false - val totalSize: Long get() = files.totalSize - object NoFiles : ShareUiState() { - override val files = emptyList() - } - - data class FilesAdded( - override val files: List, - ) : ShareUiState() + object AddingFiles : ShareUiState() data class Starting( - override val files: List, private val zipPercent: Int, private val torPercent: Int, ) : ShareUiState() { @@ -37,31 +27,25 @@ sealed class ShareUiState { } data class Sharing( - override val files: List, val onionAddress: String, ) : ShareUiState() { override val allowsModifyingFiles = false override val collapsableSheet = true } - data class Complete( - override val files: List, - ) : ShareUiState() + object Complete : ShareUiState() - data class Stopping( - override val files: List, - ) : ShareUiState() { + object Stopping : ShareUiState() { override val allowsModifyingFiles = false } data class ErrorAddingFile( - override val files: List, val errorFile: SendFile? = null, ) : ShareUiState() data class Error( - override val files: List, val torFailedToConnect: Boolean = false, + val errorMsg: String? = null, ) : ShareUiState() } From 7bbd60500b803735be2cd659f051f57b5e66f847 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Thu, 8 Jun 2023 16:12:00 -0300 Subject: [PATCH 02/17] cleanup state management after review --- .../onionshare/android/files/FileManager.kt | 14 +++++------- .../org/onionshare/android/tor/TorManager.kt | 22 ++----------------- 2 files changed, 8 insertions(+), 28 deletions(-) diff --git a/app/src/main/java/org/onionshare/android/files/FileManager.kt b/app/src/main/java/org/onionshare/android/files/FileManager.kt index 5d9dfa2d..161a8a7a 100644 --- a/app/src/main/java/org/onionshare/android/files/FileManager.kt +++ b/app/src/main/java/org/onionshare/android/files/FileManager.kt @@ -39,8 +39,8 @@ class FileManager @Inject constructor( app: Application, ) { private val ctx = app.applicationContext - private val _filesStateState = MutableStateFlow(FilesState(emptyList())) - val filesState = _filesStateState.asStateFlow() + private val _filesState = MutableStateFlow(FilesState(emptyList())) + val filesState = _filesState.asStateFlow() private val _zipState = MutableStateFlow(null) val zipState = _zipState.asStateFlow() @@ -78,7 +78,7 @@ class FileManager @Inject constructor( } SendFile(name, sizeHuman, size, uri, documentFile.type) } - _filesStateState.value = FilesState(existingFiles + files) + _filesState.value = FilesState(existingFiles + files) } fun removeFile(file: SendFile) { @@ -86,7 +86,7 @@ class FileManager @Inject constructor( file.releaseUriPermission() val newList = filesState.value.files.filterNot { it == file } - _filesStateState.value = FilesState(newList) + _filesState.value = FilesState(newList) } fun removeAll() { @@ -94,7 +94,7 @@ class FileManager @Inject constructor( filesState.value.files.iterator().forEach { file -> file.releaseUriPermission() } - _filesStateState.value = FilesState(emptyList()) + _filesState.value = FilesState(emptyList()) } suspend fun zipFiles(): ZipResult = withContext(Dispatchers.IO) { @@ -104,7 +104,7 @@ class FileManager @Inject constructor( } catch (e: FileErrorException) { // remove errorFile from list of files, so user can try again val newFiles = filesState.value.files.toMutableList().apply { remove(e.file) } - _filesStateState.value = FilesState(newFiles) + _filesState.value = FilesState(newFiles) ZipResult.Error(e.file) } catch (e: Exception) { ZipResult.Error() @@ -170,8 +170,6 @@ class FileManager @Inject constructor( fun stop() { _zipState.value?.zip?.delete() - // TODO - // if (currentState is ZipResult.Zipped) currentState.sendPage.zipFile.delete() } private fun Uri.getFallBackName(): String? { diff --git a/app/src/main/java/org/onionshare/android/tor/TorManager.kt b/app/src/main/java/org/onionshare/android/tor/TorManager.kt index e45ebbd5..98cfd602 100644 --- a/app/src/main/java/org/onionshare/android/tor/TorManager.kt +++ b/app/src/main/java/org/onionshare/android/tor/TorManager.kt @@ -16,14 +16,7 @@ import org.briarproject.moat.MoatApi import org.briarproject.onionwrapper.CircumventionProvider import org.briarproject.onionwrapper.LocationUtils import org.briarproject.onionwrapper.TorWrapper -import org.briarproject.onionwrapper.TorWrapper.TorState.CONNECTED -import org.briarproject.onionwrapper.TorWrapper.TorState.CONNECTING -import org.briarproject.onionwrapper.TorWrapper.TorState.DISABLED -import org.briarproject.onionwrapper.TorWrapper.TorState.NOT_STARTED -import org.briarproject.onionwrapper.TorWrapper.TorState.STARTED -import org.briarproject.onionwrapper.TorWrapper.TorState.STARTING import org.briarproject.onionwrapper.TorWrapper.TorState.STOPPED -import org.briarproject.onionwrapper.TorWrapper.TorState.STOPPING import org.onionshare.android.ui.settings.SettingsManager import org.slf4j.LoggerFactory.getLogger import java.io.IOException @@ -98,19 +91,8 @@ class TorManager @Inject constructor( } override fun onState(s: TorWrapper.TorState) { - when (s) { - NOT_STARTED -> LOG.info("new state: not started") - STARTING -> LOG.info("new state: starting") - STARTED -> LOG.info("new state: started") - CONNECTING -> LOG.info("new state: connecting") - CONNECTED -> LOG.info("new state: connected") - DISABLED -> LOG.info("new state: network disabled") - STOPPING -> LOG.info("new state: stopping") - STOPPED -> { - LOG.info("new state: stopped") - _state.value = TorState.Stopped - } - } + LOG.info("new state: $s") + if (s == STOPPED) _state.value = TorState.Stopped } fun publishOnionService(port: Int) { From 1c7d4844834a8f7441fae31c243f3fe98e6fde5f Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 9 Jun 2023 15:01:37 -0300 Subject: [PATCH 03/17] Show exception message in UI to aid with debugging errors --- .../org/onionshare/android/ui/share/ShareBottomSheet.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/onionshare/android/ui/share/ShareBottomSheet.kt b/app/src/main/java/org/onionshare/android/ui/share/ShareBottomSheet.kt index b3c22539..1cb8f32a 100644 --- a/app/src/main/java/org/onionshare/android/ui/share/ShareBottomSheet.kt +++ b/app/src/main/java/org/onionshare/android/ui/share/ShareBottomSheet.kt @@ -165,7 +165,11 @@ fun BottomSheet(state: ShareUiState, onSheetButtonClicked: () -> Unit) { val textRes = if (state.torFailedToConnect) R.string.share_state_error_tor_text else R.string.share_state_error_text Text( - text = stringResource(textRes), + text = if (state.errorMsg == null) { + stringResource(textRes) + } else { + stringResource(textRes) + "\n\n${state.errorMsg}" + }, modifier = Modifier.padding(16.dp), ) Divider(thickness = 2.dp) From b2f1888a3b688111f9be87318b7b81a912c47267 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 9 Jun 2023 15:07:16 -0300 Subject: [PATCH 04/17] Handle Tor errors while starting --- .../org/onionshare/android/ShareManager.kt | 39 ++++++++++++------- .../org/onionshare/android/tor/TorManager.kt | 19 ++++----- .../org/onionshare/android/tor/TorState.kt | 1 - 3 files changed, 36 insertions(+), 23 deletions(-) diff --git a/app/src/main/java/org/onionshare/android/ShareManager.kt b/app/src/main/java/org/onionshare/android/ShareManager.kt index d9ffcb25..19e0a9ab 100644 --- a/app/src/main/java/org/onionshare/android/ShareManager.kt +++ b/app/src/main/java/org/onionshare/android/ShareManager.kt @@ -40,7 +40,6 @@ class ShareManager @Inject constructor( @Volatile private var startSharingJob: Job? = null - private val shouldStop = MutableStateFlow(false) val filesState = fileManager.filesState private val _shareState = MutableStateFlow(ShareUiState.AddingFiles) @@ -134,15 +133,23 @@ class ShareManager @Inject constructor( // TODO check if this always works as expected startSharingJob?.cancelAndJoin() } + _shareState.value = ShareUiState.Starting(0, 0) // the ErrorAddingFile state is transient and needs manual reset to not persist // TODO test // if (shareState.value is ShareUiState.ErrorAddingFile) fileManager.resetError() - shouldStop.value = false // Attention: We'll launch sharing in Global scope, so it survives ViewModel death, // because this gets called implicitly by the ViewModel in ViewModelScope @Suppress("OPT_IN_USAGE") startSharingJob = GlobalScope.launch(Dispatchers.IO) { coroutineScope mainScope@{ + fun stopOnError(error: ShareUiState.Error) { + _shareState.value = error + // stop in a new scope to not cause deadlock when waiting for startSharingJob to complete + GlobalScope.launch { + stopSharing(error) + } + this@mainScope.cancel() + } // call ensureActive() before any heavy work to ensure we don't continue when cancelled ensureActive() // When the current scope gets cancelled, the async routine gets cancelled as well @@ -153,10 +160,13 @@ class ShareManager @Inject constructor( torManager.start() } catch (e: Exception) { LOG.error("Error starting Tor: ", e) - this@mainScope.cancel() - stopSharing() + stopOnError(ShareUiState.Error(errorMsg = e.toString())) } } + // wait for tor.start() to return before starting to observe, actual startup happens async + torTask.await() + LOG.info("Tor task returned") + // start progress observer task val observerTask = async { fileManager.zipState.combine(torManager.state) { zipState, torState -> onStarting(zipState, torState) @@ -165,19 +175,21 @@ class ShareManager @Inject constructor( // only continue collecting while we are starting (otherwise would never stop collecting) shareUiState is ShareUiState.Starting }.collect { shareUiState -> + LOG.info("New share state: $shareUiState") _shareState.value = shareUiState + if (shareUiState is ShareUiState.Error) stopOnError(shareUiState) } + LOG.info("Observer task finished.") } + ensureActive() val zipResult = fileTask.await() - // wait for tor.start() to return, actual startup happens async - torTask.await() if (zipResult is ZipResult.Zipped) { val port = webserverManager.start(zipResult.sendPage) torManager.publishOnionService(port) observerTask.await() } else if (zipResult is ZipResult.Error) { - this@mainScope.cancel() - stopSharing() + // TODO handle zipResult.errorFile + stopOnError(ShareUiState.Error()) } } } @@ -202,17 +214,18 @@ class ShareManager @Inject constructor( } TorState.Stopped -> { - shareState.value + ShareUiState.Error(errorMsg = "Tor stopped unexpectedly.") } } } - private suspend fun stopSharing() = withContext(Dispatchers.IO) { - _shareState.value = ShareUiState.Stopping - shouldStop.value = true + private suspend fun stopSharing(errorState: ShareUiState.Error? = null) = withContext(Dispatchers.IO) { LOG.info("Stopping sharing...") + _shareState.value = ShareUiState.Stopping if (startSharingJob?.isActive == true) { + LOG.info("Wait for start job to finish...") startSharingJob?.cancelAndJoin() + LOG.info("Start job to finished.") } startSharingJob = null @@ -221,7 +234,7 @@ class ShareManager @Inject constructor( fileManager.stop() notificationManager.onStopped() - _shareState.value = ShareUiState.AddingFiles + _shareState.value = errorState ?: ShareUiState.AddingFiles } } diff --git a/app/src/main/java/org/onionshare/android/tor/TorManager.kt b/app/src/main/java/org/onionshare/android/tor/TorManager.kt index 98cfd602..89641703 100644 --- a/app/src/main/java/org/onionshare/android/tor/TorManager.kt +++ b/app/src/main/java/org/onionshare/android/tor/TorManager.kt @@ -5,7 +5,9 @@ import android.content.Context.MODE_PRIVATE import android.content.Intent import android.os.Build.VERSION.SDK_INT import androidx.core.content.ContextCompat.startForegroundService +import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow @@ -53,11 +55,12 @@ class TorManager @Inject constructor( * Starts [ShareService] and creates a new onion service. * Suspends until the address of the onion service is available. */ + @OptIn(DelicateCoroutinesApi::class) @Throws(IOException::class, IllegalArgumentException::class, InterruptedException::class) suspend fun start() = withContext(Dispatchers.IO) { LOG.info("Starting...") val now = System.currentTimeMillis() - _state.value = TorState.Starting(progress = 0, startTime = now, lastProgressTime = now) + _state.value = TorState.Starting(progress = 0, lastProgressTime = now) Intent(app, ShareService::class.java).also { intent -> startForegroundService(app, intent) } @@ -65,7 +68,10 @@ class TorManager @Inject constructor( tor.start() changeStartingState(5) if (settingsManager.automaticBridges.value) { - startCheckJob = launch { + // we try without bridges first + tor.disableBridges() + // start the check job in global scope, so this method can return without waiting for it + startCheckJob = GlobalScope.launch { LOG.info("Starting check job") checkStartupProgress() LOG.info("Check job finished") @@ -117,14 +123,9 @@ class TorManager @Inject constructor( } private fun changeStartingState(progress: Int) { - val oldStartingState = state.value as? TorState.Starting - if (oldStartingState == null) LOG.warn("Old state was not Starting, but ${state.value}") - val now = System.currentTimeMillis() - val newState = oldStartingState?.copy(progress = progress, lastProgressTime = now) - _state.value = newState ?: TorState.Starting( + _state.value = TorState.Starting( progress = progress, - startTime = now, - lastProgressTime = now, + lastProgressTime = System.currentTimeMillis(), ) } diff --git a/app/src/main/java/org/onionshare/android/tor/TorState.kt b/app/src/main/java/org/onionshare/android/tor/TorState.kt index eb70eafc..1e169499 100644 --- a/app/src/main/java/org/onionshare/android/tor/TorState.kt +++ b/app/src/main/java/org/onionshare/android/tor/TorState.kt @@ -6,7 +6,6 @@ sealed class TorState { data class Starting( val progress: Int, - val startTime: Long, val lastProgressTime: Long, ) : TorState() From 5f9faa233770aacb79dfb18796a43bd9d88fc17c Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 9 Jun 2023 15:34:07 -0300 Subject: [PATCH 05/17] Handle zipping error by removing offending file and let user try again --- .../java/org/onionshare/android/ShareManager.kt | 15 +++++---------- .../android/ui/share/ShareBottomSheet.kt | 7 ++++--- .../onionshare/android/ui/share/ShareUiState.kt | 8 +++++--- 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/org/onionshare/android/ShareManager.kt b/app/src/main/java/org/onionshare/android/ShareManager.kt index 19e0a9ab..1fcd668a 100644 --- a/app/src/main/java/org/onionshare/android/ShareManager.kt +++ b/app/src/main/java/org/onionshare/android/ShareManager.kt @@ -124,19 +124,15 @@ class ShareManager @Inject constructor( is ShareUiState.Sharing -> stopSharing() is ShareUiState.Complete -> startSharing() is ShareUiState.ErrorAddingFile -> startSharing() - is ShareUiState.Error -> startSharing() + is ShareUiState.ErrorStarting -> startSharing() is ShareUiState.Stopping -> error("Pressing sheet button while stopping should not be possible") } private suspend fun startSharing() { if (startSharingJob?.isActive == true) { - // TODO check if this always works as expected startSharingJob?.cancelAndJoin() } _shareState.value = ShareUiState.Starting(0, 0) - // the ErrorAddingFile state is transient and needs manual reset to not persist - // TODO test -// if (shareState.value is ShareUiState.ErrorAddingFile) fileManager.resetError() // Attention: We'll launch sharing in Global scope, so it survives ViewModel death, // because this gets called implicitly by the ViewModel in ViewModelScope @Suppress("OPT_IN_USAGE") @@ -160,7 +156,7 @@ class ShareManager @Inject constructor( torManager.start() } catch (e: Exception) { LOG.error("Error starting Tor: ", e) - stopOnError(ShareUiState.Error(errorMsg = e.toString())) + stopOnError(ShareUiState.ErrorStarting(errorMsg = e.toString())) } } // wait for tor.start() to return before starting to observe, actual startup happens async @@ -188,8 +184,7 @@ class ShareManager @Inject constructor( torManager.publishOnionService(port) observerTask.await() } else if (zipResult is ZipResult.Error) { - // TODO handle zipResult.errorFile - stopOnError(ShareUiState.Error()) + stopOnError(ShareUiState.ErrorAddingFile(zipResult.errorFile)) } } } @@ -210,11 +205,11 @@ class ShareManager @Inject constructor( } TorState.FailedToConnect -> { - ShareUiState.Error(true) + ShareUiState.ErrorStarting(true) } TorState.Stopped -> { - ShareUiState.Error(errorMsg = "Tor stopped unexpectedly.") + ShareUiState.ErrorStarting(errorMsg = "Tor stopped unexpectedly.") } } } diff --git a/app/src/main/java/org/onionshare/android/ui/share/ShareBottomSheet.kt b/app/src/main/java/org/onionshare/android/ui/share/ShareBottomSheet.kt index 1cb8f32a..88144a1d 100644 --- a/app/src/main/java/org/onionshare/android/ui/share/ShareBottomSheet.kt +++ b/app/src/main/java/org/onionshare/android/ui/share/ShareBottomSheet.kt @@ -95,7 +95,8 @@ private fun getBottomSheetUi(state: ShareUiState) = when (state) { stateText = R.string.share_state_ready, buttonText = R.string.share_button_start, ) - is ShareUiState.Error -> BottomSheetUi( + + is ShareUiState.ErrorStarting -> BottomSheetUi( indicatorColor = Error, stateText = R.string.share_state_error, buttonText = R.string.share_button_error, @@ -161,7 +162,7 @@ fun BottomSheet(state: ShareUiState, onSheetButtonClicked: () -> Unit) { ) } Divider(thickness = 2.dp) - } else if (state is ShareUiState.Error) { + } else if (state is ShareUiState.ErrorStarting) { val textRes = if (state.torFailedToConnect) R.string.share_state_error_tor_text else R.string.share_state_error_text Text( @@ -302,7 +303,7 @@ fun ShareBottomSheetErrorPreview() { OnionshareTheme { Surface(color = MaterialTheme.colors.background) { BottomSheet( - state = ShareUiState.Error(Random.nextBoolean()), + state = ShareUiState.ErrorStarting(Random.nextBoolean()), onSheetButtonClicked = {}, ) } diff --git a/app/src/main/java/org/onionshare/android/ui/share/ShareUiState.kt b/app/src/main/java/org/onionshare/android/ui/share/ShareUiState.kt index 05b48ac8..59e5d552 100644 --- a/app/src/main/java/org/onionshare/android/ui/share/ShareUiState.kt +++ b/app/src/main/java/org/onionshare/android/ui/share/ShareUiState.kt @@ -39,13 +39,15 @@ sealed class ShareUiState { override val allowsModifyingFiles = false } + sealed class Error : ShareUiState() + data class ErrorAddingFile( val errorFile: SendFile? = null, - ) : ShareUiState() + ) : Error() - data class Error( + data class ErrorStarting( val torFailedToConnect: Boolean = false, val errorMsg: String? = null, - ) : ShareUiState() + ) : Error() } From 03f46f93591ff2f1dc192785115b88230a8ba13b Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 9 Jun 2023 16:00:14 -0300 Subject: [PATCH 06/17] Fix UI handling of ErrorAddingFile state We weren't properly showing the empty state when the last file got removed. --- .../onionshare/android/ui/share/ShareUi.kt | 51 +++++++++++-------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/org/onionshare/android/ui/share/ShareUi.kt b/app/src/main/java/org/onionshare/android/ui/share/ShareUi.kt index 1becd62d..4c425b62 100644 --- a/app/src/main/java/org/onionshare/android/ui/share/ShareUi.kt +++ b/app/src/main/java/org/onionshare/android/ui/share/ShareUi.kt @@ -23,6 +23,7 @@ import androidx.compose.material.IconButton import androidx.compose.material.MaterialTheme import androidx.compose.material.Scaffold import androidx.compose.material.SnackbarDuration +import androidx.compose.material.SnackbarHostState import androidx.compose.material.SnackbarResult import androidx.compose.material.Text import androidx.compose.material.TopAppBar @@ -30,6 +31,7 @@ import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.Add import androidx.compose.material.icons.filled.MoreVert import androidx.compose.material.rememberBottomSheetScaffoldState +import androidx.compose.material.rememberScaffoldState import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue @@ -65,6 +67,11 @@ import org.onionshare.android.ui.theme.topBar private val bottomSheetPeekHeight = 60.dp +private fun isEmptyState(shareState: ShareUiState, filesState: FilesState): Boolean { + return (shareState == ShareUiState.AddingFiles || shareState is ShareUiState.ErrorAddingFile) && + filesState.files.isEmpty() +} + @Composable @OptIn(ExperimentalMaterialApi::class) fun ShareUi( @@ -78,9 +85,13 @@ fun ShareUi( ) { val scaffoldState = rememberBottomSheetScaffoldState() val offset = getOffsetInDp(scaffoldState.bottomSheetState) - if (shareState == ShareUiState.AddingFiles && filesState.files.isEmpty()) { + val snackbarHostState: SnackbarHostState + if (isEmptyState(shareState, filesState)) { + val normalScaffoldState = rememberScaffoldState() + snackbarHostState = normalScaffoldState.snackbarHostState Scaffold( topBar = { ActionBar(navController, R.string.app_name, shareState.allowsModifyingFiles) }, + scaffoldState = normalScaffoldState, floatingActionButton = { Fab(scaffoldState.bottomSheetState, onFabClicked) }, @@ -92,28 +103,11 @@ fun ShareUi( scaffoldState.bottomSheetState.collapse() } } else { + snackbarHostState = scaffoldState.snackbarHostState LaunchedEffect("showSheet") { delay(750) scaffoldState.bottomSheetState.expand() } - if (shareState is ShareUiState.ErrorAddingFile) { - val errorFile = shareState.errorFile - val text = if (errorFile != null) { - stringResource(R.string.share_error_file_snackbar_text, errorFile.basename) - } else { - stringResource(R.string.share_error_snackbar_text) - } - val action = - if (filesState.files.isEmpty()) null else stringResource(R.string.share_error_snackbar_action) - LaunchedEffect("showSnackbar") { - val snackbarResult = scaffoldState.snackbarHostState.showSnackbar( - message = text, - actionLabel = action, - duration = SnackbarDuration.Long, - ) - if (snackbarResult == SnackbarResult.ActionPerformed) onSheetButtonClicked() - } - } if (!shareState.collapsableSheet && scaffoldState.bottomSheetState.isCollapsed) { // ensure the bottom sheet is visible LaunchedEffect(shareState) { @@ -135,6 +129,23 @@ fun ShareUi( MainContent(shareState, filesState, offset, onFileRemove, onRemoveAll, Modifier.padding(innerPadding)) } } + if (shareState is ShareUiState.ErrorAddingFile) { + val errorFile = shareState.errorFile + val text = if (errorFile != null) { + stringResource(R.string.share_error_file_snackbar_text, errorFile.basename) + } else { + stringResource(R.string.share_error_snackbar_text) + } + val action = if (filesState.files.isEmpty()) null else stringResource(R.string.share_error_snackbar_action) + LaunchedEffect("showSnackbar") { + val snackbarResult = snackbarHostState.showSnackbar( + message = text, + actionLabel = action, + duration = SnackbarDuration.Long, + ) + if (snackbarResult == SnackbarResult.ActionPerformed) onSheetButtonClicked() + } + } } @Composable @@ -218,7 +229,7 @@ fun MainContent( onRemoveAll: () -> Unit, modifier: Modifier, ) { - if (shareState is ShareUiState.AddingFiles && filesState.files.isEmpty()) { + if (isEmptyState(shareState, filesState)) { Box( modifier = modifier .fillMaxWidth() From 1ca147532705d86fa1c3981af1e403009b974918 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 9 Jun 2023 16:05:12 -0300 Subject: [PATCH 07/17] Remove left-over state handling block and add missing NotificationManager calls. --- .../org/onionshare/android/ShareManager.kt | 75 +------------------ 1 file changed, 2 insertions(+), 73 deletions(-) diff --git a/app/src/main/java/org/onionshare/android/ShareManager.kt b/app/src/main/java/org/onionshare/android/ShareManager.kt index 1fcd668a..a3e66bb9 100644 --- a/app/src/main/java/org/onionshare/android/ShareManager.kt +++ b/app/src/main/java/org/onionshare/android/ShareManager.kt @@ -45,79 +45,6 @@ class ShareManager @Inject constructor( private val _shareState = MutableStateFlow(ShareUiState.AddingFiles) val shareState: StateFlow = _shareState.asStateFlow() -// @OptIn(DelicateCoroutinesApi::class) -// val shareState: StateFlow = combineTransform( -// flow = fileManager.state, -// flow2 = torManager.state, -// flow3 = webserverManager.state, -// flow4 = shouldStop, -// ) { f, t, w, shouldStop -> -// if (LOG.isInfoEnabled) { -// val s = if (shouldStop) "stop!" else "" -// LOG.info("New state from: f-${f::class.simpleName} t-${t::class.simpleName} w-${w::class.simpleName} $s") -// } -// // initial state: Adding file and services stopped -// if (f is FilesState.Added && t is TorState.Stopped && w is WebServerState.Stopped && !w.downloadComplete) { -// if (f.files.isEmpty()) emit(ShareUiState.NoFiles) -// else emit(ShareUiState.FilesAdded(f.files)) -// } // handle error while adding files while Tor is still starting or started -// else if (f is FilesState.Error && (t is TorState.Starting || t is TorState.Published)) { -// stopSharing() -// } // handle error while adding files when Tor has stopped -// else if (f is FilesState.Error && t is TorState.Stopped) { -// // TODO notify the user when the app is not displayed -// emit(ShareUiState.ErrorAddingFile(f.files, f.errorFile)) -// // special case handling for error state without file left -// if (f.files.isEmpty()) { -// delay(1000) -// emit(ShareUiState.NoFiles) -// } -// } // continue with zipping and report state while doing it -// else if (f is FilesState.Zipping && t is TorState.Starting) { -// val torPercent = (t as? TorState.Starting)?.progress ?: 0 -// emit(ShareUiState.Starting(f.files, f.progress, torPercent)) -// } // after zipping is complete, and webserver still stopped, start it -// else if (f is FilesState.Zipped && !shouldStop && -// (t is TorState.Starting || t is TorState.Published) && w is WebServerState.Stopped -// ) { -// webserverManager.start(f.sendPage) -// val torPercent = (t as? TorState.Starting)?.progress ?: 0 -// emit(ShareUiState.Starting(f.files, 100, torPercent)) -// } // continue to report Tor progress after files are zipped -// else if (f is FilesState.Zipped && t is TorState.Starting) { -// emit(ShareUiState.Starting(f.files, 100, t.progress)) -// } // everything is done, show sharing state with onion address -// else if (f is FilesState.Zipped && t is TorState.Published && w is WebServerState.Started) { -// val url = "http://${t.onion}.onion" -// emit(ShareUiState.Sharing(f.files, url)) -// notificationManager.onSharing() -// } // if webserver says download is complete, report that back -// else if (w is WebServerState.Stopping && w.downloadComplete) { -// this@ShareManager.shouldStop.value = true -// } // wait with stopping Tor until download has really completed -// else if (w is WebServerState.Stopped && w.downloadComplete) { -// stopSharing() -// emit(ShareUiState.Complete(f.files)) -// } // handle stopping state -// else if (t is TorState.Stopping) { -// emit(ShareUiState.Stopping(f.files)) -// } // handle unexpected stopping/stopped only after zipped, because we start webserver only when that happens -// else if (!shouldStop && f is FilesState.Zipped && (t is TorState.Stopped || w is WebServerState.Stopped) -// ) { -// notificationManager.onError() -// val torFailed = (t as? TorState.Stopping)?.failedToConnect == true || -// (t as? TorState.Stopped)?.failedToConnect == true -// LOG.info("Tor failed: $torFailed") -// emit(ShareUiState.Error(f.files, torFailed)) -// // state hack to ensure the webserver also stops when tor fails, so we add files again -// if (webserverManager.state.value !is WebServerState.Stopped) webserverManager.stop() -// } else { -// LOG.error("Unhandled state: ↑") -// } -// }.distinctUntilChanged().onEach { -// LOG.debug("New state: ${it::class.simpleName}") -// }.stateIn(GlobalScope, SharingStarted.Lazily, ShareUiState.NoFiles) - suspend fun onStateChangeRequested() = when (shareState.value) { is ShareUiState.AddingFiles -> startSharing() is ShareUiState.Starting -> stopSharing() @@ -139,6 +66,7 @@ class ShareManager @Inject constructor( startSharingJob = GlobalScope.launch(Dispatchers.IO) { coroutineScope mainScope@{ fun stopOnError(error: ShareUiState.Error) { + notificationManager.onError() _shareState.value = error // stop in a new scope to not cause deadlock when waiting for startSharingJob to complete GlobalScope.launch { @@ -201,6 +129,7 @@ class ShareManager @Inject constructor( is TorState.Published -> { // We only create the hidden service after files have been zipped and webserver was started, // so we are in sharing state once the first HS descriptor has been published. + notificationManager.onSharing() ShareUiState.Sharing(torState.onion) } From 56cc2da1fd579c1007a0eee1aa6a9ccc13b8c790 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 21 Jun 2023 10:26:24 -0300 Subject: [PATCH 08/17] Consume ZipResult with when statement to benefit from its exhaustive nature. --- .../java/org/onionshare/android/ShareManager.kt | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/onionshare/android/ShareManager.kt b/app/src/main/java/org/onionshare/android/ShareManager.kt index a3e66bb9..aeb2b591 100644 --- a/app/src/main/java/org/onionshare/android/ShareManager.kt +++ b/app/src/main/java/org/onionshare/android/ShareManager.kt @@ -106,13 +106,16 @@ class ShareManager @Inject constructor( LOG.info("Observer task finished.") } ensureActive() - val zipResult = fileTask.await() - if (zipResult is ZipResult.Zipped) { - val port = webserverManager.start(zipResult.sendPage) - torManager.publishOnionService(port) - observerTask.await() - } else if (zipResult is ZipResult.Error) { - stopOnError(ShareUiState.ErrorAddingFile(zipResult.errorFile)) + when (val zipResult = fileTask.await()) { + is ZipResult.Zipped -> { + val port = webserverManager.start(zipResult.sendPage) + torManager.publishOnionService(port) + observerTask.await() + } + + is ZipResult.Error -> { + stopOnError(ShareUiState.ErrorAddingFile(zipResult.errorFile)) + } } } } From 4117d4c69ad1b87ce1185dd2c1b1179e06cb2367 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 21 Jun 2023 10:47:55 -0300 Subject: [PATCH 09/17] Add more logging and don't disable tor bridges on new auto-start as TorWrapper starts with a fresh config each time and doesn't use bridges by default --- app/src/main/java/org/onionshare/android/ShareManager.kt | 2 ++ app/src/main/java/org/onionshare/android/files/FileManager.kt | 1 + app/src/main/java/org/onionshare/android/tor/TorManager.kt | 2 -- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/onionshare/android/ShareManager.kt b/app/src/main/java/org/onionshare/android/ShareManager.kt index aeb2b591..afc5ea17 100644 --- a/app/src/main/java/org/onionshare/android/ShareManager.kt +++ b/app/src/main/java/org/onionshare/android/ShareManager.kt @@ -92,6 +92,7 @@ class ShareManager @Inject constructor( LOG.info("Tor task returned") // start progress observer task val observerTask = async { + LOG.info("Starting Observer task...") fileManager.zipState.combine(torManager.state) { zipState, torState -> onStarting(zipState, torState) }.transformWhile { shareUiState -> @@ -106,6 +107,7 @@ class ShareManager @Inject constructor( LOG.info("Observer task finished.") } ensureActive() + LOG.info("Awaiting file task...") when (val zipResult = fileTask.await()) { is ZipResult.Zipped -> { val port = webserverManager.start(zipResult.sendPage) diff --git a/app/src/main/java/org/onionshare/android/files/FileManager.kt b/app/src/main/java/org/onionshare/android/files/FileManager.kt index 161a8a7a..b04de51a 100644 --- a/app/src/main/java/org/onionshare/android/files/FileManager.kt +++ b/app/src/main/java/org/onionshare/android/files/FileManager.kt @@ -107,6 +107,7 @@ class FileManager @Inject constructor( _filesState.value = FilesState(newFiles) ZipResult.Error(e.file) } catch (e: Exception) { + LOG.error("Error zipping files: ", e) ZipResult.Error() } } diff --git a/app/src/main/java/org/onionshare/android/tor/TorManager.kt b/app/src/main/java/org/onionshare/android/tor/TorManager.kt index 89641703..b4f33070 100644 --- a/app/src/main/java/org/onionshare/android/tor/TorManager.kt +++ b/app/src/main/java/org/onionshare/android/tor/TorManager.kt @@ -68,8 +68,6 @@ class TorManager @Inject constructor( tor.start() changeStartingState(5) if (settingsManager.automaticBridges.value) { - // we try without bridges first - tor.disableBridges() // start the check job in global scope, so this method can return without waiting for it startCheckJob = GlobalScope.launch { LOG.info("Starting check job") From 5175a608b36a4051d3e12bd141a3da5ae79b5a4f Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 26 Jun 2023 11:54:22 -0300 Subject: [PATCH 10/17] Guard against concurrent modifications of TorState --- .../org/onionshare/android/ShareManager.kt | 5 ++- .../org/onionshare/android/tor/TorManager.kt | 32 +++++++++++++++---- .../org/onionshare/android/tor/TorState.kt | 2 ++ 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/onionshare/android/ShareManager.kt b/app/src/main/java/org/onionshare/android/ShareManager.kt index afc5ea17..4f6d0f9d 100644 --- a/app/src/main/java/org/onionshare/android/ShareManager.kt +++ b/app/src/main/java/org/onionshare/android/ShareManager.kt @@ -142,6 +142,8 @@ class ShareManager @Inject constructor( ShareUiState.ErrorStarting(true) } + TorState.Stopping -> error("Still observing TorState after calling stop().") + TorState.Stopped -> { ShareUiState.ErrorStarting(errorMsg = "Tor stopped unexpectedly.") } @@ -158,7 +160,8 @@ class ShareManager @Inject constructor( } startSharingJob = null - if (torManager.state.value !is TorState.Stopped) torManager.stop() + val torState = torManager.state.value + if (torState !is TorState.Stopped && torState !is TorState.Stopping) torManager.stop() if (webserverManager.state.value !is WebServerState.Stopped) webserverManager.stop() fileManager.stop() notificationManager.onStopped() diff --git a/app/src/main/java/org/onionshare/android/tor/TorManager.kt b/app/src/main/java/org/onionshare/android/tor/TorManager.kt index b4f33070..8c46874f 100644 --- a/app/src/main/java/org/onionshare/android/tor/TorManager.kt +++ b/app/src/main/java/org/onionshare/android/tor/TorManager.kt @@ -26,6 +26,7 @@ import java.util.concurrent.TimeUnit.MINUTES import javax.inject.Inject import javax.inject.Singleton import kotlin.math.roundToInt +import kotlin.reflect.KClass private val LOG = getLogger(TorManager::class.java) private val TOR_START_TIMEOUT_SINCE_START = MINUTES.toMillis(5) @@ -42,6 +43,9 @@ class TorManager @Inject constructor( private val locationUtils: LocationUtils, ) : TorWrapper.Observer { + /** + * Attention: Only use [updateTorState] to update this state. + */ private val _state = MutableStateFlow(TorState.Stopped) internal val state = _state.asStateFlow() @@ -51,6 +55,21 @@ class TorManager @Inject constructor( tor.setObserver(this@TorManager) } + /** + * Updates the [_state] with the given new [state] preventing concurrent modifications. + * The state only gets updated when [_state] was in [expectedState]. + * @return true if the state was updated and false if not. + */ + @Synchronized + private fun updateTorState(expectedState: KClass<*>?, newState: TorState, warn: Boolean = true): Boolean { + if (expectedState != null && _state.value::class != expectedState) { + if (warn) LOG.warn("Expected state $expectedState, but was ${state.value}") + return false + } + _state.value = newState + return true + } + /** * Starts [ShareService] and creates a new onion service. * Suspends until the address of the onion service is available. @@ -60,7 +79,7 @@ class TorManager @Inject constructor( suspend fun start() = withContext(Dispatchers.IO) { LOG.info("Starting...") val now = System.currentTimeMillis() - _state.value = TorState.Starting(progress = 0, lastProgressTime = now) + updateTorState(null, TorState.Starting(progress = 0, lastProgressTime = now)) Intent(app, ShareService::class.java).also { intent -> startForegroundService(app, intent) } @@ -86,6 +105,7 @@ class TorManager @Inject constructor( fun stop() { LOG.info("Stopping...") + updateTorState(null, TorState.Stopping) startCheckJob?.cancel() startCheckJob = null tor.stop() @@ -96,7 +116,7 @@ class TorManager @Inject constructor( override fun onState(s: TorWrapper.TorState) { LOG.info("new state: $s") - if (s == STOPPED) _state.value = TorState.Stopped + if (s == STOPPED) updateTorState(null, TorState.Stopped) } fun publishOnionService(port: Int) { @@ -109,8 +129,7 @@ class TorManager @Inject constructor( } override fun onHsDescriptorUpload(onion: String) { - if (state.value !is TorState.Published) { - _state.value = TorState.Published(onion) + if (updateTorState(TorState.Starting::class, TorState.Published(onion), warn = false)) { startCheckJob?.cancel() startCheckJob = null } @@ -121,10 +140,11 @@ class TorManager @Inject constructor( } private fun changeStartingState(progress: Int) { - _state.value = TorState.Starting( + val newState = TorState.Starting( progress = progress, lastProgressTime = System.currentTimeMillis(), ) + updateTorState(TorState.Starting::class, newState) } private suspend fun checkStartupProgress() { @@ -154,7 +174,7 @@ class TorManager @Inject constructor( useBridges(builtInBridges) if (waitForTorToStart()) return LOG.info("Could not connect to Tor") - _state.value = TorState.FailedToConnect + updateTorState(TorState.Starting::class, TorState.FailedToConnect) } /** diff --git a/app/src/main/java/org/onionshare/android/tor/TorState.kt b/app/src/main/java/org/onionshare/android/tor/TorState.kt index 1e169499..56cc4b47 100644 --- a/app/src/main/java/org/onionshare/android/tor/TorState.kt +++ b/app/src/main/java/org/onionshare/android/tor/TorState.kt @@ -15,4 +15,6 @@ sealed class TorState { object FailedToConnect : TorState() + object Stopping : TorState() + } From c8f838599615554e1b237530cf53dbf75b8ef2a3 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 26 Jun 2023 12:03:45 -0300 Subject: [PATCH 11/17] Don't stop if starting Tor was cancelled already --- app/src/main/java/org/onionshare/android/ShareManager.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/onionshare/android/ShareManager.kt b/app/src/main/java/org/onionshare/android/ShareManager.kt index 4f6d0f9d..d46af9d7 100644 --- a/app/src/main/java/org/onionshare/android/ShareManager.kt +++ b/app/src/main/java/org/onionshare/android/ShareManager.kt @@ -1,5 +1,6 @@ package org.onionshare.android +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.Job @@ -84,7 +85,9 @@ class ShareManager @Inject constructor( torManager.start() } catch (e: Exception) { LOG.error("Error starting Tor: ", e) - stopOnError(ShareUiState.ErrorStarting(errorMsg = e.toString())) + if (e !is CancellationException) { + stopOnError(ShareUiState.ErrorStarting(errorMsg = e.toString())) + } } } // wait for tor.start() to return before starting to observe, actual startup happens async From e90a022c9f3b537607331914d92632cdcc0932e1 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 28 Jun 2023 11:34:04 -0300 Subject: [PATCH 12/17] Fix race when stopping Tor --- .../org/onionshare/android/ShareManager.kt | 3 +-- .../org/onionshare/android/tor/TorManager.kt | 22 ++++++++++++------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/org/onionshare/android/ShareManager.kt b/app/src/main/java/org/onionshare/android/ShareManager.kt index d46af9d7..39830383 100644 --- a/app/src/main/java/org/onionshare/android/ShareManager.kt +++ b/app/src/main/java/org/onionshare/android/ShareManager.kt @@ -163,8 +163,7 @@ class ShareManager @Inject constructor( } startSharingJob = null - val torState = torManager.state.value - if (torState !is TorState.Stopped && torState !is TorState.Stopping) torManager.stop() + torManager.stop() if (webserverManager.state.value !is WebServerState.Stopped) webserverManager.stop() fileManager.stop() notificationManager.onStopped() diff --git a/app/src/main/java/org/onionshare/android/tor/TorManager.kt b/app/src/main/java/org/onionshare/android/tor/TorManager.kt index 8c46874f..03d356c9 100644 --- a/app/src/main/java/org/onionshare/android/tor/TorManager.kt +++ b/app/src/main/java/org/onionshare/android/tor/TorManager.kt @@ -58,7 +58,10 @@ class TorManager @Inject constructor( /** * Updates the [_state] with the given new [state] preventing concurrent modifications. * The state only gets updated when [_state] was in [expectedState]. - * @return true if the state was updated and false if not. + * + * Note that the underlying [MutableStateFlow] may reject updates that are equal to the previous state. + * + * @return true if the expected state was either null or matched the previous state. */ @Synchronized private fun updateTorState(expectedState: KClass<*>?, newState: TorState, warn: Boolean = true): Boolean { @@ -104,13 +107,16 @@ class TorManager @Inject constructor( } fun stop() { - LOG.info("Stopping...") - updateTorState(null, TorState.Stopping) - startCheckJob?.cancel() - startCheckJob = null - tor.stop() - Intent(app, ShareService::class.java).also { intent -> - app.stopService(intent) + if (updateTorState(TorState.Stopping::class, TorState.Stopping, warn = false)) { + LOG.info("Was already stopping. Not stopping again.") + } else { + LOG.info("Stopping...") + startCheckJob?.cancel() + startCheckJob = null + tor.stop() + Intent(app, ShareService::class.java).also { intent -> + app.stopService(intent) + } } } From 5c8829e3ab16cd7c440e64d80744628e148aa645 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 31 Jul 2023 16:06:36 +0200 Subject: [PATCH 13/17] Reset progress when waiting for Tor Also write first tests for TorManager --- app/build.gradle | 5 + .../main/java/org/onionshare/android/Clock.kt | 9 ++ .../onionshare/android/tor/MoatApiFactory.kt | 17 ++ .../org/onionshare/android/tor/TorManager.kt | 53 ++++-- .../org/onionshare/android/TorManagerTest.kt | 151 ++++++++++++++++++ 5 files changed, 223 insertions(+), 12 deletions(-) create mode 100644 app/src/main/java/org/onionshare/android/Clock.kt create mode 100644 app/src/main/java/org/onionshare/android/tor/MoatApiFactory.kt create mode 100644 app/src/test/java/org/onionshare/android/TorManagerTest.kt diff --git a/app/build.gradle b/app/build.gradle index 9858763c..37586add 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -142,6 +142,11 @@ dependencies { debugImplementation 'androidx.compose.ui:ui-test-manifest' testImplementation 'junit:junit:4.13.2' + testImplementation 'io.mockk:mockk:1.13.5' + testImplementation 'org.slf4j:slf4j-jdk14:2.0.7' + testImplementation 'app.cash.turbine:turbine:1.0.0' + testImplementation 'org.jetbrains.kotlin:kotlin-test' + testImplementation 'org.jetbrains.kotlinx:kotlinx-coroutines-test:1.7.3' androidTestImplementation 'androidx.test.ext:junit:1.1.5' androidTestImplementation 'androidx.test.espresso:espresso-core:3.5.1' diff --git a/app/src/main/java/org/onionshare/android/Clock.kt b/app/src/main/java/org/onionshare/android/Clock.kt new file mode 100644 index 00000000..bf703e19 --- /dev/null +++ b/app/src/main/java/org/onionshare/android/Clock.kt @@ -0,0 +1,9 @@ +package org.onionshare.android + +fun interface Clock { + fun currentTimeMillis(): Long +} + +object DefaultClock : Clock { + override fun currentTimeMillis(): Long = System.currentTimeMillis() +} diff --git a/app/src/main/java/org/onionshare/android/tor/MoatApiFactory.kt b/app/src/main/java/org/onionshare/android/tor/MoatApiFactory.kt new file mode 100644 index 00000000..4e11d2d2 --- /dev/null +++ b/app/src/main/java/org/onionshare/android/tor/MoatApiFactory.kt @@ -0,0 +1,17 @@ +package org.onionshare.android.tor + +import org.briarproject.moat.MoatApi +import java.io.File + +fun interface MoatApiFactory { + fun createMoatApi(obfs4Executable: File, obfs4Dir: File): MoatApi +} + +object DefaultMoatApiFactory : MoatApiFactory { + private const val MOAT_URL = "https://onion.azureedge.net/" + private const val MOAT_FRONT = "ajax.aspnetcdn.com" + + override fun createMoatApi(obfs4Executable: File, obfs4Dir: File): MoatApi { + return MoatApi(obfs4Executable, obfs4Dir, MOAT_URL, MOAT_FRONT) + } +} diff --git a/app/src/main/java/org/onionshare/android/tor/TorManager.kt b/app/src/main/java/org/onionshare/android/tor/TorManager.kt index 03d356c9..0f51d378 100644 --- a/app/src/main/java/org/onionshare/android/tor/TorManager.kt +++ b/app/src/main/java/org/onionshare/android/tor/TorManager.kt @@ -14,35 +14,56 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.launch import kotlinx.coroutines.withContext -import org.briarproject.moat.MoatApi import org.briarproject.onionwrapper.CircumventionProvider import org.briarproject.onionwrapper.LocationUtils import org.briarproject.onionwrapper.TorWrapper import org.briarproject.onionwrapper.TorWrapper.TorState.STOPPED +import org.onionshare.android.Clock +import org.onionshare.android.DefaultClock import org.onionshare.android.ui.settings.SettingsManager import org.slf4j.LoggerFactory.getLogger import java.io.IOException import java.util.concurrent.TimeUnit.MINUTES import javax.inject.Inject import javax.inject.Singleton +import kotlin.coroutines.CoroutineContext import kotlin.math.roundToInt import kotlin.reflect.KClass private val LOG = getLogger(TorManager::class.java) private val TOR_START_TIMEOUT_SINCE_START = MINUTES.toMillis(5) private val TOR_START_TIMEOUT_SINCE_LAST_PROGRESS = MINUTES.toMillis(2) -private const val MOAT_URL = "https://onion.azureedge.net/" -private const val MOAT_FRONT = "ajax.aspnetcdn.com" @Singleton -class TorManager @Inject constructor( +class TorManager( private val app: Application, private val tor: TorWrapper, private val settingsManager: SettingsManager, private val circumventionProvider: CircumventionProvider, private val locationUtils: LocationUtils, + private val moatApiFactory: MoatApiFactory, + private val clock: Clock, + private val dispatcher: CoroutineContext, ) : TorWrapper.Observer { + @Inject + constructor( + app: Application, + tor: TorWrapper, + settingsManager: SettingsManager, + circumventionProvider: CircumventionProvider, + locationUtils: LocationUtils, + ) : this( + app = app, + tor = tor, + settingsManager = settingsManager, + circumventionProvider = circumventionProvider, + locationUtils = locationUtils, + moatApiFactory = DefaultMoatApiFactory, + clock = DefaultClock, + dispatcher = Dispatchers.IO + ) + /** * Attention: Only use [updateTorState] to update this state. */ @@ -81,7 +102,7 @@ class TorManager @Inject constructor( @Throws(IOException::class, IllegalArgumentException::class, InterruptedException::class) suspend fun start() = withContext(Dispatchers.IO) { LOG.info("Starting...") - val now = System.currentTimeMillis() + val now = clock.currentTimeMillis() updateTorState(null, TorState.Starting(progress = 0, lastProgressTime = now)) Intent(app, ShareService::class.java).also { intent -> startForegroundService(app, intent) @@ -91,7 +112,7 @@ class TorManager @Inject constructor( changeStartingState(5) if (settingsManager.automaticBridges.value) { // start the check job in global scope, so this method can return without waiting for it - startCheckJob = GlobalScope.launch { + startCheckJob = GlobalScope.launch(dispatcher) { LOG.info("Starting check job") checkStartupProgress() LOG.info("Check job finished") @@ -148,7 +169,7 @@ class TorManager @Inject constructor( private fun changeStartingState(progress: Int) { val newState = TorState.Starting( progress = progress, - lastProgressTime = System.currentTimeMillis(), + lastProgressTime = clock.currentTimeMillis(), ) updateTorState(TorState.Starting::class, newState) } @@ -170,6 +191,8 @@ class TorManager @Inject constructor( LOG.info("Using bridges from Moat...") useBridges(bridges) if (waitForTorToStart()) return + } else { + LOG.info("No bridges received from Moat. Continuing...") } // use built-in bridges LOG.info("Using built-in bridges...") @@ -195,11 +218,16 @@ class TorManager @Inject constructor( // Measure TOR_START_TIMEOUT_SINCE_START from the time when this method was called, rather than the time // when Tor was started, otherwise if one connection method times out then all subsequent methods will be // considered to have timed out too - val start = System.currentTimeMillis() + val start = clock.currentTimeMillis() + val oldState = state.value as? TorState.Starting ?: return true + // reset time of last progress to current as well to measure progress since last changing bridge settings + if (!updateTorState(TorState.Starting::class, oldState.copy(lastProgressTime = start))) { + return true + } while (true) { val s = state.value if (s !is TorState.Starting) return true - val now = System.currentTimeMillis() + val now = clock.currentTimeMillis() if (now - start > TOR_START_TIMEOUT_SINCE_START) { LOG.info("Tor is taking too long to start") return false @@ -213,9 +241,10 @@ class TorManager @Inject constructor( } private fun getBridgesFromMoat(): List { - val obfs4Executable = tor.obfs4ExecutableFile - val stateDir = app.getDir("state", MODE_PRIVATE) - val moat = MoatApi(obfs4Executable, stateDir, MOAT_URL, MOAT_FRONT) + val moat = moatApiFactory.createMoatApi( + obfs4Executable = tor.obfs4ExecutableFile, + obfs4Dir = app.getDir("state", MODE_PRIVATE) + ) val bridges = moat.get().let { // if response was empty, try it again with what we think the country should be if (it.isEmpty()) moat.getWithCountry(locationUtils.currentCountry) diff --git a/app/src/test/java/org/onionshare/android/TorManagerTest.kt b/app/src/test/java/org/onionshare/android/TorManagerTest.kt new file mode 100644 index 00000000..7121655e --- /dev/null +++ b/app/src/test/java/org/onionshare/android/TorManagerTest.kt @@ -0,0 +1,151 @@ +package org.onionshare.android + +import android.app.Application +import androidx.compose.runtime.mutableStateOf +import app.cash.turbine.test +import io.mockk.Runs +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.runTest +import org.briarproject.moat.MoatApi +import org.briarproject.onionwrapper.CircumventionProvider +import org.briarproject.onionwrapper.LocationUtils +import org.briarproject.onionwrapper.TorWrapper +import org.junit.Test +import org.onionshare.android.tor.MoatApiFactory +import org.onionshare.android.tor.TorManager +import org.onionshare.android.tor.TorState +import org.onionshare.android.ui.settings.SettingsManager +import java.io.File +import java.util.concurrent.TimeUnit.MINUTES +import kotlin.test.assertEquals +import kotlin.test.assertIs + +class TorManagerTest { + + private val app: Application = mockk() + private val tor: TorWrapper = mockk() + private val settingsManager: SettingsManager = mockk() + private val circumventionProvider: CircumventionProvider = mockk() + private val locationUtils: LocationUtils = mockk() + private val moatApiFactory: MoatApiFactory = mockk() + private val clock: Clock = mockk() + private val dispatcher = StandardTestDispatcher() + + private val torManager: TorManager + + private val obfs4ExecutableFile = File("/usr/bin/echo") + private val stateDir = File("/tmp") + private val moatApi: MoatApi = mockk() + + init { + every { tor.setObserver(any()) } just Runs + + torManager = TorManager( + app = app, + tor = tor, + settingsManager = settingsManager, + circumventionProvider = circumventionProvider, + locationUtils = locationUtils, + moatApiFactory = moatApiFactory, + clock = clock, + dispatcher = dispatcher + ) + } + + @Test + fun testSimpleStart() = runTest { + every { clock.currentTimeMillis() } returns 1L andThen 2L + every { app.startService(any()) } returns null + every { tor.start() } just Runs + every { settingsManager.automaticBridges } returns mutableStateOf(true) + every { tor.enableNetwork(true) } just Runs + + torManager.state.test { + assertIs(awaitItem()) + torManager.start() + assertIs(awaitItem()) // 0% + assertIs(awaitItem()) // 5% + torManager.onState(TorWrapper.TorState.STARTING) + + torManager.onBootstrapPercentage(50) + val starting50 = awaitItem() + assertIs(starting50) // 50% + assertEquals(50, starting50.progress) + + torManager.onHsDescriptorUpload("foobar") + val published = awaitItem() + assertIs(published) + assertEquals("foobar", published.onion) + + // subsequent descriptor uploads are ignored + torManager.onHsDescriptorUpload("foobar") + torManager.onHsDescriptorUpload("foobar") + } + } + + @Test + fun testCircumventionFailure() = runTest { + val startTime = 1L + val startedTime = 2L + val firstWaitTime = startedTime + 1 + val firstTimeJump = firstWaitTime + MINUTES.toMillis(2) + 1 + val secondWaitTime = firstTimeJump + 1 + val secondTimeJump = secondWaitTime + MINUTES.toMillis(2) + 1 + + every { clock.currentTimeMillis() } returns startTime andThen startedTime + every { app.startService(any()) } returns null + every { tor.start() } just Runs + every { settingsManager.automaticBridges } returns mutableStateOf(true) + every { tor.enableNetwork(true) } just Runs + + // moat doesn't return bridges + every { tor.obfs4ExecutableFile } returns obfs4ExecutableFile + every { app.getDir("state", 0) } returns stateDir + every { moatApiFactory.createMoatApi(obfs4ExecutableFile, stateDir) } returns moatApi + every { moatApi.get() } returns emptyList() + every { locationUtils.currentCountry } returns "br" + every { moatApi.getWithCountry("br") } returns emptyList() + + // use built-in bridges (empty here as well) + every { circumventionProvider.getSuitableBridgeTypes("br") } returns emptyList() + every { tor.enableBridges(emptyList()) } just Runs + + torManager.state.test { + assertIs(awaitItem()) + torManager.start() + + val beforeStart = awaitItem() + assertIs(beforeStart) // 0% + assertEquals(0, beforeStart.progress) + assertEquals(startTime, beforeStart.lastProgressTime) + + val afterStart = awaitItem() + assertIs(afterStart) // 5% + assertEquals(5, afterStart.progress) + assertEquals(startedTime, afterStart.lastProgressTime) + + // now a lot of time has passed and we run the startCheckJob + every { clock.currentTimeMillis() } returnsMany listOf( + firstWaitTime, firstTimeJump, + secondWaitTime, secondTimeJump, + ) + dispatcher.scheduler.runCurrent() + + val beforeMoat = awaitItem() + assertIs(beforeMoat) + assertEquals(5, beforeMoat.progress) // progress didn't change + assertEquals(firstWaitTime, beforeMoat.lastProgressTime) + + val afterBuiltIn = awaitItem() + assertIs(afterBuiltIn) + assertEquals(5, afterBuiltIn.progress) // progress didn't change + assertEquals(secondWaitTime, afterBuiltIn.lastProgressTime) + + assertIs(awaitItem()) + } + } + +} From 9ce88152ddc716d3faf4fddc7d7da904b4172440 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 31 Jul 2023 16:17:07 +0200 Subject: [PATCH 14/17] Allow cancelling while adding a file to the ZIP as well --- .../org/onionshare/android/files/FileManager.kt | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/onionshare/android/files/FileManager.kt b/app/src/main/java/org/onionshare/android/files/FileManager.kt index b04de51a..2f23ede9 100644 --- a/app/src/main/java/org/onionshare/android/files/FileManager.kt +++ b/app/src/main/java/org/onionshare/android/files/FileManager.kt @@ -8,6 +8,7 @@ import android.text.format.Formatter.formatShortFileSize import android.util.Base64.NO_PADDING import android.util.Base64.URL_SAFE import android.util.Base64.encodeToString +import androidx.annotation.WorkerThread import androidx.documentfile.provider.DocumentFile import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers @@ -23,6 +24,8 @@ import org.slf4j.LoggerFactory.getLogger import java.io.File import java.io.FileNotFoundException import java.io.IOException +import java.io.InputStream +import java.io.OutputStream import java.util.zip.ZipEntry import java.util.zip.ZipOutputStream import javax.inject.Inject @@ -129,7 +132,7 @@ class FileManager @Inject constructor( try { ctx.contentResolver.openInputStream(file.uri)?.use { inputStream -> zipStream.putNextEntry(ZipEntry(file.basename)) - inputStream.copyTo(zipStream) + inputStream.cancelableCopyTo(zipStream) } currentCoroutineContext().ensureActive() _zipState.value = ZipState(zipFile, progress) @@ -173,6 +176,18 @@ class FileManager @Inject constructor( _zipState.value?.zip?.delete() } + @WorkerThread + @Suppress("BlockingMethodInNonBlockingContext") + private suspend fun InputStream.cancelableCopyTo(out: OutputStream, bufferSize: Int = DEFAULT_BUFFER_SIZE) { + val buffer = ByteArray(bufferSize) + var bytes = read(buffer) + while (bytes >= 0) { + currentCoroutineContext().ensureActive() + out.write(buffer, 0, bytes) + bytes = read(buffer) + } + } + private fun Uri.getFallBackName(): String? { return lastPathSegment?.split("/")?.last() } From b2a251509dbd6c7d55ae003fcd25bd6539ce51ec Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 31 Jul 2023 16:22:48 +0200 Subject: [PATCH 15/17] Fix display/copy of onion address --- app/src/main/java/org/onionshare/android/ShareManager.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/org/onionshare/android/ShareManager.kt b/app/src/main/java/org/onionshare/android/ShareManager.kt index 39830383..8f702bf1 100644 --- a/app/src/main/java/org/onionshare/android/ShareManager.kt +++ b/app/src/main/java/org/onionshare/android/ShareManager.kt @@ -138,7 +138,7 @@ class ShareManager @Inject constructor( // We only create the hidden service after files have been zipped and webserver was started, // so we are in sharing state once the first HS descriptor has been published. notificationManager.onSharing() - ShareUiState.Sharing(torState.onion) + ShareUiState.Sharing("http://${torState.onion}.onion") } TorState.FailedToConnect -> { From 60d770f0c9266be16c3616d8da796944d3e61459 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Tue, 1 Aug 2023 09:27:48 +0200 Subject: [PATCH 16/17] Fix empty state condition --- app/src/main/java/org/onionshare/android/ui/share/ShareUi.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/src/main/java/org/onionshare/android/ui/share/ShareUi.kt b/app/src/main/java/org/onionshare/android/ui/share/ShareUi.kt index 4c425b62..0f563a73 100644 --- a/app/src/main/java/org/onionshare/android/ui/share/ShareUi.kt +++ b/app/src/main/java/org/onionshare/android/ui/share/ShareUi.kt @@ -68,8 +68,7 @@ import org.onionshare.android.ui.theme.topBar private val bottomSheetPeekHeight = 60.dp private fun isEmptyState(shareState: ShareUiState, filesState: FilesState): Boolean { - return (shareState == ShareUiState.AddingFiles || shareState is ShareUiState.ErrorAddingFile) && - filesState.files.isEmpty() + return shareState.allowsModifyingFiles && filesState.files.isEmpty() } @Composable From 49ffd7847217b2346f4c04ee717d3d9a0956accc Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 2 Aug 2023 10:14:55 +0200 Subject: [PATCH 17/17] Introduce a Started TorState This is because previously we only considered uploading the HS desc as started. However, this only happens after zipping files. If we zipping takes a long time, Tor would think it hasn't started and tries circumvention methods. Using a new Started state prevents this. --- app/src/main/java/org/onionshare/android/ShareManager.kt | 4 ++++ app/src/main/java/org/onionshare/android/tor/TorManager.kt | 6 ++++-- app/src/main/java/org/onionshare/android/tor/TorState.kt | 2 ++ app/src/test/java/org/onionshare/android/TorManagerTest.kt | 4 ++++ 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/onionshare/android/ShareManager.kt b/app/src/main/java/org/onionshare/android/ShareManager.kt index 8f702bf1..2085d746 100644 --- a/app/src/main/java/org/onionshare/android/ShareManager.kt +++ b/app/src/main/java/org/onionshare/android/ShareManager.kt @@ -134,6 +134,10 @@ class ShareManager @Inject constructor( ShareUiState.Starting(zipState?.progress ?: 0, torPercent) } + is TorState.Started -> { + ShareUiState.Starting(zipState?.progress ?: 0, 95) + } + is TorState.Published -> { // We only create the hidden service after files have been zipped and webserver was started, // so we are in sharing state once the first HS descriptor has been published. diff --git a/app/src/main/java/org/onionshare/android/tor/TorManager.kt b/app/src/main/java/org/onionshare/android/tor/TorManager.kt index 0f51d378..147f31d3 100644 --- a/app/src/main/java/org/onionshare/android/tor/TorManager.kt +++ b/app/src/main/java/org/onionshare/android/tor/TorManager.kt @@ -17,6 +17,7 @@ import kotlinx.coroutines.withContext import org.briarproject.onionwrapper.CircumventionProvider import org.briarproject.onionwrapper.LocationUtils import org.briarproject.onionwrapper.TorWrapper +import org.briarproject.onionwrapper.TorWrapper.TorState.CONNECTED import org.briarproject.onionwrapper.TorWrapper.TorState.STOPPED import org.onionshare.android.Clock import org.onionshare.android.DefaultClock @@ -143,7 +144,8 @@ class TorManager( override fun onState(s: TorWrapper.TorState) { LOG.info("new state: $s") - if (s == STOPPED) updateTorState(null, TorState.Stopped) + if (s == CONNECTED) updateTorState(TorState.Starting::class, TorState.Started) + else if (s == STOPPED) updateTorState(null, TorState.Stopped) } fun publishOnionService(port: Int) { @@ -156,7 +158,7 @@ class TorManager( } override fun onHsDescriptorUpload(onion: String) { - if (updateTorState(TorState.Starting::class, TorState.Published(onion), warn = false)) { + if (updateTorState(TorState.Started::class, TorState.Published(onion), warn = false)) { startCheckJob?.cancel() startCheckJob = null } diff --git a/app/src/main/java/org/onionshare/android/tor/TorState.kt b/app/src/main/java/org/onionshare/android/tor/TorState.kt index 56cc4b47..1e8e27f7 100644 --- a/app/src/main/java/org/onionshare/android/tor/TorState.kt +++ b/app/src/main/java/org/onionshare/android/tor/TorState.kt @@ -9,6 +9,8 @@ sealed class TorState { val lastProgressTime: Long, ) : TorState() + object Started : TorState() + data class Published( val onion: String, ) : TorState() diff --git a/app/src/test/java/org/onionshare/android/TorManagerTest.kt b/app/src/test/java/org/onionshare/android/TorManagerTest.kt index 7121655e..14f39373 100644 --- a/app/src/test/java/org/onionshare/android/TorManagerTest.kt +++ b/app/src/test/java/org/onionshare/android/TorManagerTest.kt @@ -75,6 +75,10 @@ class TorManagerTest { assertIs(starting50) // 50% assertEquals(50, starting50.progress) + // getting informed about CONNECTED is required now + torManager.onState(TorWrapper.TorState.CONNECTED) + assertIs(awaitItem()) + torManager.onHsDescriptorUpload("foobar") val published = awaitItem() assertIs(published)