fix: catch SecurityException when location permission revoked while backgrounded#663
Conversation
…ackgrounded Fixes COLUMBA-5R. When a user grants location permission, backgrounds the app, then revokes the permission from Android Settings, the ViewModel retains stale hasLocationPermission=true state. On some OEMs (observed on Realme 8s 5G / Android 13) the process isn't killed on revocation, so MapLibre's LocationManager.getLastKnownLocation() throws a SecurityException. Three changes: - Re-check actual Android permission in ON_RESUME lifecycle handler to sync ViewModel state with reality after returning from background - Wrap both location component activation paths in try-catch as a TOCTOU safety net (permission can be revoked between our check and MapLibre's internal LocationManager call) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Greptile SummaryThis PR fixes a fatal Key observations:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Android OS
participant LifecycleObserver
participant MapLibre
participant ViewModel
User->>Android OS: Grant location permission
Android OS-->>ViewModel: onPermissionResult(true)
ViewModel-->>MapLibre: activateLocationComponent (try/catch SecurityException)
User->>Android OS: Background app
User->>Android OS: Revoke location permission in Settings
User->>Android OS: Return to app (ON_RESUME)
Android OS->>LifecycleObserver: ON_RESUME event
LifecycleObserver->>Android OS: hasPermission() → false
LifecycleObserver->>MapLibre: locationComponent.isLocationComponentEnabled = false
LifecycleObserver->>ViewModel: onPermissionResult(false)
LifecycleObserver->>Android OS: removeLocationUpdates (non-GMS only)
LifecycleObserver->>MapLibre: view.onResume() ⚠️ no try/catch here
Note over LifecycleObserver,MapLibre: TOCTOU window — SecurityException<br/>from view.onResume() is still uncaught
Last reviewed commit: e239543 |
- Add viewModel.onPermissionResult(false) in onMapStyleLoaded catch block to sync ViewModel state when SecurityException is caught - Reorder ON_RESUME handler to check permission BEFORE view.onResume() to prevent MapLibre from resuming the location engine with revoked permission - Clean up platformLocationListener when permission revoked on resume to prevent stale GPS queries - Disable MapLibre location component before calling view.onResume() when permission has been revoked Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| } | ||
| platformLocationListener = null | ||
| } | ||
| view.onResume() |
There was a problem hiding this comment.
Residual TOCTOU window around view.onResume()
The pre-check (lines 504–517) correctly guards the case where permission is already revoked before ON_RESUME fires. However, when both state.hasLocationPermission and stillHasPermission are true (permission appears valid at line 504), the if-block is skipped and view.onResume() is called with no exception handling. If MapLibre's internal location engine calls LocationManager.requestLocationUpdates() during onResume() and the OS revokes the permission in the narrow window between lines 504 and 518, a SecurityException will be thrown uncaught here — the same crash this PR sets out to fix.
The two other code paths (onMapStyleLoaded catch at line 253 and the LaunchedEffect catch at line 440) both use try-catch precisely for this TOCTOU reason. The view.onResume() call should be given the same treatment:
try {
view.onResume()
} catch (e: SecurityException) {
Log.w("MapScreen", "SecurityException during onResume, disabling location component", e)
mapLibreMap?.locationComponent?.let { lc ->
if (lc.isLocationComponentActivated) {
lc.isLocationComponentEnabled = false
}
}
viewModel.onPermissionResult(false)
platformLocationListener?.let {
LocationCompat.removeLocationUpdates(context, it)
}
platformLocationListener = null
}Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/main/java/com/lxmf/messenger/ui/screens/MapScreen.kt
Line: 518
Comment:
**Residual TOCTOU window around `view.onResume()`**
The pre-check (lines 504–517) correctly guards the case where permission is already revoked before `ON_RESUME` fires. However, when both `state.hasLocationPermission` and `stillHasPermission` are `true` (permission appears valid at line 504), the if-block is skipped and `view.onResume()` is called with no exception handling. If MapLibre's internal location engine calls `LocationManager.requestLocationUpdates()` during `onResume()` and the OS revokes the permission in the narrow window between lines 504 and 518, a `SecurityException` will be thrown uncaught here — the same crash this PR sets out to fix.
The two other code paths (`onMapStyleLoaded` catch at line 253 and the `LaunchedEffect` catch at line 440) both use try-catch precisely for this TOCTOU reason. The `view.onResume()` call should be given the same treatment:
```kotlin
try {
view.onResume()
} catch (e: SecurityException) {
Log.w("MapScreen", "SecurityException during onResume, disabling location component", e)
mapLibreMap?.locationComponent?.let { lc ->
if (lc.isLocationComponentActivated) {
lc.isLocationComponentEnabled = false
}
}
viewModel.onPermissionResult(false)
platformLocationListener?.let {
LocationCompat.removeLocationUpdates(context, it)
}
platformLocationListener = null
}
```
How can I resolve this? If you propose a fix, please make it concise.| platformLocationListener?.let { | ||
| LocationCompat.removeLocationUpdates(context, it) | ||
| } | ||
| platformLocationListener = null |
There was a problem hiding this comment.
GMS fusedLocationClient not stopped on permission revocation
The cleanup here only removes the platform LocationListener (the non-GMS path). Per the comment at line 270 ("Platform LocationListener for cleanup when not using GMS"), platformLocationListener is null when useGms = true, making this block a no-op for GMS devices.
On the GMS path, startLocationUpdates registers a callback with fusedLocationClient. When permission is revoked, that callback is never removed here — the fusedLocationClient reference and its registered callback continue to exist until composition leaves. While FusedLocationProviderClient will eventually stop delivering updates on its own after revocation, explicitly removing the callback is the right pattern and mirrors what the non-GMS branch already does.
Consider also removing GMS callbacks in this revocation path (requires tracking the LocationCallback returned from startLocationUpdates, similar to how platformLocationListener tracks the non-GMS callback).
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/main/java/com/lxmf/messenger/ui/screens/MapScreen.kt
Line: 513-516
Comment:
**GMS `fusedLocationClient` not stopped on permission revocation**
The cleanup here only removes the platform `LocationListener` (the non-GMS path). Per the comment at line 270 ("Platform LocationListener for cleanup when **not** using GMS"), `platformLocationListener` is `null` when `useGms = true`, making this block a no-op for GMS devices.
On the GMS path, `startLocationUpdates` registers a callback with `fusedLocationClient`. When permission is revoked, that callback is never removed here — the `fusedLocationClient` reference and its registered callback continue to exist until composition leaves. While `FusedLocationProviderClient` will eventually stop delivering updates on its own after revocation, explicitly removing the callback is the right pattern and mirrors what the non-GMS branch already does.
Consider also removing GMS callbacks in this revocation path (requires tracking the `LocationCallback` returned from `startLocationUpdates`, similar to how `platformLocationListener` tracks the non-GMS callback).
How can I resolve this? If you propose a fix, please make it concise.
Summary
Fixes COLUMBA-5R — fatal
SecurityExceptioncrash on the Map screen.hasLocationPermission=true(stale), but MapLibre callsLocationManager.getLastKnownLocation()which throwsSecurityException.onMapStyleLoadedand theLaunchedEffectlocation component activation paths now catchSecurityExceptionas a TOCTOU guardObserved on Realme 8s 5G / Android 13 — some OEMs don't kill the app process on permission revocation.
Test plan
MapViewModelTestpasses (permission state management)🤖 Generated with Claude Code