Merge "Do not crash when passing null into buildTemplateMobileAll"

framework-t/src/android/net/NetworkTemplate.java

@@ -61,6 +61,7 @@ import java.lang.annotation.RetentionPolicy;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
@@ -194,8 +195,22 @@ public final class NetworkTemplate implements Parcelable {
     @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.TIRAMISU,
             publicAlternatives = "Use {@code Builder} instead.")
     public static NetworkTemplate buildTemplateMobileAll(@NonNull String subscriberId) {
+        final Set<String> set;
+        // Prevent from crash for b/273963543, where the OEMs still call into this method
+        // with null subscriberId and get crashed.
+        final int firstSdk = Build.VERSION.DEVICE_INITIAL_SDK_INT;
+        if (firstSdk > Build.VERSION_CODES.TIRAMISU && subscriberId == null) {
+            throw new IllegalArgumentException("buildTemplateMobileAll does not accept null"
+                    + " subscriberId on Android U devices or above");
+        }
+        if (subscriberId == null) {
+            set = new HashSet<>();
+            set.add(null);
+        } else {
+            set = Set.of(subscriberId);
+        }
         return new NetworkTemplate.Builder(MATCH_MOBILE).setMeteredness(METERED_YES)
-                .setSubscriberIds(Set.of(subscriberId)).build();
+                .setSubscriberIds(set).build();
     }
 
     /**
@@ -410,12 +425,20 @@ public final class NetworkTemplate implements Parcelable {
             // subscriber ID.
             case MATCH_CARRIER:
                 if (matchSubscriberIds.length == 0) {
-                    throw new IllegalArgumentException("checkValidMatchSubscriberIds with empty"
+                    throw new IllegalArgumentException("matchSubscriberIds may not contain"
-                            + " list of ids for rule" + getMatchRuleName(matchRule));
+                            + " null for rule " + getMatchRuleName(matchRule));
                 }
-                // fall through
-            case MATCH_MOBILE:
                 if (CollectionUtils.contains(matchSubscriberIds, null)) {
+                    throw new IllegalArgumentException("matchSubscriberIds may not contain"
+                            + " null for rule " + getMatchRuleName(matchRule));
+                }
+                break;
+            case MATCH_MOBILE:
+                // Prevent from crash for b/273963543, where the OEMs still call into unsupported
+                // buildTemplateMobileAll with null subscriberId and get crashed.
+                final int firstSdk = Build.VERSION.DEVICE_INITIAL_SDK_INT;
+                if (firstSdk > Build.VERSION_CODES.TIRAMISU
+                        && CollectionUtils.contains(matchSubscriberIds, null)) {
                     throw new IllegalArgumentException("checkValidMatchSubscriberIds list of ids"
                             + " may not contain null for rule " + getMatchRuleName(matchRule));
                 }

tests/common/java/android/net/netstats/NetworkTemplateTest.kt

@@ -30,16 +30,17 @@ import android.net.NetworkTemplate.MATCH_PROXY
 import android.net.NetworkTemplate.MATCH_WIFI
 import android.net.NetworkTemplate.NETWORK_TYPE_ALL
 import android.net.NetworkTemplate.OEM_MANAGED_ALL
+import android.os.Build
 import android.telephony.TelephonyManager
 import com.android.testutils.ConnectivityModuleTest
 import com.android.testutils.DevSdkIgnoreRule
 import com.android.testutils.SC_V2
+import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
-import kotlin.test.assertEquals
-import kotlin.test.assertFailsWith
 
 private const val TEST_IMSI1 = "imsi"
 private const val TEST_WIFI_KEY1 = "wifiKey1"
@@ -96,9 +97,27 @@ class NetworkTemplateTest {
         }
 
         // Verify carrier and mobile template cannot contain one of subscriber Id is null.
-        listOf(MATCH_MOBILE, MATCH_CARRIER).forEach {
         assertFailsWith<IllegalArgumentException> {
-                NetworkTemplate.Builder(it).setSubscriberIds(setOf(null)).build()
+            NetworkTemplate.Builder(MATCH_CARRIER).setSubscriberIds(setOf(null)).build()
+        }
+        val firstSdk = Build.VERSION.DEVICE_INITIAL_SDK_INT
+        if (firstSdk > Build.VERSION_CODES.TIRAMISU) {
+            assertFailsWith<IllegalArgumentException> {
+                NetworkTemplate.Builder(MATCH_MOBILE).setSubscriberIds(setOf(null)).build()
+            }
+        } else {
+            NetworkTemplate.Builder(MATCH_MOBILE).setSubscriberIds(setOf(null)).build().let {
+                val expectedTemplate = NetworkTemplate(
+                    MATCH_MOBILE,
+                    arrayOfNulls<String>(1) /*subscriberIds*/,
+                    emptyArray<String>() /*wifiNetworkKey*/,
+                    METERED_ALL,
+                    ROAMING_ALL,
+                    DEFAULT_NETWORK_ALL,
+                    NETWORK_TYPE_ALL,
+                    OEM_MANAGED_ALL
+                )
+                assertEquals(expectedTemplate, it)
             }
         }
 

tests/unit/java/android/net/NetworkTemplateTest.kt

@@ -50,16 +50,17 @@ import android.telephony.TelephonyManager
 import com.android.testutils.DevSdkIgnoreRule
 import com.android.testutils.DevSdkIgnoreRunner
 import com.android.testutils.assertParcelSane
+import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertNotEquals
+import kotlin.test.assertTrue
 import org.junit.Before
+import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mockito.mock
 import org.mockito.Mockito.`when`
 import org.mockito.MockitoAnnotations
-import kotlin.test.assertEquals
-import kotlin.test.assertFalse
-import kotlin.test.assertNotEquals
-import kotlin.test.assertTrue
 
 private const val TEST_IMSI1 = "imsi1"
 private const val TEST_IMSI2 = "imsi2"
@@ -70,6 +71,8 @@ private const val TEST_WIFI_KEY2 = "wifiKey2"
 @RunWith(DevSdkIgnoreRunner::class)
 @DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.S_V2)
 class NetworkTemplateTest {
+    @get:Rule
+    val ignoreRule = DevSdkIgnoreRule()
     private val mockContext = mock(Context::class.java)
     private val mockWifiInfo = mock(WifiInfo::class.java)
 
@@ -215,6 +218,18 @@ class NetworkTemplateTest {
         templateNullWifiKey.assertDoesNotMatch(identWifiNullKey)
     }
 
+    @DevSdkIgnoreRule.IgnoreAfter(Build.VERSION_CODES.TIRAMISU)
+    @Test
+    fun testBuildTemplateMobileAll_nullSubscriberId() {
+        val templateMobileAllWithNullImsi = buildTemplateMobileAll(null)
+        val setWithNull = HashSet<String?>().apply {
+            add(null)
+        }
+        val templateFromBuilder = NetworkTemplate.Builder(MATCH_MOBILE).setMeteredness(METERED_YES)
+            .setSubscriberIds(setWithNull).build()
+        assertEquals(templateFromBuilder, templateMobileAllWithNullImsi)
+    }
+
     @Test
     fun testMobileMatches() {
         val templateMobileImsi1 = buildTemplateMobileAll(TEST_IMSI1)