Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 98f17c1f authored by tomnatan's avatar tomnatan
Browse files

Skip unknown change IDs when adding/removing overrides on release builds 

Otherwise an exception is thrown because CompatConfig#isOverridable
returns false for an unknown change ID.

Fix: 205297182
Test: atest FrameworksServicesTests:CompatConfigTest
Test: atest FrameworksServicesTests:PlatformCompatTest
Change-Id: Id45ddd3226555d1169d9814517d2da336c1c7b1a
parent 60316d32

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"