Skip to content

Commit 667f6d4

Browse files
committed
test(rcs): cover the URL check throwing on the listener's warning path
The crash fix that wrapped launchUrlSafetyWarning's link check in try/catch had no test, because nothing could make the check throw. RcsNotificationListener.urlCheck is now a replaceable seam, the same shape as CommunityContributor.transport, defaulting to UrlSafetyChecker.checkSmsBody with the same arguments. The new test in RcsNotificationListenerTest posts a chat notification with a link through onNotificationPosted while the check throws, and fails if anything reaches the default uncaught exception handler. A control run with the try/catch removed fails it with both escaped throws. The test is added to an existing file; no existing test changed.
1 parent 6fa0e0e commit 667f6d4

3 files changed

Lines changed: 33 additions & 10 deletions

File tree

‎README.md‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
<p align="center">
1313
<a href="https://github.com/SysAdminDoc/CallShield/releases/latest"><img src="https://img.shields.io/github/v/release/SysAdminDoc/CallShield?style=flat-square&color=a6e3a1" alt="Release"></a>
1414
<img src="https://img.shields.io/badge/Spam%20Numbers-51%2C403-f38ba8?style=flat-square" alt="51,403 Numbers">
15-
<img src="https://img.shields.io/badge/JVM%20unit%20tests-2054-94e2d5?style=flat-square" alt="2054 JVM unit tests">
15+
<img src="https://img.shields.io/badge/JVM%20unit%20tests-2055-94e2d5?style=flat-square" alt="2055 JVM unit tests">
1616
<img src="https://img.shields.io/badge/Android-10%2B-89b4fa?style=flat-square" alt="Android 10+">
1717
<img src="https://img.shields.io/badge/License-MIT-cba6f7?style=flat-square" alt="MIT License">
1818
<img src="https://img.shields.io/badge/API%20Keys-None-fab387?style=flat-square" alt="No required API keys">
@@ -592,12 +592,12 @@ RELEASE_KEY_PASSWORD=...
592592
## Testing
593593

594594
```bash
595-
./gradlew testDebugUnitTest # 2054 tests
595+
./gradlew testDebugUnitTest # 2055 tests
596596
./gradlew connectedDebugAndroidTest -Pandroid.testInstrumentationRunnerArguments.class=com.sysadmindoc.callshield.platform.TargetSdkBehaviorSmokeTest
597597
./gradlew verifyPipelineTests # Cloudflare Worker (node) + data-pipeline and translation checks (python)
598598
```
599599

600-
The suite is **2054 total JVM unit tests**, run with Robolectric wherever a screen, a service or the database is involved.
600+
The suite is **2055 total JVM unit tests**, run with Robolectric wherever a screen, a service or the database is involved.
601601

602602
Two GitHub workflows run without building the app. **Validation** runs the Worker and
603603
pipeline suites on every push except report-only ones (`run-pipeline-tests.ps1 -CorrectnessOnly`),
@@ -634,7 +634,7 @@ language in [issue #7](https://github.com/SysAdminDoc/CallShield/issues/7).
634634
| Community API | Cloudflare Workers |
635635
| URL Safety | Local spam-domain data, with optional PhishTank and OpenPhish |
636636
| Verification | Local Gradle, lint, and release-artifact checks |
637-
| Tests | 2054 JVM unit tests (JUnit) |
637+
| Tests | 2055 JVM unit tests (JUnit) |
638638
| Strings | 1794 string resources and 49 plural groups (translation-ready) |
639639
| Accessibility | 100+ content descriptions, 48dp touch targets |
640640
| Min SDK | 29 (Android 10) |

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -326,12 +326,7 @@ class RcsNotificationListener : NotificationListenerService() {
326326
// Best-effort, as in SmsReceiver. This scope has no exception
327327
// handler, so a throw here would take the whole app down.
328328
try {
329-
val malicious =
330-
UrlSafetyChecker.checkSmsBody(
331-
body,
332-
stripQuery = stripQuery,
333-
allowRemoteLookup = allowRemoteLookup,
334-
)
329+
val malicious = urlCheck(body, stripQuery, allowRemoteLookup)
335330
if (malicious.isNotEmpty()) {
336331
val threats = urlThreatLabels(applicationContext, malicious)
337332
if (source.category == NotificationScreeningCategory.RCS && filterAsciiDigits(sender).length >= 7) {
@@ -411,6 +406,10 @@ class RcsNotificationListener : NotificationListenerService() {
411406
}
412407

413408
companion object {
409+
/** The link check behind the URL warning. Tests replace it to make it throw. */
410+
internal var urlCheck: suspend (body: String, stripQuery: Boolean, allowRemoteLookup: Boolean) -> List<UrlSafetyChecker.UrlCheckResult> =
411+
{ body, stripQuery, allowRemoteLookup -> UrlSafetyChecker.checkSmsBody(body, stripQuery, allowRemoteLookup) }
412+
414413
/**
415414
* Logs a message this listener flagged. The SMS receiver has usually
416415
* logged an SMS already, and [SpamRepository.logFlaggedText] logs only

‎app/src/test/java/com/sysadmindoc/callshield/service/RcsNotificationListenerTest.kt‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ import org.robolectric.Robolectric
3030
import org.robolectric.RobolectricTestRunner
3131
import org.robolectric.Shadows.shadowOf
3232
import org.robolectric.annotation.Config
33+
import java.util.concurrent.CopyOnWriteArrayList
34+
import java.util.concurrent.CountDownLatch
3335

3436
/** Texts that only reach CallShield as a messaging app's notification, as RCS chats do. */
3537
@RunWith(RobolectricTestRunner::class)
@@ -81,6 +83,28 @@ class RcsNotificationListenerTest {
8183
}
8284
}
8385

86+
@Test
87+
fun `a link check that throws doesn't take the app down`() {
88+
val crashes = CopyOnWriteArrayList<Throwable>()
89+
val checked = CountDownLatch(1)
90+
val previousHandler = Thread.getDefaultUncaughtExceptionHandler()
91+
val realCheck = RcsNotificationListener.urlCheck
92+
Thread.setDefaultUncaughtExceptionHandler { _, e -> crashes += e }
93+
RcsNotificationListener.urlCheck = { _, _, _ ->
94+
checked.countDown()
95+
error("threat feed unreadable")
96+
}
97+
try {
98+
postUntil(chatNotification("Here are the notes from today https://example.com/notes", sender = "Dana")) { checked.count == 0L }
99+
// An escaped throw reaches the handler right after the check, on the listener's thread.
100+
Thread.sleep(500L)
101+
assertEquals(emptyList<Throwable>(), crashes.toList())
102+
} finally {
103+
RcsNotificationListener.urlCheck = realCheck
104+
Thread.setDefaultUncaughtExceptionHandler(previousHandler)
105+
}
106+
}
107+
84108
@Test
85109
fun `a meeting app's background Connected notification doesn't start meeting mode, its call does`() {
86110
MeetingModeRegistry.clear()

0 commit comments

Comments
 (0)