Stop throwing when the invalid capability is passed

When new capability is introduced and added to capaility or
forbiddenCapability by default, module rollback could cause system
server crash since JobScheduler persists network request that contains
new capability that is invalid for old code.
This CL updates capability validation not to throw exception to avoid
system crash in this case.

Bug: 343823469
Bug: 313030307
Test: NetworkCapabilitiesTest
Change-Id: I5708c475f3b4974284531212b77f9788ca9f497e
This commit is contained in:
Motomu Utsumi 2024-05-31 12:55:31 +09:00
commit ab1a0671c7
2 changed files with 50 additions and 14 deletions

View file

@ -36,6 +36,7 @@ import android.os.Parcelable;
import android.os.Process;
import android.text.TextUtils;
import android.util.ArraySet;
import android.util.Log;
import android.util.Range;
import com.android.internal.annotations.VisibleForTesting;
@ -843,7 +844,10 @@ public final class NetworkCapabilities implements Parcelable {
// If the given capability was previously added to the list of forbidden capabilities
// then the capability will also be removed from the list of forbidden capabilities.
// TODO: Add forbidden capabilities to the public API
checkValidCapability(capability);
if (!isValidCapability(capability)) {
Log.e(TAG, "addCapability is called with invalid capability: " + capability);
return this;
}
mNetworkCapabilities |= 1L << capability;
// remove from forbidden capability list
mForbiddenNetworkCapabilities &= ~(1L << capability);
@ -864,7 +868,10 @@ public final class NetworkCapabilities implements Parcelable {
* @hide
*/
public void addForbiddenCapability(@NetCapability int capability) {
checkValidCapability(capability);
if (!isValidCapability(capability)) {
Log.e(TAG, "addForbiddenCapability is called with invalid capability: " + capability);
return;
}
mForbiddenNetworkCapabilities |= 1L << capability;
mNetworkCapabilities &= ~(1L << capability); // remove from requested capabilities
}
@ -878,7 +885,10 @@ public final class NetworkCapabilities implements Parcelable {
* @hide
*/
public @NonNull NetworkCapabilities removeCapability(@NetCapability int capability) {
checkValidCapability(capability);
if (!isValidCapability(capability)) {
Log.e(TAG, "removeCapability is called with invalid capability: " + capability);
return this;
}
final long mask = ~(1L << capability);
mNetworkCapabilities &= mask;
return this;
@ -893,7 +903,11 @@ public final class NetworkCapabilities implements Parcelable {
* @hide
*/
public @NonNull NetworkCapabilities removeForbiddenCapability(@NetCapability int capability) {
checkValidCapability(capability);
if (!isValidCapability(capability)) {
Log.e(TAG,
"removeForbiddenCapability is called with invalid capability: " + capability);
return this;
}
mForbiddenNetworkCapabilities &= ~(1L << capability);
return this;
}
@ -2632,12 +2646,6 @@ public final class NetworkCapabilities implements Parcelable {
return capability >= 0 && capability <= MAX_NET_CAPABILITY;
}
private static void checkValidCapability(@NetworkCapabilities.NetCapability int capability) {
if (!isValidCapability(capability)) {
throw new IllegalArgumentException("NetworkCapability " + capability + " out of range");
}
}
private static boolean isValidEnterpriseId(
@NetworkCapabilities.EnterpriseId int enterpriseId) {
return enterpriseId >= NET_ENTERPRISE_ID_1

View file

@ -61,6 +61,7 @@ import static android.net.NetworkCapabilities.TRANSPORT_VPN;
import static android.net.NetworkCapabilities.TRANSPORT_WIFI;
import static android.net.NetworkCapabilities.TRANSPORT_WIFI_AWARE;
import static android.os.Process.INVALID_UID;
import static com.android.modules.utils.build.SdkLevel.isAtLeastS;
import static com.android.modules.utils.build.SdkLevel.isAtLeastT;
import static com.android.modules.utils.build.SdkLevel.isAtLeastV;
@ -68,6 +69,7 @@ import static com.android.testutils.DevSdkIgnoreRuleKt.SC_V2;
import static com.android.testutils.MiscAsserts.assertEmpty;
import static com.android.testutils.MiscAsserts.assertThrows;
import static com.android.testutils.ParcelUtils.assertParcelingIsLossless;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@ -83,21 +85,25 @@ import android.net.wifi.aware.WifiAwareNetworkSpecifier;
import android.os.Build;
import android.util.ArraySet;
import android.util.Range;
import androidx.test.filters.SmallTest;
import com.android.testutils.CompatUtil;
import com.android.testutils.ConnectivityModuleTest;
import com.android.testutils.DevSdkIgnoreRule;
import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo;
import com.android.testutils.DevSdkIgnoreRunner;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Set;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mockito;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Set;
@SmallTest
@RunWith(DevSdkIgnoreRunner.class)
// NetworkCapabilities is only updatable on S+, and this test covers behavior which implementation
@ -1465,4 +1471,26 @@ public class NetworkCapabilitiesTest {
assertEquals("-SUPL-VALIDATED-CAPTIVE_PORTAL+MMS+OEM_PAID",
nc1.describeCapsDifferencesFrom(nc2));
}
@Test
public void testInvalidCapability() {
final int invalidCapability = Integer.MAX_VALUE;
// Passing invalid capability does not throw
final NetworkCapabilities nc1 = new NetworkCapabilities.Builder()
.addCapability(NET_CAPABILITY_INTERNET)
.addForbiddenCapability(NET_CAPABILITY_NOT_ROAMING)
.removeCapability(invalidCapability)
.removeForbiddenCapability(invalidCapability)
.addCapability(invalidCapability)
.addForbiddenCapability(invalidCapability)
.build();
final NetworkCapabilities nc2 = new NetworkCapabilities.Builder()
.addCapability(NET_CAPABILITY_INTERNET)
.addForbiddenCapability(NET_CAPABILITY_NOT_ROAMING)
.build();
// nc1 and nc2 are the same since invalid capability is ignored
assertEquals(nc1, nc2);
}
}