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

Commit 0c368929 authored by Jack Yu's avatar Jack Yu
Browse files

Fixed crash during eSIM activation 

Adapted the pre-U behavior for returning empty
subscription list when callers do not have permissions.

Fix: 266299303
Test: Performed eSIM activation and confirmed the crash is gone
      atest SubscriptionManagerServiceTest to catch the crash case.
Change-Id: I91993a8e38606b961cdee54e858d7d882f50e57b
parent 3cb12601

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

+13 −8
Original line number Diff line number Diff line
@@ -100,6 +100,7 @@ import java.io.FileDescriptor; 
import java.io.PrintWriter;

import java.util.ArrayList;

import java.util.Arrays;

import java.util.Collections;

import java.util.Comparator;

import java.util.List;

import java.util.Map;
@@ -1638,8 +1639,6 @@ public class SubscriptionManagerService extends ISub.Stub { 
     *

     * @return Sorted list of the currently {@link SubscriptionInfo} records available on the

     * device.

     *

     * @throws SecurityException if the caller does not have required permissions.

     */

    @Override

    @NonNull
@@ -1659,8 +1658,12 @@ public class SubscriptionManagerService extends ISub.Stub { 
        if (!TelephonyPermissions.checkReadPhoneStateOnAnyActiveSub(mContext,

                Binder.getCallingPid(), Binder.getCallingUid(), callingPackage, callingFeatureId,

                "getAllSubInfoList")) {

            throw new SecurityException("Need READ_PHONE_STATE, READ_PRIVILEGED_PHONE_STATE, or "

                    + "carrier privilege");

            // Ideally we should avoid silent failure, but since this API has already been used by

            // many apps and they do not expect the security exception, we return an empty list

            // here so it's consistent with pre-U behavior.

            loge("getActiveSubscriptionInfoList: " + callingPackage + " does not have enough "

                    + "permission. Returning empty list here.");

            return Collections.emptyList();

        }



        return mSubscriptionDatabaseManager.getAllSubscriptions().stream()
@@ -2262,8 +2265,6 @@ public class SubscriptionManagerService extends ISub.Stub { 
     * @param callingFeatureId The feature in the package.

     *

     * @return The list of opportunistic subscription info that can be accessed by the callers.

     *

     * @throws SecurityException if callers do not hold the required permission.

     */

    @Override

    @NonNull
@@ -2283,8 +2284,12 @@ public class SubscriptionManagerService extends ISub.Stub { 
        if (!TelephonyPermissions.checkReadPhoneStateOnAnyActiveSub(mContext,

                Binder.getCallingPid(), Binder.getCallingUid(), callingPackage, callingFeatureId,

                "getOpportunisticSubscriptions")) {

            throw new SecurityException("Need READ_PHONE_STATE, READ_PRIVILEGED_PHONE_STATE, or "

                    + "carrier privilege");

            // Ideally we should avoid silent failure, but since this API has already been used by

            // many apps and they do not expect the security exception, we return an empty list

            // here so it's consistent with pre-U behavior.

            loge("getOpportunisticSubscriptions: " + callingPackage + " does not have enough "

                    + "permission. Returning empty list here.");

            return Collections.emptyList();

        }



        return mSubscriptionDatabaseManager.getAllSubscriptions().stream()

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

+6 −8
Original line number Diff line number Diff line
@@ -54,7 +54,6 @@ import static org.mockito.ArgumentMatchers.nullable; 
import static org.mockito.Mockito.doAnswer;

import static org.mockito.Mockito.doNothing;

import static org.mockito.Mockito.doReturn;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.times;

import static org.mockito.Mockito.verify;
@@ -65,7 +64,6 @@ import android.app.PropertyInvalidatedCache; 
import android.compat.testing.PlatformCompatChangeRule;

import android.content.Intent;

import android.content.pm.PackageManager;

import android.content.res.Resources;

import android.os.Build;

import android.os.Bundle;

import android.os.Handler;
@@ -660,9 +658,9 @@ public class SubscriptionManagerServiceTest extends TelephonyTest { 
        // Remove MODIFY_PHONE_STATE

        mContextFixture.removeCallingOrSelfPermission(Manifest.permission.MODIFY_PHONE_STATE);



        // Should fail without READ_PHONE_STATE

        assertThrows(SecurityException.class, () -> mSubscriptionManagerServiceUT

                .getActiveSubscriptionInfoList(CALLING_PACKAGE, CALLING_FEATURE));

        // Should get an empty list without READ_PHONE_STATE.

        assertThat(mSubscriptionManagerServiceUT.getActiveSubscriptionInfoList(

                CALLING_PACKAGE, CALLING_FEATURE)).isEmpty();



        // Grant READ_PHONE_STATE permission for insertion.

        mContextFixture.addCallingOrSelfPermission(Manifest.permission.READ_PHONE_STATE);
@@ -1063,9 +1061,9 @@ public class SubscriptionManagerServiceTest extends TelephonyTest { 
        testSetOpportunistic();

        insertSubscription(FAKE_SUBSCRIPTION_INFO2);



        // Should fail without READ_PHONE_STATE

        assertThrows(SecurityException.class, () -> mSubscriptionManagerServiceUT

                .getOpportunisticSubscriptions(CALLING_PACKAGE, CALLING_FEATURE));

        // Should get an empty list without READ_PHONE_STATE.

        assertThat(mSubscriptionManagerServiceUT.getOpportunisticSubscriptions(

                CALLING_PACKAGE, CALLING_FEATURE)).isEmpty();



        mContextFixture.addCallingOrSelfPermission(Manifest.permission.READ_PHONE_STATE);