Skip to content

Commit ff57dc9

Browse files
committed
fix: close the gaps a second refuting review found in lookups and list refresh
Live lookup (ce2dbd5): - With SkipCalls the only source, a failed lookup (parked page, 401, 429, timeout) scored 0 and turned a locally flagged call into a green "Looks Safe". The overlay now counts definitive answers. When every source failed it keeps the local verdict and says no source returned a definitive result. The score moved into liveLookupScore() so it can be tested. - The SkipCalls parser reported one report for every flagged number, but the service gives a verdict and a category, never a count. It now reports none. The overlay says "Flagged" instead of "1 report", and the detail header says "Flagged as spam by 1 source" in red instead of turning green. A lookup that failed everywhere is no longer shown in the clean colour. - The zh-rCN "check 4 sources" label and the overlay KDoc still named the removed sources. - The live probe only sampled a clean number, so a spam verdict moving to a shape the app reads as clean would still pass. It now also requires a known spam number to answer is_spam true. Subscription refresh (f136f95): - An empty 200 body failed with a generic error. It now carries the parser's EMPTY reason, so a background refresh holds the last good copy. - A failed refresh always recorded the same message. The row now says whether the server answered with an HTTP status, the list outgrew the size or row cap, the download wasn't a list, or the certificate failed. - "Expires: 1.5 hours" read as 1 with no unit, which means a day. Decimals are refused. A byte-order mark before the header no longer hides it. - A storage error on one list escaped the refresh loop and skipped every list after it. Each list now fails on its own. Test changes: RemoteLookupParserTest pins the zero count. The probe test now serves distinct clean and spam answers, because a single answer couldn't exercise the new spam-sample check; its assertions are otherwise unchanged. New cases cover liveLookupScore, the empty, oversized and unreadable body mapping, a 404 on the refresh row, decimal Expires values and a BOM.
1 parent 43243cf commit ff57dc9

18 files changed

Lines changed: 291 additions & 55 deletions

‎CHANGELOG.md‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,9 @@ All notable changes to CallShield will be documented in this file.
4141
with under half the list's numbers isn't applied in the background, because
4242
a broken export looks just like a list that cleaned itself up. The list
4343
keeps its last good copy and says why on its row, which also shows when it
44-
last updated.
44+
last updated. A list that fails to refresh says what went wrong: the
45+
server's HTTP status, the size or row limit, a download that isn't a list,
46+
or a certificate that couldn't be verified.
4547

4648
### Live caller lookup
4749

@@ -52,8 +54,12 @@ All notable changes to CallShield will be documented in this file.
5254
WhoCalledMe as clean on every enriched call.
5355
- SkipCalls, the one that still works, is now read correctly. It reports spam in
5456
an `is_spam` field the app never looked for, so every number it flagged showed
55-
as clean. A flagged number now shows as flagged, with the category SkipCalls
56-
gives (scam, for example).
57+
as clean. A flagged number now shows as flagged, and its detail page shows
58+
the category SkipCalls gives (scam, for example). SkipCalls never gives a
59+
count, so the app no longer shows a made-up "1 report".
60+
- When SkipCalls can't answer, the caller ID overlay keeps CallShield's own
61+
warning and says no source gave a definitive result. It used to switch to a
62+
green "Looks Safe".
5763
- A lookup answer that doesn't look like a lookup result now reads as
5864
"unavailable", not "clean". The overlay no longer has a caller-name line,
5965
because nothing free provides one any more, and a spam hit no longer shows as

‎README.md‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -453,15 +453,16 @@ For a locally suspicious call, CallShield can show a real-time overlay when the
453453
│ LIKELY SPAM │
454454
│ (212) 555-1234 │
455455
│ New York, NY │
456-
│ Spam Score: 50% │
457-
│ ⚠ SkipCalls: Flagged as scam │
456+
│ Spam Score: 50% (Flagged) │
457+
│ ⚠ SkipCalls: Flagged │
458458
│ All sources checked │
459459
│ [Search] [Block] [Dismiss] │
460460
│ 🔈 Play SIT Tone (anti-dialer) │
461461
└──────────────────────────────────┘
462462
```
463463

464-
- Shows instantly with the area code, then updates when SkipCalls answers
464+
- Shows instantly with the area code, then updates when SkipCalls answers. SkipCalls gives a verdict and a category but no report count. The number's detail page shows the category
465+
- If SkipCalls can't answer, the overlay keeps CallShield's own warning and says no source returned a definitive result
465466
- **SIT Tone**. ITU-T E.180 three-tone sequence tricks autodialers into removing your number
466467
- Color-coded: green (safe) → yellow → orange → red (spam)
467468

@@ -812,7 +813,7 @@ language in [issue #7](https://github.com/SysAdminDoc/CallShield/issues/7).
812813
| URL Safety | Local spam-domain data; optional URLhaus (abuse.ch) |
813814
| Verification | Local Gradle, lint, and release-artifact checks |
814815
| Tests | 1100 JVM unit tests (JUnit) |
815-
| Strings | 1416 string resources and 33 plural groups (translation-ready) |
816+
| Strings | 1422 string resources and 33 plural groups (translation-ready) |
816817
| Accessibility | 100+ content descriptions, 48dp touch targets |
817818
| Min SDK | 29 (Android 10) |
818819
| Target SDK | 36 |

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,10 @@ internal object ExternalBlocklistParser {
5757

5858
private const val EXPIRES_HEADER_LINES = 50
5959
private val expiresLine = Regex("""^expires\s*:\s*(.+)$""", RegexOption.IGNORE_CASE)
60-
private val expiresValue = Regex("""^(\d{1,5})(?!\d)\s*([a-z]*)""", RegexOption.IGNORE_CASE)
60+
61+
// A decimal such as "1.5 hours" is refused rather than read as "1", which
62+
// with no unit after it would mean days.
63+
private val expiresValue = Regex("""^(\d{1,5})(?![\d.])\s*([a-z]*)""", RegexOption.IGNORE_CASE)
6164

6265
fun parse(
6366
rawUrl: String,
@@ -128,7 +131,9 @@ internal object ExternalBlocklistParser {
128131
if (format == "json") {
129132
jsonExpires(body)
130133
} else {
134+
// A byte-order mark survives trim() and would hide a first-line header.
131135
body
136+
.removePrefix("\uFEFF")
132137
.lineSequence()
133138
.map { it.trim() }
134139
.filter { it.isNotEmpty() }

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

Lines changed: 40 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
package com.sysadmindoc.callshield.data.remote
22

3+
import com.sysadmindoc.callshield.data.ExternalBlocklistFailureReason
34
import com.sysadmindoc.callshield.data.ExternalBlocklistParser
5+
import com.sysadmindoc.callshield.data.ExternalBlocklistValidationException
46
import kotlinx.coroutines.Dispatchers
57
import kotlinx.coroutines.withContext
68
import okhttp3.HttpUrl
79
import okhttp3.HttpUrl.Companion.toHttpUrl
810
import okhttp3.Request
11+
import java.io.IOException
912
import java.util.concurrent.TimeUnit
1013

1114
interface ExternalBlocklistDataSource {
@@ -54,33 +57,50 @@ class OkHttpExternalBlocklistDataSource : ExternalBlocklistDataSource {
5457
return@use null
5558
}
5659
if (!response.isSuccessful) {
57-
error("HTTP ${response.code}")
58-
}
59-
when (val body = response.body?.readUtf8Bounded(ExternalBlocklistParser.MAX_SUBSCRIPTION_BYTES)) {
60-
is BoundedResponseBody.Text -> {
61-
body.value
62-
}
63-
64-
BoundedResponseBody.Empty -> {
65-
error("External blocklist returned an empty body")
66-
}
67-
68-
is BoundedResponseBody.Oversized -> {
69-
error("External blocklist exceeded ${body.maxBytes} byte cap")
70-
}
71-
72-
BoundedResponseBody.Unreadable,
73-
null,
74-
-> {
75-
error("External blocklist response could not be read")
76-
}
60+
throw ExternalBlocklistHttpException(response.code)
7761
}
62+
externalBlocklistBodyText(response.body.readUtf8Bounded(ExternalBlocklistParser.MAX_SUBSCRIPTION_BYTES))
7863
}
7964
if (bodyText != null) return bodyText
8065
}
8166
}
8267
}
8368

69+
/** The list server answered with an error status; kept apart so the row can say which. */
70+
internal class ExternalBlocklistHttpException(
71+
val code: Int,
72+
) : IOException("HTTP $code")
73+
74+
/**
75+
* A body that arrived but holds nothing is an empty list, the same answer the
76+
* parser gives for a list with no numbers, so a background refresh holds the
77+
* last good copy instead of recording a failure.
78+
*/
79+
internal fun externalBlocklistBodyText(body: BoundedResponseBody): String =
80+
when (body) {
81+
is BoundedResponseBody.Text -> {
82+
body.value
83+
}
84+
85+
BoundedResponseBody.Empty -> {
86+
throw ExternalBlocklistValidationException(
87+
ExternalBlocklistFailureReason.EMPTY,
88+
"External blocklist returned an empty body",
89+
)
90+
}
91+
92+
is BoundedResponseBody.Oversized -> {
93+
throw ExternalBlocklistValidationException(
94+
ExternalBlocklistFailureReason.OVERSIZE,
95+
"External blocklist exceeded ${body.maxBytes} byte cap",
96+
)
97+
}
98+
99+
BoundedResponseBody.Unreadable -> {
100+
throw IOException("External blocklist response could not be read")
101+
}
102+
}
103+
84104
internal fun validatedExternalBlocklistRedirect(
85105
originalUrl: HttpUrl,
86106
currentUrl: HttpUrl,

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,12 @@ internal fun parseSkipCallsBody(body: String): ExternalLookup.SourceResult {
2929
?.get(1)
3030
?.trim()
3131
?.takeUnless { it.isEmpty() || it.equals("unknown", ignoreCase = true) }
32+
// SkipCalls gives a verdict and a category, never a count. Reporting one
33+
// report would put a made-up "1 report" on the overlay and detail page.
3234
return ExternalLookup.SourceResult(
3335
source = "SkipCalls",
3436
isSpam = true,
35-
reports = 1,
37+
reports = 0,
3638
detail = if (description != null) "Flagged as $description" else "Flagged as spam",
3739
status = RemoteLookupStatus.FOUND,
3840
)

‎app/src/main/java/com/sysadmindoc/callshield/data/repository/SyncRepository.kt‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import com.sysadmindoc.callshield.data.model.SpamPrefixJson
2424
import com.sysadmindoc.callshield.data.model.SpamShardDescriptor
2525
import com.sysadmindoc.callshield.data.model.SpamShardManifest
2626
import com.sysadmindoc.callshield.data.remote.ExternalBlocklistDataSource
27+
import com.sysadmindoc.callshield.data.remote.ExternalBlocklistHttpException
2728
import com.sysadmindoc.callshield.data.remote.GitHubDataSource
2829
import com.sysadmindoc.callshield.data.remote.GitHubFeedValidationException
2930
import com.sysadmindoc.callshield.data.remote.HttpClient
@@ -287,13 +288,23 @@ class SyncRepository(
287288
* list's size is not applied: the last good rows stay and the list is
288289
* flagged, the way a collapsed database or hot-feed publish is refused.
289290
*/
291+
@Suppress("TooGenericExceptionCaught")
290292
suspend fun refreshDueExternalBlocklists(now: Long = System.currentTimeMillis()): List<ExternalBlocklistRefreshOutcome> =
291293
withContext(Dispatchers.IO) {
292294
syncMutex.withLock {
293295
settingsRepository
294296
.readExternalBlocklistSubscriptions()
295297
.filter { ExternalBlocklistRefreshPolicy.isDue(it, now) }
296-
.map { refreshExternalBlocklist(it, now) }
298+
.map { subscription ->
299+
// A storage error on one list must not skip every list after it.
300+
try {
301+
refreshExternalBlocklist(subscription, now)
302+
} catch (e: CancellationException) {
303+
throw e
304+
} catch (e: Exception) {
305+
failExternalBlocklist(subscription, now, e)
306+
}
307+
}
297308
}
298309
}
299310

@@ -722,17 +733,57 @@ class SyncRepository(
722733
return ExternalBlocklistRefreshOutcome.HELD
723734
}
724735

736+
@Suppress("TooGenericExceptionCaught")
725737
private suspend fun failExternalBlocklist(
726738
subscription: ExternalBlocklistSubscription,
727739
now: Long,
728740
error: Exception,
729741
): ExternalBlocklistRefreshOutcome {
730742
// The cause can embed the list's full URL, so it goes to the log only.
731743
android.util.Log.w("SyncRepository", "External blocklist refresh failed", error)
732-
recordExternalBlocklistAttempt(subscription.id, now, context.getString(R.string.external_blocklist_refresh_failed))
744+
try {
745+
recordExternalBlocklistAttempt(subscription.id, now, externalBlocklistFailureText(error))
746+
} catch (e: CancellationException) {
747+
throw e
748+
} catch (e: Exception) {
749+
android.util.Log.w("SyncRepository", "Could not record the failed refresh", e)
750+
}
733751
return ExternalBlocklistRefreshOutcome.FAILED
734752
}
735753

754+
/**
755+
* What went wrong, without the URL. A list that moved (HTTP 404) or grew past
756+
* the caps reads differently on its row from one that was briefly unreachable.
757+
*/
758+
private fun externalBlocklistFailureText(error: Exception): String {
759+
val reason = (error as? ExternalBlocklistValidationException)?.reason
760+
return when {
761+
error is ExternalBlocklistHttpException -> {
762+
context.getString(R.string.external_blocklist_refresh_http, error.code)
763+
}
764+
765+
reason == ExternalBlocklistFailureReason.OVERSIZE -> {
766+
context.getString(R.string.external_blocklist_refresh_too_large)
767+
}
768+
769+
reason == ExternalBlocklistFailureReason.ROW_LIMIT -> {
770+
context.getString(R.string.external_blocklist_refresh_too_many_rows)
771+
}
772+
773+
reason != null -> {
774+
context.getString(R.string.external_blocklist_refresh_unreadable)
775+
}
776+
777+
HttpClient.isCertificateTrustFailure(error) -> {
778+
context.getString(R.string.external_blocklist_refresh_certificate)
779+
}
780+
781+
else -> {
782+
context.getString(R.string.external_blocklist_refresh_failed)
783+
}
784+
}
785+
}
786+
736787
private suspend fun recordExternalBlocklistAttempt(
737788
id: String,
738789
now: Long,

0 commit comments

Comments
 (0)