From fe5f1e99e31c773ae8f97d88763bf205e34a0846 Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Mon, 29 Aug 2022 11:49:45 +0800 Subject: [PATCH] Do not crash when duplication found in merged imsi list For some unknown reason, the merged imsi list might contain duplicated items. Deduplication for better error handling. Test: NetworkPolicyManagerServiceTest Bug: 243015487 Change-Id: I69d332ee0acde6cdd7582d1aa18c82576f7090f2 --- .../server/net/NetworkPolicyManagerService.java | 12 ++++++++++-- .../server/net/NetworkPolicyManagerServiceTest.java | 13 +++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java index c73c5a0260a98..e2c4cbdc829cb 100644 --- a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java +++ b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java @@ -123,6 +123,7 @@ import static android.telephony.CarrierConfigManager.KEY_DATA_RAPID_NOTIFICATION import static android.telephony.CarrierConfigManager.KEY_DATA_WARNING_NOTIFICATION_BOOL; import static android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID; +import static com.android.internal.annotations.VisibleForTesting.Visibility.PRIVATE; import static com.android.internal.util.ArrayUtils.appendInt; import static com.android.internal.util.XmlUtils.readBooleanAttribute; import static com.android.internal.util.XmlUtils.readIntAttribute; @@ -3158,7 +3159,8 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { * active merge set [A,B], we'd return a new template that primarily matches * A, but also matches B. */ - private static NetworkTemplate normalizeTemplate(@NonNull NetworkTemplate template, + @VisibleForTesting(visibility = PRIVATE) + static NetworkTemplate normalizeTemplate(@NonNull NetworkTemplate template, @NonNull List mergedList) { // Now there are several types of network which uses Subscriber Id to store network // information. For instance: @@ -3168,6 +3170,12 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { if (template.getSubscriberIds().isEmpty()) return template; for (final String[] merged : mergedList) { + // In some rare cases (e.g. b/243015487), merged subscriberId list might contain + // duplicated items. Deduplication for better error handling. + final ArraySet mergedSet = new ArraySet(merged); + if (mergedSet.size() != merged.length) { + Log.wtf(TAG, "Duplicated merged list detected: " + Arrays.toString(merged)); + } // TODO: Handle incompatible subscriberIds if that happens in practice. for (final String subscriberId : template.getSubscriberIds()) { if (com.android.net.module.util.CollectionUtils.contains(merged, subscriberId)) { @@ -3175,7 +3183,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { // a template that matches all merged subscribers. return new NetworkTemplate.Builder(template.getMatchRule()) .setWifiNetworkKeys(template.getWifiNetworkKeys()) - .setSubscriberIds(Set.of(merged)) + .setSubscriberIds(mergedSet) .setMeteredness(template.getMeteredness()) .build(); } diff --git a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java index 0f2fe4434bfa1..821ce5edad27f 100644 --- a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java @@ -75,6 +75,7 @@ import static com.android.server.net.NetworkPolicyManagerService.TYPE_LIMIT_SNOO import static com.android.server.net.NetworkPolicyManagerService.TYPE_RAPID; import static com.android.server.net.NetworkPolicyManagerService.TYPE_WARNING; import static com.android.server.net.NetworkPolicyManagerService.UidBlockedState.getEffectiveBlockedReasons; +import static com.android.server.net.NetworkPolicyManagerService.normalizeTemplate; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -2030,6 +2031,18 @@ public class NetworkPolicyManagerServiceTest { METERED_NO, actualPolicy.template.getMeteredness()); } + @Test + public void testNormalizeTemplate_duplicatedMergedImsiList() { + final NetworkTemplate template = new NetworkTemplate.Builder(MATCH_CARRIER) + .setSubscriberIds(Set.of(TEST_IMSI)).build(); + final String[] mergedImsiGroup = new String[] {TEST_IMSI, TEST_IMSI}; + final ArrayList mergedList = new ArrayList<>(); + mergedList.add(mergedImsiGroup); + // Verify the duplicated items in the merged IMSI list won't crash the system. + final NetworkTemplate result = normalizeTemplate(template, mergedList); + assertEquals(template, result); + } + private String formatBlockedStateError(int uid, int rule, boolean metered, boolean backgroundRestricted) { return String.format(