From a80fd3debaa26e4f6e2acad0fc6c3f43c292cbca Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Tue, 10 Dec 2024 14:35:58 +0800 Subject: [PATCH] try-catch when calling createContextAsUser There is no safe way to invoke this method correctly since the package might not be installed for certain user type by OEM. Thus, improve error handling and logging into WTF counter metrics. Test: Manual: 1. Use internal main branch 2. Modify xml under vendor/google/data/etc/sysconfig to configure tethering only installs on the SYSTEM user 3. Mock the device as entitlement required (ag/29664886) 4. Create a secondary user which has the permission to enable tethering (go/hsum) 5. adb shell pm list packages --user 10 | grep tether 6. m statsd_testdrive && statsd_testdrive -e 979 7. Enable tethering Test: atest TetheringTests:EntitlementManagerTest Bug: 382628161 Change-Id: I233cad672e41d2b4687e53e61b93dcea9eb07817 --- Tethering/Android.bp | 1 + .../tethering/EntitlementManager.java | 22 ++++++++++--- .../tethering/EntitlementManagerTest.java | 31 ++++++++++++++++++- 3 files changed, 49 insertions(+), 5 deletions(-) diff --git a/Tethering/Android.bp b/Tethering/Android.bp index 4d173a5d23..091849ba26 100644 --- a/Tethering/Android.bp +++ b/Tethering/Android.bp @@ -58,6 +58,7 @@ java_defaults { ":framework-connectivity-shared-srcs", ":services-tethering-shared-srcs", ":statslog-connectivity-java-gen", + ":statslog-framework-connectivity-java-gen", ":statslog-tethering-java-gen", ], static_libs: [ diff --git a/Tethering/src/com/android/networkstack/tethering/EntitlementManager.java b/Tethering/src/com/android/networkstack/tethering/EntitlementManager.java index a9421660d9..900b505f0f 100644 --- a/Tethering/src/com/android/networkstack/tethering/EntitlementManager.java +++ b/Tethering/src/com/android/networkstack/tethering/EntitlementManager.java @@ -62,6 +62,7 @@ import androidx.annotation.Nullable; import com.android.internal.annotations.VisibleForTesting; import com.android.modules.utils.build.SdkLevel; +import com.android.net.module.util.FrameworkConnectivityStatsLog; import com.android.net.module.util.SharedLog; import java.io.PrintWriter; @@ -154,14 +155,27 @@ public class EntitlementManager { // Only launch entitlement UI for the current user if it is allowed to // change tethering. This usually means the system user or the admin users in HSUM. - // TODO (b/382624069): Figure out whether it is safe to call createContextAsUser - // from secondary user. And re-enable the check or remove the code accordingly. - if (false) { + if (SdkLevel.isAtLeastT()) { // Create a user context for the current foreground user as UserManager#isAdmin() // operates on the context user. final int currentUserId = getCurrentUser(); final UserHandle currentUser = UserHandle.of(currentUserId); - final Context userContext = mContext.createContextAsUser(currentUser, 0); + final Context userContext; + try { + // There is no safe way to invoke this method since tethering package + // might not be installed for a certain user on the OEM devices, + // refer to b/382628161. + userContext = mContext.createContextAsUser(currentUser, 0); + } catch (IllegalStateException e) { + FrameworkConnectivityStatsLog.write( + FrameworkConnectivityStatsLog.CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED, + FrameworkConnectivityStatsLog.CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_ENTITLEMENT_CREATE_CONTEXT_AS_USER_THROWS + ); + // Fallback to startActivity if createContextAsUser failed. + mLog.e("createContextAsUser failed, fallback to startActivity", e); + mContext.startActivity(intent); + return intent; + } final UserManager userManager = userContext.getSystemService(UserManager.class); if (userManager.isAdminUser()) { diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/EntitlementManagerTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/EntitlementManagerTest.java index 16ebbbbdbb..58e18940e4 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/EntitlementManagerTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/EntitlementManagerTest.java @@ -38,6 +38,7 @@ import static android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID; import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn; import static com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSession; import static com.android.networkstack.apishim.ConstantsShim.KEY_CARRIER_SUPPORTS_TETHERING_BOOL; +import static com.android.testutils.DevSdkIgnoreRule.IgnoreAfter; import static com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo; import static com.android.testutils.DevSdkIgnoreRuleKt.SC_V2; @@ -49,6 +50,7 @@ import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyLong; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; @@ -159,10 +161,20 @@ public final class EntitlementManagerTest { return super.getSystemServiceName(serviceClass); } + @NonNull @Override public Context createContextAsUser(UserHandle user, int flags) { + if (mCreateContextAsUserException != null) { + throw mCreateContextAsUserException; + } return mMockContext; // Return self for easier test injection. } + + private RuntimeException mCreateContextAsUserException = null; + + private void setCreateContextAsUserException(RuntimeException e) { + mCreateContextAsUserException = e; + } } class TestDependencies extends EntitlementManager.Dependencies { @@ -591,8 +603,24 @@ public final class EntitlementManagerTest { .onTetherProvisioningFailed(TETHERING_WIFI, FAILED_TETHERING_REASON); } + @IgnoreUpTo(SC_V2) @Test - public void testUiProvisioningMultiUser() { + public void testUiProvisioningMultiUser_aboveT_createContextAsUserThrows() { + mMockContext.setCreateContextAsUserException(new IllegalStateException()); + doTestUiProvisioningMultiUser(true, 1); + doTestUiProvisioningMultiUser(false, 1); + } + + @IgnoreUpTo(SC_V2) + @Test + public void testUiProvisioningMultiUser_aboveT() { + doTestUiProvisioningMultiUser(true, 1); + doTestUiProvisioningMultiUser(false, 0); + } + + @IgnoreAfter(SC_V2) + @Test + public void testUiProvisioningMultiUser_belowT() { doTestUiProvisioningMultiUser(true, 1); doTestUiProvisioningMultiUser(false, 1); } @@ -630,6 +658,7 @@ public final class EntitlementManagerTest { doReturn(isAdminUser).when(mUserManager).isAdminUser(); mDeps.reset(); + clearInvocations(mTetherProvisioningFailedListener); mDeps.fakeEntitlementResult = TETHER_ERROR_NO_ERROR; mEnMgr.notifyUpstream(true); mLooper.dispatchAll();