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

Commit eb8f4061 authored by Yuri Lin's avatar Yuri Lin
Browse files

Return copies of notification channels and groups instead of the cached entities. 

This fixes an issue where callers could in theory modify the returned object from getNotificationChannel or getNotificationChannelGroup and as a result modify the cache.

Also modifies NotificationChannelGroup.clone() to actually return a deep copy by also copying the list of channels and its contents.

Flag: android.app.nm_binder_perf_cache_channels
Bug: 381131846
Test: NotificationManagerTest, NotificationManagerServiceTest
Change-Id: I950695c8ff89924a998a0b79bfdbacfd27fbe4f0
parent 8c37fec1

core/api/test-current.txt

+1 −1
Original line number Diff line number Diff line
@@ -369,7 +369,7 @@ package android.app { 
  }



  public final class NotificationChannel implements android.os.Parcelable {

    method @FlaggedApi("android.service.notification.notification_conversation_channel_management") @NonNull public android.app.NotificationChannel copy();

    method @NonNull public android.app.NotificationChannel copy();

    method public int getOriginalImportance();

    method public boolean isImportanceLockedByCriticalDeviceFunction();

    method public void lockFields(int);

core/java/android/app/NotificationChannel.java

+0 −1
Original line number Diff line number Diff line
@@ -508,7 +508,6 @@ public final class NotificationChannel implements Parcelable { 
    /** @hide */

    @TestApi

    @NonNull

    @FlaggedApi(FLAG_NOTIFICATION_CONVERSATION_CHANNEL_MANAGEMENT)

    public NotificationChannel copy() {

        NotificationChannel copy = new NotificationChannel(mId, mName, mImportance);

        copy.setDescription(mDesc);

core/java/android/app/NotificationChannelGroup.java

+5 −1
Original line number Diff line number Diff line
@@ -331,7 +331,11 @@ public final class NotificationChannelGroup implements Parcelable { 
        NotificationChannelGroup cloned = new NotificationChannelGroup(getId(), getName());

        cloned.setDescription(getDescription());

        cloned.setBlocked(isBlocked());

        cloned.setChannels(getChannels());

        if (mChannels != null) {

            for (NotificationChannel c : mChannels) {

                cloned.addChannel(c.copy());

            }

        }

        cloned.lockFields(mUserLockedFields);

        return cloned;

    }

core/java/android/app/NotificationManager.java

+24 −11
Original line number Diff line number Diff line
@@ -1317,10 +1317,16 @@ public class NotificationManager { 
     */

    public List<NotificationChannel> getNotificationChannels() {

        if (Flags.nmBinderPerfCacheChannels()) {

            return mNotificationChannelListCache.query(new NotificationChannelQuery(

                    mContext.getOpPackageName(),

                    mContext.getPackageName(),

                    mContext.getUserId()));

            List<NotificationChannel> channelList = mNotificationChannelListCache.query(

                    new NotificationChannelQuery(mContext.getOpPackageName(),

                            mContext.getPackageName(), mContext.getUserId()));

            List<NotificationChannel> out = new ArrayList();

            if (channelList != null) {

                for (NotificationChannel c : channelList) {

                    out.add(c.copy());

                }

            }

            return out;

        } else {

            INotificationManager service = service();

            try {
@@ -1343,7 +1349,7 @@ public class NotificationManager { 
        }

        for (NotificationChannel channel : channels) {

            if (channelId.equals(channel.getId())) {

                return channel;

                return channel.copy();

            }

        }

        return null;
@@ -1364,12 +1370,12 @@ public class NotificationManager { 
        for (NotificationChannel channel : channels) {

            if (conversationId.equals(channel.getConversationId())

                    && channelId.equals(channel.getParentChannelId())) {

                return channel;

                return channel.copy();

            } else if (channelId.equals(channel.getId())) {

                parent = channel;

            }

        }

        return parent;

        return parent != null ? parent.copy() : null;

    }



    /**
@@ -1405,8 +1411,9 @@ public class NotificationManager { 
                    new NotificationChannelQuery(pkgName, pkgName, mContext.getUserId()));

            Map<String, NotificationChannelGroup> groupHeaders =

                    mNotificationChannelGroupsCache.query(pkgName);

            return NotificationChannelGroupsHelper.getGroupWithChannels(channelGroupId, channelList,

                    groupHeaders, /* includeDeleted= */ false);

            NotificationChannelGroup ncg = NotificationChannelGroupsHelper.getGroupWithChannels(

                    channelGroupId, channelList, groupHeaders, /* includeDeleted= */ false);

            return ncg != null ? ncg.clone() : null;

        } else {

            INotificationManager service = service();

            try {
@@ -1428,8 +1435,14 @@ public class NotificationManager { 
                    new NotificationChannelQuery(pkgName, pkgName, mContext.getUserId()));

            Map<String, NotificationChannelGroup> groupHeaders =

                    mNotificationChannelGroupsCache.query(pkgName);

            return NotificationChannelGroupsHelper.getGroupsWithChannels(channelList, groupHeaders,

            List<NotificationChannelGroup> populatedGroupList =

                    NotificationChannelGroupsHelper.getGroupsWithChannels(channelList, groupHeaders,

                            NotificationChannelGroupsHelper.Params.forAllGroups());

            List<NotificationChannelGroup> out = new ArrayList<>();

            for (NotificationChannelGroup g : populatedGroupList) {

                out.add(g.clone());

            }

            return out;

        } else {

            INotificationManager service = service();

            try {

core/tests/coretests/src/android/app/NotificationManagerTest.java

+69 −0
Original line number Diff line number Diff line
@@ -440,6 +440,44 @@ public class NotificationManagerTest { 
                .getNotificationChannels(any(), any(), anyInt());

    }



    @Test

    @EnableFlags(Flags.FLAG_NM_BINDER_PERF_CACHE_CHANNELS)

    public void getNotificationChannel_localModificationDoesNotChangeCache() throws Exception {

        NotificationManager.invalidateNotificationChannelCache();

        NotificationChannel original = new NotificationChannel("id", "name",

                NotificationManager.IMPORTANCE_DEFAULT);

        NotificationChannel originalConv = new NotificationChannel("", "name_conversation",

                NotificationManager.IMPORTANCE_DEFAULT);

        originalConv.setConversationId("id", "id_conversation");

        when(mNotificationManager.mBackendService.getNotificationChannels(any(), any(),

                anyInt())).thenReturn(new ParceledListSlice<>(

                        List.of(original.copy(), originalConv.copy())));



        // modify the output channel, but only locally

        NotificationChannel out = mNotificationManager.getNotificationChannel("id");

        out.setName("modified");



        // This should not change the result of getNotificationChannel

        assertThat(mNotificationManager.getNotificationChannel("id")).isEqualTo(original);

        assertThat(mNotificationManager.getNotificationChannel("id")).isNotEqualTo(out);



        // and also check the conversation channel

        NotificationChannel outConv = mNotificationManager.getNotificationChannel("id",

                "id_conversation");

        outConv.setName("conversation_modified");

        assertThat(mNotificationManager.getNotificationChannel("id", "id_conversation")).isEqualTo(

                originalConv);

        assertThat(

                mNotificationManager.getNotificationChannel("id", "id_conversation")).isNotEqualTo(

                outConv);



        // nonexistent conversation returns the (not modified) parent channel

        assertThat(mNotificationManager.getNotificationChannel("id", "nonexistent")).isEqualTo(

                original);

        assertThat(mNotificationManager.getNotificationChannel("id", "nonexistent")).isNotEqualTo(

                out);

    }



    @Test

    @EnableFlags(Flags.FLAG_NM_BINDER_PERF_CACHE_CHANNELS)

    public void getNotificationChannelGroup_cachedUntilInvalidated() throws Exception {
@@ -520,6 +558,37 @@ public class NotificationManagerTest { 
        assertThat(result).containsExactly(expectedG1, expectedG2);

    }



    @Test

    @EnableFlags(Flags.FLAG_NM_BINDER_PERF_CACHE_CHANNELS)

    public void getNotificationChannelGroup_localModificationDoesNotChangeCache() throws Exception {

        // Group setup

        NotificationChannelGroup g1 = new NotificationChannelGroup("g1", "group one");

        NotificationChannel nc1 = new NotificationChannel("nc1", "channel one",

                NotificationManager.IMPORTANCE_DEFAULT);

        nc1.setGroup("g1");

        NotificationChannel nc2 = new NotificationChannel("nc2", "channel two",

                NotificationManager.IMPORTANCE_DEFAULT);

        nc2.setGroup("g1");



        NotificationManager.invalidateNotificationChannelCache();

        NotificationManager.invalidateNotificationChannelGroupCache();

        when(mNotificationManager.mBackendService.getNotificationChannelGroupsWithoutChannels(

                any())).thenReturn(new ParceledListSlice<>(List.of(g1.clone())));

        when(mNotificationManager.mBackendService.getNotificationChannels(any(), any(), anyInt()))

                .thenReturn(new ParceledListSlice<>(List.of(nc1.copy(), nc2.copy())));



        NotificationChannelGroup g1result = mNotificationManager.getNotificationChannelGroup("g1");

        g1result.setDescription("something different!");

        for (NotificationChannel c : g1result.getChannels()) {

            c.setDescription("also something different");

        }



        // expected output equivalent to original, unchanged

        NotificationChannelGroup expectedG1 = g1.clone();

        expectedG1.setChannels(List.of(nc1, nc2));

        assertThat(mNotificationManager.getNotificationChannelGroup("g1")).isEqualTo(expectedG1);

    }



    @Test

    @EnableFlags(Flags.FLAG_MODES_UI)

    public void areAutomaticZenRulesUserManaged_handheld_isTrue() {