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

Commit 761c4439 authored by Matías Hernández's avatar Matías Hernández
Browse files

Fix collection of Notification allPendingIntents 

Notification.writeToParcel calls PendingIntent.setOnMarshaledListener() in order to "collect" all PendingIntents included anywhere inside it while serializing. This means that if _another notification_ is also serialized as part of this process, then the listener is overwritten -- and any fields written to the parcel after it will not participate in this collection. Notifications can contain other notifications (most obviously as their publicVersion, but also potentially inside any Bundle such as extras).

There are two approaches to fixing this:
1) Make setOnMarshaledListener() *add* a listener instead of replacing it (renaming to add/remove). This will collect every PendingIntent wherever it is located, including recursively in other Notification objects.
2) Keep listeners exclusive but have writeToParcel() restore the previous one when exiting. This will not include other Notification objects, but also won't let those "break" the collection of their container.

This CL goes with #1 because it seems the original intent of this code, collecting every PendingIntent (instead of an explicit list like contentIntent, deleteIntent, actions, etc). It also now collects the publicVersion's intents which is a bugfix. It will continue to collect PendingIntents present in whatever apps decide to stuff into extras -- but now including Notifications put in there too.

Tests are added for the fixed behavior and to verify the allowlist calls from NotificationManagerService as well.

Fixes: 282229007
Fixes: 289198162
Test: atest NotificationTest NotificationManagerServiceTest
Change-Id: I836685c6aebe30276945e33a3dfdccd236fe89ec
parent f0958868

core/java/android/app/Notification.java

+7 −7
Original line number Diff line number Diff line
@@ -3038,10 +3038,9 @@ public class Notification implements Parcelable 
        // cannot look into the extras as there may be parcelables there that

        // the platform does not know how to handle. To go around that we have

        // an explicit list of the pending intents in the extras bundle.

        final boolean collectPendingIntents = (allPendingIntents == null);

        if (collectPendingIntents) {

            PendingIntent.setOnMarshaledListener(

                    (PendingIntent intent, Parcel out, int outFlags) -> {

        PendingIntent.OnMarshaledListener addedListener = null;

        if (allPendingIntents == null) {

            addedListener = (PendingIntent intent, Parcel out, int outFlags) -> {

                if (parcel == out) {

                    synchronized (this) {

                        if (allPendingIntents == null) {
@@ -3050,7 +3049,8 @@ public class Notification implements Parcelable 
                        allPendingIntents.add(intent);

                    }

                }

            });

            };

            PendingIntent.addOnMarshaledListener(addedListener);

        }

        try {

            // IMPORTANT: Add marshaling code in writeToParcelImpl as we
@@ -3061,8 +3061,8 @@ public class Notification implements Parcelable 
                parcel.writeArraySet(allPendingIntents);

            }

        } finally {

            if (collectPendingIntents) {

                PendingIntent.setOnMarshaledListener(null);

            if (addedListener != null) {

                PendingIntent.removeOnMarshaledListener(addedListener);

            }

        }

    }

core/java/android/app/PendingIntent.java

+34 −11
Original line number Diff line number Diff line
@@ -64,6 +64,7 @@ import com.android.internal.os.IResultReceiver; 


import java.lang.annotation.Retention;

import java.lang.annotation.RetentionPolicy;

import java.util.ArrayList;

import java.util.Collections;

import java.util.List;

import java.util.Objects;
@@ -391,11 +392,12 @@ public final class PendingIntent implements Parcelable { 
        void onMarshaled(PendingIntent intent, Parcel parcel, int flags);

    }



    private static final ThreadLocal<OnMarshaledListener> sOnMarshaledListener

            = new ThreadLocal<>();

    private static final ThreadLocal<List<OnMarshaledListener>> sOnMarshaledListener =

            ThreadLocal.withInitial(ArrayList::new);



    /**

     * Registers an listener for pending intents being written to a parcel.

     * Registers an listener for pending intents being written to a parcel. This replaces any

     * listeners previously added.

     *

     * @param listener The listener, null to clear.

     *
@@ -403,7 +405,27 @@ public final class PendingIntent implements Parcelable { 
     */

    @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)

    public static void setOnMarshaledListener(OnMarshaledListener listener) {

        sOnMarshaledListener.set(listener);

        final List<OnMarshaledListener> listeners = sOnMarshaledListener.get();

        listeners.clear();

        if (listener != null) {

            listeners.add(listener);

        }

    }



    /**

     * Adds a listener for pending intents being written to a parcel.

     * @hide

     */

    static void addOnMarshaledListener(OnMarshaledListener listener) {

        sOnMarshaledListener.get().add(listener);

    }



    /**

     * Removes a listener for pending intents being written to a parcel.

     * @hide

     */

    static void removeOnMarshaledListener(OnMarshaledListener listener) {

        sOnMarshaledListener.get().remove(listener);

    }



    private static void checkPendingIntent(int flags, @NonNull Intent intent,
@@ -1451,11 +1473,11 @@ public final class PendingIntent implements Parcelable { 


    public void writeToParcel(Parcel out, int flags) {

        out.writeStrongBinder(mTarget.asBinder());

        OnMarshaledListener listener = sOnMarshaledListener.get();

        if (listener != null) {

            listener.onMarshaled(this, out, flags);

        final List<OnMarshaledListener> listeners = sOnMarshaledListener.get();

        final int numListeners = listeners.size();

        for (int i = 0; i < numListeners; i++) {

            listeners.get(i).onMarshaled(this, out, flags);

        }



    }



    public static final @NonNull Creator<PendingIntent> CREATOR = new Creator<PendingIntent>() {
@@ -1483,9 +1505,10 @@ public final class PendingIntent implements Parcelable { 
            @NonNull Parcel out) {

        out.writeStrongBinder(sender != null ? sender.mTarget.asBinder() : null);

        if (sender != null) {

            OnMarshaledListener listener = sOnMarshaledListener.get();

            if (listener != null) {

                listener.onMarshaled(sender, out, 0 /* flags */);

            final List<OnMarshaledListener> listeners = sOnMarshaledListener.get();

            final int numListeners = listeners.size();

            for (int i = 0; i < numListeners; i++) {

                listeners.get(i).onMarshaled(sender, out, 0 /* flags */);

            }

        }

    }

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

+48 −0
Original line number Diff line number Diff line
@@ -270,6 +270,54 @@ public class NotificationTest { 
        assertTrue(n.allPendingIntents.contains(intent));

    }



    @Test

    public void allPendingIntents_resilientToAnotherNotificationInExtras() {

        PendingIntent contentIntent = createPendingIntent("content");

        PendingIntent actionIntent = createPendingIntent("action");

        Notification another = new Notification.Builder(mContext, "channel").build();

        Bundle bundleContainingAnotherNotification = new Bundle();

        bundleContainingAnotherNotification.putParcelable(null, another);

        Notification source = new Notification.Builder(mContext, "channel")

                .setContentIntent(contentIntent)

                .addAction(new Notification.Action.Builder(null, "action", actionIntent).build())

                .setExtras(bundleContainingAnotherNotification)

                .build();



        Parcel p = Parcel.obtain();

        source.writeToParcel(p, 0);

        p.setDataPosition(0);

        Notification unparceled = new Notification(p);



        assertThat(unparceled.allPendingIntents).containsExactly(contentIntent, actionIntent);

    }



    @Test

    public void allPendingIntents_alsoInPublicVersion() {

        PendingIntent contentIntent = createPendingIntent("content");

        PendingIntent actionIntent = createPendingIntent("action");

        PendingIntent publicContentIntent = createPendingIntent("publicContent");

        PendingIntent publicActionIntent = createPendingIntent("publicAction");

        Notification source = new Notification.Builder(mContext, "channel")

                .setContentIntent(contentIntent)

                .addAction(new Notification.Action.Builder(null, "action", actionIntent).build())

                .setPublicVersion(new Notification.Builder(mContext, "channel")

                        .setContentIntent(publicContentIntent)

                        .addAction(new Notification.Action.Builder(

                                null, "publicAction", publicActionIntent).build())

                        .build())

                .build();



        Parcel p = Parcel.obtain();

        source.writeToParcel(p, 0);

        p.setDataPosition(0);

        Notification unparceled = new Notification(p);



        assertThat(unparceled.allPendingIntents).containsExactly(contentIntent, actionIntent,

                publicContentIntent, publicActionIntent);

        assertThat(unparceled.publicVersion.allPendingIntents).containsExactly(publicContentIntent,

                publicActionIntent);

    }



    @Test

    public void messagingStyle_isGroupConversation() {

        mContext.getApplicationInfo().targetSdkVersion = Build.VERSION_CODES.P;

services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java

+71 −0
Original line number Diff line number Diff line
@@ -62,6 +62,8 @@ import static android.content.pm.PackageManager.PERMISSION_GRANTED; 
import static android.os.Build.VERSION_CODES.O_MR1;

import static android.os.Build.VERSION_CODES.P;

import static android.os.PowerManager.PARTIAL_WAKE_LOCK;

import static android.os.PowerWhitelistManager.REASON_NOTIFICATION_SERVICE;

import static android.os.PowerWhitelistManager.TEMPORARY_ALLOWLIST_TYPE_FOREGROUND_SERVICE_ALLOWED;

import static android.os.UserHandle.USER_SYSTEM;

import static android.os.UserManager.USER_TYPE_FULL_SECONDARY;

import static android.os.UserManager.USER_TYPE_PROFILE_CLONE;
@@ -83,6 +85,9 @@ import static com.android.internal.config.sysui.SystemUiSystemPropertiesFlags.No 
import static com.android.internal.config.sysui.SystemUiSystemPropertiesFlags.NotificationFlags.SHOW_STICKY_HUN_FOR_DENIED_FSI;

import static com.android.internal.config.sysui.SystemUiSystemPropertiesFlags.NotificationFlags.WAKE_LOCK_FOR_POSTING_NOTIFICATION;

import static com.android.internal.widget.LockPatternUtils.StrongAuthTracker.STRONG_AUTH_REQUIRED_AFTER_USER_LOCKDOWN;

import static com.android.server.am.PendingIntentRecord.FLAG_ACTIVITY_SENDER;

import static com.android.server.am.PendingIntentRecord.FLAG_BROADCAST_SENDER;

import static com.android.server.am.PendingIntentRecord.FLAG_SERVICE_SENDER;

import static com.android.server.notification.NotificationManagerService.DEFAULT_MAX_NOTIFICATION_ENQUEUE_RATE;

import static com.android.server.notification.NotificationRecordLogger.NotificationReportedEvent.NOTIFICATION_ADJUSTED;

import static com.android.server.notification.NotificationRecordLogger.NotificationReportedEvent.NOTIFICATION_POSTED;
@@ -186,6 +191,7 @@ import android.os.Bundle; 
import android.os.IBinder;

import android.os.Looper;

import android.os.Parcel;

import android.os.Parcelable;

import android.os.PowerManager;

import android.os.PowerManager.WakeLock;

import android.os.Process;
@@ -716,6 +722,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { 














    @After















    public void assertAllWakeLocksReleased() {













        waitForIdle(); // Finish async work.















        for (WakeLock wakeLock : mAcquiredWakeLocks) {















            assertThat(wakeLock.isHeld()).isFalse();















        }










@@ -12023,6 +12030,70 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {













        assertThat(mService.mNotificationList.get(0).getNotification().when).isEqualTo(111); // old















    }



























    @Test













    public void enqueueNotification_allowlistsPendingIntents() throws RemoteException {













        PendingIntent contentIntent = createPendingIntent("content");













        PendingIntent actionIntent1 = createPendingIntent("action1");













        PendingIntent actionIntent2 = createPendingIntent("action2");













        Notification n = new Notification.Builder(mContext, TEST_CHANNEL_ID)













                .setContentIntent(contentIntent)













                .addAction(new Notification.Action.Builder(null, "action1", actionIntent1).build())













                .addAction(new Notification.Action.Builder(null, "action2", actionIntent2).build())













                .build();

























        mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", 1,













                parcelAndUnparcel(n, Notification.CREATOR), mUserId);

























        verify(mAmi, times(3)).setPendingIntentAllowlistDuration(













                any(), any(), anyLong(),













                eq(TEMPORARY_ALLOWLIST_TYPE_FOREGROUND_SERVICE_ALLOWED),













                eq(REASON_NOTIFICATION_SERVICE), any());













        verify(mAmi, times(3)).setPendingIntentAllowBgActivityStarts(any(),













                any(), eq(FLAG_ACTIVITY_SENDER | FLAG_BROADCAST_SENDER | FLAG_SERVICE_SENDER));













    }

























    @Test













    public void enqueueNotification_allowlistsPendingIntents_includingFromPublicVersion()













            throws RemoteException {













        PendingIntent contentIntent = createPendingIntent("content");













        PendingIntent actionIntent = createPendingIntent("action");













        PendingIntent publicContentIntent = createPendingIntent("publicContent");













        PendingIntent publicActionIntent = createPendingIntent("publicAction");













        Notification source = new Notification.Builder(mContext, TEST_CHANNEL_ID)













                .setContentIntent(contentIntent)













                .addAction(new Notification.Action.Builder(null, "action", actionIntent).build())













                .setPublicVersion(new Notification.Builder(mContext, "channel")













                        .setContentIntent(publicContentIntent)













                        .addAction(new Notification.Action.Builder(













                                null, "publicAction", publicActionIntent).build())













                        .build())













                .build();

























        mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", 1,













                parcelAndUnparcel(source, Notification.CREATOR), mUserId);

























        verify(mAmi, times(4)).setPendingIntentAllowlistDuration(













                any(), any(), anyLong(),













                eq(TEMPORARY_ALLOWLIST_TYPE_FOREGROUND_SERVICE_ALLOWED),













                eq(REASON_NOTIFICATION_SERVICE), any());













        verify(mAmi, times(4)).setPendingIntentAllowBgActivityStarts(any(),













                any(), eq(FLAG_ACTIVITY_SENDER | FLAG_BROADCAST_SENDER | FLAG_SERVICE_SENDER));













    }

























    private static <T extends Parcelable> T parcelAndUnparcel(T source,













            Parcelable.Creator<T> creator) {













        Parcel parcel = Parcel.obtain();













        source.writeToParcel(parcel, 0);













        parcel.setDataPosition(0);













        return creator.createFromParcel(parcel);













    }

























    private PendingIntent createPendingIntent(String action) {













        return PendingIntent.getActivity(mContext, 0,













                new Intent(action).setPackage(mContext.getPackageName()),













                PendingIntent.FLAG_MUTABLE);













    }



























    private void setDpmAppOppsExemptFromDismissal(boolean isOn) {















        DeviceConfig.setProperty(















                DeviceConfig.NAMESPACE_DEVICE_POLICY_MANAGER,