[android] Preflight argument validity before constructing LatLng

mapbox · Apr 3, 2017 · 1586ffa · 1586ffa
1 parent c456679
commit 1586ffa
Show file tree

Hide file tree

Showing 3 changed files with 104 additions and 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
@@ -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/...pboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngBoundsTest.java b/...pboxGLAndroidSDKTestApp/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/...oid/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngTest.java b/...oid/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);