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
diff --git a/framework/src/android/net/NetworkCapabilities.java b/framework/src/android/net/NetworkCapabilities.java
index 45efbfe..a5a6723 100644
--- a/framework/src/android/net/NetworkCapabilities.java
+++ b/framework/src/android/net/NetworkCapabilities.java
@@ -36,6 +36,7 @@
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 @@
// 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 @@
* @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 @@
* @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 @@
* @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 @@
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
diff --git a/tests/common/java/android/net/NetworkCapabilitiesTest.java b/tests/common/java/android/net/NetworkCapabilitiesTest.java
index 6eb56c7b..0f0e2f1 100644
--- a/tests/common/java/android/net/NetworkCapabilitiesTest.java
+++ b/tests/common/java/android/net/NetworkCapabilitiesTest.java
@@ -61,6 +61,7 @@
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.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.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 @@
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);
+ }
}