Fix data safety, security, and coroutine correctness
- Wrap deleteAll+insertAll in Room transaction to prevent data loss
on crash during refresh
- Add CancellationException rethrow in ShelterRepository and
MapCacheManager to preserve structured concurrency
- Close OkHttp response body on error paths (response.use{})
- Add ZIP bomb protection (10MB cap) in GeoJSON parser
- Add per-feature error handling — skip malformed records instead of
losing all shelters
- Validate coordinates within Norway's bounding box
- Enforce HTTPS-only via network_security_config (remove cleartext
allowance for tile.openstreetmap.org)
- Disable android:allowBackup to prevent DB extraction via ADB
- Strip Log.v/d/i in release builds via ProGuard to prevent location
data leakage
- Restore map position in MapCacheManager.finally block on
cancellation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
9639ad44f4
commit
e93273bff4
6 changed files with 136 additions and 55 deletions
7
app/proguard-rules.pro
vendored
7
app/proguard-rules.pro
vendored
|
|
@ -10,3 +10,10 @@
|
||||||
# OkHttp
|
# OkHttp
|
||||||
-dontwarn okhttp3.**
|
-dontwarn okhttp3.**
|
||||||
-dontwarn okio.**
|
-dontwarn okio.**
|
||||||
|
|
||||||
|
# Strip verbose/debug/info logs in release builds (prevent location data leakage)
|
||||||
|
-assumenosideeffects class android.util.Log {
|
||||||
|
public static int v(...);
|
||||||
|
public static int d(...);
|
||||||
|
public static int i(...);
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -12,7 +12,7 @@
|
||||||
|
|
||||||
<application
|
<application
|
||||||
android:name=".TilfluktsromApp"
|
android:name=".TilfluktsromApp"
|
||||||
android:allowBackup="true"
|
android:allowBackup="false"
|
||||||
android:icon="@mipmap/ic_launcher"
|
android:icon="@mipmap/ic_launcher"
|
||||||
android:label="@string/app_name"
|
android:label="@string/app_name"
|
||||||
android:supportsRtl="true"
|
android:supportsRtl="true"
|
||||||
|
|
|
||||||
|
|
@ -2,7 +2,9 @@ package no.naiv.tilfluktsrom.data
|
||||||
|
|
||||||
import android.content.Context
|
import android.content.Context
|
||||||
import android.util.Log
|
import android.util.Log
|
||||||
|
import kotlinx.coroutines.CancellationException
|
||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
|
import kotlinx.coroutines.NonCancellable
|
||||||
import kotlinx.coroutines.delay
|
import kotlinx.coroutines.delay
|
||||||
import kotlinx.coroutines.withContext
|
import kotlinx.coroutines.withContext
|
||||||
import org.osmdroid.util.GeoPoint
|
import org.osmdroid.util.GeoPoint
|
||||||
|
|
@ -68,8 +70,12 @@ class MapCacheManager(private val context: Context) {
|
||||||
longitude: Double,
|
longitude: Double,
|
||||||
onProgress: (Float) -> Unit = {}
|
onProgress: (Float) -> Unit = {}
|
||||||
): Boolean = withContext(Dispatchers.Main) {
|
): Boolean = withContext(Dispatchers.Main) {
|
||||||
|
// Save map state so we can restore it even on cancellation
|
||||||
|
val savedZoom = mapView.zoomLevelDouble
|
||||||
|
val savedCenter = mapView.mapCenter
|
||||||
|
|
||||||
try {
|
try {
|
||||||
Log.i(TAG, "Seeding tile cache for area around $latitude, $longitude")
|
Log.d(TAG, "Seeding tile cache around user location")
|
||||||
|
|
||||||
val totalSteps = CACHE_ZOOM_LEVELS.size * GRID_SIZE * GRID_SIZE
|
val totalSteps = CACHE_ZOOM_LEVELS.size * GRID_SIZE * GRID_SIZE
|
||||||
var step = 0
|
var step = 0
|
||||||
|
|
@ -86,22 +92,16 @@ class MapCacheManager(private val context: Context) {
|
||||||
(2 * CACHE_RADIUS_DEGREES * col) / (GRID_SIZE - 1)
|
(2 * CACHE_RADIUS_DEGREES * col) / (GRID_SIZE - 1)
|
||||||
|
|
||||||
mapView.controller.setCenter(GeoPoint(lat, lon))
|
mapView.controller.setCenter(GeoPoint(lat, lon))
|
||||||
// Force a layout pass so tiles are requested
|
|
||||||
mapView.invalidate()
|
mapView.invalidate()
|
||||||
|
|
||||||
step++
|
step++
|
||||||
onProgress(step.toFloat() / totalSteps)
|
onProgress(step.toFloat() / totalSteps)
|
||||||
|
|
||||||
// Brief delay to allow tile loading to start
|
|
||||||
delay(300)
|
delay(300)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Restore to user's location
|
|
||||||
mapView.controller.setZoom(14.0)
|
|
||||||
mapView.controller.setCenter(GeoPoint(latitude, longitude))
|
|
||||||
|
|
||||||
prefs.edit()
|
prefs.edit()
|
||||||
.putLong(KEY_CACHED_LAT, latitude.toBits())
|
.putLong(KEY_CACHED_LAT, latitude.toBits())
|
||||||
.putLong(KEY_CACHED_LON, longitude.toBits())
|
.putLong(KEY_CACHED_LON, longitude.toBits())
|
||||||
|
|
@ -109,11 +109,21 @@ class MapCacheManager(private val context: Context) {
|
||||||
.putBoolean(KEY_CACHE_COMPLETE, true)
|
.putBoolean(KEY_CACHE_COMPLETE, true)
|
||||||
.apply()
|
.apply()
|
||||||
|
|
||||||
Log.i(TAG, "Tile cache seeding complete")
|
Log.d(TAG, "Tile cache seeding complete")
|
||||||
true
|
true
|
||||||
|
} catch (e: CancellationException) {
|
||||||
|
throw e // Never swallow coroutine cancellation
|
||||||
} catch (e: Exception) {
|
} catch (e: Exception) {
|
||||||
Log.e(TAG, "Failed to seed tile cache", e)
|
Log.e(TAG, "Failed to seed tile cache", e)
|
||||||
false
|
false
|
||||||
|
} finally {
|
||||||
|
// Always restore map to user's location, even on cancellation
|
||||||
|
withContext(NonCancellable) {
|
||||||
|
mapView.controller.setZoom(savedZoom)
|
||||||
|
mapView.controller.setCenter(
|
||||||
|
GeoPoint(savedCenter.latitude, savedCenter.longitude)
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,6 @@
|
||||||
package no.naiv.tilfluktsrom.data
|
package no.naiv.tilfluktsrom.data
|
||||||
|
|
||||||
|
import android.util.Log
|
||||||
import no.naiv.tilfluktsrom.util.CoordinateConverter
|
import no.naiv.tilfluktsrom.util.CoordinateConverter
|
||||||
import org.json.JSONObject
|
import org.json.JSONObject
|
||||||
import java.io.ByteArrayOutputStream
|
import java.io.ByteArrayOutputStream
|
||||||
|
|
@ -12,6 +13,17 @@ import java.util.zip.ZipInputStream
|
||||||
*/
|
*/
|
||||||
object ShelterGeoJsonParser {
|
object ShelterGeoJsonParser {
|
||||||
|
|
||||||
|
private const val TAG = "ShelterGeoJsonParser"
|
||||||
|
|
||||||
|
// Maximum uncompressed size to prevent ZIP bomb attacks (10 MB)
|
||||||
|
private const val MAX_UNCOMPRESSED_SIZE = 10 * 1024 * 1024L
|
||||||
|
|
||||||
|
// Norway's approximate bounding box in WGS84
|
||||||
|
private const val NORWAY_LAT_MIN = 57.0
|
||||||
|
private const val NORWAY_LAT_MAX = 72.0
|
||||||
|
private const val NORWAY_LON_MIN = 3.0
|
||||||
|
private const val NORWAY_LON_MAX = 33.0
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Extract and parse GeoJSON from a ZIP input stream.
|
* Extract and parse GeoJSON from a ZIP input stream.
|
||||||
*/
|
*/
|
||||||
|
|
@ -26,7 +38,18 @@ object ShelterGeoJsonParser {
|
||||||
while (entry != null) {
|
while (entry != null) {
|
||||||
if (entry.name.endsWith(".geojson") || entry.name.endsWith(".json")) {
|
if (entry.name.endsWith(".geojson") || entry.name.endsWith(".json")) {
|
||||||
val buffer = ByteArrayOutputStream()
|
val buffer = ByteArrayOutputStream()
|
||||||
zis.copyTo(buffer)
|
val chunk = ByteArray(8192)
|
||||||
|
var totalRead = 0L
|
||||||
|
var bytesRead: Int
|
||||||
|
while (zis.read(chunk).also { bytesRead = it } != -1) {
|
||||||
|
totalRead += bytesRead
|
||||||
|
if (totalRead > MAX_UNCOMPRESSED_SIZE) {
|
||||||
|
throw IllegalStateException(
|
||||||
|
"GeoJSON exceeds maximum size (${MAX_UNCOMPRESSED_SIZE / 1024}KB)"
|
||||||
|
)
|
||||||
|
}
|
||||||
|
buffer.write(chunk, 0, bytesRead)
|
||||||
|
}
|
||||||
return buffer.toString(Charsets.UTF_8.name())
|
return buffer.toString(Charsets.UTF_8.name())
|
||||||
}
|
}
|
||||||
entry = zis.nextEntry
|
entry = zis.nextEntry
|
||||||
|
|
@ -37,13 +60,16 @@ object ShelterGeoJsonParser {
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Parse raw GeoJSON string into Shelter objects.
|
* Parse raw GeoJSON string into Shelter objects.
|
||||||
|
* Malformed individual features are skipped rather than failing the entire parse.
|
||||||
*/
|
*/
|
||||||
fun parseGeoJson(json: String): List<Shelter> {
|
fun parseGeoJson(json: String): List<Shelter> {
|
||||||
val root = JSONObject(json)
|
val root = JSONObject(json)
|
||||||
val features = root.getJSONArray("features")
|
val features = root.getJSONArray("features")
|
||||||
val shelters = mutableListOf<Shelter>()
|
val shelters = mutableListOf<Shelter>()
|
||||||
|
var skipped = 0
|
||||||
|
|
||||||
for (i in 0 until features.length()) {
|
for (i in 0 until features.length()) {
|
||||||
|
try {
|
||||||
val feature = features.getJSONObject(i)
|
val feature = features.getJSONObject(i)
|
||||||
val geometry = feature.getJSONObject("geometry")
|
val geometry = feature.getJSONObject("geometry")
|
||||||
val properties = feature.getJSONObject("properties")
|
val properties = feature.getJSONObject("properties")
|
||||||
|
|
@ -55,16 +81,47 @@ object ShelterGeoJsonParser {
|
||||||
// Convert UTM33N to WGS84
|
// Convert UTM33N to WGS84
|
||||||
val latLon = CoordinateConverter.utm33nToWgs84(easting, northing)
|
val latLon = CoordinateConverter.utm33nToWgs84(easting, northing)
|
||||||
|
|
||||||
|
// Validate coordinates are within Norway
|
||||||
|
if (latLon.latitude !in NORWAY_LAT_MIN..NORWAY_LAT_MAX ||
|
||||||
|
latLon.longitude !in NORWAY_LON_MIN..NORWAY_LON_MAX
|
||||||
|
) {
|
||||||
|
Log.w(TAG, "Skipping shelter at index $i: coordinates outside Norway " +
|
||||||
|
"(${latLon.latitude}, ${latLon.longitude})")
|
||||||
|
skipped++
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
val plasser = properties.optInt("plasser", 0)
|
||||||
|
if (plasser < 0) {
|
||||||
|
Log.w(TAG, "Skipping shelter at index $i: negative capacity ($plasser)")
|
||||||
|
skipped++
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
shelters.add(
|
shelters.add(
|
||||||
Shelter(
|
Shelter(
|
||||||
lokalId = properties.optString("lokalId", "unknown-$i"),
|
lokalId = properties.optString("lokalId", "unknown-$i"),
|
||||||
romnr = properties.optInt("romnr", 0),
|
romnr = properties.optInt("romnr", 0),
|
||||||
plasser = properties.optInt("plasser", 0),
|
plasser = plasser,
|
||||||
adresse = properties.optString("adresse", ""),
|
adresse = properties.optString("adresse", ""),
|
||||||
latitude = latLon.latitude,
|
latitude = latLon.latitude,
|
||||||
longitude = latLon.longitude
|
longitude = latLon.longitude
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
} catch (e: Exception) {
|
||||||
|
Log.w(TAG, "Skipping malformed shelter feature at index $i", e)
|
||||||
|
skipped++
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (skipped > 0) {
|
||||||
|
Log.w(TAG, "Skipped $skipped malformed features out of ${features.length()}")
|
||||||
|
}
|
||||||
|
|
||||||
|
if (shelters.isEmpty()) {
|
||||||
|
throw IllegalStateException(
|
||||||
|
"No valid shelters found in GeoJSON (${features.length()} features were malformed)"
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
return shelters
|
return shelters
|
||||||
|
|
|
||||||
|
|
@ -2,6 +2,8 @@ package no.naiv.tilfluktsrom.data
|
||||||
|
|
||||||
import android.content.Context
|
import android.content.Context
|
||||||
import android.util.Log
|
import android.util.Log
|
||||||
|
import androidx.room.withTransaction
|
||||||
|
import kotlinx.coroutines.CancellationException
|
||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
import kotlinx.coroutines.flow.Flow
|
import kotlinx.coroutines.flow.Flow
|
||||||
import kotlinx.coroutines.withContext
|
import kotlinx.coroutines.withContext
|
||||||
|
|
@ -55,7 +57,7 @@ class ShelterRepository(context: Context) {
|
||||||
*/
|
*/
|
||||||
suspend fun refreshData(): Boolean = withContext(Dispatchers.IO) {
|
suspend fun refreshData(): Boolean = withContext(Dispatchers.IO) {
|
||||||
try {
|
try {
|
||||||
Log.i(TAG, "Downloading shelter data from Geonorge...")
|
Log.d(TAG, "Downloading shelter data from Geonorge...")
|
||||||
|
|
||||||
val request = Request.Builder()
|
val request = Request.Builder()
|
||||||
.url(SHELTER_DATA_URL)
|
.url(SHELTER_DATA_URL)
|
||||||
|
|
@ -63,12 +65,13 @@ class ShelterRepository(context: Context) {
|
||||||
.build()
|
.build()
|
||||||
|
|
||||||
val response = client.newCall(request).execute()
|
val response = client.newCall(request).execute()
|
||||||
if (!response.isSuccessful) {
|
response.use { resp ->
|
||||||
Log.e(TAG, "Download failed: HTTP ${response.code}")
|
if (!resp.isSuccessful) {
|
||||||
|
Log.e(TAG, "Download failed: HTTP ${resp.code}")
|
||||||
return@withContext false
|
return@withContext false
|
||||||
}
|
}
|
||||||
|
|
||||||
val body = response.body ?: run {
|
val body = resp.body ?: run {
|
||||||
Log.e(TAG, "Empty response body")
|
Log.e(TAG, "Empty response body")
|
||||||
return@withContext false
|
return@withContext false
|
||||||
}
|
}
|
||||||
|
|
@ -77,14 +80,20 @@ class ShelterRepository(context: Context) {
|
||||||
ShelterGeoJsonParser.parseFromZip(stream)
|
ShelterGeoJsonParser.parseFromZip(stream)
|
||||||
}
|
}
|
||||||
|
|
||||||
Log.i(TAG, "Parsed ${shelters.size} shelters, saving to database...")
|
Log.d(TAG, "Parsed ${shelters.size} shelters, saving to database...")
|
||||||
|
|
||||||
|
// Atomic replace: delete + insert in a single transaction
|
||||||
|
db.withTransaction {
|
||||||
dao.deleteAll()
|
dao.deleteAll()
|
||||||
dao.insertAll(shelters)
|
dao.insertAll(shelters)
|
||||||
|
}
|
||||||
|
|
||||||
prefs.edit().putLong(KEY_LAST_UPDATE, System.currentTimeMillis()).apply()
|
prefs.edit().putLong(KEY_LAST_UPDATE, System.currentTimeMillis()).apply()
|
||||||
Log.i(TAG, "Shelter data updated successfully")
|
Log.d(TAG, "Shelter data updated successfully")
|
||||||
true
|
true
|
||||||
|
}
|
||||||
|
} catch (e: CancellationException) {
|
||||||
|
throw e // Never swallow coroutine cancellation
|
||||||
} catch (e: Exception) {
|
} catch (e: Exception) {
|
||||||
Log.e(TAG, "Failed to refresh shelter data", e)
|
Log.e(TAG, "Failed to refresh shelter data", e)
|
||||||
false
|
false
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,5 @@
|
||||||
<?xml version="1.0" encoding="utf-8"?>
|
<?xml version="1.0" encoding="utf-8"?>
|
||||||
<network-security-config>
|
<network-security-config>
|
||||||
<!-- Allow cleartext for OSMDroid tile servers that may use HTTP -->
|
<!-- Default: cleartext traffic not permitted (HTTPS only) -->
|
||||||
<domain-config cleartextTrafficPermitted="true">
|
<base-config cleartextTrafficPermitted="false" />
|
||||||
<domain includeSubdomains="true">tile.openstreetmap.org</domain>
|
|
||||||
</domain-config>
|
|
||||||
</network-security-config>
|
</network-security-config>
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue