Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 83e03f55 authored by Maggie's avatar Maggie
Browse files

Fix location settings bug on non-GPS devices 

The old location_mode API hardcoded gps and network location provider when it enables/disables location, without checking whether the providers exist on device.
It causes bugs when used together with the new
LocationManager.setLocationEnabled() APIs.

This fix modified LocationManager.setLocationEnabled() API when user
tries to disable location on device. Besides turning off the providers
from LocationManager.getAllProviders(), it also turns off GPS and
network provider explicitly.

To reduce times of binding to the service and chance of race condition, we also
modified SettingsProvider.updateLocationProvidersAllowedLocked() to
accept a string param with multiple location providers to be
enabled or disalbed at the same time.

Bug: 73261572
Test: Manual on chromebook
Change-Id: I2e59e0d4cf395b98cd481af5d7f3c762274d7826
parent 211078e1

location/java/android/location/LocationManager.java

+33 −3
Original line number Diff line number Diff line
@@ -42,12 +42,14 @@ import android.os.RemoteException; 
import android.os.UserHandle;

import android.provider.Settings;

import android.text.TextUtils;

import android.util.ArraySet;

import android.util.Log;

import com.android.internal.location.ProviderProperties;

import java.util.ArrayList;

import java.util.Arrays;

import java.util.HashMap;

import java.util.List;

import java.util.Set;



/**

 * This class provides access to the system location services.  These
@@ -1252,12 +1254,40 @@ public class LocationManager { 
    @SystemApi

    @RequiresPermission(WRITE_SECURE_SETTINGS)

    public void setLocationEnabledForUser(boolean enabled, UserHandle userHandle) {

        for (String provider : getAllProviders()) {

        final List<String> allProvidersList = getAllProviders();

        // Update all providers on device plus gps and network provider when disabling location.

        Set<String> allProvidersSet = new ArraySet<>(allProvidersList.size() + 2);

        allProvidersSet.addAll(allProvidersList);

        // When disabling location, disable gps and network provider that could have been enabled by

        // location mode api.

        if (enabled == false) {

            allProvidersSet.add(GPS_PROVIDER);

            allProvidersSet.add(NETWORK_PROVIDER);

        }

        if (allProvidersSet.isEmpty()) {

            return;

        }

        // to ensure thread safety, we write the provider name with a '+' or '-'

        // and let the SettingsProvider handle it rather than reading and modifying

        // the list of enabled providers.

        final String prefix = enabled ? "+" : "-";

        StringBuilder locationProvidersAllowed = new StringBuilder();

        for (String provider : allProvidersSet) {

            checkProvider(provider);

            if (provider.equals(PASSIVE_PROVIDER)) {

                continue;

            }

            setProviderEnabledForUser(provider, enabled, userHandle);

            locationProvidersAllowed.append(prefix);

            locationProvidersAllowed.append(provider);

            locationProvidersAllowed.append(",");

        }

        // Remove the trailing comma

        locationProvidersAllowed.setLength(locationProvidersAllowed.length() - 1);

        Settings.Secure.putStringForUser(

                mContext.getContentResolver(),

                Settings.Secure.LOCATION_PROVIDERS_ALLOWED,

                locationProvidersAllowed.toString(),

                userHandle.getIdentifier());

    }



    /**

packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java

+34 −58
Original line number Diff line number Diff line
@@ -1798,76 +1798,52 @@ public class SettingsProvider extends ContentProvider { 
        if (TextUtils.isEmpty(value)) {

            return false;

        }



        final char prefix = value.charAt(0);

        if (prefix != '+' && prefix != '-') {

            if (forceNotify) {

                final int key = makeKey(SETTINGS_TYPE_SECURE, owningUserId);

                mSettingsRegistry.notifyForSettingsChange(key,

                        Settings.Secure.LOCATION_PROVIDERS_ALLOWED);

            }

            return false;

        }



        // skip prefix

        value = value.substring(1);



        Setting settingValue = getSecureSetting(

        Setting oldSetting = getSecureSetting(

                Settings.Secure.LOCATION_PROVIDERS_ALLOWED, owningUserId);

        if (settingValue == null) {

        if (oldSetting == null) {

            return false;

        }



        String oldProviders = !settingValue.isNull() ? settingValue.getValue() : "";



        int index = oldProviders.indexOf(value);

        int end = index + value.length();



        // check for commas to avoid matching on partial string

        if (index > 0 && oldProviders.charAt(index - 1) != ',') {

            index = -1;

        }



        // check for commas to avoid matching on partial string

        if (end < oldProviders.length() && oldProviders.charAt(end) != ',') {

            index = -1;

        String oldProviders = oldSetting.getValue();

        List<String> oldProvidersList = TextUtils.isEmpty(oldProviders)

                ? new ArrayList<>() : new ArrayList<>(Arrays.asList(oldProviders.split(",")));

        Set<String> newProvidersSet = new ArraySet<>();

        newProvidersSet.addAll(oldProvidersList);



        String[] providerUpdates = value.split(",");

        boolean inputError = false;

        for (String provider : providerUpdates) {

            // do not update location_providers_allowed when input is invalid

            if (TextUtils.isEmpty(provider)) {

                inputError = true;

                break;

            }



        String newProviders;



        if (prefix == '+' && index < 0) {

            // append the provider to the list if not present

            if (oldProviders.length() == 0) {

                newProviders = value;

            } else {

                newProviders = oldProviders + ',' + value;

            final char prefix = provider.charAt(0);

            // do not update location_providers_allowed when input is invalid

            if (prefix != '+' && prefix != '-') {

                inputError = true;

                break;

            }

        } else if (prefix == '-' && index >= 0) {

            // remove the provider from the list if present

            // remove leading or trailing comma

            if (index > 0) {

                index--;

            } else if (end < oldProviders.length()) {

                end++;

            // skip prefix

            provider = provider.substring(1);

            if (prefix == '+') {

                newProvidersSet.add(provider);

            } else if (prefix == '-') {

                newProvidersSet.remove(provider);

            }



            newProviders = oldProviders.substring(0, index);

            if (end < oldProviders.length()) {

                newProviders += oldProviders.substring(end);

        }

        } else {

        String newProviders = TextUtils.join(",", newProvidersSet.toArray());

        if (inputError == true || newProviders.equals(oldProviders)) {

            // nothing changed, so no need to update the database

            if (forceNotify) {

                final int key = makeKey(SETTINGS_TYPE_SECURE, owningUserId);

                mSettingsRegistry.notifyForSettingsChange(key,

                mSettingsRegistry.notifyForSettingsChange(

                        makeKey(SETTINGS_TYPE_SECURE, owningUserId),

                        Settings.Secure.LOCATION_PROVIDERS_ALLOWED);

            }

            return false;

        }



        return mSettingsRegistry.insertSettingLocked(SETTINGS_TYPE_SECURE,

                owningUserId, Settings.Secure.LOCATION_PROVIDERS_ALLOWED, newProviders,

                tag, makeDefault, getCallingPackage(), forceNotify, CRITICAL_SECURE_SETTINGS);

                owningUserId, Settings.Secure.LOCATION_PROVIDERS_ALLOWED, newProviders, tag,

                makeDefault, getCallingPackage(), forceNotify, CRITICAL_SECURE_SETTINGS);

    }



    private static void warnOrThrowForUndesiredSecureSettingsMutationForTargetSdk(

packages/SettingsProvider/test/src/com/android/providers/settings/SettingsProviderTest.java

+110 −3
Original line number Diff line number Diff line
@@ -17,8 +17,8 @@ 
package com.android.providers.settings;



import static junit.framework.Assert.assertEquals;

import static junit.framework.Assert.assertSame;

import static junit.framework.Assert.assertNull;

import static junit.framework.Assert.assertSame;

import static junit.framework.Assert.fail;



import android.content.ContentResolver;
@@ -32,9 +32,8 @@ import android.os.SystemClock; 
import android.os.UserHandle;

import android.provider.Settings;

import android.util.Log;

import org.junit.Test;



import java.util.concurrent.atomic.AtomicBoolean;

import org.junit.Test;



/**

 * Tests for the SettingContentProvider.
@@ -688,4 +687,112 @@ public class SettingsProviderTest extends BaseSettingsProviderTest { 
            cursor.close();

        }

    }



    @Test

    public void testUpdateLocationProvidersAllowedLocked_enableProviders() throws Exception {

        setSettingViaFrontEndApiAndAssertSuccessfulChange(

                SETTING_TYPE_SECURE,

                Settings.Secure.LOCATION_MODE,

                String.valueOf(Settings.Secure.LOCATION_MODE_OFF),

                UserHandle.USER_SYSTEM);



        // Enable one provider

        updateStringViaProviderApiSetting(

                SETTING_TYPE_SECURE, Settings.Secure.LOCATION_PROVIDERS_ALLOWED, "+gps");



        assertEquals(

                "Wrong location providers",

                "gps",

                queryStringViaProviderApi(

                        SETTING_TYPE_SECURE, Settings.Secure.LOCATION_PROVIDERS_ALLOWED));



        // Enable a list of providers, including the one that is already enabled

        updateStringViaProviderApiSetting(

                SETTING_TYPE_SECURE,

                Settings.Secure.LOCATION_PROVIDERS_ALLOWED,

                "+gps,+network,+network");



        assertEquals(

                "Wrong location providers",

                "gps,network",

                queryStringViaProviderApi(

                        SETTING_TYPE_SECURE, Settings.Secure.LOCATION_PROVIDERS_ALLOWED));

    }



    @Test

    public void testUpdateLocationProvidersAllowedLocked_disableProviders() throws Exception {

        setSettingViaFrontEndApiAndAssertSuccessfulChange(

                SETTING_TYPE_SECURE,

                Settings.Secure.LOCATION_MODE,

                String.valueOf(Settings.Secure.LOCATION_MODE_HIGH_ACCURACY),

                UserHandle.USER_SYSTEM);



        // Disable providers that were enabled

        updateStringViaProviderApiSetting(

                SETTING_TYPE_SECURE,

                Settings.Secure.LOCATION_PROVIDERS_ALLOWED,

                "-gps,-network");



        assertEquals(

                "Wrong location providers",

                "",

                queryStringViaProviderApi(

                        SETTING_TYPE_SECURE, Settings.Secure.LOCATION_PROVIDERS_ALLOWED));



        // Disable a provider that was not enabled

        updateStringViaProviderApiSetting(

                SETTING_TYPE_SECURE,

                Settings.Secure.LOCATION_PROVIDERS_ALLOWED,

                "-test");



        assertEquals(

                "Wrong location providers",

                "",

                queryStringViaProviderApi(

                        SETTING_TYPE_SECURE, Settings.Secure.LOCATION_PROVIDERS_ALLOWED));

    }



    @Test

    public void testUpdateLocationProvidersAllowedLocked_enableAndDisable() throws Exception {

        setSettingViaFrontEndApiAndAssertSuccessfulChange(

                SETTING_TYPE_SECURE,

                Settings.Secure.LOCATION_MODE,

                String.valueOf(Settings.Secure.LOCATION_MODE_OFF),

                UserHandle.USER_SYSTEM);



        updateStringViaProviderApiSetting(

                SETTING_TYPE_SECURE,

                Settings.Secure.LOCATION_PROVIDERS_ALLOWED,

                "+gps,+network,+test");

        updateStringViaProviderApiSetting(

                SETTING_TYPE_SECURE, Settings.Secure.LOCATION_PROVIDERS_ALLOWED, "-test");



        assertEquals(

                "Wrong location providers",

                "gps,network",

                queryStringViaProviderApi(

                        SETTING_TYPE_SECURE, Settings.Secure.LOCATION_PROVIDERS_ALLOWED));

    }



    @Test

    public void testUpdateLocationProvidersAllowedLocked_invalidInput() throws Exception {

        setSettingViaFrontEndApiAndAssertSuccessfulChange(

                SETTING_TYPE_SECURE,

                Settings.Secure.LOCATION_MODE,

                String.valueOf(Settings.Secure.LOCATION_MODE_OFF),

                UserHandle.USER_SYSTEM);



        // update providers with a invalid string

        updateStringViaProviderApiSetting(

                SETTING_TYPE_SECURE,

                Settings.Secure.LOCATION_PROVIDERS_ALLOWED,

                "+gps, invalid-string");



        // Verifies providers list does not change

        assertEquals(

                "Wrong location providers",

                "",

                queryStringViaProviderApi(

                        SETTING_TYPE_SECURE, Settings.Secure.LOCATION_PROVIDERS_ALLOWED));

    }

}