Skip to content

Commit b5c266b

Browse files
SysAdminDocSysAdminDoc
committed
fix(blocklist): make block, allow and unblock see every spelling
After 680ee83 screening matched a national-format number's +1 form, but the edit paths still worked on one spelling. A refuting review reproduced three regressions: an older allow of +1 649 555 0123 beat a newer block of 649 555 0123 (it was blocked before 680ee83), Remove my rule on the national form left a +1 block in force while saying it was removed, and a temporary allow of the national form was accepted against a permanent +1 block that kept winning. PhoneIdentityCanonicalizer.equivalentForms() now lists every spelling a line can be stored under: the canonical one, its +1 form, and the national spellings canonicalization leaves alone (only numbers libphonenumber rejects, so a valid number still has one form and costs no extra queries). Screening (CheckContext.alternateForms replaces e164Fallback) and the outgoing risk warning match all of them, and it now works from the +1 side too. blockNumber clears allows, addToWhitelist resolves blocks and refuses a temporary allow against a permanent block, and unblockByNumber removes blocks, in every form. Lookup's Remove my rule and Number Detail's unblock go through unblockByNumber, and Number Detail's blocked state matches every form. Tests: PhoneIdentityCanonicalizerTest adds the pure equivalentForms cases. NationalFormatLookupTest adds a case for each regression plus the reverse spelling, and its precondition now expects the 11-digit national spelling among the forms (the helper's list grew). Single-form edits failed four of the new cases; single-form screening failed eight.
1 parent a23caa1 commit b5c266b

12 files changed

Lines changed: 163 additions & 34 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,10 @@ All notable changes to CallShield will be documented in this file.
3333
When Android's number library didn't recognize the exchange or the area
3434
code (a made-up number, or an area code newer than the phone's copy of the
3535
library), the number kept its bare form and slipped past both. Numbers you
36-
blocked or allowed match either form too, and so does the outgoing call
37-
hold.
36+
blocked or allowed match every spelling too, and so does the outgoing call
37+
hold. Blocking, allowing and removing a block also work across spellings:
38+
a newer block of 649 555 0123 replaces an older allow of +1 649 555 0123,
39+
and Remove my rule clears the block however it was saved.
3840

3941
- Your call category rules now reach more suspicious-pattern blocks. A call
4042
flagged as possible neighbor spoofing counts as a scam, one from an active

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,14 @@ internal object OutgoingRiskPolicy {
3333
nowElapsed: Long = SystemClock.elapsedRealtime(),
3434
): OutgoingRiskWarning? {
3535
val normalized = repository.normalizeNumber(rawNumber)
36-
val match = normalized.takeIf(String::isNotBlank)?.let { repository.findExactSpamNumber(it) }
36+
// Every stored spelling, the way screening matches them, for the row
37+
// and for the allow alike.
38+
val forms = normalized.takeIf(String::isNotBlank)?.let(repository::lookupForms).orEmpty()
39+
val match = forms.firstNotNullOfOrNull { repository.findExactSpamNumber(it) }
3740
// A whitelist entry is the user's standing false-positive correction —
3841
// the community DB row usually still exists for exactly those numbers,
3942
// and warning on every dial would re-litigate a decision already made.
40-
if (match != null && repository.hasActiveWhitelistEntry(normalized)) return null
43+
if (match != null && forms.any { repository.hasActiveWhitelistEntry(it) }) return null
4144
val warning = match?.toOutgoingWarning(normalized)
4245
return warning?.takeIf { shouldShow(normalized, nowElapsed) }
4346
}

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

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,21 @@ class PhoneIdentityCanonicalizer internal constructor(
3434
?: normalized
3535
}
3636

37+
/**
38+
* Every spelling under which this phone may have stored the line behind
39+
* [canonical], [canonical] first. For a NANP number on a NANP home region
40+
* that's its +1 form plus whichever national spellings canonicalization
41+
* leaves without a "+" (only numbers libphonenumber rejects stay
42+
* national, so a valid number has just the one form). Lookups and edits
43+
* both go through this, so a block, an allow and its removal all see the
44+
* same set of rows.
45+
*/
46+
fun equivalentForms(canonical: String): List<String> {
47+
val national = nanpNational(canonical, homeRegionIso) ?: return listOf(canonical)
48+
val storedNational = listOf(national, "1$national").filter { canonicalizePhone(it) == it }
49+
return (listOf(canonical, "+1$national") + storedNational).distinct()
50+
}
51+
3752
fun canonicalizeIdentity(raw: String): String {
3853
val trimmed = raw.trim()
3954
if (trimmed.any(Char::isLetter)) {
@@ -123,7 +138,8 @@ class PhoneIdentityCanonicalizer internal constructor(
123138
* number without "+" whenever libphonenumber rejects it (a fictional
124139
* exchange, an unassigned area code, or one newer than the phone's
125140
* metadata), while the spam database and prefix list are keyed by
126-
* E.164. Lookups try this form too; stored identities keep theirs.
141+
* E.164. It names one key for every spelling (the outgoing hold's
142+
* Call anyway pass); lookups use [equivalentForms].
127143
*/
128144
fun nanpE164Fallback(
129145
canonical: String,
@@ -136,9 +152,23 @@ class PhoneIdentityCanonicalizer internal constructor(
136152
else -> null
137153
}
138154

155+
/** The ten national digits of a NANP number on a NANP home region, whichever way it's spelled. */
156+
private fun nanpNational(
157+
canonical: String,
158+
homeRegionIso: String?,
159+
): String? =
160+
when {
161+
normalizeRegion(homeRegionIso) !in NANP_REGIONS -> null
162+
NANP_E164.matches(canonical) -> canonical.substring(2)
163+
NANP_NATIONAL.matches(canonical) -> canonical
164+
NANP_WITH_TRUNK_PREFIX.matches(canonical) -> canonical.substring(1)
165+
else -> null
166+
}
167+
139168
// Every region that shares country code 1.
140169
private val NANP_REGIONS =
141170
setOf("US", "CA", "AG", "AI", "AS", "BB", "BM", "BS", "DM", "DO", "GD", "GU", "JM", "KN", "KY", "LC", "MP", "MS", "PR", "SX", "TC", "TT", "VC", "VG", "VI")
171+
private val NANP_E164 = Regex("\\+1[2-9][0-9]{9}")
142172
private val NANP_NATIONAL = Regex("[2-9][0-9]{9}")
143173
private val NANP_WITH_TRUNK_PREFIX = Regex("1[2-9][0-9]{9}")
144174
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ class SpamRepository(
108108
normalizePhone = phoneIdentityCanonicalizer::canonicalizePhone,
109109
normalizeSenderIdentity = phoneIdentityCanonicalizer::canonicalizeIdentity,
110110
senderRegionIso = phoneIdentityCanonicalizer.homeRegionIso,
111+
equivalentForms = phoneIdentityCanonicalizer::equivalentForms,
111112
)
112113
private val syncRepository =
113114
SyncRepository(
@@ -126,6 +127,7 @@ class SpamRepository(
126127
settingsRepository = settingsRepository,
127128
normalizeNumber = phoneIdentityCanonicalizer::canonicalizePhone,
128129
normalizeLogIdentity = phoneIdentityCanonicalizer::canonicalizeIdentity,
130+
equivalentForms = phoneIdentityCanonicalizer::equivalentForms,
129131
invalidateWildcardCache = spamRepositoryImpl::invalidateWildcardCache,
130132
invalidateKeywordCache = spamRepositoryImpl::invalidateKeywordCache,
131133
invalidateHashWildcardCache = spamRepositoryImpl::invalidateHashWildcardCache,
@@ -635,7 +637,7 @@ class SpamRepository(
635637
internal suspend fun findExactSpamNumber(normalized: String) = spamRepositoryImpl.findByNumberInternal(normalized)
636638

637639
/** Forms of a normalized number to match stored rows against, as the screening pipeline does. */
638-
internal fun lookupForms(normalized: String): List<String> = listOfNotNull(normalized, PhoneIdentityCanonicalizer.nanpE164Fallback(normalized, phoneIdentityCanonicalizer.homeRegionIso))
640+
internal fun lookupForms(normalized: String): List<String> = phoneIdentityCanonicalizer.equivalentForms(normalized)
639641

640642
internal suspend fun warmScreeningCaches() = spamRepositoryImpl.warmScreeningCaches()
641643

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -134,19 +134,19 @@ data class CheckContext(
134134
/** Region-scoped sender-ID/numbering evidence for SMS only. */
135135
val senderProvenance: SenderProvenance? = null,
136136
/**
137-
* The +1 form of [number] when it is a national-format NANP number that
138-
* canonicalization left without "+" (see
139-
* [com.sysadmindoc.callshield.data.PhoneIdentityCanonicalizer.nanpE164Fallback]).
137+
* Other spellings the same line may be stored under: the +1 and national
138+
* forms of a NANP number libphonenumber rejects (see
139+
* [com.sysadmindoc.callshield.data.PhoneIdentityCanonicalizer.equivalentForms]).
140140
*/
141-
val e164Fallback: String? = null,
141+
val alternateForms: List<String> = emptyList(),
142142
) {
143143
/**
144144
* Forms to try when matching stored numbers exactly, canonical first.
145-
* Allow and block lookups both use it, so a number saved in either form
145+
* Allow and block lookups both use it, so a number saved in any form
146146
* is recognized the same way on both sides.
147147
*/
148148
val lookupForms: List<String>
149-
get() = listOfNotNull(number, e164Fallback)
149+
get() = listOf(number) + alternateForms
150150

151151
/**
152152
* Milliseconds remaining before the 5-second Android CallScreeningService

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

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ class BlocklistRepository(
3838
private val settingsRepository: SettingsRepository,
3939
private val normalizeNumber: (String) -> String,
4040
private val normalizeLogIdentity: (String) -> String,
41+
/**
42+
* Every stored spelling of a canonical number, canonical first. Screening
43+
* matches all of them, so an edit that only touched one form could leave
44+
* an older allow beating a newer block, or a removed block still in force.
45+
*/
46+
private val equivalentForms: (String) -> List<String> = { listOf(it) },
4147
private val invalidateWildcardCache: () -> Unit,
4248
private val invalidateKeywordCache: () -> Unit,
4349
private val invalidateHashWildcardCache: () -> Unit,
@@ -74,10 +80,10 @@ class BlocklistRepository(
7480
var blocked = false
7581
runInTransaction {
7682
if (cleanupExpired) cleanupExpiredTemporaryDecisions()
77-
val existingWhitelist = dao.findWhitelistEntry(normalized)
78-
val permanentAllowExists = expiresAt != null && existingWhitelist != null && existingWhitelist.expiresAt == null
83+
val existingWhitelist = equivalentForms(normalized).mapNotNull { dao.findWhitelistEntry(it) }
84+
val permanentAllowExists = expiresAt != null && existingWhitelist.any { it.expiresAt == null }
7985
if (!permanentAllowExists) {
80-
existingWhitelist?.let { dao.deleteWhitelistEntry(it) }
86+
existingWhitelist.forEach { dao.deleteWhitelistEntry(it) }
8187
when (val existing = dao.findByNumber(normalized)) {
8288
null -> {
8389
dao.insertNumber(
@@ -143,7 +149,11 @@ class BlocklistRepository(
143149
/** Undo a block by number string (e.g. an accidental blocked-log swipe). */
144150
suspend fun unblockByNumber(number: String) {
145151
val normalized = normalizeNumber(number)
146-
dao.findByNumber(normalized)?.let { unblockNumber(it) }
152+
if (normalized.isBlank()) return
153+
equivalentForms(normalized)
154+
.mapNotNull { dao.findByNumber(it) }
155+
.filter { it.isUserBlocked }
156+
.forEach { unblockNumber(it) }
147157
}
148158

149159
fun getAllWildcardRules(): Flow<List<WildcardRule>> = dao.getAllWildcardRules()
@@ -571,16 +581,18 @@ class BlocklistRepository(
571581
var whitelisted = false
572582
runInTransaction {
573583
cleanupExpiredTemporaryDecisions()
574-
val existingSpam = dao.findByNumber(normalized)
575-
val permanentUserBlock = existingSpam?.isUserBlocked == true && existingSpam.expiresAt == null
584+
val existingSpam = equivalentForms(normalized).mapNotNull { dao.findByNumber(it) }
585+
val permanentUserBlock = existingSpam.any { it.isUserBlocked && it.expiresAt == null }
576586
if (!(expiresAt != null && permanentUserBlock)) {
577-
if (expiresAt == null || existingSpam?.expiresAt != null) {
578-
when (val resolution = resolveSpamNumberForWhitelist(existingSpam)) {
579-
SpamNumberWhitelistResolution.None -> Unit
580-
is SpamNumberWhitelistResolution.Update -> dao.insertNumber(resolution.number)
581-
is SpamNumberWhitelistResolution.Delete -> dao.deleteNumber(resolution.number)
587+
existingSpam
588+
.filter { expiresAt == null || it.expiresAt != null }
589+
.forEach { row ->
590+
when (val resolution = resolveSpamNumberForWhitelist(row)) {
591+
SpamNumberWhitelistResolution.None -> Unit
592+
is SpamNumberWhitelistResolution.Update -> dao.insertNumber(resolution.number)
593+
is SpamNumberWhitelistResolution.Delete -> dao.deleteNumber(resolution.number)
594+
}
582595
}
583-
}
584596
dao.insertWhitelistEntry(
585597
WhitelistEntry(
586598
number = normalized,

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package com.sysadmindoc.callshield.data.repository
33
import android.content.Context
44
import androidx.datastore.preferences.core.Preferences
55
import com.sysadmindoc.callshield.data.CategoryCallPolicy
6-
import com.sysadmindoc.callshield.data.PhoneIdentityCanonicalizer
76
import com.sysadmindoc.callshield.data.SenderProvenance
87
import com.sysadmindoc.callshield.data.SenderProvenanceResolver
98
import com.sysadmindoc.callshield.data.checker.CheckContext
@@ -36,6 +35,8 @@ class SpamRepositoryImpl(
3635
private val normalizeSenderIdentity: (String) -> String,
3736
private val senderProvenanceResolver: SenderProvenanceResolver = SenderProvenanceResolver(),
3837
private val senderRegionIso: String? = null,
38+
/** Every stored spelling of a canonical number, canonical first. */
39+
private val equivalentForms: (String) -> List<String> = { listOf(it) },
3940
) {
4041
// isSpam() is the critical real-time path. Loading all prefixes,
4142
// wildcard rules, and keyword rules from Room on every call adds
@@ -175,7 +176,7 @@ class SpamRepositoryImpl(
175176
callerIdentity = callerIdentity,
176177
smsContextTrusted = smsContextTrusted,
177178
senderProvenance = senderProvenance,
178-
e164Fallback = PhoneIdentityCanonicalizer.nanpE164Fallback(normalized, senderRegionIso),
179+
alternateForms = equivalentForms(normalized).drop(1),
179180
)
180181

181182
val pipelineRun = CheckerPipeline.runWithDiagnostics(callChain, ctx)
@@ -265,7 +266,7 @@ class SpamRepositoryImpl(
265266
number = normalized,
266267
realtimeCall = false,
267268
prefs = prefs,
268-
e164Fallback = PhoneIdentityCanonicalizer.nanpE164Fallback(normalized, senderRegionIso),
269+
alternateForms = equivalentForms(normalized).drop(1),
269270
)
270271
return CheckerPipeline.traceAll(callChain, ctx)
271272
}

‎app/src/main/java/com/sysadmindoc/callshield/ui/MainViewModel.kt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,9 @@ class MainViewModel
237237

238238
fun observeSpamNumber(number: String): Flow<SpamNumber?> = repo.observeNumber(number)
239239

240+
/** Every stored spelling of [number], the way screening matches it. */
241+
fun lookupForms(number: String): List<String> = repo.lookupForms(number)
242+
240243
val userBlockedNumbers: StateFlow<List<SpamNumber>> =
241244
repo
242245
.getUserBlockedNumbers()

‎app/src/main/java/com/sysadmindoc/callshield/ui/screens/details/NumberDetailScreen.kt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ fun NumberDetailScreen(
7070
.collectAsStateWithLifecycle(initialValue = null)
7171
val userBlocked by viewModel.userBlockedNumbers.collectAsStateWithLifecycle()
7272

73-
val isBlocked = userBlocked.any { it.number == number }
73+
val forms = remember(number) { viewModel.lookupForms(number) }
74+
val isBlocked = userBlocked.any { it.number in forms }
7475
val callCount = numberCalls.count { it.isCall }
7576
val smsCount = numberCalls.count { !it.isCall }
7677
val firstSeen = numberCalls.minByOrNull { it.timestamp }?.timestamp
@@ -270,7 +271,8 @@ fun NumberDetailScreen(
270271
color = CatRed,
271272
onClick = {
272273
if (isBlocked) {
273-
userBlocked.find { it.number == number }?.let { viewModel.unblockNumber(it) }
274+
// By number, so a block saved in another spelling of it goes too.
275+
viewModel.unblockByNumber(number)
274276
hapticTick(context)
275277
Toast.makeText(context, numberUnblockedMessage, Toast.LENGTH_SHORT).show()
276278
} else {

‎app/src/main/java/com/sysadmindoc/callshield/ui/screens/lookup/LookupScreen.kt‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,6 @@ fun LookupScreen(viewModel: MainViewModel) {
141141
// rotation, and so the verdict stays bound to the number that was actually
142142
// checked rather than to whatever is currently typed in the field.
143143
val outcome by viewModel.lookupOutcome.collectAsStateWithLifecycle()
144-
val userBlocked by viewModel.userBlockedNumbers.collectAsStateWithLifecycle()
145144
val result = outcome?.result
146145
val trace = outcome?.trace
147146
val checkedNumber = outcome?.number
@@ -338,7 +337,6 @@ fun LookupScreen(viewModel: MainViewModel) {
338337
}
339338
val probabilistic = lookupResult.isSpam && BlockReasoning.isProbabilistic(lookupResult.reasonCode)
340339
val userRule = BlockReasoning.isUserRule(lookupResult.reasonCode)
341-
val userRuleEntry = userBlocked.firstOrNull { it.number == resultNumber }
342340

343341
SectionHeader(
344342
stringResource(R.string.lookup_risk_assessment),
@@ -451,8 +449,8 @@ fun LookupScreen(viewModel: MainViewModel) {
451449
onClick = {
452450
when {
453451
userRule -> {
454-
userRuleEntry?.let { viewModel.unblockNumber(it) }
455-
?: viewModel.unblockByNumber(resultNumber)
452+
// By number, so a block saved in another spelling of it goes too.
453+
viewModel.unblockByNumber(resultNumber)
456454
hapticTick(context)
457455
scope.launch {
458456
snackbarHostState.showSnackbar(resources.getString(R.string.lookup_rule_removed))

0 commit comments

Comments
 (0)