From 1586ffaf739849ed2adfb586c18b4c793b5b7ca4 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Mon, 3 Apr 2017 12:18:13 -0700 Subject: [PATCH] [android] Preflight argument validity before constructing LatLng --- .../com/mapbox/mapboxsdk/geometry/LatLng.java | 28 +++++-- .../mapboxsdk/geometry/LatLngBoundsTest.java | 23 +++--- .../mapbox/mapboxsdk/geometry/LatLngTest.java | 75 +++++++++++++++++++ 3 files changed, 104 insertions(+), 22 deletions(-) diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/geometry/LatLng.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/geometry/LatLng.java index 5e3064f75fd..ca2d3673b27 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/geometry/LatLng.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/geometry/LatLng.java @@ -50,8 +50,8 @@ public LatLng() { * @param longitude Longitude in degrees */ public LatLng(double latitude, double longitude) { - this.latitude = latitude; - this.longitude = longitude; + setLatitude(latitude); + setLongitude(longitude); } /** @@ -62,9 +62,9 @@ public LatLng(double latitude, double longitude) { * @param altitude Altitude in meters */ public LatLng(double latitude, double longitude, double altitude) { - this.latitude = latitude; - this.longitude = longitude; - this.altitude = altitude; + setLatitude(latitude); + setLongitude(longitude); + setAltitude(altitude); } /** @@ -88,12 +88,18 @@ public LatLng(LatLng aLatLng) { } protected LatLng(Parcel in) { - latitude = in.readDouble(); - longitude = in.readDouble(); - altitude = in.readDouble(); + setLatitude(in.readDouble()); + setLongitude(in.readDouble()); + setAltitude(in.readDouble()); } public void setLatitude(double latitude) { + if (Double.isNaN(latitude)) { + throw new IllegalArgumentException("latitude must not be NaN"); + } + if (Math.abs(latitude) > 90.0) { + throw new IllegalArgumentException("latitude must be between -90 and 90"); + } this.latitude = latitude; } @@ -103,6 +109,12 @@ public double getLatitude() { } public void setLongitude(double longitude) { + if (Double.isNaN(longitude)) { + throw new IllegalArgumentException("longitude must not be NaN"); + } + if (Double.isInfinite(longitude)) { + throw new IllegalArgumentException("longitude must not be infinite"); + } this.longitude = longitude; } diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngBoundsTest.java b/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngBoundsTest.java index 4630f9f8b66..8d9a360714e 100644 --- a/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngBoundsTest.java +++ b/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngBoundsTest.java @@ -4,7 +4,6 @@ import com.mapbox.mapboxsdk.exceptions.InvalidLatLngBoundsException; import com.mapbox.mapboxsdk.utils.MockParcel; -import com.mapbox.services.android.telemetry.constants.GeoConstants; import org.junit.Before; import org.junit.Test; @@ -138,21 +137,17 @@ public void containsBoundsInWorld() { } @Test - public void containsNotBiggerBoundsInWorld() { - LatLngBounds biggerWorldBounds = new LatLngBounds.Builder() - .include(new LatLng(GeoConstants.MAX_LATITUDE + 10, GeoConstants.MIN_LONGITUDE - 10)) - .include(new LatLng(GeoConstants.MIN_LATITUDE - 10, GeoConstants.MAX_LONGITUDE + 10)) + public void containsBounds() { + LatLngBounds inner = new LatLngBounds.Builder() + .include(new LatLng(-5, -5)) + .include(new LatLng(5, 5)) .build(); - assertFalse("Bounds should not be contained ", LatLngBounds.world().contains(biggerWorldBounds)); - } - - @Test - public void containsNotBoundsInWorld() { - LatLngBounds outSideWorldBounds = new LatLngBounds.Builder() - .include(new LatLng(GeoConstants.MAX_LATITUDE + 10, GeoConstants.MAX_LONGITUDE + 10)) - .include(new LatLng(GeoConstants.MAX_LATITUDE + 20, GeoConstants.MAX_LONGITUDE + 20)) + LatLngBounds outer = new LatLngBounds.Builder() + .include(new LatLng(-10, -10)) + .include(new LatLng(10, 10)) .build(); - assertFalse("Bounds should not be contained ", LatLngBounds.world().contains(outSideWorldBounds)); + assertTrue(outer.contains(inner)); + assertFalse(inner.contains(outer)); } @Test diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngTest.java b/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngTest.java index 06fe1c91b96..06e93b9d2f4 100644 --- a/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngTest.java +++ b/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngTest.java @@ -6,6 +6,8 @@ import com.mapbox.mapboxsdk.utils.MockParcel; import org.junit.Test; +import org.junit.Rule; +import org.junit.rules.ExpectedException; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; @@ -79,6 +81,79 @@ public void testLongitudeSetter() { assertEquals("longitude should match", 3, latLng.getLongitude(), DELTA); } + @Rule + public final ExpectedException exception = ExpectedException.none(); + + @Test + public void testConstructorChecksLatitudeNaN() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("latitude must not be NaN"); + new LatLng(Double.NaN, 0); + } + + @Test + public void testConstructorChecksLongitudeNaN() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("longitude must not be NaN"); + new LatLng(0, Double.NaN); + } + + @Test + public void testConstructorChecksLatitudeGreaterThan90() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("latitude must be between -90 and 90"); + new LatLng(95, 0); + } + + @Test + public void testConstructorChecksLatitudeLessThanThanNegative90() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("latitude must be between -90 and 90"); + new LatLng(-95, 0); + } + + @Test + public void testConstructorChecksLongitudeInfinity() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("longitude must not be infinite"); + new LatLng(0, Double.POSITIVE_INFINITY); + } + + @Test + public void testLatitudeSetterChecksNaN() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("latitude must not be NaN"); + new LatLng().setLatitude(Double.NaN); + } + + @Test + public void testLongitudeSetterChecksNaN() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("longitude must not be NaN"); + new LatLng().setLongitude(Double.NaN); + } + + @Test + public void testLatitudeSetterChecksGreaterThan90() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("latitude must be between -90 and 90"); + new LatLng().setLatitude(95); + } + + @Test + public void testLatitudeSetterChecksLessThanThanNegative90() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("latitude must be between -90 and 90"); + new LatLng().setLatitude(-95); + } + + @Test + public void testLongitudeSetterChecksInfinity() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("longitude must not be infinite"); + new LatLng().setLongitude(Double.NEGATIVE_INFINITY); + } + @Test public void testAltitudeSetter() { LatLng latLng = new LatLng(1.2, 3.4);