diff options
7 files changed, 130 insertions, 33 deletions
diff --git a/location/java/android/location/LocationResult.java b/location/java/android/location/LocationResult.java index 8423000b6276..67f47756984f 100644 --- a/location/java/android/location/LocationResult.java +++ b/location/java/android/location/LocationResult.java @@ -19,8 +19,11 @@ package android.location; import android.annotation.IntRange; import android.annotation.NonNull; import android.annotation.Nullable; +import android.location.flags.Flags; import android.os.Parcel; import android.os.Parcelable; +import android.os.SystemClock; +import android.util.Log; import com.android.internal.util.Preconditions; @@ -37,6 +40,23 @@ import java.util.function.Predicate; * @hide */ public final class LocationResult implements Parcelable { + private static final String TAG = "LocationResult"; + + // maximum reasonable accuracy, somewhat arbitrarily chosen. this is a very high upper limit, it + // could likely be lower, but we only want to throw out really absurd values. + private static final float MAX_ACCURACY_M = 1000000; + + // maximum reasonable speed we expect a device to travel at is currently mach 1 (top speed of + // current fastest private jet). Higher speed than the value is considered as a malfunction + // than a correct reading. + private static final float MAX_SPEED_MPS = 343; + + /** Exception representing an invalid location within a {@link LocationResult}. */ + public static class BadLocationException extends Exception { + public BadLocationException(String message) { + super(message); + } + } /** * Creates a new LocationResult from the given locations, making a copy of each location. @@ -101,18 +121,60 @@ public final class LocationResult implements Parcelable { * * @hide */ - public @NonNull LocationResult validate() { + public @NonNull LocationResult validate() throws BadLocationException { long prevElapsedRealtimeNs = 0; final int size = mLocations.size(); for (int i = 0; i < size; ++i) { Location location = mLocations.get(i); - if (!location.isComplete()) { - throw new IllegalArgumentException( - "incomplete location at index " + i + ": " + mLocations); - } - if (location.getElapsedRealtimeNanos() < prevElapsedRealtimeNs) { - throw new IllegalArgumentException( - "incorrectly ordered location at index " + i + ": " + mLocations); + if (Flags.locationValidation()) { + if (location.getLatitude() < -90.0 + || location.getLatitude() > 90.0 + || location.getLongitude() < -180.0 + || location.getLongitude() > 180.0 + || Double.isNaN(location.getLatitude()) + || Double.isNaN(location.getLongitude())) { + throw new BadLocationException("location must have valid lat/lng"); + } + if (!location.hasAccuracy()) { + throw new BadLocationException("location must have accuracy"); + } + if (location.getAccuracy() < 0 || location.getAccuracy() > MAX_ACCURACY_M) { + throw new BadLocationException("location must have reasonable accuracy"); + } + if (location.getTime() < 0) { + throw new BadLocationException("location must have valid time"); + } + if (prevElapsedRealtimeNs > location.getElapsedRealtimeNanos()) { + throw new BadLocationException( + "location must have valid monotonically increasing realtime"); + } + if (location.getElapsedRealtimeNanos() + > SystemClock.elapsedRealtimeNanos()) { + throw new BadLocationException("location must not have realtime in the future"); + } + if (!location.isMock()) { + if (location.getProvider() == null) { + throw new BadLocationException("location must have valid provider"); + } + if (location.getLatitude() == 0 && location.getLongitude() == 0) { + throw new BadLocationException("location must not be at 0,0"); + } + } + + if (location.hasSpeed() && (location.getSpeed() < 0 + || location.getSpeed() > MAX_SPEED_MPS)) { + Log.w(TAG, "removed bad location speed: " + location.getSpeed()); + location.removeSpeed(); + } + } else { + if (!location.isComplete()) { + throw new IllegalArgumentException( + "incomplete location at index " + i + ": " + mLocations); + } + if (location.getElapsedRealtimeNanos() < prevElapsedRealtimeNs) { + throw new IllegalArgumentException( + "incorrectly ordered location at index " + i + ": " + mLocations); + } } prevElapsedRealtimeNs = location.getElapsedRealtimeNanos(); } diff --git a/location/java/android/location/flags/gnss.aconfig b/location/java/android/location/flags/gnss.aconfig index f4b1056019cb..a8464d3f86ec 100644 --- a/location/java/android/location/flags/gnss.aconfig +++ b/location/java/android/location/flags/gnss.aconfig @@ -27,3 +27,10 @@ flag { description: "Flag for releasing SUPL connection on timeout" bug: "315024652" } + +flag { + name: "location_validation" + namespace: "location" + description: "Flag for location validation" + bug: "314328533" +} diff --git a/services/core/java/com/android/server/location/gnss/GnssLocationProvider.java b/services/core/java/com/android/server/location/gnss/GnssLocationProvider.java index 27b01a5424b8..9c4225dc2542 100644 --- a/services/core/java/com/android/server/location/gnss/GnssLocationProvider.java +++ b/services/core/java/com/android/server/location/gnss/GnssLocationProvider.java @@ -67,6 +67,7 @@ import android.location.LocationListener; import android.location.LocationManager; import android.location.LocationRequest; import android.location.LocationResult; +import android.location.LocationResult.BadLocationException; import android.location.flags.Flags; import android.location.provider.ProviderProperties; import android.location.provider.ProviderRequest; @@ -1380,7 +1381,11 @@ public class GnssLocationProvider extends AbstractLocationProvider implements location.setExtras(mLocationExtras.getBundle()); - reportLocation(LocationResult.wrap(location).validate()); + try { + reportLocation(LocationResult.wrap(location).validate()); + } catch (BadLocationException e) { + throw new IllegalArgumentException(e); + } if (mStarted) { mGnssMetrics.logReceivedLocationStatus(hasLatLong); @@ -1751,7 +1756,11 @@ public class GnssLocationProvider extends AbstractLocationProvider implements } } - reportLocation(LocationResult.wrap(locations).validate()); + try { + reportLocation(LocationResult.wrap(locations).validate()); + } catch (BadLocationException e) { + throw new IllegalArgumentException(e); + } } Runnable[] listeners; diff --git a/services/core/java/com/android/server/location/provider/LocationProviderManager.java b/services/core/java/com/android/server/location/provider/LocationProviderManager.java index 91e6a80a5bbb..7d44aecf5607 100644 --- a/services/core/java/com/android/server/location/provider/LocationProviderManager.java +++ b/services/core/java/com/android/server/location/provider/LocationProviderManager.java @@ -64,6 +64,7 @@ import android.location.LocationManagerInternal; import android.location.LocationManagerInternal.ProviderEnabledListener; import android.location.LocationRequest; import android.location.LocationResult; +import android.location.LocationResult.BadLocationException; import android.location.altitude.AltitudeConverter; import android.location.provider.IProviderRequestListener; import android.location.provider.ProviderProperties; @@ -910,7 +911,8 @@ public class LocationProviderManager extends < getRequest().getMinUpdateIntervalMillis() - maxJitterMs) { if (D) { Log.v(TAG, mName + " provider registration " + getIdentity() - + " dropped delivery - too fast"); + + " dropped delivery - too fast (deltaMs=" + + deltaMs + ")."); } return false; } @@ -2574,29 +2576,17 @@ public class LocationProviderManager extends @GuardedBy("mMultiplexerLock") @Nullable private LocationResult processReportedLocation(LocationResult locationResult) { - LocationResult processed = locationResult.filter(location -> { - if (!location.isMock()) { - if (location.getLatitude() == 0 && location.getLongitude() == 0) { - Log.e(TAG, "blocking 0,0 location from " + mName + " provider"); - return false; - } - } - - if (!location.isComplete()) { - Log.e(TAG, "blocking incomplete location from " + mName + " provider"); - return false; - } - - return true; - }); - if (processed == null) { + try { + locationResult.validate(); + } catch (BadLocationException e) { + Log.e(TAG, "Dropping invalid locations: " + e); return null; } // Attempt to add a missing MSL altitude on behalf of the provider. if (DeviceConfig.getBoolean(DeviceConfig.NAMESPACE_LOCATION, "enable_location_provider_manager_msl", true)) { - return processed.map(location -> { + return locationResult.map(location -> { if (!location.hasMslAltitude() && location.hasAltitude()) { try { Location locationCopy = new Location(location); @@ -2626,7 +2616,7 @@ public class LocationProviderManager extends return location; }); } - return processed; + return locationResult; } @GuardedBy("mMultiplexerLock") diff --git a/services/core/java/com/android/server/location/provider/MockLocationProvider.java b/services/core/java/com/android/server/location/provider/MockLocationProvider.java index 52b04d4bab14..4efacd7a2995 100644 --- a/services/core/java/com/android/server/location/provider/MockLocationProvider.java +++ b/services/core/java/com/android/server/location/provider/MockLocationProvider.java @@ -21,6 +21,7 @@ import static com.android.internal.util.ConcurrentUtils.DIRECT_EXECUTOR; import android.annotation.Nullable; import android.location.Location; import android.location.LocationResult; +import android.location.LocationResult.BadLocationException; import android.location.provider.ProviderProperties; import android.location.provider.ProviderRequest; import android.location.util.identity.CallerIdentity; @@ -55,7 +56,11 @@ public class MockLocationProvider extends AbstractLocationProvider { Location location = new Location(l); location.setIsFromMockProvider(true); mLocation = location; - reportLocation(LocationResult.wrap(location).validate()); + try { + reportLocation(LocationResult.wrap(location).validate()); + } catch (BadLocationException e) { + throw new IllegalArgumentException(e); + } } @Override diff --git a/services/core/java/com/android/server/location/provider/proxy/ProxyLocationProvider.java b/services/core/java/com/android/server/location/provider/proxy/ProxyLocationProvider.java index 05966da28217..a597edd93515 100644 --- a/services/core/java/com/android/server/location/provider/proxy/ProxyLocationProvider.java +++ b/services/core/java/com/android/server/location/provider/proxy/ProxyLocationProvider.java @@ -305,7 +305,7 @@ public class ProxyLocationProvider extends AbstractLocationProvider implements return; } - reportLocation(LocationResult.wrap(location).validate()); + reportLocation(LocationResult.wrap(location)); } } @@ -316,8 +316,7 @@ public class ProxyLocationProvider extends AbstractLocationProvider implements if (mProxy != this) { return; } - - reportLocation(LocationResult.wrap(locations).validate()); + reportLocation(LocationResult.wrap(locations)); } } diff --git a/services/tests/mockingservicestests/src/com/android/server/location/provider/LocationProviderManagerTest.java b/services/tests/mockingservicestests/src/com/android/server/location/provider/LocationProviderManagerTest.java index 293003dcda18..32878b3e199f 100644 --- a/services/tests/mockingservicestests/src/com/android/server/location/provider/LocationProviderManagerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/location/provider/LocationProviderManagerTest.java @@ -67,6 +67,7 @@ import android.location.LocationManagerInternal; import android.location.LocationManagerInternal.ProviderEnabledListener; import android.location.LocationRequest; import android.location.LocationResult; +import android.location.flags.Flags; import android.location.provider.IProviderRequestListener; import android.location.provider.ProviderProperties; import android.location.provider.ProviderRequest; @@ -78,8 +79,10 @@ import android.os.PackageTagsList; import android.os.PowerManager; import android.os.Process; import android.os.RemoteException; +import android.os.SystemClock; import android.os.WorkSource; import android.platform.test.annotations.Presubmit; +import android.platform.test.flag.junit.SetFlagsRule; import android.provider.DeviceConfig; import android.provider.Settings; import android.util.Log; @@ -97,6 +100,7 @@ import com.android.server.location.injector.TestInjector; import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; @@ -140,6 +144,9 @@ public class LocationProviderManagerTest { private static final WorkSource WORK_SOURCE = new WorkSource(IDENTITY.getUid()); private static final String MISSING_PERMISSION = "missing_permission"; + @Rule + public final SetFlagsRule mSetFlagsRule = new SetFlagsRule(); + private Random mRandom; @Mock @@ -1347,6 +1354,24 @@ public class LocationProviderManagerTest { assertThat(mManager.isVisibleToCaller()).isFalse(); } + @Test + public void testValidateLocation_futureLocation() { + mSetFlagsRule.enableFlags(Flags.FLAG_LOCATION_VALIDATION); + Location location = createLocation(NAME, mRandom); + mProvider.setProviderLocation(location); + + assertThat(mPassive.getLastLocation(new LastLocationRequest.Builder().build(), IDENTITY, + PERMISSION_FINE)).isEqualTo(location); + + Location futureLocation = createLocation(NAME, mRandom); + futureLocation.setElapsedRealtimeNanos( + SystemClock.elapsedRealtimeNanos() + TimeUnit.SECONDS.toNanos(2)); + mProvider.setProviderLocation(futureLocation); + + assertThat(mPassive.getLastLocation(new LastLocationRequest.Builder().build(), IDENTITY, + PERMISSION_FINE)).isEqualTo(location); + } + @MediumTest @Test public void testEnableMsl_expectedBehavior() throws Exception { |