From acdff333312aff15f50926e44421155abe7b414b Mon Sep 17 00:00:00 2001 From: Peng Liu Date: Thu, 22 Sep 2022 13:02:34 +0300 Subject: [PATCH] Throw exception when constructing Value with NaN or infinite numbers. (#1681) --- CHANGELOG.md | 1 + .../maps/testapp/utils/NavigationSimulator.kt | 8 ++- .../style/layers/properties/PropertyValue.kt | 6 +- .../maps/extension/style/utils/TypeUtils.kt | 12 ++-- .../extension/style/utils/TypeUtilsTest.kt | 72 +++++++++++++++++-- 5 files changed, 85 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e1166a80c..8a937529d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Mapbox welcomes participation and contributions from everyone. * Symbol flickering after runtime styling caused by using outdated symbol placement on updated symbol buckets. ([1679](https://github.com/mapbox/mapbox-maps-android/pull/1679)) * Allow Light intensity and Light shadow intensity to accept expressions when using setStyleLight API call. ([1679](https://github.com/mapbox/mapbox-maps-android/pull/1679)) * Fix a bug in cameraForGeometry when called on a map with padding set. ([1679](https://github.com/mapbox/mapbox-maps-android/pull/1679)) +* Throw an exception with the meaningful stacktrace when constructing `Value` from an infinite or NaN number. ([1681](https://github.com/mapbox/mapbox-maps-android/pull/1681)) ## Dependencies * Update mapbox-gestures-android dependency to [v0.8.0](https://github.com/mapbox/mapbox-gestures-android/releases/tag/v0.8.0). ([1645] (https://github.com/mapbox/mapbox-maps-android/pull/1645)) diff --git a/app/src/main/java/com/mapbox/maps/testapp/utils/NavigationSimulator.kt b/app/src/main/java/com/mapbox/maps/testapp/utils/NavigationSimulator.kt index 44cc34d377..51c5d146bd 100644 --- a/app/src/main/java/com/mapbox/maps/testapp/utils/NavigationSimulator.kt +++ b/app/src/main/java/com/mapbox/maps/testapp/utils/NavigationSimulator.kt @@ -335,8 +335,12 @@ class NavigationSimulator( override fun onIndicatorPositionChanged(point: Point) { // use altitude to pass through interpolated progress data, reduce the overhead to // calculate the progress on each frame. - routeLayer.lineTrimOffset(listOf(0.0, point.altitude())) - casingLayer.lineTrimOffset(listOf(0.0, point.altitude())) + if (point.hasAltitude()) { + with(point.altitude()) { + routeLayer.lineTrimOffset(listOf(0.0, this)) + casingLayer.lineTrimOffset(listOf(0.0, this)) + } + } } fun onDestroy() { diff --git a/extension-style/src/main/java/com/mapbox/maps/extension/style/layers/properties/PropertyValue.kt b/extension-style/src/main/java/com/mapbox/maps/extension/style/layers/properties/PropertyValue.kt index a7df270cfb..d98ce4b6c8 100644 --- a/extension-style/src/main/java/com/mapbox/maps/extension/style/layers/properties/PropertyValue.kt +++ b/extension-style/src/main/java/com/mapbox/maps/extension/style/layers/properties/PropertyValue.kt @@ -19,7 +19,11 @@ open class PropertyValue internal constructor(val propertyName: String, val p /** * The [Value] representation of the property. */ - val value: Value = TypeUtils.wrapToValue(propertyValue as Any) + val value: Value = try { + TypeUtils.wrapToValue(propertyValue as Any) + } catch (e: IllegalArgumentException) { + throw IllegalArgumentException("Incorrect property value for $propertyName: ${e.message}", e.cause) + } /** * Returns if this is a expression. diff --git a/extension-style/src/main/java/com/mapbox/maps/extension/style/utils/TypeUtils.kt b/extension-style/src/main/java/com/mapbox/maps/extension/style/utils/TypeUtils.kt index 645e2cd3b4..43292d4ca2 100644 --- a/extension-style/src/main/java/com/mapbox/maps/extension/style/utils/TypeUtils.kt +++ b/extension-style/src/main/java/com/mapbox/maps/extension/style/utils/TypeUtils.kt @@ -44,32 +44,34 @@ internal object TypeUtils { Value(value) } is Double -> { + require(value.isFinite()) { "Value can not be Double.NaN, Double.POSITIVE_INFINITY or Double.NEGATIVE_INFINITY" } Value(value) } is Float -> { + require(value.isFinite()) { "Value can not be Float.NaN, Float.POSITIVE_INFINITY or Float.NEGATIVE_INFINITY" } Value(value.toDouble()) } is Long -> { Value(value) } is IntArray -> { - val valueArray = value.map { Value(it.toLong()) } + val valueArray = value.map { wrapToValue(it.toLong()) } Value(valueArray) } is BooleanArray -> { - val valueArray = value.map(::Value) + val valueArray = value.map(::wrapToValue) Value(valueArray) } is DoubleArray -> { - val valueArray = value.map(::Value) + val valueArray = value.map(::wrapToValue) Value(valueArray) } is FloatArray -> { - val valueArray = value.map { Value(it.toDouble()) } + val valueArray = value.map { wrapToValue(it.toDouble()) } Value(valueArray) } is LongArray -> { - val valueArray = value.map(::Value) + val valueArray = value.map(::wrapToValue) Value(valueArray) } is Array<*> -> { diff --git a/extension-style/src/test/java/com/mapbox/maps/extension/style/utils/TypeUtilsTest.kt b/extension-style/src/test/java/com/mapbox/maps/extension/style/utils/TypeUtilsTest.kt index 993a5e219a..ed7bb8854a 100644 --- a/extension-style/src/test/java/com/mapbox/maps/extension/style/utils/TypeUtilsTest.kt +++ b/extension-style/src/test/java/com/mapbox/maps/extension/style/utils/TypeUtilsTest.kt @@ -35,27 +35,42 @@ class TypeUtilsTest { @Test fun wrapToValue_Int() { - testwrapToValue(123) + testWrapToValue(123) } @Test fun wrapToValue_Long() { - testwrapToValue(123L) + testWrapToValue(123L) } @Test fun wrapToValue_String() { - testwrapToValue("abc") + testWrapToValue("abc") } @Test fun wrapToValue_Boolean() { - testwrapToValue(true) + testWrapToValue(true) } @Test fun wrapToValue_Double() { - testwrapToValue(0.01) + testWrapToValue(0.01) + } + + @Test(expected = IllegalArgumentException::class) + fun wrapToValue_Double_NaN() { + testWrapToValue(Double.NaN) + } + + @Test(expected = IllegalArgumentException::class) + fun wrapToValue_Double_POSITIVE_INFINITY() { + testWrapToValue(Double.POSITIVE_INFINITY) + } + + @Test(expected = IllegalArgumentException::class) + fun wrapToValue_Double_NEGATIVE_INFINITY() { + testWrapToValue(Double.NEGATIVE_INFINITY) } @Test @@ -64,7 +79,22 @@ class TypeUtilsTest { assertEquals(0.01f.toDouble(), result.contents) } - private fun testwrapToValue(value: Any) { + @Test(expected = IllegalArgumentException::class) + fun wrapToValue_Float_NaN() { + testWrapToValue(Float.NaN) + } + + @Test(expected = IllegalArgumentException::class) + fun wrapToValue_Float_POSITIVE_INFINITY() { + testWrapToValue(Float.POSITIVE_INFINITY) + } + + @Test(expected = IllegalArgumentException::class) + fun wrapToValue_Float_NEGATIVE_INFINITY() { + testWrapToValue(Float.NEGATIVE_INFINITY) + } + + private fun testWrapToValue(value: Any) { val result = TypeUtils.wrapToValue(value) assertEquals(value.toString(), result.toString()) } @@ -87,12 +117,42 @@ class TypeUtilsTest { assertEquals("[0.01, 0.02, 0.03]", result.toString()) } + @Test(expected = IllegalArgumentException::class) + fun wrapToValue_DoubleArray_contains_NaN() { + TypeUtils.wrapToValue(doubleArrayOf(0.01, 0.02, Double.NaN)) + } + + @Test(expected = IllegalArgumentException::class) + fun wrapToValue_DoubleArray_contains_POSITIVE_INFINITY() { + TypeUtils.wrapToValue(doubleArrayOf(0.01, 0.02, Double.POSITIVE_INFINITY)) + } + + @Test(expected = IllegalArgumentException::class) + fun wrapToValue_DoubleArray_contains_NEGATIVE_INFINITY() { + TypeUtils.wrapToValue(doubleArrayOf(0.01, 0.02, Double.NEGATIVE_INFINITY)) + } + @Test fun wrapToValue_FloatArray() { val result = TypeUtils.wrapToValue(floatArrayOf(1.0f, 2.0f, 3.0f)) assertEquals("[1.0, 2.0, 3.0]", result.toString()) } + @Test(expected = IllegalArgumentException::class) + fun wrapToValue_FloatArray_contains_NaN() { + TypeUtils.wrapToValue(floatArrayOf(0.01f, 0.02f, Float.NaN)) + } + + @Test(expected = IllegalArgumentException::class) + fun wrapToValue_FloatArray_contains_POSITIVE_INFINITY() { + TypeUtils.wrapToValue(floatArrayOf(0.01f, 0.02f, Float.POSITIVE_INFINITY)) + } + + @Test(expected = IllegalArgumentException::class) + fun wrapToValue_FloatArray_contains_NEGATIVE_INFINITY() { + TypeUtils.wrapToValue(floatArrayOf(0.01f, 0.02f, Float.NEGATIVE_INFINITY)) + } + @Test fun wrapToValue_LongArray() { val result = TypeUtils.wrapToValue(longArrayOf(1L, 2L, 3L))