Merge "Fix backward compatibility issue for removed wildcard match rule" am: dbdb1d972c

Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/2424491

Change-Id: I3bb79b9bb89e227ad12b2626c3fcecf15c349f0b
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

framework-t/src/android/net/NetworkTemplate.java

@@ -48,6 +48,7 @@ import android.os.Build;
 import android.os.Parcel;
 import android.os.Parcelable;
 import android.util.ArraySet;
+import android.util.Log;
 
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.net.module.util.CollectionUtils;
@@ -70,6 +71,8 @@ import java.util.TreeSet;
  */
 @SystemApi(client = MODULE_LIBRARIES)
 public final class NetworkTemplate implements Parcelable {
+    private static final String TAG = NetworkTemplate.class.getSimpleName();
+
     /** @hide */
     @Retention(RetentionPolicy.SOURCE)
     @IntDef(prefix = { "MATCH_" }, value = {
@@ -270,12 +273,17 @@ public final class NetworkTemplate implements Parcelable {
 
     private static void checkValidMatchSubscriberIds(int matchRule, String[] matchSubscriberIds) {
         switch (matchRule) {
-            case MATCH_CARRIER:
             // CARRIER templates must always specify a valid subscriber ID.
+            // MOBILE templates can have empty matchSubscriberIds but it must not contain a null
+            // subscriber ID.
+            case MATCH_CARRIER:
                 if (matchSubscriberIds.length == 0) {
                     throw new IllegalArgumentException("checkValidMatchSubscriberIds with empty"
                             + " list of ids for rule" + getMatchRuleName(matchRule));
-                } else if (CollectionUtils.contains(matchSubscriberIds, null)) {
+                }
+                // fall through
+            case MATCH_MOBILE:
+                if (CollectionUtils.contains(matchSubscriberIds, null)) {
                     throw new IllegalArgumentException("checkValidMatchSubscriberIds list of ids"
                             + " may not contain null for rule " + getMatchRuleName(matchRule));
                 }
@@ -296,12 +304,36 @@ public final class NetworkTemplate implements Parcelable {
         // Older versions used to only match MATCH_MOBILE and MATCH_MOBILE_WILDCARD templates
         // to metered networks. It is now possible to match mobile with any meteredness, but
         // in order to preserve backward compatibility of @UnsupportedAppUsage methods, this
-        //constructor passes METERED_YES for these types.
+        // constructor passes METERED_YES for these types.
-        this(matchRule, new String[] { subscriberId },
+        // For backwards compatibility, still accept old wildcard match rules (6 and 7 for
+        // MATCH_{MOBILE,WIFI}_WILDCARD) but convert into functionally equivalent non-wildcard
+        // ones.
+        this(getBackwardsCompatibleMatchRule(matchRule),
+                subscriberId != null ? new String[] { subscriberId } : new String[0],
                 wifiNetworkKey != null ? new String[] { wifiNetworkKey } : new String[0],
-                (matchRule == MATCH_MOBILE || matchRule == MATCH_CARRIER)
+                getMeterednessForBackwardsCompatibility(matchRule), ROAMING_ALL,
-                        ? METERED_YES : METERED_ALL, ROAMING_ALL, DEFAULT_NETWORK_ALL,
+                DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, OEM_MANAGED_ALL);
-                NETWORK_TYPE_ALL, OEM_MANAGED_ALL);
+        if (matchRule == 6 || matchRule == 7) {
+            Log.e(TAG, "Use MATCH_MOBILE with empty subscriberIds or MATCH_WIFI with empty "
+                    + "wifiNetworkKeys instead of template with matchRule=" + matchRule);
+        }
+    }
+
+    private static int getBackwardsCompatibleMatchRule(int matchRule) {
+        // Backwards compatibility old constants
+        // Old MATCH_MOBILE_WILDCARD
+        if (6 == matchRule) return MATCH_MOBILE;
+        // Old MATCH_WIFI_WILDCARD
+        if (7 == matchRule) return MATCH_WIFI;
+        return matchRule;
+    }
+
+    private static int getMeterednessForBackwardsCompatibility(int matchRule) {
+        if (getBackwardsCompatibleMatchRule(matchRule) == MATCH_MOBILE
+                || matchRule == MATCH_CARRIER) {
+            return METERED_YES;
+        }
+        return METERED_ALL;
     }
 
     /** @hide */

tests/common/java/android/net/netstats/NetworkTemplateTest.kt

@@ -95,6 +95,13 @@ class NetworkTemplateTest {
             NetworkTemplate.Builder(MATCH_CARRIER).build()
         }
 
+        // Verify carrier and mobile template cannot contain one of subscriber Id is null.
+        listOf(MATCH_MOBILE, MATCH_CARRIER).forEach {
+            assertFailsWith<IllegalArgumentException> {
+                NetworkTemplate.Builder(it).setSubscriberIds(setOf(null)).build()
+            }
+        }
+
         // Verify template which matches metered cellular networks,
         // regardless of IMSI. See buildTemplateMobileWildcard.
         NetworkTemplate.Builder(MATCH_MOBILE).setMeteredness(METERED_YES).build().let {
@@ -157,6 +164,23 @@ class NetworkTemplateTest {
         }
     }
 
+    @Test
+    fun testUnsupportedAppUsageConstructor() {
+        val templateMobile = NetworkTemplate(MATCH_MOBILE, null /* subscriberId */,
+                null /* wifiNetworkKey */)
+        val templateMobileWildcard = NetworkTemplate(6 /* MATCH_MOBILE_WILDCARD */,
+                null /* subscriberId */, null /* wifiNetworkKey */)
+        val templateWifiWildcard = NetworkTemplate(7 /* MATCH_WIFI_WILDCARD */,
+                null /* subscriberId */,
+                null /* wifiNetworkKey */)
+
+        assertEquals(NetworkTemplate.Builder(MATCH_MOBILE).setMeteredness(METERED_YES).build(),
+                templateMobile)
+        assertEquals(NetworkTemplate.Builder(MATCH_MOBILE).setMeteredness(METERED_YES).build(),
+                templateMobileWildcard)
+        assertEquals(NetworkTemplate.Builder(MATCH_WIFI).build(), templateWifiWildcard)
+    }
+
     @Test
     fun testBuilderWifiNetworkKeys() {
         // Verify template builder which generates same template with the given different