Merge "Fix an NPE when a user is added and then removed quickly" into main

2 files changed, 36 insertions, 0 deletions

diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java
index 4c5f65285a9e..ac0892b92646 100644
--- a/services/core/java/com/android/server/connectivity/Vpn.java
+++ b/services/core/java/com/android/server/connectivity/Vpn.java
@@ -1960,6 +1960,10 @@ public class Vpn {
     public void onUserAdded(int userId) {
         // If the user is restricted tie them to the parent user's VPN
         UserInfo user = mUserManager.getUserInfo(userId);
+        if (user == null) {
+            Log.e(TAG, "Can not retrieve UserInfo for userId=" + userId);
+            return;
+        }
         if (user.isRestricted() && user.restrictedProfileParentId == mUserId) {
             synchronized(Vpn.this) {
                 final Set<Range<Integer>> existingRanges = mNetworkCapabilities.getUids();
@@ -1989,6 +1993,14 @@ public class Vpn {
     public void onUserRemoved(int userId) {
         // clean up if restricted
         UserInfo user = mUserManager.getUserInfo(userId);
+        // TODO: Retrieving UserInfo upon receiving the USER_REMOVED intent is not guaranteed.
+        //  This could prevent the removal of associated ranges. To ensure proper range removal,
+        //  store the user info when adding ranges. This allows using the user ID in the
+        //  USER_REMOVED intent to handle the removal process.
+        if (user == null) {
+            Log.e(TAG, "Can not retrieve UserInfo for userId=" + userId);
+            return;
+        }
         if (user.isRestricted() && user.restrictedProfileParentId == mUserId) {
             synchronized(Vpn.this) {
                 final Set<Range<Integer>> existingRanges = mNetworkCapabilities.getUids();
diff --git a/services/tests/VpnTests/java/com/android/server/connectivity/VpnTest.java b/services/tests/VpnTests/java/com/android/server/connectivity/VpnTest.java
index 5db6a8f12e52..9117cc8e5ab8 100644
--- a/services/tests/VpnTests/java/com/android/server/connectivity/VpnTest.java
+++ b/services/tests/VpnTests/java/com/android/server/connectivity/VpnTest.java
@@ -918,6 +918,30 @@ public class VpnTest extends VpnTestBase {
     }
 
     @Test
+    public void testOnUserAddedAndRemoved_nullUserInfo() throws Exception {
+        final Vpn vpn = createVpn(PRIMARY_USER.id);
+        final Set<Range<Integer>> initialRange = rangeSet(PRIMARY_USER_RANGE);
+        // Note since mVpnProfile is a Ikev2VpnProfile, this starts an IkeV2VpnRunner.
+        startLegacyVpn(vpn, mVpnProfile);
+        // Set an initial Uid range and mock the network agent
+        vpn.mNetworkCapabilities.setUids(initialRange);
+        vpn.mNetworkAgent = mMockNetworkAgent;
+
+        // Add the restricted user and then remove it immediately. So the getUserInfo() will return
+        // null for the given restricted user id.
+        setMockedUsers(PRIMARY_USER, RESTRICTED_PROFILE_A);
+        doReturn(null).when(mUserManager).getUserInfo(RESTRICTED_PROFILE_A.id);
+        vpn.onUserAdded(RESTRICTED_PROFILE_A.id);
+        // Expect no range change to the NetworkCapabilities.
+        assertEquals(initialRange, vpn.mNetworkCapabilities.getUids());
+
+        // Remove the restricted user
+        vpn.onUserRemoved(RESTRICTED_PROFILE_A.id);
+        // Expect no range change to the NetworkCapabilities.
+        assertEquals(initialRange, vpn.mNetworkCapabilities.getUids());
+    }
+
+    @Test
     public void testPrepare_throwSecurityExceptionWhenGivenPackageDoesNotBelongToTheCaller()
             throws Exception {
         mTestDeps.mIgnoreCallingUidChecks = false;