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

Commit 69734a3e authored by Nathan Harold's avatar Nathan Harold
Browse files

Make getPhoneNumber methods safer and faster 

Make the getPhoneNumber() methods a little bit safer
and easier to work with by having both APIs call a
single internal utility method. This eliminates redundant
permissions checks that could all result from a single
method call to getPhoneNumber. In very rare cases, these
permissions checks could have resulted in inconsistent
results due to context switching.

Also fix an issue where DEFAULT_SUBSCRIPTION_ID had some
inconsistent behavior. It would work "correctly" for
PHONE_NUMBER_SOURCE_UICC but wouldn't for the other two
sources.

Bug: 317673478
Test: atest FrameworksTelephonyTests
Change-Id: I53fe0e7afefe1f2bc7d280fa74768b77962bc078
parent 7f2efe4b

flags/subscription.aconfig

+13 −1
Original line number Diff line number Diff line
@@ -53,3 +53,15 @@ flag { 
  description: "Supports querying if a subscription is associated with the caller"

  bug: "325045841"

}



# OWNER=nharold TARGET=24Q3

flag {

  name: "safer_get_phone_number"

  namespace: "telephony"

  description: "Safety and performance improvements for getPhoneNumber()"

  bug: "317673478"



  metadata {

    purpose: PURPOSE_BUGFIX

  }

}

src/java/com/android/internal/telephony/subscription/SubscriptionManagerService.java

+141 −42
Original line number Diff line number Diff line
@@ -3712,14 +3712,25 @@ public class SubscriptionManagerService extends ISub.Stub { 
            "carrier privileges",

    })

    public String getPhoneNumber(int subId, @PhoneNumberSource int source,

            @NonNull String callingPackage, @Nullable String callingFeatureId) {

            @NonNull String callingPackage, @Nullable String callingFeatureId /* unused */) {

        TelephonyPermissions.enforceAnyPermissionGrantedOrCarrierPrivileges(

                mContext, subId, Binder.getCallingUid(), "getPhoneNumber",

                Manifest.permission.READ_PHONE_NUMBERS,

                Manifest.permission.READ_PRIVILEGED_PHONE_STATE);



        enforceTelephonyFeatureWithException(callingPackage, "getPhoneNumber");



        if (mFeatureFlags.saferGetPhoneNumber()) {

            checkPhoneNumberSource(source);

            subId = checkAndGetSubId(subId);

            if (subId == SubscriptionManager.INVALID_SUBSCRIPTION_ID) return "";



            final long identity = Binder.clearCallingIdentity();

            try {

                return getPhoneNumberFromSourceInternal(subId, source);

            } finally {

                Binder.restoreCallingIdentity(identity);

            }

        } else {

            final long identity = Binder.clearCallingIdentity();

            try {

                SubscriptionInfoInternal subInfo = mSubscriptionDatabaseManager
@@ -3749,6 +3760,68 @@ public class SubscriptionManagerService extends ISub.Stub { 
                Binder.restoreCallingIdentity(identity);

            }

        }

    }



    /**

     * Get a resolved subId based on what the user passed in.

     *

     * Only use this before clearing the calling binder. Used for compatibility (only).

     * Do not use this behavior for new methods.

     *

     * @param subId the subId passed in by the user.

     */

    private int checkAndGetSubId(int subId) {

        if (subId == SubscriptionManager.INVALID_SUBSCRIPTION_ID) {

            // for historical reasons, INVALID_SUB_ID fails gracefully

            return subId;

        } else if (subId == SubscriptionManager.DEFAULT_SUBSCRIPTION_ID) {

            return getDefaultSubId();

        } else if (!SubscriptionManager.isValidSubscriptionId(subId)) {

            throw new IllegalArgumentException("Invalid SubId=" + subId);

        } else {

            return subId;

        }

    }



    private void checkPhoneNumberSource(int source) {

        if (source == SubscriptionManager.PHONE_NUMBER_SOURCE_UICC

                || source == SubscriptionManager.PHONE_NUMBER_SOURCE_CARRIER

                || source == SubscriptionManager.PHONE_NUMBER_SOURCE_IMS) {

            return;

        }



        throw new IllegalArgumentException("Invalid number source " + source);

    }



    private @NonNull String getPhoneNumberFromSourceInternal(

            int subId,

            @PhoneNumberSource int source) {



        final SubscriptionInfoInternal subInfo = mSubscriptionDatabaseManager

                .getSubscriptionInfoInternal(subId);



        if (subInfo == null) {

            loge("No SubscriptionInfo found for subId=" + subId);

            return "";

        }



        switch(source) {

            case SubscriptionManager.PHONE_NUMBER_SOURCE_UICC:

                final Phone phone = PhoneFactory.getPhone(getSlotIndex(subId));

                if (phone != null) {

                    return TextUtils.emptyIfNull(phone.getLine1Number());

                } else {

                    return subInfo.getNumber();

                }

            case SubscriptionManager.PHONE_NUMBER_SOURCE_CARRIER:

                return subInfo.getNumberFromCarrier();

            case SubscriptionManager.PHONE_NUMBER_SOURCE_IMS:

                return subInfo.getNumberFromIms();

            default:

                loge("No SubscriptionInfo found for subId=" + subId);

                return "";

        }

    }



    /**

     * Get phone number from first available source. The order would be
@@ -3782,6 +3855,31 @@ public class SubscriptionManagerService extends ISub.Stub { 
        enforceTelephonyFeatureWithException(callingPackage,

                "getPhoneNumberFromFirstAvailableSource");



        if (mFeatureFlags.saferGetPhoneNumber()) {

            subId = checkAndGetSubId(subId);

            if (subId == SubscriptionManager.INVALID_SUBSCRIPTION_ID) return "";



            final long identity = Binder.clearCallingIdentity();

            try {

                String number;

                number = getPhoneNumberFromSourceInternal(

                        subId,

                        SubscriptionManager.PHONE_NUMBER_SOURCE_CARRIER);

                if (!TextUtils.isEmpty(number)) return number;



                number = getPhoneNumberFromSourceInternal(

                        subId,

                        SubscriptionManager.PHONE_NUMBER_SOURCE_UICC);

                if (!TextUtils.isEmpty(number)) return number;



                number = getPhoneNumberFromSourceInternal(

                        subId,

                        SubscriptionManager.PHONE_NUMBER_SOURCE_IMS);

                return TextUtils.emptyIfNull(number);

            } finally {

                Binder.restoreCallingIdentity(identity);

            }

        } else {

            String numberFromCarrier = getPhoneNumber(subId,

                    SubscriptionManager.PHONE_NUMBER_SOURCE_CARRIER, callingPackage,

                    callingFeatureId);
@@ -3802,6 +3900,7 @@ public class SubscriptionManagerService extends ISub.Stub { 
            }

            return "";

        }

    }



    /**

     * Set the phone number of the subscription.

tests/telephonytests/src/com/android/internal/telephony/subscription/SubscriptionDatabaseManagerTest.java

+2 −2
Original line number Diff line number Diff line
@@ -76,8 +76,8 @@ public class SubscriptionDatabaseManagerTest extends TelephonyTest { 
    static final String FAKE_DEFAULT_CARD_NAME = "CARD %d";

    static final String FAKE_ICCID1 = "123456";

    static final String FAKE_ICCID2 = "456789";

    static final String FAKE_PHONE_NUMBER1 = "6502530000";

    static final String FAKE_PHONE_NUMBER2 = "4089961010";

    static final String FAKE_PHONE_NUMBER1 = "9995551234";

    static final String FAKE_PHONE_NUMBER2 = "9998887777";

    static final String FAKE_CARRIER_NAME1 = "A-Mobile";

    static final String FAKE_CARRIER_NAME2 = "B-Mobile";

    static final int FAKE_COLOR1 = 1;

tests/telephonytests/src/com/android/internal/telephony/subscription/SubscriptionManagerServiceTest.java

+72 −1
Original line number Diff line number Diff line
@@ -243,7 +243,6 @@ public class SubscriptionManagerServiceTest extends TelephonyTest { 
        doReturn(false).when(mFlags).enforceTelephonyFeatureMappingForPublicApis();

        doReturn(true).when(mPackageManager).hasSystemFeature(

                eq(PackageManager.FEATURE_TELEPHONY_SUBSCRIPTION));



        logd("SubscriptionManagerServiceTest -Setup!");

    }
@@ -1787,6 +1786,45 @@ public class SubscriptionManagerServiceTest extends TelephonyTest { 
                1, CALLING_PACKAGE, CALLING_FEATURE)).isEqualTo(FAKE_PHONE_NUMBER1);

    }



    @Test

    public void testGetPhoneNumberSourcePriority() throws Exception {

        mContextFixture.addCallingOrSelfPermission(Manifest.permission.READ_PHONE_NUMBERS);



        String phoneNumberFromCarrier = "8675309";

        String phoneNumberFromUicc = "1112223333";

        String phoneNumberFromIms = "5553466";

        String phoneNumberFromPhoneObject = "8001234567";



        doReturn(phoneNumberFromPhoneObject).when(mPhone).getLine1Number();



        SubscriptionInfoInternal multiNumberSubInfo =

                new SubscriptionInfoInternal.Builder(FAKE_SUBSCRIPTION_INFO1)

                        .setNumberFromCarrier(phoneNumberFromCarrier)

                        .setNumber(phoneNumberFromUicc)

                        .setNumberFromIms(phoneNumberFromIms)

                        .build();

        int subId = insertSubscription(multiNumberSubInfo);



        assertThat(mSubscriptionManagerServiceUT.getPhoneNumberFromFirstAvailableSource(

                subId, CALLING_PACKAGE, CALLING_FEATURE)).isEqualTo(phoneNumberFromCarrier);



        multiNumberSubInfo =

                new SubscriptionInfoInternal.Builder(multiNumberSubInfo)

                        .setNumberFromCarrier("")

                        .setNumber(phoneNumberFromUicc)

                        .setNumberFromIms(phoneNumberFromIms)

                        .build();

        subId = insertSubscription(multiNumberSubInfo);



        assertThat(mSubscriptionManagerServiceUT.getPhoneNumberFromFirstAvailableSource(

                subId, CALLING_PACKAGE, CALLING_FEATURE)).isEqualTo(phoneNumberFromPhoneObject);



        doReturn("").when(mPhone).getLine1Number();



        assertThat(mSubscriptionManagerServiceUT.getPhoneNumberFromFirstAvailableSource(

                subId, CALLING_PACKAGE, CALLING_FEATURE)).isEqualTo(phoneNumberFromIms);

    }



    @Test

    public void testSetUiccApplicationsEnabled() {

        insertSubscription(FAKE_SUBSCRIPTION_INFO1);
@@ -2482,6 +2520,39 @@ public class SubscriptionManagerServiceTest extends TelephonyTest { 
                .isEqualTo(FAKE_PHONE_NUMBER2);

    }



    @Test

    public void testGetPhoneNumberFromDefaultSubscription() {

        doReturn(true).when(mFlags).saferGetPhoneNumber();



        mContextFixture.addCallingOrSelfPermission(Manifest.permission.READ_PRIVILEGED_PHONE_STATE);

        mContextFixture.addCallingOrSelfPermission(Manifest.permission.MODIFY_PHONE_STATE);

        int subId = insertSubscription(FAKE_SUBSCRIPTION_INFO1);



        mSubscriptionManagerServiceUT.setDefaultVoiceSubId(subId);



        assertThat(

                mSubscriptionManagerServiceUT.getPhoneNumberFromFirstAvailableSource(

                        subId, CALLING_PACKAGE, CALLING_FEATURE)).isEqualTo(FAKE_PHONE_NUMBER1);

        assertThat(

                mSubscriptionManagerServiceUT.getPhoneNumber(

                        SubscriptionManager.DEFAULT_SUBSCRIPTION_ID,

                        SubscriptionManager.PHONE_NUMBER_SOURCE_UICC,

                        CALLING_PACKAGE,

                        CALLING_FEATURE)).isEqualTo(FAKE_PHONE_NUMBER1);

        assertThat(

                mSubscriptionManagerServiceUT.getPhoneNumber(

                        SubscriptionManager.DEFAULT_SUBSCRIPTION_ID,

                        SubscriptionManager.PHONE_NUMBER_SOURCE_CARRIER,

                        CALLING_PACKAGE,

                        CALLING_FEATURE)).isEqualTo(FAKE_PHONE_NUMBER1);

        assertThat(

                mSubscriptionManagerServiceUT.getPhoneNumber(

                        SubscriptionManager.DEFAULT_SUBSCRIPTION_ID,

                        SubscriptionManager.PHONE_NUMBER_SOURCE_IMS,

                        CALLING_PACKAGE,

                        CALLING_FEATURE)).isEqualTo(FAKE_PHONE_NUMBER1);

    }



    @Test

    public void testEsimActivation() {

        mContextFixture.addCallingOrSelfPermission(Manifest.permission.READ_PRIVILEGED_PHONE_STATE);