Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Skip unknown change IDs when adding/removing overrides on release builds" into sc-v2-dev

Browse Source
This commit is contained in:
Tom Natan
2021-11-16 15:41:56 +00:00
committed by Android (Google) Code Review
parent 7d75b59495 98f17c1f70
commit 7ff3fe9ca5
3 changed files with 93 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
services/core/java/com/android/server/compat/CompatConfig.java
View File

@@ -220,11 +220,18 @@ final class CompatConfig {
* Overrides the enabled state for a given change and app.
*
*
* @param overrides list of overrides to default changes config.
* @param packageName app for which the overrides will be applied.
* @param overrides list of overrides to default changes config.
* @param packageName app for which the overrides will be applied.
* @param skipUnknownChangeIds whether to skip unknown change IDs in {@code overrides}.
*/
synchronized void addOverrides(CompatibilityOverrideConfig overrides, String packageName) {
synchronized void addPackageOverrides(CompatibilityOverrideConfig overrides,
String packageName, boolean skipUnknownChangeIds) {
for (Long changeId : overrides.overrides.keySet()) {
if (skipUnknownChangeIds && !isKnownChangeId(changeId)) {
Slog.w(TAG, "Trying to add overrides for unknown Change ID " + changeId + ". "
+ "Skipping Change ID.");
continue;
}
addOverrideUnsafe(changeId, packageName, overrides.overrides.get(changeId));
}
saveOverrides();
@@ -335,7 +342,8 @@ final class CompatConfig {
/**
* Removes all overrides previously added via {@link #addOverride(long, String, boolean)} or
* {@link #addOverrides(CompatibilityOverrideConfig, String)} for a certain package.
* {@link #addPackageOverrides(CompatibilityOverrideConfig, String, boolean)} for a certain
* package.
*
* <p>This restores the default behaviour for the given app.
*
@@ -356,7 +364,8 @@ final class CompatConfig {
/**
* Removes overrides whose change ID is specified in {@code overridesToRemove} that were
* previously added via {@link #addOverride(long, String, boolean)} or
* {@link #addOverrides(CompatibilityOverrideConfig, String)} for a certain package.
* {@link #addPackageOverrides(CompatibilityOverrideConfig, String, boolean)} for a certain
* package.
*
* <p>This restores the default behaviour for the given change IDs and app.
*
@@ -367,6 +376,11 @@ final class CompatConfig {
String packageName) {
boolean shouldInvalidateCache = false;
for (Long changeId : overridesToRemove.changeIds) {
if (!isKnownChangeId(changeId)) {
Slog.w(TAG, "Trying to remove overrides for unknown Change ID " + changeId + ". "
+ "Skipping Change ID.");
continue;
}
shouldInvalidateCache |= removeOverrideUnsafe(changeId, packageName);
}
if (shouldInvalidateCache) {

10
services/core/java/com/android/server/compat/PlatformCompat.java
View File

@@ -205,7 +205,8 @@ public class PlatformCompat extends IPlatformCompat.Stub {
overridesMap.put(change, new PackageOverride.Builder().setEnabled(false)
.build());
}
mCompatConfig.addOverrides(new CompatibilityOverrideConfig(overridesMap), packageName);
mCompatConfig.addPackageOverrides(new CompatibilityOverrideConfig(overridesMap),
packageName, /* skipUnknownChangeIds */ false);
killPackage(packageName);
}
@@ -220,7 +221,8 @@ public class PlatformCompat extends IPlatformCompat.Stub {
overridesMap.put(change, new PackageOverride.Builder().setEnabled(false)
.build());
}
mCompatConfig.addOverrides(new CompatibilityOverrideConfig(overridesMap), packageName);
mCompatConfig.addPackageOverrides(new CompatibilityOverrideConfig(overridesMap),
packageName, /* skipUnknownChangeIds */ false);
}
@Override
@@ -229,7 +231,7 @@ public class PlatformCompat extends IPlatformCompat.Stub {
// TODO(b/183630314): Unify the permission enforcement with the other setOverrides* methods.
checkCompatChangeOverrideOverridablePermission();
checkAllCompatOverridesAreOverridable(overrides.overrides.keySet());
mCompatConfig.addOverrides(overrides, packageName);
mCompatConfig.addPackageOverrides(overrides, packageName, /* skipUnknownChangeIds= */ true);
}
@Override
@@ -435,7 +437,7 @@ public class PlatformCompat extends IPlatformCompat.Stub {
private void checkAllCompatOverridesAreOverridable(Collection<Long> changeIds) {
for (Long changeId : changeIds) {
if (!mCompatConfig.isOverridable(changeId)) {
if (isKnownChangeId(changeId) && !mCompatConfig.isOverridable(changeId)) {
throw new SecurityException("Only change ids marked as Overridable can be "
+ "overridden.");
}

88
services/tests/servicestests/src/com/android/server/compat/CompatConfigTest.java
View File

@@ -265,9 +265,10 @@ public class CompatConfigTest {
}
@Test
public void testInstallerCanSetOverrides() throws Exception {
public void testInstallerCanAddOverrides() throws Exception {
final long disabledChangeId1 = 1234L;
final long disabledChangeId2 = 1235L;
final long unknownChangeId = 1236L;
// We make disabledChangeId2 non-overridable to make sure it is ignored.
CompatConfig compatConfig = CompatConfigBuilder.create(mBuildClassifier, mContext)
.addDisabledOverridableChangeWithId(disabledChangeId1)
@@ -284,19 +285,25 @@ public class CompatConfigTest {
// Force the validator to prevent overriding non-overridable changes by using a user build.
when(mBuildClassifier.isDebuggableBuild()).thenReturn(false);
when(mBuildClassifier.isFinalBuild()).thenReturn(true);
Map<Long, PackageOverride> overrides = new HashMap<>();
overrides.put(disabledChangeId1, new PackageOverride.Builder()
.setMaxVersionCode(99L)
.setEnabled(true)
.build());
// Adding an unknown change ID to make sure it's skipped if skipUnknownChangeIds is true.
overrides.put(unknownChangeId, new PackageOverride.Builder().setEnabled(false).build());
CompatibilityOverrideConfig config = new CompatibilityOverrideConfig(overrides);
CompatibilityOverrideConfig config = new CompatibilityOverrideConfig(
Collections.singletonMap(disabledChangeId1,
new PackageOverride.Builder()
.setMaxVersionCode(99L)
.setEnabled(true)
.build()));
compatConfig.addOverrides(config, "com.some.package");
compatConfig.addPackageOverrides(config, "com.some.package", /* skipUnknownChangeIds */
true);
assertThat(compatConfig.isChangeEnabled(disabledChangeId1, applicationInfo)).isTrue();
assertThat(compatConfig.isChangeEnabled(disabledChangeId2, applicationInfo)).isFalse();
// Making sure the unknown change ID is still unknown and isChangeEnabled returns true.
assertThat(compatConfig.isKnownChangeId(unknownChangeId)).isFalse();
assertThat(compatConfig.isChangeEnabled(unknownChangeId, applicationInfo)).isTrue();
}
@Test
public void testPreventInstallerSetNonOverridable() throws Exception {
final long disabledChangeId1 = 1234L;
@@ -326,13 +333,46 @@ public class CompatConfigTest {
CompatibilityOverrideConfig config = new CompatibilityOverrideConfig(overrides);
assertThrows(SecurityException.class,
() -> compatConfig.addOverrides(config, "com.some.package")
() -> compatConfig.addPackageOverrides(config, "com.some.package",
/* skipUnknownChangeIds */ true)
);
assertThat(compatConfig.isChangeEnabled(disabledChangeId1, applicationInfo)).isTrue();
assertThat(compatConfig.isChangeEnabled(disabledChangeId2, applicationInfo)).isFalse();
assertThat(compatConfig.isChangeEnabled(disabledChangeId3, applicationInfo)).isFalse();
}
@Test
public void testCanAddOverridesForUnknownChangeIdOnDebugBuild() throws Exception {
final long disabledChangeId = 1234L;
final long unknownChangeId = 1235L;
// We make disabledChangeId2 non-overridable to make sure it is ignored.
CompatConfig compatConfig = CompatConfigBuilder.create(mBuildClassifier, mContext)
.addDisabledChangeWithId(disabledChangeId)
.build();
ApplicationInfo applicationInfo = ApplicationInfoBuilder.create()
.withPackageName("com.some.package")
.build();
PackageManager packageManager = mock(PackageManager.class);
when(mContext.getPackageManager()).thenReturn(packageManager);
when(packageManager.getApplicationInfo(eq("com.some.package"), anyInt()))
.thenReturn(applicationInfo);
when(mBuildClassifier.isDebuggableBuild()).thenReturn(true);
Map<Long, PackageOverride> overrides = new HashMap<>();
overrides.put(disabledChangeId, new PackageOverride.Builder().setEnabled(true).build());
// Adding an unknown change ID to make sure it isn't skipped if skipUnknownChangeIds is
// false.
overrides.put(unknownChangeId, new PackageOverride.Builder().setEnabled(false).build());
CompatibilityOverrideConfig config = new CompatibilityOverrideConfig(overrides);
compatConfig.addPackageOverrides(config, "com.some.package", /* skipUnknownChangeIds */
false);
assertThat(compatConfig.isChangeEnabled(disabledChangeId, applicationInfo)).isTrue();
// Making sure the unknown change ID is now known and has an override.
assertThat(compatConfig.isKnownChangeId(unknownChangeId)).isTrue();
assertThat(compatConfig.isChangeEnabled(unknownChangeId, applicationInfo)).isFalse();
}
@Test
public void testApplyDeferredOverridesAfterInstallingApp() throws Exception {
ApplicationInfo applicationInfo = ApplicationInfoBuilder.create()
@@ -377,7 +417,8 @@ public class CompatConfigTest {
.setMaxVersionCode(99L)
.setEnabled(true)
.build()));
compatConfig.addOverrides(config, "com.installed.foo");
compatConfig.addPackageOverrides(config, "com.installed.foo", /* skipUnknownChangeIds */
true);
assertThat(compatConfig.isChangeEnabled(1234L, applicationInfo)).isFalse();
// Add override that does include the installed app version
@@ -388,7 +429,8 @@ public class CompatConfigTest {
.setMaxVersionCode(100L)
.setEnabled(true)
.build()));
compatConfig.addOverrides(config, "com.installed.foo");
compatConfig.addPackageOverrides(config, "com.installed.foo", /* skipUnknownChangeIds */
true);
assertThat(compatConfig.isChangeEnabled(1234L, applicationInfo)).isTrue();
}
@@ -411,7 +453,8 @@ public class CompatConfigTest {
.setMaxVersionCode(99L)
.setEnabled(true)
.build()));
compatConfig.addOverrides(config, "com.notinstalled.foo");
compatConfig.addPackageOverrides(config, "com.notinstalled.foo", /* skipUnknownChangeIds */
true);
assertThat(compatConfig.isChangeEnabled(1234L, applicationInfo)).isFalse();
// Pretend the app is now installed.
@@ -557,6 +600,7 @@ public class CompatConfigTest {
final long disabledChangeId1 = 1234L;
final long disabledChangeId2 = 1235L;
final long enabledChangeId = 1236L;
final long unknownChangeId = 1237L;
// We make disabledChangeId2 non-overridable to make sure it is ignored.
CompatConfig compatConfig = CompatConfigBuilder.create(mBuildClassifier, mContext)
.addDisabledOverridableChangeWithId(disabledChangeId1)
@@ -583,6 +627,8 @@ public class CompatConfigTest {
Set<Long> overridesToRemove = new HashSet<>();
overridesToRemove.add(disabledChangeId1);
overridesToRemove.add(enabledChangeId);
// Adding an unknown change ID to make sure it's skipped.
overridesToRemove.add(unknownChangeId);
CompatibilityOverridesToRemoveConfig config = new CompatibilityOverridesToRemoveConfig(
overridesToRemove);
@@ -590,6 +636,8 @@ public class CompatConfigTest {
assertThat(compatConfig.isChangeEnabled(disabledChangeId1, applicationInfo)).isFalse();
assertThat(compatConfig.isChangeEnabled(disabledChangeId2, applicationInfo)).isTrue();
assertThat(compatConfig.isChangeEnabled(enabledChangeId, applicationInfo)).isTrue();
// Making sure the unknown change ID is still unknown.
assertThat(compatConfig.isKnownChangeId(unknownChangeId)).isFalse();
}
@Test
@@ -797,18 +845,18 @@ public class CompatConfigTest {
.build());
when(mPackageManager.getApplicationInfo(eq("bar.baz"), anyInt()))
.thenThrow(new NameNotFoundException());
compatConfig.addOverrides(
compatConfig.addPackageOverrides(
new CompatibilityOverrideConfig(
Collections.singletonMap(
1L,
new PackageOverride.Builder().setEnabled(true).build())),
"foo.bar");
compatConfig.addOverrides(
"foo.bar", /* skipUnknownChangeIds */ true);
compatConfig.addPackageOverrides(
new CompatibilityOverrideConfig(
Collections.singletonMap(
2L,
new PackageOverride.Builder().setEnabled(false).build())),
"bar.baz");
"bar.baz", /* skipUnknownChangeIds */ true);
assertThat(readFile(overridesFile)).isEqualTo("<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
+ "<overrides>\n"
@@ -847,12 +895,12 @@ public class CompatConfigTest {
compatConfig.forceNonDebuggableFinalForTest(true);
compatConfig.initOverrides(overridesFile, new File(""));
compatConfig.addOverrides(new CompatibilityOverrideConfig(Collections.singletonMap(1L,
new PackageOverride.Builder()
compatConfig.addPackageOverrides(new CompatibilityOverrideConfig(
Collections.singletonMap(1L, new PackageOverride.Builder()
.setMinVersionCode(99L)
.setMaxVersionCode(101L)
.setEnabled(true)
.build())), "foo.bar");
.build())), "foo.bar", /* skipUnknownChangeIds */ true);
assertThat(readFile(overridesFile)).isEqualTo("<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
+ "<overrides>\n"