From 606ba79f2078b5dc69ee307818caf03fbbef8159 Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Mon, 21 Sep 2026 13:29:24 +0200 Subject: [PATCH 1/2] fix: skip an invalid camera instead of crashing the map `camera` reached both SDKs unchecked, the way `region` did before #158. Both crash paths were reproduced against the real SDKs rather than assumed: - MapKit raises `NSInvalidArgumentException` ("Invalid camera centerCoordinate") from `-[MKMapCamera _validate]` inside `-[MKMapView setCamera:]` for a `NaN`, infinite or out-of-range center. Swift cannot catch it, so the check has to happen first. - `CameraPosition.Builder.tilt` throws `IllegalArgumentException` for any pitch outside 0..90 - `NaN` and ordinary values such as 120 alike - which unwinds the Fabric mount transaction. - The remaining non-finite framing values throw on neither side, but collapse the altitude to the map's minimum, or leave the camera and `MKMapView.region` reading back as `NaN`. - JS: an unusable `camera` - or one unset after the view accepted it - is held at the last accepted value, with a `__DEV__` warning for the invalid case. It cannot become `undefined`, for the reason `resolveRegionProp` documents: React rewrites a removed prop to `null` and the generated struct converter throws on it before any native guard runs. - Swift: `Camera.isValid` pairs `Coordinate.isValid` with `CameraFraming`, which lives in the `Geometry` SPM target so `swift test` covers it. Both adapters guard `updateMapCamera`, and the Google one also checks the camera its map is created with - that never passes through `updateMapCamera`. - Kotlin: `Camera.isValid()` mirrors `Region.isValid()`, and the adapter checks before the main-thread hop, where a throw would surface as an uncaught main-looper exception no JS caller can catch. A finite pitch outside 0..90 is clamped rather than treated as invalid, so an unsupported tilt no longer discards a usable center; MapKit flattens such a camera instead of refusing it, so this keeps the two platforms aligned. `hybridRef.setCamera` and `animateCamera` funnel through the same guarded `updateMapCamera`, so JS validation alone was never enough. --- README.md | 9 +- .../nitro/nitromaps/Camera+CameraPosition.kt | 15 +++- .../nitro/nitromaps/Camera+Validity.kt | 16 ++++ .../nitromaps/GoogleMapProviderAdapter.kt | 8 ++ .../nitro/nitromaps/CameraValidityTest.kt | 74 +++++++++++++++ package/ios/AppleMapProviderAdapter.swift | 9 ++ package/ios/Camera+Validity.swift | 12 +++ package/ios/Geometry/CameraFraming.swift | 31 +++++++ package/ios/GoogleMapProviderAdapter.swift | 14 ++- .../Tests/Geometry/CameraFramingTests.swift | 31 +++++++ .../camera/__tests__/isValidCamera.test.ts | 75 ++++++++++++++++ .../__tests__/resolveCameraProp.test.ts | 90 +++++++++++++++++++ package/src/camera/isValidCamera.ts | 25 ++++++ package/src/camera/resolveCameraProp.ts | 31 +++++++ package/src/camera/useValidCamera.ts | 25 ++++++ package/src/camera/warnCamera.ts | 3 + package/src/components/MapView.tsx | 4 +- 17 files changed, 462 insertions(+), 10 deletions(-) create mode 100644 package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+Validity.kt create mode 100644 package/android/src/test/java/com/margelo/nitro/nitromaps/CameraValidityTest.kt create mode 100644 package/ios/Camera+Validity.swift create mode 100644 package/ios/Geometry/CameraFraming.swift create mode 100644 package/ios/Tests/Geometry/CameraFramingTests.swift create mode 100644 package/src/camera/__tests__/isValidCamera.test.ts create mode 100644 package/src/camera/__tests__/resolveCameraProp.test.ts create mode 100644 package/src/camera/isValidCamera.ts create mode 100644 package/src/camera/resolveCameraProp.ts create mode 100644 package/src/camera/useValidCamera.ts create mode 100644 package/src/camera/warnCamera.ts diff --git a/README.md b/README.md index 5a7a89d0..c4aed221 100644 --- a/README.md +++ b/README.md @@ -574,14 +574,15 @@ setMarkers((current) => 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 invalid `camera` is ignored the same way, and a pitch past the range the SDKs draw is pulled back to it rather than rejected. - An overlay whose coordinates, ring length or radius cannot be drawn is skipped; its neighbours still render. -- Anything supplied through `region` or through a `` / `` / `` / `` child is reported through `console.warn` in development. +- Anything supplied through `region`, `camera`, or a `` / `` / `` / `` 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`. +Where the check runs depends on the entry point. `region`, `camera` and `fitToCoordinates` are guarded natively on both platforms, so a `hybridRef` call - `setCamera` and `animateCamera` included - 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 `` child is. +One gap is worth knowing about: descriptors passed through the bulk `markers` prop are checked on neither side - only the `` 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. +Valid means: latitude and longitude finite and within ±90 / ±180, region deltas finite and greater than 0, camera `zoom` / `heading` / `pitch` / `altitude` finite when supplied, 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 diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+CameraPosition.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+CameraPosition.kt index 55c271dc..4323e0e4 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+CameraPosition.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+CameraPosition.kt @@ -10,10 +10,23 @@ fun Camera.toCameraPosition(current: CameraPosition? = null): CameraPosition { .target(LatLng(center.latitude, center.longitude)) .zoom((zoom ?: current?.zoom?.toDouble() ?: 10.0).toFloat()) .bearing((heading ?: current?.bearing?.toDouble() ?: 0.0).toFloat()) - .tilt((pitch ?: current?.tilt?.toDouble() ?: 0.0).toFloat()) + .tilt(drawableTilt(pitch ?: current?.tilt?.toDouble() ?: 0.0)) .build() } +/** + * `CameraPosition.Builder.tilt` throws for anything outside 0..90, and that throw would unwind the + * Fabric mount transaction, so a pitch past the limit is pulled back to it. MapKit flattens such a + * camera rather than refusing it, which is the behaviour this matches. A non-finite pitch never + * reaches here - `Camera.isValid()` drops the whole camera first - but it is handled so this stays + * safe on its own. + */ +private fun drawableTilt(pitch: Double): Float = + if (pitch.isFinite()) pitch.coerceIn(MINIMUM_TILT, MAXIMUM_TILT).toFloat() else MINIMUM_TILT.toFloat() + +private const val MINIMUM_TILT = 0.0 +private const val MAXIMUM_TILT = 90.0 + fun CameraPosition.toCamera(): Camera { return Camera( center = Coordinate(latitude = target.latitude, longitude = target.longitude), diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+Validity.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+Validity.kt new file mode 100644 index 00000000..02baddb9 --- /dev/null +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+Validity.kt @@ -0,0 +1,16 @@ +package com.margelo.nitro.nitromaps + +/** + * `CameraPosition.Builder.tilt` throws for a non-finite pitch, which unwinds the Fabric mount + * transaction, and a non-finite zoom or heading is taken silently and leaves the camera reading + * back as `NaN`. + */ +internal fun Camera.isValid(): Boolean = + isValidCoordinate(center.latitude, center.longitude) && + zoom.isFiniteOrAbsent() && + heading.isFiniteOrAbsent() && + pitch.isFiniteOrAbsent() && + altitude.isFiniteOrAbsent() + +/** An absent value is filled in from the camera the map already has, so only a supplied one is checked. */ +private fun Double?.isFiniteOrAbsent(): Boolean = this == null || isFinite() diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt index bc1e31e4..fa588a72 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt @@ -639,6 +639,14 @@ class GoogleMapProviderAdapter( animated: Boolean, durationMs: Int = 0, ) { + // Checked before the main-thread hop: `runOnMain` posts to the looper when called from + // anywhere else, so a throw out of `CameraPosition` would surface as an uncaught main-looper + // exception the JS caller cannot catch. + if (!camera.isValid()) { + Log.w(NITRO_MAPS_LOG_TAG, "Ignored an invalid camera: $camera.") + return + } + runOnMain { val map = googleMap ?: return@runOnMain val target = camera.toCameraPosition(map.cameraPosition) diff --git a/package/android/src/test/java/com/margelo/nitro/nitromaps/CameraValidityTest.kt b/package/android/src/test/java/com/margelo/nitro/nitromaps/CameraValidityTest.kt new file mode 100644 index 00000000..5275dd88 --- /dev/null +++ b/package/android/src/test/java/com/margelo/nitro/nitromaps/CameraValidityTest.kt @@ -0,0 +1,74 @@ +package com.margelo.nitro.nitromaps + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class CameraValidityTest { + @Test + fun acceptsACameraTheSdkCanRepresent() { + assertTrue(camera().isValid()) + assertTrue(camera(latitude = -90.0, longitude = 180.0).isValid()) + } + + @Test + fun acceptsACameraThatSuppliesNoFramingValues() { + assertTrue(camera(zoom = null, heading = null, pitch = null, altitude = null).isValid()) + } + + @Test + fun rejectsANonFiniteCenter() { + assertFalse(camera(latitude = Double.NaN, longitude = Double.NaN).isValid()) + assertFalse(camera(longitude = Double.POSITIVE_INFINITY).isValid()) + } + + @Test + fun rejectsACenterOutsideTheWorld() { + assertFalse(camera(latitude = 1000.0).isValid()) + assertFalse(camera(longitude = -180.0001).isValid()) + } + + @Test + fun rejectsANonFiniteFramingValue() { + assertFalse(camera(zoom = Double.NaN).isValid()) + assertFalse(camera(heading = Double.POSITIVE_INFINITY).isValid()) + assertFalse(camera(pitch = Double.NaN).isValid()) + assertFalse(camera(altitude = Double.NEGATIVE_INFINITY).isValid()) + } + + /** A pitch past the drawable range is clamped rather than rejected, so the camera still applies. */ + @Test + fun acceptsAPitchOutsideTheDrawableRange() { + assertTrue(camera(pitch = 120.0).isValid()) + assertTrue(camera(pitch = -10.0).isValid()) + } + + /** + * `CameraPosition.Builder.tilt` throws `IllegalArgumentException` for anything outside 0..90, so + * the clamp is what keeps a valid camera with an unsupported pitch from taking the mount + * transaction down. + */ + @Test + fun clampsAPitchTheSdkWouldRefuse() { + assertEquals(90.0f, camera(pitch = 120.0).toCameraPosition().tilt, 0.0f) + assertEquals(0.0f, camera(pitch = -10.0).toCameraPosition().tilt, 0.0f) + assertEquals(45.0f, camera(pitch = 45.0).toCameraPosition().tilt, 0.0f) + } + + private fun camera( + latitude: Double = 52.23, + longitude: Double = 21.01, + zoom: Double? = 12.0, + heading: Double? = 90.0, + pitch: Double? = 45.0, + altitude: Double? = 1000.0, + ): Camera = + Camera( + center = Coordinate(latitude = latitude, longitude = longitude), + zoom = zoom, + heading = heading, + pitch = pitch, + altitude = altitude, + ) +} diff --git a/package/ios/AppleMapProviderAdapter.swift b/package/ios/AppleMapProviderAdapter.swift index c7b792b3..ac0b1276 100644 --- a/package/ios/AppleMapProviderAdapter.swift +++ b/package/ios/AppleMapProviderAdapter.swift @@ -271,6 +271,15 @@ final class AppleMapProviderAdapter: MapProviderAdapter { } func updateMapCamera(_ camera: Camera, animated: Bool, duration: Double = 0) { + // `setCamera` raises an Objective-C NSException - `Invalid camera + // centerCoordinate` - from `-[MKMapCamera _validate]` for a center MapKit + // cannot place, and Swift cannot catch that. The framing values do not + // raise, but a non-finite one collapses the altitude or leaves + // `view.region` reading back as `NaN`. + guard camera.isValid else { + return + } + let mapCamera = camera.toMKMapCamera() guard !view.camera.approximatelyEquals(mapCamera) else { return diff --git a/package/ios/Camera+Validity.swift b/package/ios/Camera+Validity.swift new file mode 100644 index 00000000..1ca276d9 --- /dev/null +++ b/package/ios/Camera+Validity.swift @@ -0,0 +1,12 @@ +extension Camera { + /// Whether the camera can be handed to `MKMapView.camera` without MapKit raising. + var isValid: Bool { + center.isValid + && CameraFraming.isDrawable( + zoom: zoom, + heading: heading, + pitch: pitch, + altitude: altitude + ) + } +} diff --git a/package/ios/Geometry/CameraFraming.swift b/package/ios/Geometry/CameraFraming.swift new file mode 100644 index 00000000..c789bfb4 --- /dev/null +++ b/package/ios/Geometry/CameraFraming.swift @@ -0,0 +1,31 @@ +import Foundation + +/// The framing half of a camera - zoom, heading, pitch and altitude - which +/// `CLLocationCoordinate2DIsValid` says nothing about. +enum CameraFraming { + /// Every value the caller supplied has to be a real number. `MKMapCamera` + /// does not reject a `NaN` one: it collapses the altitude to the minimum the + /// map allows, or leaves `MKMapView.region` reading back as `NaN`. On Android + /// a non-finite tilt throws out of `CameraPosition` instead. + static func isDrawable( + zoom: Double?, + heading: Double?, + pitch: Double?, + altitude: Double? + ) -> Bool { + isRealOrAbsent(zoom) + && isRealOrAbsent(heading) + && isRealOrAbsent(pitch) + && isRealOrAbsent(altitude) + } + + /// An omitted value is filled in from the camera the map already has, so only + /// a supplied one has to be checked. + private static func isRealOrAbsent(_ value: Double?) -> Bool { + guard let value else { + return true + } + + return value.isFinite + } +} diff --git a/package/ios/GoogleMapProviderAdapter.swift b/package/ios/GoogleMapProviderAdapter.swift index 898f0713..b33a9679 100644 --- a/package/ios/GoogleMapProviderAdapter.swift +++ b/package/ios/GoogleMapProviderAdapter.swift @@ -32,8 +32,10 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { } lazy var view: GMSMapView = { - let camera = - self.camera?.toGMSCameraPosition() + // The map is created from the stored prop directly, so it is checked here + // too: `updateMapCamera` never runs for the camera the map starts with. + let initialCamera = + self.camera.flatMap { $0.isValid ? $0.toGMSCameraPosition() : nil } ?? GMSCameraPosition(latitude: 0, longitude: 0, zoom: 10) let mapView: GMSMapView if let googleMapId = _googleMapId?.trimmingCharacters(in: .whitespacesAndNewlines), @@ -42,10 +44,10 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { mapView = GMSMapView( frame: .zero, mapID: GMSMapID(identifier: googleMapId), - camera: camera + camera: initialCamera ) } else { - mapView = GMSMapView(frame: .zero, camera: camera) + mapView = GMSMapView(frame: .zero, camera: initialCamera) } mapView.delegate = self @@ -320,6 +322,10 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { } private func updateMapCamera(_ camera: Camera, animated: Bool, duration: Double? = nil) { + guard camera.isValid else { + return + } + let target = camera.toGMSCameraPosition(current: view.camera) guard !view.camera.approximatelyEquals(target) else { return diff --git a/package/ios/Tests/Geometry/CameraFramingTests.swift b/package/ios/Tests/Geometry/CameraFramingTests.swift new file mode 100644 index 00000000..56091868 --- /dev/null +++ b/package/ios/Tests/Geometry/CameraFramingTests.swift @@ -0,0 +1,31 @@ +import Testing + +@testable import NitroMapsGeometry + +@Test +func acceptsFramingValuesTheMapCanUse() { + #expect(CameraFraming.isDrawable(zoom: 12, heading: 90, pitch: 45, altitude: 1000)) + #expect(CameraFraming.isDrawable(zoom: 0, heading: 0, pitch: 0, altitude: 0)) +} + +@Test +func acceptsAbsentFramingValues() { + #expect(CameraFraming.isDrawable(zoom: nil, heading: nil, pitch: nil, altitude: nil)) + #expect(CameraFraming.isDrawable(zoom: 12, heading: nil, pitch: nil, altitude: nil)) +} + +@Test +func rejectsANonFiniteFramingValue() { + #expect(!CameraFraming.isDrawable(zoom: .nan, heading: nil, pitch: nil, altitude: nil)) + #expect(!CameraFraming.isDrawable(zoom: nil, heading: .infinity, pitch: nil, altitude: nil)) + #expect(!CameraFraming.isDrawable(zoom: nil, heading: nil, pitch: .nan, altitude: nil)) + #expect(!CameraFraming.isDrawable(zoom: nil, heading: nil, pitch: nil, altitude: -.infinity)) +} + +/// A pitch past the range the SDKs draw is pulled back to it rather than +/// rejected, so the camera still reaches the map. +@Test +func acceptsAPitchOutsideTheDrawableRange() { + #expect(CameraFraming.isDrawable(zoom: nil, heading: nil, pitch: 120, altitude: nil)) + #expect(CameraFraming.isDrawable(zoom: nil, heading: nil, pitch: -10, altitude: nil)) +} diff --git a/package/src/camera/__tests__/isValidCamera.test.ts b/package/src/camera/__tests__/isValidCamera.test.ts new file mode 100644 index 00000000..77d486ad --- /dev/null +++ b/package/src/camera/__tests__/isValidCamera.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, test } from 'bun:test'; +import { isValidCamera } from '../isValidCamera'; + +const validCamera = { + center: { latitude: 52.23, longitude: 21.01 }, + zoom: 12, + heading: 90, + pitch: 45, + altitude: 1000, +}; + +describe('isValidCamera', () => { + test('accepts a camera both SDKs can represent', () => { + expect(isValidCamera(validCamera)).toBe(true); + expect(isValidCamera({ center: { latitude: -90, longitude: 180 } })).toBe( + true, + ); + }); + + test('accepts a camera that supplies no framing values', () => { + expect(isValidCamera({ center: { latitude: 0, longitude: 0 } })).toBe(true); + }); + + test('rejects a missing camera', () => { + expect(isValidCamera(undefined)).toBe(false); + }); + + test('rejects a non-finite center', () => { + expect( + isValidCamera({ + ...validCamera, + center: { latitude: Number.NaN, longitude: Number.NaN }, + }), + ).toBe(false); + expect( + isValidCamera({ + ...validCamera, + center: { latitude: 0, longitude: Number.POSITIVE_INFINITY }, + }), + ).toBe(false); + }); + + test('rejects a center outside the world', () => { + expect( + isValidCamera({ + ...validCamera, + center: { latitude: 1000, longitude: 0 }, + }), + ).toBe(false); + expect( + isValidCamera({ + ...validCamera, + center: { latitude: 0, longitude: 181 }, + }), + ).toBe(false); + }); + + test('rejects a non-finite framing value', () => { + expect(isValidCamera({ ...validCamera, zoom: Number.NaN })).toBe(false); + expect( + isValidCamera({ ...validCamera, heading: Number.POSITIVE_INFINITY }), + ).toBe(false); + expect(isValidCamera({ ...validCamera, pitch: Number.NaN })).toBe(false); + expect( + isValidCamera({ ...validCamera, altitude: Number.NEGATIVE_INFINITY }), + ).toBe(false); + }); + + // Android throws on a tilt past 90 and MapKit flattens it; both are handled + // natively by clamping, so the camera itself stays usable. + test('accepts a pitch outside the range the SDKs draw', () => { + expect(isValidCamera({ ...validCamera, pitch: 120 })).toBe(true); + expect(isValidCamera({ ...validCamera, pitch: -10 })).toBe(true); + }); +}); diff --git a/package/src/camera/__tests__/resolveCameraProp.test.ts b/package/src/camera/__tests__/resolveCameraProp.test.ts new file mode 100644 index 00000000..86e69c4e --- /dev/null +++ b/package/src/camera/__tests__/resolveCameraProp.test.ts @@ -0,0 +1,90 @@ +import { afterEach, beforeEach, describe, expect, spyOn, test } from 'bun:test'; +import { resolveCameraProp } from '../resolveCameraProp'; + +const warnSpy = spyOn(console, 'warn'); +const previousDev = (globalThis as { __DEV__?: boolean }).__DEV__; + +const validCamera = { + center: { latitude: 52.23, longitude: 21.01 }, + zoom: 12, +}; + +function restoreDevFlag(): void { + const globalDev = globalThis as { __DEV__?: boolean }; + if (previousDev === undefined) { + delete globalDev.__DEV__; + return; + } + + globalDev.__DEV__ = previousDev; +} + +beforeEach(() => { + warnSpy.mockClear(); +}); + +afterEach(() => { + warnSpy.mockClear(); + restoreDevFlag(); +}); + +describe('resolveCameraProp', () => { + test('passes a valid camera through unchanged', () => { + (globalThis as { __DEV__?: boolean }).__DEV__ = true; + + expect(resolveCameraProp(validCamera, undefined)).toBe(validCamera); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + test('leaves a missing camera alone without warning', () => { + (globalThis as { __DEV__?: boolean }).__DEV__ = true; + + expect(resolveCameraProp(undefined, undefined)).toBeUndefined(); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + test('holds the last accepted camera when the prop is unset', () => { + (globalThis as { __DEV__?: boolean }).__DEV__ = true; + + // `camera={following ? camera : undefined}` on a mounted view: unsetting it + // reaches the native view as `null`, which throws in the struct converter. + expect(resolveCameraProp(undefined, validCamera)).toBe(validCamera); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + test('drops an invalid camera and warns in development', () => { + (globalThis as { __DEV__?: boolean }).__DEV__ = true; + + expect( + resolveCameraProp( + { center: { latitude: Number.NaN, longitude: Number.NaN } }, + undefined, + ), + ).toBeUndefined(); + + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy.mock.calls[0]?.[0]).toContain('camera ignored'); + }); + + test('holds the last accepted camera instead of unsetting the prop', () => { + (globalThis as { __DEV__?: boolean }).__DEV__ = true; + + // Unsetting it would reach the native view as `null`, which the generated + // struct converter rejects before any native guard runs. + expect( + resolveCameraProp({ ...validCamera, pitch: Number.NaN }, validCamera), + ).toBe(validCamera); + }); + + test('drops an invalid camera silently outside development', () => { + (globalThis as { __DEV__?: boolean }).__DEV__ = false; + + expect( + resolveCameraProp( + { center: { latitude: 1000, longitude: 0 } }, + undefined, + ), + ).toBeUndefined(); + expect(warnSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/package/src/camera/isValidCamera.ts b/package/src/camera/isValidCamera.ts new file mode 100644 index 00000000..50a6aadb --- /dev/null +++ b/package/src/camera/isValidCamera.ts @@ -0,0 +1,25 @@ +import type { Camera } from '../types/camera'; +import { isValidCoordinate } from '../utils/validateGeometry'; + +// An omitted value is filled in from the camera the map already has, so only a +// supplied one has to be a real number. +function isFiniteOrAbsent(value: number | undefined): boolean { + return value == null || Number.isFinite(value); +} + +/** + * A camera reaches `MKMapCamera` and `CameraPosition` unchanged. MapKit raises + * an uncatchable `NSException` for a center it cannot place, and Google Maps + * throws for a non-finite pitch; the remaining non-finite values are taken + * silently and leave the camera reading back as `NaN`. + */ +export function isValidCamera(value: Camera | undefined): boolean { + return ( + value != null && + isValidCoordinate(value.center) && + isFiniteOrAbsent(value.zoom) && + isFiniteOrAbsent(value.heading) && + isFiniteOrAbsent(value.pitch) && + isFiniteOrAbsent(value.altitude) + ); +} diff --git a/package/src/camera/resolveCameraProp.ts b/package/src/camera/resolveCameraProp.ts new file mode 100644 index 00000000..877c0f80 --- /dev/null +++ b/package/src/camera/resolveCameraProp.ts @@ -0,0 +1,31 @@ +import type { Camera } from '../types/camera'; +import { isValidCamera } from './isValidCamera'; +import { warnCamera } from './warnCamera'; + +// Once the view has accepted a camera the prop must never go back to +// `undefined`, whether it was unset or is unusable - the invariant +// `resolveRegionProp` documents: React rewrites a removed prop to `null`, the +// optional JSI converter only short-circuits on `undefined`, and the generated +// struct converter then calls `asObject` on a null and throws, before any +// native guard runs. Holding the last accepted camera leaves the prop +// untouched instead, which is also what the map should show. Before anything +// has been accepted there is nothing to hold, and `undefined` at mount is +// safe: React omits the key entirely. +export function resolveCameraProp( + camera: Camera | undefined, + lastAccepted: Camera | undefined, +): Camera | undefined { + if (camera == null) { + return lastAccepted; + } + + if (isValidCamera(camera)) { + return camera; + } + + warnCamera( + 'camera ignored: invalid center coordinate, or non-finite zoom, heading, pitch or altitude', + ); + + return lastAccepted; +} diff --git a/package/src/camera/useValidCamera.ts b/package/src/camera/useValidCamera.ts new file mode 100644 index 00000000..3196b463 --- /dev/null +++ b/package/src/camera/useValidCamera.ts @@ -0,0 +1,25 @@ +import { useLayoutEffect, useMemo, useRef } from 'react'; +import type { Camera } from '../types/camera'; +import { resolveCameraProp } from './resolveCameraProp'; + +/** + * Holds the last camera the native view accepted, so an unset or invalid one + * never makes the prop transition back to `undefined`. + * + * The counterpart of `useValidRegion`, and memoized the same way: on camera + * identity, so a stable object is checked - and warned about - once rather than + * on every render, with the ref written after commit. + */ +export function useValidCamera(camera: Camera | undefined): Camera | undefined { + const lastAccepted = useRef(undefined); + const accepted = useMemo( + () => resolveCameraProp(camera, lastAccepted.current), + [camera], + ); + + useLayoutEffect(() => { + lastAccepted.current = accepted; + }, [accepted]); + + return accepted; +} diff --git a/package/src/camera/warnCamera.ts b/package/src/camera/warnCamera.ts new file mode 100644 index 00000000..0fc99daa --- /dev/null +++ b/package/src/camera/warnCamera.ts @@ -0,0 +1,3 @@ +import { createWarn } from '../utils/warn'; + +export const warnCamera = createWarn('Camera'); diff --git a/package/src/components/MapView.tsx b/package/src/components/MapView.tsx index 040a0467..7c6f9af7 100644 --- a/package/src/components/MapView.tsx +++ b/package/src/components/MapView.tsx @@ -6,6 +6,7 @@ import { type Ref, type RefObject, } from 'react'; +import { useValidCamera } from '../camera/useValidCamera'; import { useCollectedOverlays } from '../hooks/useCollectedOverlays'; import { useNitroCallback } from '../hooks/useNitroCallback'; import { useStableValue } from '../hooks/useStableValue'; @@ -134,6 +135,7 @@ export function MapView({ enteringAnimationsEqual, ); const validRegion = useValidRegion(region); + const validCamera = useValidCamera(camera); const hasMarkerPress = onMarkerPressProp != null || hasCollectedMarkerPress; @@ -285,7 +287,7 @@ export function MapView({ googleMapId={googleMapId} mapType={mapType} region={validRegion} - camera={camera} + camera={validCamera} scrollEnabled={scrollEnabled} zoomEnabled={zoomEnabled} rotateEnabled={rotateEnabled} From dde956e6fc1f1a622818d7e329d4e5a1ebd05eac Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Tue, 22 Sep 2026 10:06:30 +0200 Subject: [PATCH 2/2] fix: reject a camera zoom or heading that overflows a Float `Camera.isValid()` checked `Double.isFinite()`, but `CameraPosition` keeps zoom and bearing as `Float`, so the value the SDK receives is the narrowed one. A `Double` past `Float.MAX_VALUE` becomes `Infinity`, and the builder takes it without complaint - probed against the real SDK on the JVM, it returns `CameraPosition{zoom=Infinity, tilt=45.0, bearing=NaN}`, because the builder's `% 360` normalization turns an infinite bearing into `NaN`. That `NaN` readback is exactly what the guard exists to prevent, so the check has to run on the converted value. - Kotlin: zoom and heading are checked after the conversion. Pitch keeps the `Double` check - it is coerced into 0..90 before it is narrowed, so an absurd pitch still clamps rather than dropping the camera - and altitude never reaches `CameraPosition` at all. - Swift: `GMSCameraPosition` narrows zoom the same way, while heading, pitch and altitude stay `Double` through `CLLocationDirection` and `MKMapCamera`, so `CameraFraming` checks `Float(zoom)` and leaves the rest alone. - JS: the same bound on zoom and heading, so an overflowing value reaches the developer as the `__DEV__` warning rather than being dropped natively with nothing said. It is spelled as an explicit constant rather than `Math.fround`, to keep the validator independent of the engine's `Math` implementation, and it rejects the last ulp either way. The warning text drops the word "non-finite", which an overflowing value is not. --- README.md | 2 +- .../nitro/nitromaps/Camera+Validity.kt | 18 ++++++++++-- .../nitro/nitromaps/CameraValidityTest.kt | 29 +++++++++++++++++++ package/ios/Geometry/CameraFraming.swift | 15 +++++++++- .../Tests/Geometry/CameraFramingTests.swift | 18 ++++++++++++ .../camera/__tests__/isValidCamera.test.ts | 24 +++++++++++++++ package/src/camera/isValidCamera.ts | 18 ++++++++++-- package/src/camera/resolveCameraProp.ts | 2 +- 8 files changed, 118 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index c4aed221..1005466f 100644 --- a/README.md +++ b/README.md @@ -582,7 +582,7 @@ Where the check runs depends on the entry point. `region`, `camera` and `fitToCo One gap is worth knowing about: descriptors passed through the bulk `markers` prop are checked on neither side - only the `` child is. -Valid means: latitude and longitude finite and within ±90 / ±180, region deltas finite and greater than 0, camera `zoom` / `heading` / `pitch` / `altitude` finite when supplied, 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. +Valid means: latitude and longitude finite and within ±90 / ±180, region deltas finite and greater than 0, camera `zoom` / `heading` / `pitch` / `altitude` finite when supplied (with `zoom` and `heading` also small enough for the 32-bit float the SDKs keep them in), 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 diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+Validity.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+Validity.kt index 02baddb9..0f7551d1 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+Validity.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+Validity.kt @@ -7,10 +7,22 @@ package com.margelo.nitro.nitromaps */ internal fun Camera.isValid(): Boolean = isValidCoordinate(center.latitude, center.longitude) && - zoom.isFiniteOrAbsent() && - heading.isFiniteOrAbsent() && + zoom.isDrawableAsFloatOrAbsent() && + heading.isDrawableAsFloatOrAbsent() && pitch.isFiniteOrAbsent() && altitude.isFiniteOrAbsent() -/** An absent value is filled in from the camera the map already has, so only a supplied one is checked. */ +/** + * `CameraPosition` holds zoom and bearing as `Float`, so the value the SDK receives is the + * converted one: a `Double` past `Float.MAX_VALUE` - `Double.MAX_VALUE` among them - becomes + * `Infinity`, which the builder takes without complaint and then normalizes into a `NaN` bearing. + * Checking after the conversion is what makes this guard match what the map is handed. + */ +private fun Double?.isDrawableAsFloatOrAbsent(): Boolean = this == null || toFloat().isFinite() + +/** + * Pitch and altitude stay `Double`: pitch is coerced into the drawable range before it is narrowed, + * and altitude never reaches `CameraPosition` at all. An absent value is filled in from the camera + * the map already has, so only a supplied one is checked. + */ private fun Double?.isFiniteOrAbsent(): Boolean = this == null || isFinite() diff --git a/package/android/src/test/java/com/margelo/nitro/nitromaps/CameraValidityTest.kt b/package/android/src/test/java/com/margelo/nitro/nitromaps/CameraValidityTest.kt index 5275dd88..de9a3e88 100644 --- a/package/android/src/test/java/com/margelo/nitro/nitromaps/CameraValidityTest.kt +++ b/package/android/src/test/java/com/margelo/nitro/nitromaps/CameraValidityTest.kt @@ -56,6 +56,35 @@ class CameraValidityTest { assertEquals(45.0f, camera(pitch = 45.0).toCameraPosition().tilt, 0.0f) } + /** + * Zoom and bearing are narrowed to `Float` on the way into `CameraPosition`, so a `Double` past + * `Float.MAX_VALUE` is not something the map can be handed, however finite it is as a `Double`. + */ + @Test + fun rejectsAFramingValueThatOverflowsAFloat() { + assertFalse(camera(zoom = Double.MAX_VALUE).isValid()) + assertFalse(camera(heading = -Double.MAX_VALUE).isValid()) + } + + /** + * What the guard above prevents, pinned against the real SDK: the builder takes the overflowed + * value without complaint, and its `% 360` normalization turns an infinite bearing into `NaN`. + */ + @Test + fun anOverflowingFramingValueWouldReachTheSdkAsInfinity() { + val position = camera(zoom = Double.MAX_VALUE, heading = Double.MAX_VALUE).toCameraPosition() + + assertEquals(Float.POSITIVE_INFINITY, position.zoom, 0.0f) + assertTrue(position.bearing.isNaN()) + } + + /** Pitch is coerced into the drawable range before it is narrowed, so an absurd one still draws. */ + @Test + fun clampsAPitchThatOverflowsAFloat() { + assertTrue(camera(pitch = Double.MAX_VALUE).isValid()) + assertEquals(90.0f, camera(pitch = Double.MAX_VALUE).toCameraPosition().tilt, 0.0f) + } + private fun camera( latitude: Double = 52.23, longitude: Double = 21.01, diff --git a/package/ios/Geometry/CameraFraming.swift b/package/ios/Geometry/CameraFraming.swift index c789bfb4..af48414a 100644 --- a/package/ios/Geometry/CameraFraming.swift +++ b/package/ios/Geometry/CameraFraming.swift @@ -13,12 +13,25 @@ enum CameraFraming { pitch: Double?, altitude: Double? ) -> Bool { - isRealOrAbsent(zoom) + isRealAsFloatOrAbsent(zoom) && isRealOrAbsent(heading) && isRealOrAbsent(pitch) && isRealOrAbsent(altitude) } + /// `GMSCameraPosition` holds zoom as a `Float`, and `CameraPosition` does the + /// same on Android, so the value the SDK receives is the narrowed one: a + /// `Double` past `Float.greatestFiniteMagnitude` arrives as `infinity`. Zoom + /// is the only value narrowed that way - heading, pitch and altitude stay + /// `Double` through `CLLocationDirection` and `MKMapCamera`. + private static func isRealAsFloatOrAbsent(_ value: Double?) -> Bool { + guard let value else { + return true + } + + return value.isFinite && Float(value).isFinite + } + /// An omitted value is filled in from the camera the map already has, so only /// a supplied one has to be checked. private static func isRealOrAbsent(_ value: Double?) -> Bool { diff --git a/package/ios/Tests/Geometry/CameraFramingTests.swift b/package/ios/Tests/Geometry/CameraFramingTests.swift index 56091868..28b08040 100644 --- a/package/ios/Tests/Geometry/CameraFramingTests.swift +++ b/package/ios/Tests/Geometry/CameraFramingTests.swift @@ -29,3 +29,21 @@ func acceptsAPitchOutsideTheDrawableRange() { #expect(CameraFraming.isDrawable(zoom: nil, heading: nil, pitch: 120, altitude: nil)) #expect(CameraFraming.isDrawable(zoom: nil, heading: nil, pitch: -10, altitude: nil)) } + +/// Zoom is narrowed to a `Float` on its way into both SDKs, so a `Double` too +/// large for one is not a zoom the map can be handed, however finite it is. +@Test +func rejectsAZoomThatOverflowsAFloat() { + #expect(!CameraFraming.isDrawable(zoom: .greatestFiniteMagnitude, heading: nil, pitch: nil, altitude: nil)) + #expect(!CameraFraming.isDrawable(zoom: -.greatestFiniteMagnitude, heading: nil, pitch: nil, altitude: nil)) + #expect(CameraFraming.isDrawable(zoom: 3.4e38, heading: nil, pitch: nil, altitude: nil)) +} + +/// Heading, pitch and altitude stay `Double` all the way to `MKMapCamera` and +/// `CLLocationDirection`, so a large one is still a value the map can take. +@Test +func keepsALargeHeadingPitchOrAltitude() { + #expect(CameraFraming.isDrawable(zoom: nil, heading: .greatestFiniteMagnitude, pitch: nil, altitude: nil)) + #expect(CameraFraming.isDrawable(zoom: nil, heading: nil, pitch: .greatestFiniteMagnitude, altitude: nil)) + #expect(CameraFraming.isDrawable(zoom: nil, heading: nil, pitch: nil, altitude: .greatestFiniteMagnitude)) +} diff --git a/package/src/camera/__tests__/isValidCamera.test.ts b/package/src/camera/__tests__/isValidCamera.test.ts index 77d486ad..d63348b4 100644 --- a/package/src/camera/__tests__/isValidCamera.test.ts +++ b/package/src/camera/__tests__/isValidCamera.test.ts @@ -72,4 +72,28 @@ describe('isValidCamera', () => { expect(isValidCamera({ ...validCamera, pitch: 120 })).toBe(true); expect(isValidCamera({ ...validCamera, pitch: -10 })).toBe(true); }); + + // Zoom, and bearing on Android, are narrowed to a 32-bit float before the SDK + // sees them, so a number past that range arrives as `Infinity` however finite + // it is here. + test('rejects a framing value that overflows a 32-bit float', () => { + expect(isValidCamera({ ...validCamera, zoom: Number.MAX_VALUE })).toBe( + false, + ); + expect(isValidCamera({ ...validCamera, heading: -Number.MAX_VALUE })).toBe( + false, + ); + expect(isValidCamera({ ...validCamera, zoom: 3.4e38 })).toBe(true); + }); + + // Pitch and altitude stay 64-bit on both platforms - pitch is clamped before + // it is narrowed, and altitude never reaches the Android camera at all. + test('accepts a large pitch or altitude', () => { + expect(isValidCamera({ ...validCamera, pitch: Number.MAX_VALUE })).toBe( + true, + ); + expect(isValidCamera({ ...validCamera, altitude: Number.MAX_VALUE })).toBe( + true, + ); + }); }); diff --git a/package/src/camera/isValidCamera.ts b/package/src/camera/isValidCamera.ts index 50a6aadb..9b57d73a 100644 --- a/package/src/camera/isValidCamera.ts +++ b/package/src/camera/isValidCamera.ts @@ -7,6 +7,20 @@ function isFiniteOrAbsent(value: number | undefined): boolean { return value == null || Number.isFinite(value); } +// Both SDKs keep zoom as a 32-bit float, and `CameraPosition` keeps bearing as +// one too, so the value they receive is the narrowed one, and anything past +// this range arrives as `Infinity`. The native guards check the narrowed value +// itself; this one is what turns it into a warning, and rejects the last ulp +// either way rather than depending on how the runtime rounds it. +const LARGEST_FLOAT_32 = 3.4028234663852886e38; + +function isDrawableAsFloatOrAbsent(value: number | undefined): boolean { + return ( + value == null || + (Number.isFinite(value) && Math.abs(value) <= LARGEST_FLOAT_32) + ); +} + /** * A camera reaches `MKMapCamera` and `CameraPosition` unchanged. MapKit raises * an uncatchable `NSException` for a center it cannot place, and Google Maps @@ -17,8 +31,8 @@ export function isValidCamera(value: Camera | undefined): boolean { return ( value != null && isValidCoordinate(value.center) && - isFiniteOrAbsent(value.zoom) && - isFiniteOrAbsent(value.heading) && + isDrawableAsFloatOrAbsent(value.zoom) && + isDrawableAsFloatOrAbsent(value.heading) && isFiniteOrAbsent(value.pitch) && isFiniteOrAbsent(value.altitude) ); diff --git a/package/src/camera/resolveCameraProp.ts b/package/src/camera/resolveCameraProp.ts index 877c0f80..8b4b4a8e 100644 --- a/package/src/camera/resolveCameraProp.ts +++ b/package/src/camera/resolveCameraProp.ts @@ -24,7 +24,7 @@ export function resolveCameraProp( } warnCamera( - 'camera ignored: invalid center coordinate, or non-finite zoom, heading, pitch or altitude', + 'camera ignored: invalid center coordinate, or a zoom, heading, pitch or altitude the map cannot use', ); return lastAccepted;