Skip to content

Commit ff916c8

Browse files
committed
fix(feeds): retry a refused feed at its commit, and fail over fast when GitHub is unreachable
Three findings from a refuting review of the feed mirror and the signature retry. The retry after a signature refusal appended ?cb=<nanoTime>. Live headers show raw.githubusercontent.com and jsDelivr both leave the query out of the cache key (X-Cache: HIT, Source-Age continuing), so the retry re-read the same stale body and signature, up to 16 MB for the database. It now asks GitHub for the commit the branch points to (one small API call, Accept: application/vnd.github.sha) and fetches both files at /<sha>/, a path whose body and signature always come from one publish. A mirror gets no second try. The test fake served a fresh copy to any URL with a "?", which is how the no-op passed. It now ignores queries like the real CDN. A blackholed GitHub cost about 90 s per file (the default-branch lookup, then main and master at 30 s each) before the mirror was asked, so a 256-shard sync could never finish. A network-level GitHub failure (timeout, refused or unreachable connection, a name that won't resolve) now puts the mirror first for ten minutes, a failed default-branch lookup isn't repeated for ten minutes, and each call has a five-minute ceiling. An HTTP answer, a 404 included, doesn't count as unreachable. FeedMirror was filled asynchronously at process start, so a worker in a cold process could build its sources before the setting arrived and skip the mirror. The app marks the setting as loading before it reads it, and a fetch waits up to three seconds for it. The Settings text, README and data README said GitHub is always tried first and that a mirror "can't change what you get". Both are now accurate: GitHub goes first except right after it couldn't be reached, and a mirror can't alter the data but can hold back updates. Tests: GitHubDataSourceFetchTest's two past-cache tests become commit-retry tests, plus a refusal that stands when the commit can't be resolved, a mirror copy that isn't retried, the outage window opening and closing, a 404 not counting, the branch lookup not repeating, and a fetch waiting for the mirror setting. FeedMirrorTest covers the wait, no wait when nothing loads, the timeout, and a failed read. Eight mutants are each killed by name.
1 parent b313d8b commit ff916c8

10 files changed

Lines changed: 382 additions & 46 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ All notable changes to CallShield will be documented in this file.
2525
- A download whose signature arrives damaged or late is handled for what it
2626
is. Right after a new feed is published, the download network can hand out
2727
the new file with the old signature for a few minutes, so a mismatch is now
28-
fetched once more past its cache before it counts as a failure. A signature
28+
fetched again, both files at the exact commit, before it counts as a
29+
failure. A signature
2930
the server fails to send is a network hiccup to retry, not an unsigned feed.
3031
A trending feed that is refused shows as refused in Protection Test instead
3132
of "unreachable".
@@ -39,11 +40,13 @@ All notable changes to CallShield will be documented in this file.
3940
longer hides why. Its "not found" answer used to replace a passing server
4041
error and stop the app from retrying.
4142
- If GitHub is blocked where you live, Settings > Feed mirror takes a second
42-
address for the protection data. CallShield still asks GitHub first and only
43-
then tries the mirror, before falling back to the copy built into the app.
44-
One tap fills in jsDelivr, which serves the same files and can lag by up to
45-
12 hours. The signature check applies to every mirrored file, so a mirror
46-
can't slip in anything the project didn't sign.
43+
address for the protection data. CallShield asks GitHub first and then the
44+
mirror, before falling back to the copy built into the app. For ten minutes
45+
after GitHub couldn't be reached at all, the mirror goes first, so a sync
46+
behind a block doesn't wait out a timeout for every file. One tap fills in
47+
jsDelivr, which serves the same files and can lag by up to 12 hours. The
48+
signature check applies to every mirrored file, so a mirror can't slip in
49+
anything the project didn't sign, though it can hold back updates.
4750
- The ML scorer no longer judges callers outside North America. It treated any
4851
ten-digit number as a US or Canadian one once the `+` was stripped, so an
4952
unknown Singapore mobile, or a landline in New Zealand, Belgium, Thailand or

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,7 @@ by locale and message type without shipping personal data:
558558
- Selective backup/restore for rules, non-secret settings, and opt-in logs, plus CSV log export and auto-cleanup (7/14/30/90 days)
559559
- Database sync every 6 hours, a trending-feed check every 30 minutes, and a daily digest notification
560560
- External blocklist subscriptions (Settings > External blocklists) take HTTPS CSV, TXT or JSON number lists of up to 1 MB and 20,000 rows. Each list is fetched again once a day, or on the interval it declares in its header (`# Expires: 12 hours`, or `"expires": "12h"` in JSON), never more often than every six hours and at least weekly. A download that comes back empty or with under half the list's numbers isn't applied in the background. The list keeps its last good copy and says why on its row. Each row shows the list's name, the host it comes from, how many numbers it holds and when it last updated, and TalkBack reads it as one item. Tap a row to switch its list off or on. Removing a list takes its numbers out straight away, and Undo puts both back without downloading the list again
561-
- If GitHub is blocked where you live, Settings > Feed mirror takes a second address for the protection data. CallShield asks GitHub first, then the mirror, then falls back to the copy bundled with the app. One tap fills in jsDelivr (`https://cdn.jsdelivr.net/gh/SysAdminDoc/CallShield@master/`), which serves the same files and can run up to 12 hours behind. Mirrored files go through the same signature check, so a mirror can't change what you get. [data/README.md](data/README.md#mirrors-and-recovery) covers running your own
561+
- If GitHub is blocked where you live, Settings > Feed mirror takes a second address for the protection data. CallShield asks GitHub first, then the mirror, then falls back to the copy bundled with the app. For ten minutes after GitHub couldn't be reached at all, the mirror goes first. One tap fills in jsDelivr (`https://cdn.jsdelivr.net/gh/SysAdminDoc/CallShield@master/`), which serves the same files and can run up to 12 hours behind. Mirrored files go through the same signature check, so a mirror can't alter the data, though it can hold back updates. [data/README.md](data/README.md#mirrors-and-recovery) covers running your own
562562
- Quick Settings tile, app shortcuts, home screen widget
563563
- Protection test validates all layers and permissions, including checker errors and deadline cutoffs
564564
- Rules surface priority conflicts after sync and edits, with the winning rule and a review path

‎app/src/main/java/com/sysadmindoc/callshield/CallShieldApp.kt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,13 @@ class CallShieldApp :
102102

103103
registerCacheInvalidationObservers()
104104

105+
// Before the read starts, so a worker that runs first waits for it.
106+
FeedMirror.startLoading()
105107
appScope.launch {
106108
try {
107109
SpamRepository.getInstance(this@CallShieldApp).feedMirrorUrl.collect { FeedMirror.set(it) }
108110
} catch (e: Exception) {
111+
FeedMirror.loadFailed()
109112
Log.w("CallShieldApp", "Failed to follow the feed mirror setting", e)
110113
}
111114
}

‎app/src/main/java/com/sysadmindoc/callshield/data/remote/FeedMirror.kt‎

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
11
package com.sysadmindoc.callshield.data.remote
22

3+
import kotlinx.coroutines.CompletableDeferred
4+
import kotlinx.coroutines.withTimeoutOrNull
35
import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
46

57
/**
68
* A second place to download the protection feeds from, for when
79
* raw.githubusercontent.com is blocked where someone lives or the repository
810
* moves. Off unless the user sets it.
911
*
10-
* The mirror is tried only after every GitHub branch has failed, and before
11-
* the callers fall back to the bundled snapshot. It needs no certificate pins
12-
* of its own: every signed feed is checked against [FeedSignature] whichever
13-
* host serves it, so a mirror can't hand a device anything the maintainer
14-
* didn't sign. The database's rollback checks run on the parsed manifest, so
15-
* they apply to a mirrored one the same way.
12+
* The mirror is tried after every GitHub branch has failed, or first for a
13+
* few minutes after GitHub couldn't be reached at all, and before the callers
14+
* fall back to the bundled snapshot. It needs no certificate pins of its own:
15+
* every signed feed is checked against [FeedSignature] whichever host serves
16+
* it, so a mirror can't hand a device anything the maintainer didn't sign.
17+
* The database's rollback checks run on the parsed manifest, so they apply to
18+
* a mirrored one the same way. A mirror can still hold back updates.
1619
*/
1720
internal object FeedMirror {
1821
/**
@@ -27,11 +30,47 @@ internal object FeedMirror {
2730
var baseUrl: String? = null
2831
private set
2932

33+
@Volatile
34+
private var loading = false
35+
36+
@Volatile
37+
private var loaded = CompletableDeferred<Unit>()
38+
39+
/**
40+
* Called just before the app starts reading the stored setting. Until
41+
* [set] or [loadFailed] follows, [awaitLoaded] holds a fetch briefly, so a
42+
* worker that starts in a fresh process doesn't build its source list
43+
* before the setting arrives and skip the mirror.
44+
*/
45+
fun startLoading() {
46+
loading = true
47+
}
48+
49+
/** The stored setting couldn't be read, so stop holding fetches for it. */
50+
fun loadFailed() {
51+
loaded.complete(Unit)
52+
}
53+
54+
/** Waits up to [timeoutMs] for the stored setting while the app is still reading it. */
55+
suspend fun awaitLoaded(timeoutMs: Long = LOAD_WAIT_MS) {
56+
if (loading) withTimeoutOrNull(timeoutMs) { loaded.await() }
57+
}
58+
3059
/** Sets the mirror from a stored value; anything that isn't a usable base URL clears it. */
3160
fun set(raw: String?) {
3261
baseUrl = raw?.let(::normalize)
62+
loaded.complete(Unit)
3363
}
3464

65+
/** Back to the state of a fresh process, for tests. */
66+
internal fun resetForTest() {
67+
baseUrl = null
68+
loading = false
69+
loaded = CompletableDeferred()
70+
}
71+
72+
private const val LOAD_WAIT_MS = 3_000L
73+
3574
/** The mirror's URL for a repository path such as `data/hot_numbers.json`, or null with no mirror. */
3675
fun urlFor(path: String): String? = baseUrl?.let { it + path }
3776

‎app/src/main/java/com/sysadmindoc/callshield/data/remote/GitHubDataSource.kt‎

Lines changed: 133 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ import okhttp3.Interceptor
1616
import okhttp3.Request
1717
import okhttp3.Response
1818
import okio.Buffer
19+
import java.io.IOException
20+
import java.io.InterruptedIOException
21+
import java.net.ConnectException
22+
import java.net.NoRouteToHostException
23+
import java.net.UnknownHostException
1924
import java.util.concurrent.TimeUnit
2025

2126
internal enum class GitHubFeedFailureReason {
@@ -61,18 +66,23 @@ private val SHARD_PATH_REGEX = Regex("data/spam_number_shards/[0-9a-f]{2}\\.json
6166
class GitHubDataSource internal constructor(
6267
/** Tests serve canned responses through this instead of the network. */
6368
testInterceptor: Interceptor?,
69+
/** Tests move time on to end a GitHub outage window. */
70+
private val clock: () -> Long = System::currentTimeMillis,
6471
) : SpamDataSource,
6572
HotFeedDataSource {
6673
constructor() : this(null)
6774

6875
// Derived client with longer timeouts for large database downloads;
6976
// shares the connection pool with other callers via HttpClient.shared.
77+
// The call timeout bounds a whole download: connect and read timeouts
78+
// alone let a host that trickles bytes hold a sync open indefinitely.
7079
private val client =
7180
HttpClient.shared
7281
.newBuilder()
7382
.apply { testInterceptor?.let(::addInterceptor) }
7483
.connectTimeout(30, TimeUnit.SECONDS)
7584
.readTimeout(30, TimeUnit.SECONDS)
85+
.callTimeout(CALL_TIMEOUT_MINUTES, TimeUnit.MINUTES)
7686
.build()
7787

7888
private val moshi =
@@ -100,6 +110,13 @@ class GitHubDataSource internal constructor(
100110
// Cache of resolved default branch per "owner/repo" → (branch, resolvedAtMs).
101111
private val defaultBranchLock = Any()
102112
private val defaultBranchCache = mutableMapOf<String, Pair<String, Long>>()
113+
private val defaultBranchFailedAt = mutableMapOf<String, Long>()
114+
115+
// When a GitHub download last failed without an answer: a timeout, a
116+
// refused or unreachable connection, a name that won't resolve. For
117+
// GITHUB_OUTAGE_MS after that, fetches ask the mirror first.
118+
@Volatile
119+
private var gitHubUnreachableAt: Long? = null
103120

104121
companion object {
105122
const val DEFAULT_REPO_OWNER = "SysAdminDoc"
@@ -144,6 +161,18 @@ class GitHubDataSource internal constructor(
144161
/** How long a resolved default branch stays cached before re-querying. */
145162
private const val DEFAULT_BRANCH_TTL_MS = 6L * 60L * 60L * 1000L // 6 hours
146163
private val FALLBACK_BRANCHES = listOf("main", "master")
164+
165+
/** How long a failed default-branch lookup stands before GitHub's API is asked again. */
166+
private const val DEFAULT_BRANCH_RETRY_MS = 10L * 60L * 1000L
167+
168+
/** How long the mirror goes first after GitHub couldn't be reached at all. */
169+
internal const val GITHUB_OUTAGE_MS = 10L * 60L * 1000L
170+
171+
/** Bounds one whole download, the 16 MB legacy database included. */
172+
private const val CALL_TIMEOUT_MINUTES = 5L
173+
174+
private val COMMIT_SHA_REGEX = Regex("[0-9a-f]{40}(?:[0-9a-f]{24})?")
175+
private const val MAX_COMMIT_SHA_BYTES = 128L
147176
private val RAW_FEED_SPECS =
148177
mapOf(
149178
DATA_PATH to RawFeedSpec("spam database", MAX_SPAM_DATABASE_BYTES),
@@ -563,9 +592,13 @@ class GitHubDataSource internal constructor(
563592
var lastError: Exception? = null
564593
for (source in feedSources(path, owner, repo)) {
565594
try {
566-
fetchVerified(path, source)?.let { return Result.success(it) }
595+
fetchVerified(path, source)?.let {
596+
if (!source.mirror) gitHubUnreachableAt = null
597+
return Result.success(it)
598+
}
567599
lastError = moreTelling(lastError, Exception("Empty response body"), source.mirror)
568600
} catch (e: Exception) {
601+
if (!source.mirror && isUnreachable(e)) gitHubUnreachableAt = clock()
569602
lastError = moreTelling(lastError, e, source.mirror)
570603
}
571604
}
@@ -577,26 +610,59 @@ class GitHubDataSource internal constructor(
577610
* the user's [FeedMirror] when one is set. Every source carries its own
578611
* signature URL on the same host and branch, so a copy refused on one
579612
* source can't borrow another source's signature.
613+
*
614+
* For [GITHUB_OUTAGE_MS] after GitHub couldn't be reached at all, the
615+
* mirror goes first and the branch lookup is skipped. Behind a GitHub
616+
* block every GitHub request costs a full timeout, three of them per file,
617+
* so a sync of 256 shards would never end. GitHub stays in the list, after
618+
* the mirror, in case the mirror fails too.
580619
*/
581620
private suspend fun feedSources(
582621
path: String,
583622
owner: String,
584623
repo: String,
585624
): List<FeedSource> {
586-
val github =
587-
resolveCandidateBranches(owner, repo).map { branch ->
588-
FeedSource(buildRawUrl(owner, repo, branch, path), buildRawUrl(owner, repo, branch, "$path.sig"))
589-
}
625+
FeedMirror.awaitLoaded()
590626
val mirror = FeedMirror.urlFor(path)?.let { FeedSource(it, "$it.sig", mirror = true) }
591-
return github + listOfNotNull(mirror)
627+
if (mirror != null && gitHubRecentlyUnreachable()) {
628+
return listOf(mirror) + gitHubSources(path, owner, repo, knownBranches(owner, repo))
629+
}
630+
return gitHubSources(path, owner, repo, resolveCandidateBranches(owner, repo)) + listOfNotNull(mirror)
592631
}
593632

633+
private fun gitHubSources(
634+
path: String,
635+
owner: String,
636+
repo: String,
637+
branches: List<String>,
638+
): List<FeedSource> =
639+
branches.map { branch ->
640+
FeedSource(
641+
url = buildRawUrl(owner, repo, branch, path),
642+
signatureUrl = buildRawUrl(owner, repo, branch, "$path.sig"),
643+
pin = GitHubPin(owner, repo, branch),
644+
)
645+
}
646+
647+
private fun gitHubRecentlyUnreachable(): Boolean = gitHubUnreachableAt?.let { clock() - it < GITHUB_OUTAGE_MS } == true
648+
649+
/** A GitHub download that got no answer at all, as opposed to one GitHub refused or failed. */
650+
private fun isUnreachable(error: Exception): Boolean =
651+
error is InterruptedIOException ||
652+
error is ConnectException ||
653+
error is NoRouteToHostException ||
654+
error is UnknownHostException
655+
594656
/**
595657
* [path] from [source], size-checked and signature-checked, or null for
596-
* an empty body. The CDN caches a file and its signature separately, for
597-
* minutes, so right after a publish it can pair the new body with the old
598-
* signature. A signature refusal gets one more try past the cache, and
599-
* stands if that copy doesn't verify either.
658+
* an empty body. GitHub's CDN caches a file and its signature separately,
659+
* for minutes, and leaves the query string out of its cache key, so right
660+
* after a publish it can pair the new body with the old signature and no
661+
* `?` gets past that. A signature refusal from a GitHub branch is tried
662+
* once more at the commit the branch points to: that path never changes,
663+
* so its body and signature come from one publish. The refusal stands if
664+
* that copy fails too or the commit can't be resolved. A mirror gets no
665+
* second try, since its cache turns over on its own schedule.
600666
*/
601667
private fun fetchVerified(
602668
path: String,
@@ -606,14 +672,41 @@ class GitHubDataSource internal constructor(
606672
fetchVerifiedOnce(path, source.url, source.signatureUrl)
607673
} catch (refused: GitHubFeedValidationException) {
608674
if (refused.reason != GitHubFeedFailureReason.SIGNATURE) throw refused
609-
val pastCache = "cb=${System.nanoTime()}"
675+
val pin = source.pin ?: throw refused
676+
val commit = resolveCommit(pin) ?: throw refused
610677
try {
611-
fetchVerifiedOnce(path, "${source.url}?$pastCache", "${source.signatureUrl}?$pastCache")
678+
fetchVerifiedOnce(
679+
path,
680+
buildRawUrl(pin.owner, pin.repo, commit, path),
681+
buildRawUrl(pin.owner, pin.repo, commit, "$path.sig"),
682+
)
612683
} catch (_: Exception) {
613684
throw refused
614685
}
615686
}
616687

688+
/** The commit [pin]'s branch points to, or null when GitHub won't say. One small API call, made only after a refusal. */
689+
private fun resolveCommit(pin: GitHubPin): String? =
690+
try {
691+
val request =
692+
Request
693+
.Builder()
694+
.url("$GITHUB_API_BASE/${pin.owner}/${pin.repo}/commits/${pin.branch}")
695+
.header("Accept", "application/vnd.github.sha")
696+
.header("User-Agent", USER_AGENT)
697+
.build()
698+
client.newCall(request).execute().use { response ->
699+
if (!response.isSuccessful) return null
700+
readLimitedBody(response, "GitHub commit API", MAX_COMMIT_SHA_BYTES)
701+
?.trim()
702+
?.takeIf(COMMIT_SHA_REGEX::matches)
703+
}
704+
} catch (_: IOException) {
705+
null
706+
} catch (_: GitHubFeedValidationException) {
707+
null
708+
}
709+
617710
private fun fetchVerifiedOnce(
618711
path: String,
619712
url: String,
@@ -716,20 +809,38 @@ class GitHubDataSource internal constructor(
716809
repo: String,
717810
): String? {
718811
val key = "$owner/$repo"
719-
val now = System.currentTimeMillis()
812+
val now = clock()
720813
synchronized(defaultBranchLock) {
721814
val cached = defaultBranchCache[key]
722815
if (cached != null && now - cached.second < DEFAULT_BRANCH_TTL_MS) {
723816
return cached.first
724817
}
818+
// A lookup that just failed isn't repeated for every file in a
819+
// sync: behind a GitHub block each attempt costs a full timeout.
820+
val failedAt = defaultBranchFailedAt[key]
821+
if (failedAt != null && now - failedAt < DEFAULT_BRANCH_RETRY_MS) return cached?.first
725822
}
726-
val resolved = fetchDefaultBranch(owner, repo).getOrNull() ?: return null
823+
val resolved = fetchDefaultBranch(owner, repo).getOrNull()
727824
synchronized(defaultBranchLock) {
825+
if (resolved == null) {
826+
defaultBranchFailedAt[key] = now
827+
return defaultBranchCache[key]?.first
828+
}
728829
defaultBranchCache[key] = resolved to now
830+
defaultBranchFailedAt.remove(key)
729831
}
730832
return resolved
731833
}
732834

835+
/** The candidate branches without asking the API: the cached default, then the fallbacks. */
836+
private fun knownBranches(
837+
owner: String,
838+
repo: String,
839+
): List<String> {
840+
val cached = synchronized(defaultBranchLock) { defaultBranchCache["$owner/$repo"]?.first }
841+
return listOfNotNull(cached).plus(FALLBACK_BRANCHES).distinct()
842+
}
843+
733844
private suspend fun fetchDefaultBranch(
734845
owner: String,
735846
repo: String,
@@ -889,6 +1000,14 @@ class GitHubDataSource internal constructor(
8891000
val url: String,
8901001
val signatureUrl: String,
8911002
val mirror: Boolean = false,
1003+
/** The GitHub branch a source reads, for the retry at its commit. Null for the mirror. */
1004+
val pin: GitHubPin? = null,
1005+
)
1006+
1007+
private data class GitHubPin(
1008+
val owner: String,
1009+
val repo: String,
1010+
val branch: String,
8921011
)
8931012

8941013
private data class GitHubReleasePayload(

0 commit comments

Comments
 (0)