Do not crash when passing null into buildTemplateMobileAll

Test: atest FrameworksNetTests:android.net.connectivity.android.net.NetworkTemplateTest
Test: atest FrameworksNetTests:android.net.connectivity.android.net.netstats.NetworkTemplateTest
Bug: 273963543
Change-Id: I0a8f94df124147e92d35cf474b3d69d1dee6902c
This commit is contained in:
Junyu Lai
2023-03-28 17:28:01 +08:00
parent dafaf2a08b
commit 2c12f922de
3 changed files with 70 additions and 13 deletions

View File

@@ -61,6 +61,7 @@ import java.lang.annotation.RetentionPolicy;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.Comparator; import java.util.Comparator;
import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Objects; import java.util.Objects;
import java.util.Set; import java.util.Set;
@@ -194,8 +195,22 @@ public final class NetworkTemplate implements Parcelable {
@UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.TIRAMISU, @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.TIRAMISU,
publicAlternatives = "Use {@code Builder} instead.") publicAlternatives = "Use {@code Builder} instead.")
public static NetworkTemplate buildTemplateMobileAll(@NonNull String subscriberId) { 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) 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. // subscriber ID.
case MATCH_CARRIER: case MATCH_CARRIER:
if (matchSubscriberIds.length == 0) { 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)) { 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" throw new IllegalArgumentException("checkValidMatchSubscriberIds list of ids"
+ " may not contain null for rule " + getMatchRuleName(matchRule)); + " may not contain null for rule " + getMatchRuleName(matchRule));
} }

View File

@@ -30,16 +30,17 @@ import android.net.NetworkTemplate.MATCH_PROXY
import android.net.NetworkTemplate.MATCH_WIFI import android.net.NetworkTemplate.MATCH_WIFI
import android.net.NetworkTemplate.NETWORK_TYPE_ALL import android.net.NetworkTemplate.NETWORK_TYPE_ALL
import android.net.NetworkTemplate.OEM_MANAGED_ALL import android.net.NetworkTemplate.OEM_MANAGED_ALL
import android.os.Build
import android.telephony.TelephonyManager import android.telephony.TelephonyManager
import com.android.testutils.ConnectivityModuleTest import com.android.testutils.ConnectivityModuleTest
import com.android.testutils.DevSdkIgnoreRule import com.android.testutils.DevSdkIgnoreRule
import com.android.testutils.SC_V2 import com.android.testutils.SC_V2
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
import org.junit.Rule import org.junit.Rule
import org.junit.Test import org.junit.Test
import org.junit.runner.RunWith import org.junit.runner.RunWith
import org.junit.runners.JUnit4 import org.junit.runners.JUnit4
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
private const val TEST_IMSI1 = "imsi" private const val TEST_IMSI1 = "imsi"
private const val TEST_WIFI_KEY1 = "wifiKey1" 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. // Verify carrier and mobile template cannot contain one of subscriber Id is null.
listOf(MATCH_MOBILE, MATCH_CARRIER).forEach { assertFailsWith<IllegalArgumentException> {
NetworkTemplate.Builder(MATCH_CARRIER).setSubscriberIds(setOf(null)).build()
}
val firstSdk = Build.VERSION.DEVICE_INITIAL_SDK_INT
if (firstSdk > Build.VERSION_CODES.TIRAMISU) {
assertFailsWith<IllegalArgumentException> { assertFailsWith<IllegalArgumentException> {
NetworkTemplate.Builder(it).setSubscriberIds(setOf(null)).build() 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)
} }
} }

View File

@@ -50,16 +50,17 @@ import android.telephony.TelephonyManager
import com.android.testutils.DevSdkIgnoreRule import com.android.testutils.DevSdkIgnoreRule
import com.android.testutils.DevSdkIgnoreRunner import com.android.testutils.DevSdkIgnoreRunner
import com.android.testutils.assertParcelSane 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.Before
import org.junit.Rule
import org.junit.Test import org.junit.Test
import org.junit.runner.RunWith import org.junit.runner.RunWith
import org.mockito.Mockito.mock import org.mockito.Mockito.mock
import org.mockito.Mockito.`when` import org.mockito.Mockito.`when`
import org.mockito.MockitoAnnotations 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_IMSI1 = "imsi1"
private const val TEST_IMSI2 = "imsi2" private const val TEST_IMSI2 = "imsi2"
@@ -70,6 +71,8 @@ private const val TEST_WIFI_KEY2 = "wifiKey2"
@RunWith(DevSdkIgnoreRunner::class) @RunWith(DevSdkIgnoreRunner::class)
@DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.S_V2) @DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.S_V2)
class NetworkTemplateTest { class NetworkTemplateTest {
@get:Rule
val ignoreRule = DevSdkIgnoreRule()
private val mockContext = mock(Context::class.java) private val mockContext = mock(Context::class.java)
private val mockWifiInfo = mock(WifiInfo::class.java) private val mockWifiInfo = mock(WifiInfo::class.java)
@@ -215,6 +218,18 @@ class NetworkTemplateTest {
templateNullWifiKey.assertDoesNotMatch(identWifiNullKey) 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 @Test
fun testMobileMatches() { fun testMobileMatches() {
val templateMobileImsi1 = buildTemplateMobileAll(TEST_IMSI1) val templateMobileImsi1 = buildTemplateMobileAll(TEST_IMSI1)