Use default package version in isFeatureEnabled if package is not found
Bug: 287359634
Bug: 289173013
Test: atest HostsideVpnTests
Change-Id: I379cde0391f96a8ac5dcdb2ff7723ea950684e36
diff --git a/staticlibs/device/com/android/net/module/util/DeviceConfigUtils.java b/staticlibs/device/com/android/net/module/util/DeviceConfigUtils.java
index dae4eb9..138c1e5 100644
--- a/staticlibs/device/com/android/net/module/util/DeviceConfigUtils.java
+++ b/staticlibs/device/com/android/net/module/util/DeviceConfigUtils.java
@@ -53,6 +53,8 @@
"com.android.server.connectivity.intent.action.SERVICE_CONNECTIVITY_RESOURCES_APK";
private static final String CONNECTIVITY_RES_PKG_DIR = "/apex/" + TETHERING_MODULE_NAME + "/";
+ private static final long DEFAULT_PACKAGE_VERSION = 1000;
+
@VisibleForTesting
public static void resetPackageVersionCacheForTest() {
sPackageVersion = -1;
@@ -60,17 +62,21 @@
}
private static volatile long sPackageVersion = -1;
- private static long getPackageVersion(@NonNull final Context context)
- throws PackageManager.NameNotFoundException {
+ private static long getPackageVersion(@NonNull final Context context) {
// sPackageVersion may be set by another thread just after this check, but querying the
// package version several times on rare occasions is fine.
if (sPackageVersion >= 0) {
return sPackageVersion;
}
- final long version = context.getPackageManager().getPackageInfo(
- context.getPackageName(), 0).getLongVersionCode();
- sPackageVersion = version;
- return version;
+ try {
+ final long version = context.getPackageManager().getPackageInfo(
+ context.getPackageName(), 0).getLongVersionCode();
+ sPackageVersion = version;
+ return version;
+ } catch (PackageManager.NameNotFoundException e) {
+ Log.e(TAG, "Failed to get package info: " + e);
+ return DEFAULT_PACKAGE_VERSION;
+ }
}
/**
@@ -173,13 +179,8 @@
*/
public static boolean isFeatureEnabled(@NonNull Context context, @NonNull String namespace,
@NonNull String name, boolean defaultEnabled) {
- try {
- final long packageVersion = getPackageVersion(context);
- return isFeatureEnabled(context, packageVersion, namespace, name, defaultEnabled);
- } catch (PackageManager.NameNotFoundException e) {
- Log.e(TAG, "Could not find the package name", e);
- return false;
- }
+ final long packageVersion = getPackageVersion(context);
+ return isFeatureEnabled(context, packageVersion, namespace, name, defaultEnabled);
}
/**
@@ -205,18 +206,12 @@
throw new IllegalArgumentException(
"This method is only usable by the tethering module");
}
- try {
- final long packageVersion = getTetheringModuleVersion(context);
- return isFeatureEnabled(context, packageVersion, namespace, name, defaultEnabled);
- } catch (PackageManager.NameNotFoundException e) {
- Log.e(TAG, "Could not find the module name", e);
- return false;
- }
+ final long packageVersion = getTetheringModuleVersion(context);
+ return isFeatureEnabled(context, packageVersion, namespace, name, defaultEnabled);
}
private static boolean isFeatureEnabled(@NonNull Context context, long packageVersion,
- @NonNull String namespace, String name, boolean defaultEnabled)
- throws PackageManager.NameNotFoundException {
+ @NonNull String namespace, String name, boolean defaultEnabled) {
final int propertyVersion = getDeviceConfigPropertyInt(namespace, name,
0 /* default value */);
return (propertyVersion == 0 && defaultEnabled)
@@ -251,11 +246,17 @@
}
private static volatile long sModuleVersion = -1;
- private static long getTetheringModuleVersion(@NonNull Context context)
- throws PackageManager.NameNotFoundException {
+ private static long getTetheringModuleVersion(@NonNull Context context) {
if (sModuleVersion >= 0) return sModuleVersion;
- sModuleVersion = resolveTetheringModuleVersion(context);
+ try {
+ sModuleVersion = resolveTetheringModuleVersion(context);
+ } catch (PackageManager.NameNotFoundException e) {
+ // It's expected to fail tethering module version resolution on the devices with
+ // flattened apex
+ Log.e(TAG, "Failed to resolve tethering module version: " + e);
+ return DEFAULT_PACKAGE_VERSION;
+ }
return sModuleVersion;
}
diff --git a/staticlibs/tests/unit/src/com/android/net/module/util/DeviceConfigUtilsTest.java b/staticlibs/tests/unit/src/com/android/net/module/util/DeviceConfigUtilsTest.java
index b75939b..328c39a 100644
--- a/staticlibs/tests/unit/src/com/android/net/module/util/DeviceConfigUtilsTest.java
+++ b/staticlibs/tests/unit/src/com/android/net/module/util/DeviceConfigUtilsTest.java
@@ -246,10 +246,34 @@
@Test
public void testFeatureIsEnabledWithException() throws Exception {
doThrow(NameNotFoundException.class).when(mPm).getPackageInfo(anyString(), anyInt());
+
+ // Feature should be enabled by flag value "1".
+ doReturn("1").when(() -> DeviceConfig.getProperty(eq(TEST_NAME_SPACE),
+ eq(TEST_EXPERIMENT_FLAG)));
+ assertTrue(DeviceConfigUtils.isFeatureEnabled(mContext, TEST_NAME_SPACE,
+ TEST_EXPERIMENT_FLAG));
+ assertTrue(DeviceConfigUtils.isFeatureEnabled(mContext, TEST_NAME_SPACE,
+ TEST_EXPERIMENT_FLAG, TEST_APEX_NAME, false /* defaultEnabled */));
+
+ // Feature should be disabled by flag value "999999999".
+ doReturn("999999999").when(() -> DeviceConfig.getProperty(eq(TEST_NAME_SPACE),
+ eq(TEST_EXPERIMENT_FLAG)));
assertFalse(DeviceConfigUtils.isFeatureEnabled(mContext, TEST_NAME_SPACE,
TEST_EXPERIMENT_FLAG));
assertFalse(DeviceConfigUtils.isFeatureEnabled(mContext, TEST_NAME_SPACE,
TEST_EXPERIMENT_FLAG, TEST_APEX_NAME, false /* defaultEnabled */));
+
+ // Follow defaultEnabled if the flag is not set
+ doReturn(null).when(() -> DeviceConfig.getProperty(eq(TEST_NAME_SPACE),
+ eq(TEST_EXPERIMENT_FLAG)));
+ assertFalse(DeviceConfigUtils.isFeatureEnabled(mContext, TEST_NAME_SPACE,
+ TEST_EXPERIMENT_FLAG, false /* defaultEnabled */));
+ assertTrue(DeviceConfigUtils.isFeatureEnabled(mContext, TEST_NAME_SPACE,
+ TEST_EXPERIMENT_FLAG, true /* defaultEnabled */));
+ assertFalse(DeviceConfigUtils.isFeatureEnabled(mContext, TEST_NAME_SPACE,
+ TEST_EXPERIMENT_FLAG, TEST_APEX_NAME, false /* defaultEnabled */));
+ assertTrue(DeviceConfigUtils.isFeatureEnabled(mContext, TEST_NAME_SPACE,
+ TEST_EXPERIMENT_FLAG, TEST_APEX_NAME, true /* defaultEnabled */));
}
@Test