Skip to content

Bulk markers with a NaN coordinate crash the app when clustering or above 500 markers #171

Description

@jkasprzyk17

Problem

Descriptors passed through the bulk markers prop are not validated anywhere — the README says
so itself (README.md:582: "descriptors passed through the bulk markers prop are checked on
neither side"
), and #158 listed it as a known gap with "Neither crashes; both are follow-ups."

It does crash, on both platforms, as soon as the markers go through the background viewport
pipeline — that is, with clusteringEnabled, or with more than 500 markers
(MarkerClusterEngine.swift:360, MarkerRenderState.kt:86). The follow-up was never filed.

// package/src/overlays/normalizeMarkerDescriptors.ts — coordinate copied verbatim
function normalizeDescriptor(descriptor: PublicMarkerDescriptor): MarkerDescriptor {
  return {
    id: descriptor.id,
    coordinate: descriptor.coordinate,

iOS — Int(Double.nan) traps

Int(_:) from a Double is a runtime trap for NaN and ±infinity ("Double value cannot be
converted to Int because it is either infinite or NaN"
). Swift.min ignores NaN while the
spatial index computes its bounding box, so the NaN reaches the conversion:

// package/ios/MarkerSpatialIndex.swift:116 — every marker, while the index is built
min(cellsPerSide - 1, max(0, Int((lat - minLat) / latStep)))

// package/ios/MarkerClusterEngine.swift:194-195 — every clusterable candidate
let row = Int((lat / cellLat).rounded(.down))
let col = Int((lon / cellLon).rounded(.down))

The same pattern is in MarkerViewportFilter.swift:45-46. All three run on computeQueue, so
the trap aborts the process. One bad marker is enough.

Android — LatLngBounds rejects a NaN latitude

floor(NaN).toInt() is 0 in Kotlin, so nothing throws while bucketing; the NaN marker lands in
bucket "0:0", and minOf/maxOf propagate the NaN into the bucket's bounds:

// package/android/.../MarkerClusterEngine.kt:195-198
bounds =
  LatLngBounds(
    LatLng(bucket.minLat, wrapTo180(bucket.minLon)),
    LatLng(bucket.maxLat, wrapTo180(bucket.maxLon)),
  ),

LatLngBounds checks southern latitude exceeds northern latitude and throws
IllegalArgumentException for NaN (the same message #125 recorded for region). This runs on
computeExecutor (MapOverlayController.kt:159, :186) with no try/catch, so the exception
kills the process. It needs that bucket to become a cluster — two invalid markers, or one next to
a real marker in the same cell — so it is less certain to trigger than on iOS.

Reproduction

const markers = [
  ...validMarkers,                                   // any number
  { id: 'bad-1', coordinate: { latitude: NaN, longitude: NaN } },
  { id: 'bad-2', coordinate: { latitude: NaN, longitude: NaN } },
];

<MapView clusteringEnabled markers={markers} />

Realistically this is a record from an API with an empty or unparsable position.

Not checked: the synchronous path (≤ 500 markers, no clustering), where the descriptor goes
straight to addAnnotation / addMarker.

Where

  • package/src/overlays/normalizeMarkerDescriptors.ts:6-25 — no validation
  • package/ios/MarkerSpatialIndex.swift:49-55, :116, :120
  • package/ios/MarkerClusterEngine.swift:194-195
  • package/ios/MarkerViewportFilter.swift:45-46
  • package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerClusterEngine.kt:153-154, :195-198
  • package/android/src/main/java/com/margelo/nitro/nitromaps/MapOverlayController.kt:159, :186 — executor tasks without try/catch

Suggested fix

  1. JS: drop descriptors with !isValidCoordinate(descriptor.coordinate) in
    normalizeMarkerDescriptors, with the same __DEV__ warning the <Marker> child path uses
    (utils/validateGeometry.ts already has the predicate).
  2. Native: filter the same way where the dataset enters the pipeline — MarkerRenderState.setMarkers
    on Android (Coordinate.isValid() exists in Coordinate+Validity.kt) and
    MarkerRenderPipeline.setMarkers on iOS (Coordinate+Validity.swift). hybridRef and future
    entry points (e.g. MarkerCollection in feat: native marker store fed by packed delta batches #69) must not be able to bypass it.
  3. Android: wrap computeExecutor tasks in try/catch with Log.e, so a future bug in the
    index or clustering degrades to "no refresh" instead of a crash. Use shutdownNow() in
    clear() (MapOverlayController.kt:115) so an index build does not keep running after unmount.
  4. Remove the bulk-markers sentence from the README's "Invalid input" gaps.

Acceptance criteria

  • NaN, ±Infinity and out-of-range coordinates in bulk markers are skipped on both platforms,
    with and without clustering, above and below 500 markers
  • Development builds warn once per invalid descriptor, naming its id
  • An exception inside an Android compute task is logged and does not terminate the app
  • README "Invalid input" no longer lists bulk markers as a gap

Testing

  • bun: normalizeMarkerDescriptors with NaN / Infinity / out-of-range entries
  • JUnit: MarkerClusterEngine.clusters with two NaN descriptors (LatLngBounds and LatLng are
    plain Java, so this runs on the JVM)
  • swift test: once the index can be built from plain values; until then a manual run in the
    example app with the reproduction above

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingkotlinThe Kotlin / Android native layer (package/android)platform: androidAffects Androidplatform: iosAffects iOSswiftThe Swift / iOS native layer (package/ios)typescriptThe TypeScript layer (package/src)

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions