commit 8128beb12a6dea98700321a716d7e64105df6536
author Matías Hernández <matiashe@google.com> Wed Feb 22 16:51:30 2023 +0000
committer Android (Google) Code Review <android-gerrit@google.com> Wed Feb 22 16:51:30 2023 +0000
tree 3bca3e941e512e7533745c9435dec282abad81e0
parent 5b826d695d37ff88b13e2c85c84f7d407e4baf27
parent f13406f3a0e8e8260778e091ab1f6075d8af84b2

Merge "Disallow setting invalid importance when updating a NotificationChannel" into udc-dev

services/core/java/com/android/server/notification/PreferencesHelper.java

diff --git a/services/core/java/com/android/server/notification/PreferencesHelper.java b/services/core/java/com/android/server/notification/PreferencesHelper.java
index 69ea559..c63cddd 100644
--- a/services/core/java/com/android/server/notification/PreferencesHelper.java
+++ b/services/core/java/com/android/server/notification/PreferencesHelper.java
@@ -22,6 +22,7 @@
 import static android.app.NotificationManager.BUBBLE_PREFERENCE_ALL;
 import static android.app.NotificationManager.BUBBLE_PREFERENCE_NONE;
 import static android.app.NotificationManager.IMPORTANCE_DEFAULT;
+import static android.app.NotificationManager.IMPORTANCE_MAX;
 import static android.app.NotificationManager.IMPORTANCE_NONE;
 import static android.app.NotificationManager.IMPORTANCE_UNSPECIFIED;
 import static android.util.StatsLog.ANNOTATION_ID_IS_UID;
@@ -905,6 +906,9 @@
         Objects.requireNonNull(channel);
         Objects.requireNonNull(channel.getId());
         Preconditions.checkArgument(!TextUtils.isEmpty(channel.getName()));
+        Preconditions.checkArgument(channel.getImportance() >= IMPORTANCE_NONE
+                && channel.getImportance() <= IMPORTANCE_MAX, "Invalid importance level");
+
         boolean needsPolicyFileChange = false, wasUndeleted = false, needsDndChange = false;
         synchronized (mPackagePreferences) {
             PackagePreferences r = getOrCreatePackagePreferencesLocked(pkg, uid);
@@ -993,11 +997,6 @@
 
                 needsPolicyFileChange = true;
 
-                if (channel.getImportance() < IMPORTANCE_NONE
-                        || channel.getImportance() > NotificationManager.IMPORTANCE_MAX) {
-                    throw new IllegalArgumentException("Invalid importance level");
-                }
-
                 // Reset fields that apps aren't allowed to set.
                 if (fromTargetApp && !hasDndAccess) {
                     channel.setBypassDnd(r.priority == Notification.PRIORITY_MAX);

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

diff --git a/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java
index f0f9204..1ecd4a1 100644
--- a/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java
@@ -57,6 +57,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
@@ -1576,6 +1577,47 @@
                 new NotificationChannel("bananas", "bananas", IMPORTANCE_MAX), true, false));
     }
 
+    @Test
+    public void testUpdateChannel_downgradeImportance() {
+        mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1,
+                new NotificationChannel("bananas", "bananas", IMPORTANCE_DEFAULT),
+                true, false);
+
+        assertTrue(mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1,
+                new NotificationChannel("bananas", "bananas", IMPORTANCE_LOW), true, false));
+    }
+
+    @Test
+    public void testUpdateChannel_upgradeImportance_ignored() {
+        mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1,
+                new NotificationChannel("bananas", "bananas", IMPORTANCE_DEFAULT),
+                true, false);
+
+        assertFalse(mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1,
+                new NotificationChannel("bananas", "bananas", IMPORTANCE_MAX), true, false));
+    }
+
+    @Test
+    public void testUpdateChannel_badImportance() {
+        mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1,
+                new NotificationChannel("bananas", "bananas", IMPORTANCE_DEFAULT),
+                true, false);
+
+        assertThrows(IllegalArgumentException.class,
+                () -> mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1,
+                        new NotificationChannel("bananas", "bananas", IMPORTANCE_NONE - 1), true,
+                        false));
+
+        assertThrows(IllegalArgumentException.class,
+                () -> mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1,
+                        new NotificationChannel("bananas", "bananas", IMPORTANCE_UNSPECIFIED), true,
+                        false));
+
+        assertThrows(IllegalArgumentException.class,
+                () -> mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1,
+                        new NotificationChannel("bananas", "bananas", IMPORTANCE_MAX + 1), true,
+                        false));
+    }
 
     @Test
     public void testUpdate() throws Exception {