Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ Built with [Nitro Modules](https://nitro.margelo.com/) for high-performance nati
- [Google Maps setup](#google-maps-setup)
- [Marker entering animations](#marker-entering-animations)
- [Re-renders](#re-renders)
- [Invalid input](#invalid-input)
- [Capability matrix](#capability-matrix)
- [Public API](#public-api)
- [Example app](#example-app)
Expand Down Expand Up @@ -540,6 +541,20 @@ setMarkers((current) =>
);
```

## Invalid input

A coordinate that arrives as `NaN` or out of range is dropped instead of being forwarded to MapKit and the Google Maps SDK, which throw on it:

- An invalid `region` is ignored, and the map keeps the region it already had.
- An overlay whose coordinates, ring length or radius cannot be drawn is skipped; its neighbours still render.
- Anything supplied through `region` or through a `<Marker>` / `<Polyline>` / `<Polygon>` / `<Circle>` child is reported through `console.warn` in development.

Where the check runs depends on the entry point. `region` and `fitToCoordinates` are guarded natively on both platforms, so a `hybridRef` call cannot reach the SDKs either. Overlay descriptors are additionally filtered natively on Android, where an undrawable overlay throws inside the Fabric mount transaction and would otherwise take the whole screen down; those skips are reported to logcat rather than `console.warn`.

Two gaps are worth knowing about: the `camera` prop is not validated anywhere, and descriptors passed through the bulk `markers` prop are checked on neither side - only the `<Marker>` child is.

Valid means: latitude and longitude finite and within ±90 / ±180, region deltas finite and greater than 0, two coordinates for a polyline, three per polygon ring, and a finite radius of at least 0 for a circle. A region whose span would run past a pole is pulled back to what the map can show rather than rejected.

## Capability matrix

| Capability | `apple` iOS | `google` iOS | `google` Android |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.margelo.nitro.nitromaps

/** Google Maps throws on a coordinate it cannot place, which unwinds the Fabric mount transaction. */
internal fun Coordinate.isValid(): Boolean = isValidCoordinate(latitude, longitude)

/** Scalar form, so a `Region` can check its center without building a `Coordinate` for it. */
internal fun isValidCoordinate(
latitude: Double,
longitude: Double,
): Boolean =
latitude.isFinite() &&
latitude >= -90.0 &&
latitude <= 90.0 &&
longitude.isFinite() &&
longitude >= -180.0 &&
longitude <= 180.0

/** Minimum size is 2 for a polyline and 3 for a polygon ring. */
internal fun Array<Coordinate>.isValidPath(minimumSize: Int): Boolean = size >= minimumSize && all { it.isValid() }
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import android.content.pm.PackageManager
import android.content.res.Configuration
import android.os.Handler
import android.os.Looper
import android.util.Log
import android.view.View
import android.view.ViewTreeObserver
import androidx.annotation.Keep
Expand Down Expand Up @@ -361,14 +362,21 @@ class GoogleMapProviderAdapter(
padding: EdgePadding?,
animated: Boolean?,
) {
if (coordinates.isEmpty()) {
// Filtered before the main-thread hop: a throw out of `LatLngBounds` inside
// `runOnMain` lands on the looper, where the JS caller cannot catch it.
val validCoordinates = coordinates.filter { it.isValid() }
val skipped = coordinates.size - validCoordinates.size
if (skipped > 0) {
Log.w(NITRO_MAPS_LOG_TAG, "fitToCoordinates skipped $skipped coordinate(s) outside the world.")
}
if (validCoordinates.isEmpty()) {
return
}

runOnMain {
val map = googleMap ?: return@runOnMain
val builder = LatLngBounds.Builder()
for (coordinate in coordinates) {
for (coordinate in validCoordinates) {
builder.include(LatLng(coordinate.latitude, coordinate.longitude))
}
val bounds = builder.build()
Expand Down Expand Up @@ -602,6 +610,11 @@ class GoogleMapProviderAdapter(
region: Region,
animated: Boolean = false,
) {
if (!region.isValid()) {
Log.w(NITRO_MAPS_LOG_TAG, "Ignored an invalid region: $region.")
return
}

val map = googleMap ?: return
val bounds = region.toLatLngBounds()
val paddingPx = _mapPadding.toPaddingPixels()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import android.animation.ValueAnimator
import android.os.Handler
import android.os.Looper
import android.os.SystemClock
import android.util.Log
import com.facebook.react.uimanager.ThemedReactContext
import com.google.android.gms.maps.CameraUpdateFactory
import com.google.android.gms.maps.GoogleMap
Expand Down Expand Up @@ -544,17 +545,27 @@ class MapOverlayController(
val map = googleMap ?: return
reconcile(
current = polylines,
next = descriptors?.associateBy { it.id } ?: emptyMap(),
next =
validDescriptorsById(
descriptors = descriptors,
kind = "polyline",
id = { it.id },
isValid = { it.isValid() },
),
remove = { it.remove() },
add = { descriptor ->
map.addPolyline(descriptor.toPolylineOptions()).also { polyline ->
polyline.tag = descriptor.id
addedOrNull(kind = "polyline", id = descriptor.id) {
map.addPolyline(descriptor.toPolylineOptions()).also { polyline ->
polyline.tag = descriptor.id
}
}
},
update = { polyline, descriptor ->
polyline.remove()
map.addPolyline(descriptor.toPolylineOptions()).also { replacement ->
replacement.tag = descriptor.id
addedOrNull(kind = "polyline", id = descriptor.id) {
map.addPolyline(descriptor.toPolylineOptions()).also { replacement ->
replacement.tag = descriptor.id
}
}
},
)
Expand All @@ -564,17 +575,27 @@ class MapOverlayController(
val map = googleMap ?: return
reconcile(
current = polygons,
next = descriptors?.associateBy { it.id } ?: emptyMap(),
next =
validDescriptorsById(
descriptors = descriptors,
kind = "polygon",
id = { it.id },
isValid = { it.isValid() },
),
remove = { it.remove() },
add = { descriptor ->
map.addPolygon(descriptor.toPolygonOptions()).also { polygon ->
polygon.tag = descriptor.id
addedOrNull(kind = "polygon", id = descriptor.id) {
map.addPolygon(descriptor.toPolygonOptions()).also { polygon ->
polygon.tag = descriptor.id
}
}
},
update = { polygon, descriptor ->
polygon.remove()
map.addPolygon(descriptor.toPolygonOptions()).also { replacement ->
replacement.tag = descriptor.id
addedOrNull(kind = "polygon", id = descriptor.id) {
map.addPolygon(descriptor.toPolygonOptions()).also { replacement ->
replacement.tag = descriptor.id
}
}
},
)
Expand All @@ -584,28 +605,80 @@ class MapOverlayController(
val map = googleMap ?: return
reconcile(
current = circles,
next = descriptors?.associateBy { it.id } ?: emptyMap(),
next =
validDescriptorsById(
descriptors = descriptors,
kind = "circle",
id = { it.id },
isValid = { it.isValid() },
),
remove = { it.remove() },
add = { descriptor ->
map.addCircle(descriptor.toCircleOptions()).also { circle ->
circle.tag = descriptor.id
addedOrNull(kind = "circle", id = descriptor.id) {
map.addCircle(descriptor.toCircleOptions()).also { circle ->
circle.tag = descriptor.id
}
}
},
update = { circle, descriptor ->
circle.remove()
map.addCircle(descriptor.toCircleOptions()).also { replacement ->
replacement.tag = descriptor.id
addedOrNull(kind = "circle", id = descriptor.id) {
map.addCircle(descriptor.toCircleOptions()).also { replacement ->
replacement.tag = descriptor.id
}
}
},
)
}

/** Dropping the id also removes what it used to render, since `reconcile` treats a missing id as a removal. */
private fun <Descriptor> validDescriptorsById(
descriptors: Array<Descriptor>?,
kind: String,
id: (Descriptor) -> String,
isValid: (Descriptor) -> Boolean,
): Map<String, Descriptor> {
if (descriptors == null) {
return emptyMap()
}

val valid = LinkedHashMap<String, Descriptor>(descriptors.size)
for (descriptor in descriptors) {
if (!isValid(descriptor)) {
Log.w(NITRO_MAPS_LOG_TAG, "Skipped $kind \"${id(descriptor)}\": it cannot be drawn.")
continue
}

valid[id(descriptor)] = descriptor
}

return valid
}

/**
* The pre-filter only models what the descriptors declare; `GoogleMap.add*`
* can still reject a value for a reason of its own, and `reconcile` runs
* inside a view prop setter, where an escaping throw aborts the whole mount
* transaction.
*/
private fun <T> addedOrNull(
kind: String,
id: String,
add: () -> T,
): T? =
try {
add()
} catch (error: IllegalArgumentException) {
Log.w(NITRO_MAPS_LOG_TAG, "Skipped $kind \"$id\": the Google Maps SDK rejected it.", error)
null
}

private fun <T, Descriptor> reconcile(
current: MutableMap<String, T>,
next: Map<String, Descriptor>,
remove: (T) -> Unit,
add: (Descriptor) -> T?,
update: (T, Descriptor) -> T,
update: (T, Descriptor) -> T?,
) {
val nextIds = next.keys
val existingIds = current.keys
Expand All @@ -621,7 +694,12 @@ class MapOverlayController(
current[id] = created
}
} else {
current[id] = update(existing, descriptor)
val updated = update(existing, descriptor)
if (updated == null) {
current.remove(id)
} else {
current[id] = updated
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ internal class MarkerIconFactory(
cacheBitmap(key, resizeBitmap(bitmap, image))
}
} catch (error: Exception) {
Log.w(TAG, "Failed to load marker image: ${image.uri}", error)
Log.w(NITRO_MAPS_LOG_TAG, "Failed to load marker image: ${image.uri}", error)
null
}
}
Expand Down Expand Up @@ -503,7 +503,7 @@ internal class MarkerIconFactory(
uri: String,
reason: String,
) {
Log.w(TAG, "Rejected remote marker image URI ($reason): $uri")
Log.w(NITRO_MAPS_LOG_TAG, "Rejected remote marker image URI ($reason): $uri")
}

private fun resizeBitmap(
Expand All @@ -523,7 +523,6 @@ internal class MarkerIconFactory(
}

private companion object {
const val TAG = "NitroMaps"
const val DEFAULT_ICON_KEY = "__default__"
private const val DEFAULT_MARKER_WIDTH_DP = 40f
private const val DEFAULT_MARKER_HEIGHT_DP = 52f
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package com.margelo.nitro.nitromaps

/** Single logcat tag for the whole module. */
internal const val NITRO_MAPS_LOG_TAG = "NitroMaps"
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.margelo.nitro.nitromaps

/** A polyline needs two placeable points before `GoogleMap.addPolyline` accepts it. */
internal fun PolylineDescriptor.isValid(): Boolean = coordinates.isValidPath(MINIMUM_POLYLINE_SIZE)

/** Every hole is passed to `PolygonOptions.addHole` as a ring of its own, so each one has to hold up too. */
internal fun PolygonDescriptor.isValid(): Boolean {
if (!coordinates.isValidPath(MINIMUM_RING_SIZE)) {
return false
}

val rings = holes ?: return true
return rings.all { ring -> ring.isValidPath(MINIMUM_RING_SIZE) }
}

/** `GoogleMap.addCircle` throws on a negative radius and on an unplaceable center. */
internal fun CircleDescriptor.isValid(): Boolean = center.isValid() && radius.isFinite() && radius >= 0.0

private const val MINIMUM_POLYLINE_SIZE = 2
private const val MINIMUM_RING_SIZE = 3
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.margelo.nitro.nitromaps

/** A `NaN` or non-positive span puts the southern edge above the northern one, which `LatLngBounds` rejects. */
internal fun Region.isValid(): Boolean =
isValidCoordinate(latitude, longitude) &&
latitudeDelta.isFinite() &&
latitudeDelta > 0.0 &&
longitudeDelta.isFinite() &&
longitudeDelta > 0.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package com.margelo.nitro.nitromaps

import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Test

class CoordinateValidityTest {
@Test
fun acceptsCoordinatesOnTheEdgeOfTheWorld() {
assertTrue(Coordinate(latitude = 90.0, longitude = 180.0).isValid())
assertTrue(Coordinate(latitude = -90.0, longitude = -180.0).isValid())
assertTrue(Coordinate(latitude = 52.23, longitude = 21.01).isValid())
}

@Test
fun rejectsNonFiniteCoordinates() {
assertFalse(Coordinate(latitude = Double.NaN, longitude = 0.0).isValid())
assertFalse(Coordinate(latitude = 0.0, longitude = Double.NaN).isValid())
assertFalse(Coordinate(latitude = Double.POSITIVE_INFINITY, longitude = 0.0).isValid())
assertFalse(Coordinate(latitude = 0.0, longitude = Double.NEGATIVE_INFINITY).isValid())
}

@Test
fun rejectsCoordinatesOutsideTheWorld() {
assertFalse(Coordinate(latitude = 1000.0, longitude = 0.0).isValid())
assertFalse(Coordinate(latitude = 90.0001, longitude = 0.0).isValid())
assertFalse(Coordinate(latitude = -90.0001, longitude = 0.0).isValid())
assertFalse(Coordinate(latitude = 0.0, longitude = 180.0001).isValid())
assertFalse(Coordinate(latitude = 0.0, longitude = -180.0001).isValid())
}

@Test
fun requiresEnoughPointsForAPath() {
val point = Coordinate(latitude = 1.0, longitude = 2.0)

assertFalse(emptyArray<Coordinate>().isValidPath(minimumSize = 2))
assertFalse(arrayOf(point).isValidPath(minimumSize = 2))
assertTrue(arrayOf(point, point).isValidPath(minimumSize = 2))
assertFalse(arrayOf(point, point).isValidPath(minimumSize = 3))
assertTrue(arrayOf(point, point, point).isValidPath(minimumSize = 3))
}

@Test
fun rejectsAPathHoldingAnUnplaceablePoint() {
val point = Coordinate(latitude = 1.0, longitude = 2.0)
val broken = Coordinate(latitude = Double.NaN, longitude = 2.0)

assertFalse(arrayOf(point, broken).isValidPath(minimumSize = 2))
assertFalse(arrayOf(broken, point, point).isValidPath(minimumSize = 3))
}
}
Loading
Loading