nik/bitchat-android
1
0
Fork 0
mirror of https://github.com/permissionlesstech/bitchat-android.git synced 2026-08-08 06:46:11 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: key the rate-limit gate to the selected route, and drop the force flag

Browse Source 
Addresses review on #812.

isProxyEnabled() reports readiness, not route selection: it is false
while Tor is bootstrapping or restarting, even though requests will
still go through Tor. Using it as the route identity cleared a
Tor-earned gate mid-bootstrap and applied a direct-earned one to the
first Tor request -- the opposite of what scoping the gate was for. The
identity is now the selected mode from statusFlow.

The force flag turned out to be both unnecessary and harmful. Leaving
Downloading synchronously before the check already clears the entry
guard, so force only reached the completion guard -- which must stay
armed. A cancellation check can take a minute on the route timeout, and
WorkManager can surface Resumable meanwhile, so the user may start a new
download before it returns; force let the stale result overwrite work
that was running and strip the progress and stop controls. Removing it
restores that protection and needs no generation counter.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Moe Hamade 2026-07-29 15:41:05 +03:00
parent 77f3c0cc05
commit 4200871ee9
2 changed files with 37 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
app/src/main/java/com/bitchat/android/ui/ApkDownloadViewModel.kt
View File

@ -206,9 +206,10 @@ class ApkDownloadViewModel(application: Application) : AndroidViewModel(applicat
it.copy(apkStatus = ApkPreparationStatus.Loading, downloadProgress = 0)
}
// force, because the guards in checkStatus() protect a job that is still
// running, which this one is not.
checkStatus(force = true)
// No force needed, and it would be harmful: leaving Downloading above already
// clears the entry guard, while the completion guard must stay armed so a
// download the user restarts during this check is not overwritten by its result.
checkStatus()
}
private fun startDownload() {
@ -222,23 +223,21 @@ class ApkDownloadViewModel(application: Application) : AndroidViewModel(applicat
downloader.startDownload()
}
/**
* @param force resolve even while the state says Downloading. Only cancellation
* should pass true: the guard below exists so a running job is never second-guessed
* from cache contents, but a cancelled one has no other route out of that state.
*/
private fun checkStatus(force: Boolean = false) {
private fun checkStatus() {
viewModelScope.launch {
// WorkManager is the source of truth for active work. A queued or
// newly started job legitimately has no partial file yet, so never
// infer that it is orphaned from cache contents.
if (!force && _state.value.apkStatus is ApkPreparationStatus.Downloading) {
if (_state.value.apkStatus is ApkPreparationStatus.Downloading) {
return@launch
}
val resolvedStatus = resolveApkStatus()
_state.update { current ->
if (!force && current.apkStatus is ApkPreparationStatus.Downloading) {
// Re-checked rather than trusted from entry: this resolve reaches the
// network and can take a minute, in which time the user may have started
// a download. Its result must not overwrite work that is now running.
if (current.apkStatus is ApkPreparationStatus.Downloading) {
current
} else {
current.copy(apkStatus = resolvedStatus, downloadProgress = 0)

41
app/src/main/java/com/bitchat/android/util/GitHubReleaseClient.kt
View File

@ -3,6 +3,7 @@ package com.bitchat.android.util
import android.util.Log
import com.bitchat.android.net.ArtiTorManager
import com.bitchat.android.net.OkHttpProvider
import com.bitchat.android.net.TorMode
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.delay
import kotlinx.coroutines.sync.Mutex
@ -48,23 +49,36 @@ object GitHubReleaseClient {
private var blockedUntilMillis = 0L
/**
* Whether the route was proxied when [blockedUntilMillis] was recorded, or null when
* no gate is set. GitHub counts unauthenticated requests per IP, so a Tor exit and a
* direct connection have separate quotas -- a cooldown earned on one must not be
* served to the other.
* Whether Tor was the selected route when [blockedUntilMillis] was recorded, or null
* when no gate is set. GitHub counts unauthenticated requests per IP, so a Tor exit
* and a direct connection have separate quotas -- a cooldown earned on one must not
* be served to the other.
*/
private var blockedRouteUsedProxy: Boolean? = null
@Volatile
private var blockedRouteUsedTor: Boolean? = null
/**
* The route requests will take, which is what the quota belongs to.
*
* Deliberately the selected mode rather than `isProxyEnabled()`: that reports
* readiness, and is false while Tor is still bootstrapping or restarting even though
* requests will still go through Tor once it is up. Using it as the route identity
* would clear a Tor-earned gate mid-bootstrap and apply a direct-earned one to the
* first Tor request.
*/
private fun selectedRouteUsesTor(): Boolean? =
runCatching { ArtiTorManager.getInstance().statusFlow.value.mode != TorMode.OFF }
.getOrNull()
/** Drops a gate earned on a route the app is no longer using. */
private fun clearGateIfRouteChanged() {
val recordedRoute = blockedRouteUsedProxy ?: return
val currentRoute = runCatching { ArtiTorManager.getInstance().isProxyEnabled() }
.getOrNull() ?: return
val recordedRoute = blockedRouteUsedTor ?: return
val currentRoute = selectedRouteUsesTor() ?: return
if (recordedRoute != currentRoute) {
Log.i(TAG, "Route changed since the rate limit was recorded; clearing the gate")
blockedUntilMillis = 0L
blockedRouteUsedProxy = null
blockedRouteUsedTor = null
}
}
@ -104,11 +118,11 @@ object GitHubReleaseClient {
return@withLock Result.success(cached.release)
}
clearGateIfRouteChanged()
// Honoured even on an explicit refresh: sending a request GitHub has already said
// it will reject helps nobody and pushes the reset further out. A stale release is
// a better answer than an error the user cannot act on.
clearGateIfRouteChanged()
if (now < blockedUntilMillis) {
val waitMinutes = (blockedUntilMillis - now) / 60_000 + 1
Log.w(TAG, "Rate limited; not contacting GitHub for another ${waitMinutes}min")
@ -218,8 +232,7 @@ object GitHubReleaseClient {
if (blockedUntil != null) {
blockedUntilMillis = blockedUntil
// Record which route earned it: the quota belongs to that IP.
blockedRouteUsedProxy =
runCatching { ArtiTorManager.getInstance().isProxyEnabled() }.getOrNull()
blockedRouteUsedTor = selectedRouteUsesTor()
}
val message = if (blockedUntil != null) {
@ -265,7 +278,7 @@ object GitHubReleaseClient {
// revalidate to a 304 that confirms a release we no longer hold.
cachedEtag = response.header("ETag")
blockedUntilMillis = 0L
blockedRouteUsedProxy = null
blockedRouteUsedTor = null
Result.success(release)
}
} catch (e: IOException) {