Merge "Skip unknown change IDs when adding/removing overrides on release builds" into sc-v2-dev (7ff3fe9c) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/compat/CompatConfig.java

+19 −5

Original line number	Diff line number	Diff line
		@@ -222,9 +222,16 @@ final class CompatConfig {
		*
		* @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) {

services/core/java/com/android/server/compat/PlatformCompat.java

+6 −4

Original line number	Diff line number	Diff line
		@@ -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.");
		}

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

+68 −20

Original line number	Diff line number	Diff line
		@@ -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);

		CompatibilityOverrideConfig config = new CompatibilityOverrideConfig(
		Collections.singletonMap(disabledChangeId1,
		new PackageOverride.Builder()
		Map<Long, PackageOverride> overrides = new HashMap<>();
		overrides.put(disabledChangeId1, new PackageOverride.Builder()
		.setMaxVersionCode(99L)
		.setEnabled(true)
		.build()));
		.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);

		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"