Don't throw if the matching key contains ":".

If a client adds custom crafted strings as part of matching key,
it is possible for this restriction to go unnoticed. Instead,
just all allow characters in the key. This means that we cannot
solely rely on getDeliveryGroupMatchingKey() for matching but
don't think this would be a problem for clients of this API
outside the System.

Bug: 275312433
Test: atest ./tests/app/src/android/app/cts/BroadcastOptionsTest.java
Test: atest ./services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java
Test: atest ./services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java
Test: atest ./services/tests/mockingservicestests/src/com/android/server/am/BroadcastRecordTest.java
Change-Id: I76e307800017d4dd1e66233b1ff4cc5cbcef7246

2 files changed, 85 insertions, 20 deletions

diff --git a/core/java/android/app/BroadcastOptions.java b/core/java/android/app/BroadcastOptions.java
index d23d3cd87fdb..6357798a6cab 100644
--- a/core/java/android/app/BroadcastOptions.java
+++ b/core/java/android/app/BroadcastOptions.java
@@ -37,8 +37,6 @@ import android.os.PowerExemptionManager;
 import android.os.PowerExemptionManager.ReasonCode;
 import android.os.PowerExemptionManager.TempAllowListType;
 
-import com.android.internal.util.Preconditions;
-
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.util.Objects;
@@ -61,7 +59,8 @@ public class BroadcastOptions extends ComponentOptions {
     private long mRequireCompatChangeId = CHANGE_INVALID;
     private long mIdForResponseEvent;
     private @DeliveryGroupPolicy int mDeliveryGroupPolicy;
-    private @Nullable String mDeliveryGroupMatchingKey;
+    private @Nullable String mDeliveryGroupMatchingNamespaceFragment;
+    private @Nullable String mDeliveryGroupMatchingKeyFragment;
     private @Nullable BundleMerger mDeliveryGroupExtrasMerger;
     private @Nullable IntentFilter mDeliveryGroupMatchingFilter;
     private @DeferralPolicy int mDeferralPolicy;
@@ -196,7 +195,13 @@ public class BroadcastOptions extends ComponentOptions {
             "android:broadcast.deliveryGroupPolicy";
 
     /**
-     * Corresponds to {@link #setDeliveryGroupMatchingKey(String, String)}.
+     * Corresponds to namespace fragment of {@link #setDeliveryGroupMatchingKey(String, String)}.
+     */
+    private static final String KEY_DELIVERY_GROUP_NAMESPACE =
+            "android:broadcast.deliveryGroupMatchingNamespace";
+
+    /**
+     * Corresponds to key fragment of {@link #setDeliveryGroupMatchingKey(String, String)}.
      */
     private static final String KEY_DELIVERY_GROUP_KEY =
             "android:broadcast.deliveryGroupMatchingKey";
@@ -337,7 +342,8 @@ public class BroadcastOptions extends ComponentOptions {
         mIdForResponseEvent = opts.getLong(KEY_ID_FOR_RESPONSE_EVENT);
         mDeliveryGroupPolicy = opts.getInt(KEY_DELIVERY_GROUP_POLICY,
                 DELIVERY_GROUP_POLICY_ALL);
-        mDeliveryGroupMatchingKey = opts.getString(KEY_DELIVERY_GROUP_KEY);
+        mDeliveryGroupMatchingNamespaceFragment = opts.getString(KEY_DELIVERY_GROUP_NAMESPACE);
+        mDeliveryGroupMatchingKeyFragment = opts.getString(KEY_DELIVERY_GROUP_KEY);
         mDeliveryGroupExtrasMerger = opts.getParcelable(KEY_DELIVERY_GROUP_EXTRAS_MERGER,
                 BundleMerger.class);
         mDeliveryGroupMatchingFilter = opts.getParcelable(KEY_DELIVERY_GROUP_MATCHING_FILTER,
@@ -851,11 +857,8 @@ public class BroadcastOptions extends ComponentOptions {
     @NonNull
     public BroadcastOptions setDeliveryGroupMatchingKey(@NonNull String namespace,
             @NonNull String key) {
-        Preconditions.checkArgument(!namespace.contains(":"),
-                "namespace should not contain ':'");
-        Preconditions.checkArgument(!key.contains(":"),
-                "key should not contain ':'");
-        mDeliveryGroupMatchingKey = namespace + ":" + key;
+        mDeliveryGroupMatchingNamespaceFragment = Objects.requireNonNull(namespace);
+        mDeliveryGroupMatchingKeyFragment = Objects.requireNonNull(key);
         return this;
     }
 
@@ -868,7 +871,38 @@ public class BroadcastOptions extends ComponentOptions {
      */
     @Nullable
     public String getDeliveryGroupMatchingKey() {
-        return mDeliveryGroupMatchingKey;
+        if (mDeliveryGroupMatchingNamespaceFragment == null
+                || mDeliveryGroupMatchingKeyFragment == null) {
+            return null;
+        }
+        return String.join(":", mDeliveryGroupMatchingNamespaceFragment,
+                mDeliveryGroupMatchingKeyFragment);
+    }
+
+    /**
+     * Return the namespace fragment that is used to identify the delivery group that this
+     * broadcast belongs to.
+     *
+     * @return the delivery group namespace fragment that was previously set using
+     *         {@link #setDeliveryGroupMatchingKey(String, String)}.
+     * @hide
+     */
+    @Nullable
+    public String getDeliveryGroupMatchingNamespaceFragment() {
+        return mDeliveryGroupMatchingNamespaceFragment;
+    }
+
+    /**
+     * Return the key fragment that is used to identify the delivery group that this
+     * broadcast belongs to.
+     *
+     * @return the delivery group key fragment that was previously set using
+     *         {@link #setDeliveryGroupMatchingKey(String, String)}.
+     * @hide
+     */
+    @Nullable
+    public String getDeliveryGroupMatchingKeyFragment() {
+        return mDeliveryGroupMatchingKeyFragment;
     }
 
     /**
@@ -876,7 +910,8 @@ public class BroadcastOptions extends ComponentOptions {
      * {@link #setDeliveryGroupMatchingKey(String, String)}.
      */
     public void clearDeliveryGroupMatchingKey() {
-        mDeliveryGroupMatchingKey = null;
+        mDeliveryGroupMatchingNamespaceFragment = null;
+        mDeliveryGroupMatchingKeyFragment = null;
     }
 
     /**
@@ -1094,8 +1129,11 @@ public class BroadcastOptions extends ComponentOptions {
         if (mDeliveryGroupPolicy != DELIVERY_GROUP_POLICY_ALL) {
             b.putInt(KEY_DELIVERY_GROUP_POLICY, mDeliveryGroupPolicy);
         }
-        if (mDeliveryGroupMatchingKey != null) {
-            b.putString(KEY_DELIVERY_GROUP_KEY, mDeliveryGroupMatchingKey);
+        if (mDeliveryGroupMatchingNamespaceFragment != null) {
+            b.putString(KEY_DELIVERY_GROUP_NAMESPACE, mDeliveryGroupMatchingNamespaceFragment);
+        }
+        if (mDeliveryGroupMatchingKeyFragment != null) {
+            b.putString(KEY_DELIVERY_GROUP_KEY, mDeliveryGroupMatchingKeyFragment);
         }
         if (mDeliveryGroupPolicy == DELIVERY_GROUP_POLICY_MERGED) {
             if (mDeliveryGroupExtrasMerger != null) {
diff --git a/services/core/java/com/android/server/am/BroadcastRecord.java b/services/core/java/com/android/server/am/BroadcastRecord.java
index 6bd3c7953e01..e6ef3b468d2d 100644
--- a/services/core/java/com/android/server/am/BroadcastRecord.java
+++ b/services/core/java/com/android/server/am/BroadcastRecord.java
@@ -999,23 +999,50 @@ final class BroadcastRecord extends Binder {
 
     private static boolean matchesDeliveryGroup(@NonNull BroadcastRecord newRecord,
             @NonNull BroadcastRecord oldRecord) {
-        final String newMatchingKey = getDeliveryGroupMatchingKey(newRecord);
-        final String oldMatchingKey = getDeliveryGroupMatchingKey(oldRecord);
         final IntentFilter newMatchingFilter = getDeliveryGroupMatchingFilter(newRecord);
         // If neither delivery group key nor matching filter is specified, then use
         // Intent.filterEquals() to identify the delivery group.
-        if (newMatchingKey == null && oldMatchingKey == null && newMatchingFilter == null) {
+        if (isMatchingKeyNull(newRecord) && isMatchingKeyNull(oldRecord)
+                && newMatchingFilter == null) {
             return newRecord.intent.filterEquals(oldRecord.intent);
         }
         if (newMatchingFilter != null && !newMatchingFilter.asPredicate().test(oldRecord.intent)) {
             return false;
         }
-        return Objects.equals(newMatchingKey, oldMatchingKey);
+        return areMatchingKeysEqual(newRecord, oldRecord);
+    }
+
+    private static boolean isMatchingKeyNull(@NonNull BroadcastRecord record) {
+        final String namespace = getDeliveryGroupMatchingNamespaceFragment(record);
+        final String key = getDeliveryGroupMatchingKeyFragment(record);
+        // If either namespace or key part is null, then treat the entire matching key as null.
+        return namespace == null || key == null;
+    }
+
+    private static boolean areMatchingKeysEqual(@NonNull BroadcastRecord newRecord,
+            @NonNull BroadcastRecord oldRecord) {
+        final String newNamespaceFragment = getDeliveryGroupMatchingNamespaceFragment(newRecord);
+        final String oldNamespaceFragment = getDeliveryGroupMatchingNamespaceFragment(oldRecord);
+        if (!Objects.equals(newNamespaceFragment, oldNamespaceFragment)) {
+            return false;
+        }
+
+        final String newKeyFragment = getDeliveryGroupMatchingKeyFragment(newRecord);
+        final String oldKeyFragment = getDeliveryGroupMatchingKeyFragment(oldRecord);
+        return Objects.equals(newKeyFragment, oldKeyFragment);
+    }
+
+    @Nullable
+    private static String getDeliveryGroupMatchingNamespaceFragment(
+            @NonNull BroadcastRecord record) {
+        return record.options == null
+                ? null : record.options.getDeliveryGroupMatchingNamespaceFragment();
     }
 
     @Nullable
-    private static String getDeliveryGroupMatchingKey(@NonNull BroadcastRecord record) {
-        return record.options == null ? null : record.options.getDeliveryGroupMatchingKey();
+    private static String getDeliveryGroupMatchingKeyFragment(@NonNull BroadcastRecord record) {
+        return record.options == null
+                ? null : record.options.getDeliveryGroupMatchingKeyFragment();
     }
 
     @Nullable