Skip to content

Commit 680ee83

Browse files
SysAdminDocSysAdminDoc
committed
fix(screening): match stored numbers in +1 form for national caller IDs
canonicalizePhone adds "+1" only when PhoneNumberUtils.formatNumberToE164 accepts the number, and that returns null for anything libphonenumber calls invalid: fictional exchanges, unassigned area codes, and area codes newer than the phone's metadata. Such a caller ID delivered in national form stayed plus-less and missed the E.164-keyed spam database and prefix list. On the API 35 emulator a call from 16495550123 rang through while Lookup flagged +16495550123 as a prefix match. On a NANP home region, a national-format NANP number now carries its +1 form as CheckContext.e164Fallback, and every exact stored-number lookup tries both forms: the allow side (manual whitelist, temporary allow, the STIR trust database probe) as well as the block side (user blocklist, database, prefix, database prefix expansion), so an allow saved in either form still wins. The outgoing call hold uses the same forms. Stored identities keep their canonical form, so nothing is migrated. Tests: PhoneIdentityCanonicalizerTest gains the fallback's pure cases. NationalFormatLookupTest (new, Robolectric) asserts the scenario number stays plus-less, then checks the database, prefix, E.164 user block and E.164 allow cases; dropping the fallback fails the four behaviour cases, and leaving only the whitelist on the canonical form fails the allow case. No existing test changed.
1 parent e19299e commit 680ee83

9 files changed

Lines changed: 178 additions & 12 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,14 @@ All notable changes to CallShield will be documented in this file.
2424
payslip, a part-time shift rate, a house-cleaning job or a cashback offer
2525
stays clean.
2626

27+
- A caller ID that arrives without its +1, such as 649 555 0123, now matches
28+
the spam database and the prefix list, which store numbers in +1 form.
29+
When Android's number library didn't recognize the exchange or the area
30+
code (a made-up number, or an area code newer than the phone's copy of the
31+
library), the number kept its bare form and slipped past both. Numbers you
32+
blocked or allowed match either form too, and so does the outgoing call
33+
hold.
34+
2735
- Your call category rules now reach more suspicious-pattern blocks. A call
2836
flagged as possible neighbor spoofing counts as a scam, one from an active
2937
campaign range as a robocall, and a toll-free one as a telemarketer, so

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,5 +112,30 @@ class PhoneIdentityCanonicalizer internal constructor(
112112
?.trim()
113113
?.uppercase(Locale.ROOT)
114114
?.takeIf { it.length == 2 && it.all { character -> character in 'A'..'Z' } }
115+
116+
/**
117+
* The +1 form of a national-format NANP number on a phone whose home
118+
* region is in the NANP, or null. [canonicalizePhone] leaves such a
119+
* number without "+" whenever libphonenumber rejects it (a fictional
120+
* exchange, an unassigned area code, or one newer than the phone's
121+
* metadata), while the spam database and prefix list are keyed by
122+
* E.164. Lookups try this form too; stored identities keep theirs.
123+
*/
124+
fun nanpE164Fallback(
125+
canonical: String,
126+
homeRegionIso: String?,
127+
): String? =
128+
when {
129+
normalizeRegion(homeRegionIso) !in NANP_REGIONS -> null
130+
NANP_NATIONAL.matches(canonical) -> "+1$canonical"
131+
NANP_WITH_TRUNK_PREFIX.matches(canonical) -> "+$canonical"
132+
else -> null
133+
}
134+
135+
// Every region that shares country code 1.
136+
private val NANP_REGIONS =
137+
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")
138+
private val NANP_NATIONAL = Regex("[2-9][0-9]{9}")
139+
private val NANP_WITH_TRUNK_PREFIX = Regex("1[2-9][0-9]{9}")
115140
}
116141
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,9 @@ class SpamRepository(
634634

635635
internal suspend fun findExactSpamNumber(normalized: String) = spamRepositoryImpl.findByNumberInternal(normalized)
636636

637+
/** 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))
639+
637640
internal suspend fun warmScreeningCaches() = spamRepositoryImpl.warmScreeningCaches()
638641

639642
internal suspend fun hasActiveWhitelistEntry(normalized: String): Boolean =

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

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ internal class WhitelistChecker(
8080
override val name = "manual_whitelist"
8181

8282
override suspend fun check(ctx: CheckContext): BlockResult? {
83-
val entry = repo.findWhitelistEntryInternal(ctx.number) ?: return null
83+
val entry = ctx.lookupForms.firstNotNullOfOrNull { repo.findWhitelistEntryInternal(it) } ?: return null
8484
return BlockResult.allow(
8585
matchSource = if (entry.isEmergency) "emergency_contact" else "manual_whitelist",
8686
ruleId = entry.id,
@@ -192,8 +192,10 @@ internal class StirShakenTrustChecker(
192192

193193
override suspend fun check(ctx: CheckContext): BlockResult? {
194194
if (ctx.verificationStatus != VERIFICATION_STATUS_PASSED) return null
195-
val trending = isTrendingNow(ctx.prefs, ctx.number, wallClock())
196-
return decidePure(ctx.verificationStatus, repo.findByNumberInternal(ctx.number), trending = trending)
195+
val now = wallClock()
196+
val trending = ctx.lookupForms.any { isTrendingNow(ctx.prefs, it, now) }
197+
val entry = ctx.lookupForms.firstNotNullOfOrNull { repo.findByNumberInternal(it) }
198+
return decidePure(ctx.verificationStatus, entry, trending = trending)
197199
}
198200

199201
companion object {
@@ -338,7 +340,7 @@ internal class TemporaryAllowChecker(
338340
override val name = "temporary_allow"
339341

340342
override suspend fun check(ctx: CheckContext): BlockResult? {
341-
repo.findTemporaryWhitelistEntryInternal(ctx.number) ?: return null
343+
ctx.lookupForms.firstNotNullOfOrNull { repo.findTemporaryWhitelistEntryInternal(it) } ?: return null
342344
return BlockResult.allow("temporary_allow")
343345
}
344346
}
@@ -378,8 +380,8 @@ internal class UserBlocklistChecker(
378380
override val name = "user_blocklist"
379381

380382
override suspend fun check(ctx: CheckContext): BlockResult? {
381-
val entry = repo.findByNumberInternal(ctx.number)
382-
return if (entry?.isUserBlocked == true) {
383+
val entry = ctx.lookupForms.firstNotNullOfOrNull { form -> repo.findByNumberInternal(form)?.takeIf { it.isUserBlocked } }
384+
return if (entry != null) {
383385
val source = if (entry.expiresAt == null) "user_blocklist" else "temporary_block"
384386
val description =
385387
if (entry.expiresAt == null) {
@@ -401,8 +403,8 @@ internal class DatabaseChecker(
401403
override val name = "database"
402404

403405
override suspend fun check(ctx: CheckContext): BlockResult? {
404-
val entry = repo.findByNumberInternal(ctx.number)
405-
return if (entry != null && !entry.isUserBlocked) {
406+
val entry = ctx.lookupForms.firstNotNullOfOrNull { form -> repo.findByNumberInternal(form)?.takeUnless { it.isUserBlocked } }
407+
return if (entry != null) {
406408
BlockResult.block("database", entry.type, entry.description)
407409
} else {
408410
null
@@ -424,7 +426,7 @@ internal class DbPrefixExpansionChecker(
424426
override suspend fun isEnabled(ctx: CheckContext): Boolean = ctx.prefs[SpamRepository.KEY_DB_PREFIX_EXPANSION] ?: false
425427

426428
override suspend fun check(ctx: CheckContext): BlockResult? {
427-
if (!repo.hasDbPrefixMatch(ctx.number)) return null
429+
if (ctx.lookupForms.none { repo.hasDbPrefixMatch(it) }) return null
428430
return BlockResult.block(
429431
"db_prefix_expansion",
430432
description = ctx.appContext.getString(R.string.block_reason_db_prefix),
@@ -444,8 +446,9 @@ internal class PrefixChecker(
444446
override val name = "prefix"
445447

446448
override suspend fun check(ctx: CheckContext): BlockResult? {
449+
val forms = ctx.lookupForms
447450
for (prefix in repo.getPrefixesCachedInternal()) {
448-
if (ctx.number.startsWith(prefix.prefix)) {
451+
if (forms.any { it.startsWith(prefix.prefix) }) {
449452
return BlockResult.block("prefix", prefix.type, prefix.description)
450453
}
451454
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,21 @@ data class CheckContext(
133133
val smsContextTrusted: Boolean = false,
134134
/** Region-scoped sender-ID/numbering evidence for SMS only. */
135135
val senderProvenance: SenderProvenance? = null,
136+
/**
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]).
140+
*/
141+
val e164Fallback: String? = null,
136142
) {
143+
/**
144+
* 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
146+
* is recognized the same way on both sides.
147+
*/
148+
val lookupForms: List<String>
149+
get() = listOfNotNull(number, e164Fallback)
150+
137151
/**
138152
* Milliseconds remaining before the 5-second Android CallScreeningService
139153
* deadline. Effective budget is 4500 ms — 500 ms buffer for the response

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ 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
67
import com.sysadmindoc.callshield.data.SenderProvenance
78
import com.sysadmindoc.callshield.data.SenderProvenanceResolver
89
import com.sysadmindoc.callshield.data.checker.CheckContext
@@ -174,6 +175,7 @@ class SpamRepositoryImpl(
174175
callerIdentity = callerIdentity,
175176
smsContextTrusted = smsContextTrusted,
176177
senderProvenance = senderProvenance,
178+
e164Fallback = PhoneIdentityCanonicalizer.nanpE164Fallback(normalized, senderRegionIso),
177179
)
178180

179181
val pipelineRun = CheckerPipeline.runWithDiagnostics(callChain, ctx)
@@ -263,6 +265,7 @@ class SpamRepositoryImpl(
263265
number = normalized,
264266
realtimeCall = false,
265267
prefs = prefs,
268+
e164Fallback = PhoneIdentityCanonicalizer.nanpE164Fallback(normalized, senderRegionIso),
266269
)
267270
return CheckerPipeline.traceAll(callChain, ctx)
268271
}

‎app/src/main/java/com/sysadmindoc/callshield/service/CallShieldRedirectionService.kt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,11 @@ class CallShieldRedirectionService : CallRedirectionService() {
8383
OutgoingCallGuard.decide(
8484
number = number,
8585
nowElapsed = SystemClock.elapsedRealtime(),
86-
trusted = { n -> repository.hasActiveWhitelistEntry(n) || SpamHeuristics.isInContacts(context, n) },
86+
trusted = { n ->
87+
repository.lookupForms(n).any { repository.hasActiveWhitelistEntry(it) } || SpamHeuristics.isInContacts(context, n)
88+
},
8789
listed = { n ->
88-
repository.findExactSpamNumber(n)?.let { row ->
90+
repository.lookupForms(n).firstNotNullOfOrNull { repository.findExactSpamNumber(it) }?.let { row ->
8991
if (row.isUserBlocked) OutgoingCallGuard.Reason.USER_BLOCKLIST else OutgoingCallGuard.Reason.DATABASE
9092
}
9193
},

‎app/src/test/java/com/sysadmindoc/callshield/data/NationalFormatLookupTest.kt‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
package com.sysadmindoc.callshield.data
2+
3+
import android.content.Context
4+
import android.telephony.TelephonyManager
5+
import androidx.test.core.app.ApplicationProvider
6+
import com.sysadmindoc.callshield.data.model.SpamNumber
7+
import com.sysadmindoc.callshield.data.model.SpamPrefix
8+
import com.sysadmindoc.callshield.data.model.WhitelistEntry
9+
import kotlinx.coroutines.runBlocking
10+
import org.junit.After
11+
import org.junit.Assert.assertEquals
12+
import org.junit.Assert.assertFalse
13+
import org.junit.Before
14+
import org.junit.Test
15+
import org.junit.runner.RunWith
16+
import org.robolectric.RobolectricTestRunner
17+
import org.robolectric.Shadows.shadowOf
18+
import org.robolectric.annotation.Config
19+
20+
/**
21+
* A caller ID in national form that libphonenumber rejects (here a fictional
22+
* Turks and Caicos exchange) keeps no "+" after canonicalization, while the
23+
* spam database and prefix list are keyed by E.164.
24+
*/
25+
@RunWith(RobolectricTestRunner::class)
26+
@Config(sdk = [34])
27+
class NationalFormatLookupTest {
28+
private val context: Context = ApplicationProvider.getApplicationContext()
29+
private lateinit var fixture: IsolatedRepositoryFixture
30+
31+
@Before
32+
fun setUp() {
33+
shadowOf(context.getSystemService(TelephonyManager::class.java)).setSimCountryIso("us")
34+
fixture = IsolatedRepositoryFixture(context)
35+
}
36+
37+
@After
38+
fun tearDown() {
39+
fixture.close()
40+
}
41+
42+
@Test
43+
fun `the scenario really leaves the number without a plus`() {
44+
// Without this the other cases would pass for the wrong reason.
45+
assertEquals("6495550123", fixture.repository.normalizeNumber("649-555-0123"))
46+
assertEquals("16495550123", fixture.repository.normalizeNumber("1 649 555 0123"))
47+
assertEquals(listOf("6495550123", "+16495550123"), fixture.repository.lookupForms("6495550123"))
48+
}
49+
50+
@Test
51+
fun `a national-format number matches its E164 database row`() =
52+
runBlocking {
53+
fixture.dao.insertNumbers(listOf(SpamNumber(number = "+16495550123", type = "robocall", reports = 9, source = "github")))
54+
55+
assertEquals("database", fixture.repository.isSpam("6495550123").matchSource)
56+
assertEquals("database", fixture.repository.isSpam("16495550123").matchSource)
57+
}
58+
59+
@Test
60+
fun `a national-format number matches the prefix list`() =
61+
runBlocking {
62+
fixture.dao.insertPrefixes(listOf(SpamPrefix(prefix = "+1649", type = "wangiri_scam", description = "Turks and Caicos")))
63+
64+
assertEquals("prefix", fixture.repository.isSpam("6495550199").matchSource)
65+
}
66+
67+
@Test
68+
fun `a block saved in E164 form applies to the national form`() =
69+
runBlocking {
70+
fixture.dao.insertNumbers(
71+
listOf(SpamNumber(number = "+16495550177", type = "spam", source = "user", isUserBlocked = true)),
72+
)
73+
74+
assertEquals("user_blocklist", fixture.repository.isSpam("6495550177").matchSource)
75+
}
76+
77+
@Test
78+
fun `an allow saved in E164 form still wins over a database match`() =
79+
runBlocking {
80+
// Matching only the block side in both forms would newly block a
81+
// number the user had allowed.
82+
fixture.dao.insertNumbers(listOf(SpamNumber(number = "+16495550123", type = "robocall", reports = 9, source = "github")))
83+
fixture.dao.insertWhitelistEntry(WhitelistEntry(number = "+16495550123"))
84+
85+
val result = fixture.repository.isSpam("6495550123")
86+
87+
assertFalse(result.isSpam)
88+
assertEquals("manual_whitelist", result.matchSource)
89+
}
90+
}

‎app/src/test/java/com/sysadmindoc/callshield/data/PhoneIdentityCanonicalizerTest.kt‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package com.sysadmindoc.callshield.data
22

33
import org.junit.Assert.assertEquals
44
import org.junit.Assert.assertNotEquals
5+
import org.junit.Assert.assertNull
56
import org.junit.Assert.assertTrue
67
import org.junit.Test
78

@@ -27,6 +28,23 @@ class PhoneIdentityCanonicalizerTest {
2728
assertEquals("2125551234", canonicalizer.canonicalizePhone("212-555-1234"))
2829
}
2930

31+
@Test
32+
fun `a national NANP number gets a +1 lookup form only on a NANP home region`() {
33+
assertEquals("+16495550123", PhoneIdentityCanonicalizer.nanpE164Fallback("6495550123", "US"))
34+
assertEquals("+16495550123", PhoneIdentityCanonicalizer.nanpE164Fallback("16495550123", "ca"))
35+
assertEquals("+18765550123", PhoneIdentityCanonicalizer.nanpE164Fallback("8765550123", "JM"))
36+
37+
// Already E.164, not NANP-shaped (area codes start 2-9), wrong length,
38+
// or a home region outside the NANP: nothing to add.
39+
assertNull(PhoneIdentityCanonicalizer.nanpE164Fallback("+16495550123", "US"))
40+
assertNull(PhoneIdentityCanonicalizer.nanpE164Fallback("0495550123", "US"))
41+
assertNull(PhoneIdentityCanonicalizer.nanpE164Fallback("10495550123", "US"))
42+
assertNull(PhoneIdentityCanonicalizer.nanpE164Fallback("5550123", "US"))
43+
assertNull(PhoneIdentityCanonicalizer.nanpE164Fallback("116495550123", "US"))
44+
assertNull(PhoneIdentityCanonicalizer.nanpE164Fallback("6495550123", "GB"))
45+
assertNull(PhoneIdentityCanonicalizer.nanpE164Fallback("6495550123", null))
46+
}
47+
3048
@Test
3149
fun `existing E164 numbers and short codes bypass regional formatting`() {
3250
var formatterCalls = 0

0 commit comments

Comments
 (0)