Make the NetworkSpecifier a class instead of a string.

Bug: 27533960
Bug: 36053921
Bug: 36275276
Test: connectivity, wifi, telephony unit tests
Change-Id: Idd9b10a8418c53c8cf386d9ff8252226b076bbf9

core/java/android/net/NetworkCapabilities.java

@@ -18,8 +18,9 @@ package android.net;
 
 import android.os.Parcel;
 import android.os.Parcelable;
-import android.text.TextUtils;
+import android.util.Log;
-import java.lang.IllegalArgumentException;
+
+import java.util.Objects;
 
 /**
  * This class represents the capabilities of a network.  This is used both to specify
@@ -33,6 +34,8 @@ import java.lang.IllegalArgumentException;
  * all cellular based connections are metered and all Wi-Fi based connections are not.
  */
 public final class NetworkCapabilities implements Parcelable {
+    private static final String TAG = "NetworkCapabilities";
+
     /**
      * @hide
      */
@@ -204,19 +207,6 @@ public final class NetworkCapabilities implements Parcelable {
             (1 << NET_CAPABILITY_CAPTIVE_PORTAL) |
             (1 << NET_CAPABILITY_FOREGROUND);
 
-    /**
-     * Network specifier for factories which want to match any network specifier
-     * (NS) in a request. Behavior:
-     * <li>Empty NS in request matches any network factory NS</li>
-     * <li>Empty NS in the network factory NS only matches a request with an
-     * empty NS</li>
-     * <li>"*" (this constant) NS in the network factory matches requests with
-     * any NS</li>
-     *
-     * @hide
-     */
-    public static final String MATCH_ALL_REQUESTS_NETWORK_SPECIFIER = "*";
-
     /**
      * Network capabilities that are not allowed in NetworkRequests. This exists because the
      * NetworkFactory / NetworkAgent model does not deal well with the situation where a
@@ -581,63 +571,56 @@ public final class NetworkCapabilities implements Parcelable {
                 this.mLinkDownBandwidthKbps == nc.mLinkDownBandwidthKbps);
     }
 
-    private String mNetworkSpecifier;
+    private NetworkSpecifier mNetworkSpecifier = null;
+
     /**
      * Sets the optional bearer specific network specifier.
      * This has no meaning if a single transport is also not specified, so calling
      * this without a single transport set will generate an exception, as will
      * subsequently adding or removing transports after this is set.
      * </p>
-     * The interpretation of this {@code String} is bearer specific and bearers that use
-     * it should document their particulars.  For example, Bluetooth may use some sort of
-     * device id while WiFi could used SSID and/or BSSID.  Cellular may use carrier SPN (name)
-     * or Subscription ID.
      *
-     * @param networkSpecifier An {@code String} of opaque format used to specify the bearer
+     * @param networkSpecifier A concrete, parcelable framework class that extends
-     *                         specific network specifier where the bearer has a choice of
+     *                         NetworkSpecifier.
-     *                         networks.
      * @return This NetworkCapabilities instance, to facilitate chaining.
      * @hide
      */
-    public NetworkCapabilities setNetworkSpecifier(String networkSpecifier) {
+    public NetworkCapabilities setNetworkSpecifier(NetworkSpecifier networkSpecifier) {
-        if (TextUtils.isEmpty(networkSpecifier) == false && Long.bitCount(mTransportTypes) != 1) {
+        if (networkSpecifier != null && Long.bitCount(mTransportTypes) != 1) {
             throw new IllegalStateException("Must have a single transport specified to use " +
                     "setNetworkSpecifier");
         }
+
         mNetworkSpecifier = networkSpecifier;
+
         return this;
     }
 
     /**
      * Gets the optional bearer specific network specifier.
      *
-     * @return The optional {@code String} specifying the bearer specific network specifier.
+     * @return The optional {@link NetworkSpecifier} specifying the bearer specific network
-     *         See {@link #setNetworkSpecifier}.
+     *         specifier. See {@link #setNetworkSpecifier}.
      * @hide
      */
-    public String getNetworkSpecifier() {
+    public NetworkSpecifier getNetworkSpecifier() {
         return mNetworkSpecifier;
     }
 
     private void combineSpecifiers(NetworkCapabilities nc) {
-        String otherSpecifier = nc.getNetworkSpecifier();
+        if (mNetworkSpecifier != null && !mNetworkSpecifier.equals(nc.mNetworkSpecifier)) {
-        if (TextUtils.isEmpty(otherSpecifier)) return;
-        if (TextUtils.isEmpty(mNetworkSpecifier) == false) {
             throw new IllegalStateException("Can't combine two networkSpecifiers");
         }
-        setNetworkSpecifier(otherSpecifier);
+        setNetworkSpecifier(nc.mNetworkSpecifier);
     }
+
     private boolean satisfiedBySpecifier(NetworkCapabilities nc) {
-        return (TextUtils.isEmpty(mNetworkSpecifier) ||
+        return mNetworkSpecifier == null || mNetworkSpecifier.satisfiedBy(nc.mNetworkSpecifier)
-                mNetworkSpecifier.equals(nc.mNetworkSpecifier) ||
+                || nc.mNetworkSpecifier instanceof MatchAllNetworkSpecifier;
-                MATCH_ALL_REQUESTS_NETWORK_SPECIFIER.equals(nc.mNetworkSpecifier));
     }
+
     private boolean equalsSpecifier(NetworkCapabilities nc) {
-        if (TextUtils.isEmpty(mNetworkSpecifier)) {
+        return Objects.equals(mNetworkSpecifier, nc.mNetworkSpecifier);
-            return TextUtils.isEmpty(nc.mNetworkSpecifier);
-        } else {
-            return mNetworkSpecifier.equals(nc.mNetworkSpecifier);
-        }
     }
 
     /**
@@ -799,7 +782,7 @@ public final class NetworkCapabilities implements Parcelable {
                 ((int)(mTransportTypes >> 32) * 7) +
                 (mLinkUpBandwidthKbps * 11) +
                 (mLinkDownBandwidthKbps * 13) +
-                (TextUtils.isEmpty(mNetworkSpecifier) ? 0 : mNetworkSpecifier.hashCode() * 17) +
+                Objects.hashCode(mNetworkSpecifier) * 17 +
                 (mSignalStrength * 19));
     }
 
@@ -813,7 +796,7 @@ public final class NetworkCapabilities implements Parcelable {
         dest.writeLong(mTransportTypes);
         dest.writeInt(mLinkUpBandwidthKbps);
         dest.writeInt(mLinkDownBandwidthKbps);
-        dest.writeString(mNetworkSpecifier);
+        dest.writeParcelable((Parcelable) mNetworkSpecifier, flags);
         dest.writeInt(mSignalStrength);
     }
 
@@ -827,7 +810,7 @@ public final class NetworkCapabilities implements Parcelable {
                 netCap.mTransportTypes = in.readLong();
                 netCap.mLinkUpBandwidthKbps = in.readInt();
                 netCap.mLinkDownBandwidthKbps = in.readInt();
-                netCap.mNetworkSpecifier = in.readString();
+                netCap.mNetworkSpecifier = in.readParcelable(null);
                 netCap.mSignalStrength = in.readInt();
                 return netCap;
             }

core/java/android/net/NetworkRequest.java

@@ -18,6 +18,7 @@ package android.net;
 
 import android.os.Parcel;
 import android.os.Parcelable;
+import android.text.TextUtils;
 
 import java.util.Objects;
 
@@ -259,10 +260,28 @@ public class NetworkRequest implements Parcelable {
          *                         networks.
          */
         public Builder setNetworkSpecifier(String networkSpecifier) {
-            if (NetworkCapabilities.MATCH_ALL_REQUESTS_NETWORK_SPECIFIER.equals(networkSpecifier)) {
+            /*
-                throw new IllegalArgumentException("Invalid network specifier - must not be '"
+             * A StringNetworkSpecifier does not accept null or empty ("") strings. When network
-                        + NetworkCapabilities.MATCH_ALL_REQUESTS_NETWORK_SPECIFIER + "'");
+             * specifiers were strings a null string and an empty string were considered equivalent.
-            }
+             * Hence no meaning is attached to a null or empty ("") string.
+             */
+            return setNetworkSpecifier(TextUtils.isEmpty(networkSpecifier) ? null
+                    : new StringNetworkSpecifier(networkSpecifier));
+        }
+
+        /**
+         * Sets the optional bearer specific network specifier.
+         * This has no meaning if a single transport is also not specified, so calling
+         * this without a single transport set will generate an exception, as will
+         * subsequently adding or removing transports after this is set.
+         * </p>
+         *
+         * @param networkSpecifier A concrete, parcelable framework class that extends
+         *                         NetworkSpecifier.
+         * @hide
+         */
+        public Builder setNetworkSpecifier(NetworkSpecifier networkSpecifier) {
+            MatchAllNetworkSpecifier.checkNotMatchAllNetworkSpecifier(networkSpecifier);
             mNetworkCapabilities.setNetworkSpecifier(networkSpecifier);
             return this;
         }

services/core/java/com/android/server/ConnectivityService.java

@@ -62,6 +62,7 @@ import android.net.LinkProperties.CompareResult;
 import android.net.Network;
 import android.net.NetworkAgent;
 import android.net.NetworkCapabilities;
+import android.net.MatchAllNetworkSpecifier;
 import android.net.NetworkConfig;
 import android.net.NetworkInfo;
 import android.net.NetworkInfo.DetailedState;
@@ -4128,11 +4129,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
             throw new IllegalArgumentException("Bad timeout specified");
         }
 
-        if (NetworkCapabilities.MATCH_ALL_REQUESTS_NETWORK_SPECIFIER
+        MatchAllNetworkSpecifier.checkNotMatchAllNetworkSpecifier(
-                .equals(networkCapabilities.getNetworkSpecifier())) {
+                networkCapabilities.getNetworkSpecifier());
-            throw new IllegalArgumentException("Invalid network specifier - must not be '"
-                    + NetworkCapabilities.MATCH_ALL_REQUESTS_NETWORK_SPECIFIER + "'");
-        }
 
         NetworkRequest networkRequest = new NetworkRequest(networkCapabilities, legacyType,
                 nextNetworkRequestId(), type);
@@ -4205,6 +4203,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
         enforceMeteredApnPolicy(networkCapabilities);
         ensureRequestableCapabilities(networkCapabilities);
 
+        MatchAllNetworkSpecifier.checkNotMatchAllNetworkSpecifier(
+                networkCapabilities.getNetworkSpecifier());
+
         NetworkRequest networkRequest = new NetworkRequest(networkCapabilities, TYPE_NONE,
                 nextNetworkRequestId(), NetworkRequest.Type.REQUEST);
         NetworkRequestInfo nri = new NetworkRequestInfo(networkRequest, operation);
@@ -4266,6 +4267,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
             nc.addCapability(NET_CAPABILITY_FOREGROUND);
         }
 
+        MatchAllNetworkSpecifier.checkNotMatchAllNetworkSpecifier(
+                networkCapabilities.getNetworkSpecifier());
+
         NetworkRequest networkRequest = new NetworkRequest(nc, TYPE_NONE, nextNetworkRequestId(),
                 NetworkRequest.Type.LISTEN);
         NetworkRequestInfo nri = new NetworkRequestInfo(messenger, networkRequest, binder);
@@ -4283,6 +4287,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
             enforceAccessPermission();
         }
 
+        MatchAllNetworkSpecifier.checkNotMatchAllNetworkSpecifier(
+                networkCapabilities.getNetworkSpecifier());
+
         NetworkRequest networkRequest = new NetworkRequest(
                 new NetworkCapabilities(networkCapabilities), TYPE_NONE, nextNetworkRequestId(),
                 NetworkRequest.Type.LISTEN);

tests/net/java/com/android/server/ConnectivityServiceTest.java

@@ -48,6 +48,7 @@ import android.net.INetworkStatsService;
 import android.net.IpPrefix;
 import android.net.LinkAddress;
 import android.net.LinkProperties;
+import android.net.MatchAllNetworkSpecifier;
 import android.net.Network;
 import android.net.NetworkAgent;
 import android.net.NetworkCapabilities;
@@ -57,7 +58,9 @@ import android.net.NetworkInfo;
 import android.net.NetworkInfo.DetailedState;
 import android.net.NetworkMisc;
 import android.net.NetworkRequest;
+import android.net.NetworkSpecifier;
 import android.net.RouteInfo;
+import android.net.StringNetworkSpecifier;
 import android.net.metrics.IpConnectivityLog;
 import android.net.util.MultinetworkPolicyTracker;
 import android.os.ConditionVariable;
@@ -70,12 +73,15 @@ import android.os.Message;
 import android.os.MessageQueue;
 import android.os.Messenger;
 import android.os.MessageQueue.IdleHandler;
+import android.os.Parcel;
+import android.os.Parcelable;
 import android.os.Process;
 import android.os.SystemClock;
 import android.provider.Settings;
 import android.test.AndroidTestCase;
 import android.test.mock.MockContentResolver;
 import android.test.suitebuilder.annotation.SmallTest;
+import android.text.TextUtils;
 import android.util.Log;
 import android.util.LogPrinter;
 
@@ -345,8 +351,8 @@ public class ConnectivityServiceTest extends AndroidTestCase {
             mNetworkAgent.sendNetworkCapabilities(mNetworkCapabilities);
         }
 
-        public void setNetworkSpecifier(String specifier) {
+        public void setNetworkSpecifier(NetworkSpecifier networkSpecifier) {
-            mNetworkCapabilities.setNetworkSpecifier(specifier);
+            mNetworkCapabilities.setNetworkSpecifier(networkSpecifier);
             mNetworkAgent.sendNetworkCapabilities(mNetworkCapabilities);
         }
 
@@ -1860,16 +1866,19 @@ public class ConnectivityServiceTest extends AndroidTestCase {
 
     @SmallTest
     public void testNetworkSpecifier() {
-        NetworkRequest.Builder b = new NetworkRequest.Builder().addTransportType(TRANSPORT_WIFI);
         NetworkRequest rEmpty1 = newWifiRequestBuilder().build();
-        NetworkRequest rEmpty2 = newWifiRequestBuilder().setNetworkSpecifier(null).build();
+        NetworkRequest rEmpty2 = newWifiRequestBuilder().setNetworkSpecifier((String) null).build();
         NetworkRequest rEmpty3 = newWifiRequestBuilder().setNetworkSpecifier("").build();
+        NetworkRequest rEmpty4 = newWifiRequestBuilder().setNetworkSpecifier(
+            (NetworkSpecifier) null).build();
         NetworkRequest rFoo = newWifiRequestBuilder().setNetworkSpecifier("foo").build();
-        NetworkRequest rBar = newWifiRequestBuilder().setNetworkSpecifier("bar").build();
+        NetworkRequest rBar = newWifiRequestBuilder().setNetworkSpecifier(
+                new StringNetworkSpecifier("bar")).build();
 
         TestNetworkCallback cEmpty1 = new TestNetworkCallback();
         TestNetworkCallback cEmpty2 = new TestNetworkCallback();
         TestNetworkCallback cEmpty3 = new TestNetworkCallback();
+        TestNetworkCallback cEmpty4 = new TestNetworkCallback();
         TestNetworkCallback cFoo = new TestNetworkCallback();
         TestNetworkCallback cBar = new TestNetworkCallback();
         TestNetworkCallback[] emptyCallbacks = new TestNetworkCallback[] {
@@ -1878,6 +1887,7 @@ public class ConnectivityServiceTest extends AndroidTestCase {
         mCm.registerNetworkCallback(rEmpty1, cEmpty1);
         mCm.registerNetworkCallback(rEmpty2, cEmpty2);
         mCm.registerNetworkCallback(rEmpty3, cEmpty3);
+        mCm.registerNetworkCallback(rEmpty4, cEmpty4);
         mCm.registerNetworkCallback(rFoo, cFoo);
         mCm.registerNetworkCallback(rBar, cBar);
 
@@ -1886,9 +1896,10 @@ public class ConnectivityServiceTest extends AndroidTestCase {
         cEmpty1.expectAvailableCallbacks(mWiFiNetworkAgent);
         cEmpty2.expectAvailableCallbacks(mWiFiNetworkAgent);
         cEmpty3.expectAvailableCallbacks(mWiFiNetworkAgent);
+        cEmpty4.expectAvailableCallbacks(mWiFiNetworkAgent);
         assertNoCallbacks(cFoo, cBar);
 
-        mWiFiNetworkAgent.setNetworkSpecifier("foo");
+        mWiFiNetworkAgent.setNetworkSpecifier(new StringNetworkSpecifier("foo"));
         cFoo.expectAvailableCallbacks(mWiFiNetworkAgent);
         for (TestNetworkCallback c: emptyCallbacks) {
             c.expectCallback(CallbackState.NETWORK_CAPABILITIES, mWiFiNetworkAgent);
@@ -1896,7 +1907,7 @@ public class ConnectivityServiceTest extends AndroidTestCase {
         cFoo.expectCallback(CallbackState.NETWORK_CAPABILITIES, mWiFiNetworkAgent);
         cFoo.assertNoCallback();
 
-        mWiFiNetworkAgent.setNetworkSpecifier("bar");
+        mWiFiNetworkAgent.setNetworkSpecifier(new StringNetworkSpecifier("bar"));
         cFoo.expectCallback(CallbackState.LOST, mWiFiNetworkAgent);
         cBar.expectAvailableCallbacks(mWiFiNetworkAgent);
         for (TestNetworkCallback c: emptyCallbacks) {
@@ -1916,32 +1927,63 @@ public class ConnectivityServiceTest extends AndroidTestCase {
 
     @SmallTest
     public void testInvalidNetworkSpecifier() {
-        boolean execptionCalled = true;
-
         try {
             NetworkRequest.Builder builder = new NetworkRequest.Builder();
-            builder.setNetworkSpecifier(MATCH_ALL_REQUESTS_NETWORK_SPECIFIER);
+            builder.setNetworkSpecifier(new MatchAllNetworkSpecifier());
-            execptionCalled = false;
+            fail("NetworkRequest builder with MatchAllNetworkSpecifier");
-        } catch (IllegalArgumentException e) {
+        } catch (IllegalArgumentException expected) {
-            // do nothing - should get here
+            // expected
         }
 
-        assertTrue("NetworkRequest builder with MATCH_ALL_REQUESTS_NETWORK_SPECIFIER",
-                execptionCalled);
-
         try {
             NetworkCapabilities networkCapabilities = new NetworkCapabilities();
             networkCapabilities.addTransportType(TRANSPORT_WIFI)
-                    .setNetworkSpecifier(NetworkCapabilities.MATCH_ALL_REQUESTS_NETWORK_SPECIFIER);
+                    .setNetworkSpecifier(new MatchAllNetworkSpecifier());
             mService.requestNetwork(networkCapabilities, null, 0, null,
                     ConnectivityManager.TYPE_WIFI);
-            execptionCalled = false;
+            fail("ConnectivityService requestNetwork with MatchAllNetworkSpecifier");
-        } catch (IllegalArgumentException e) {
+        } catch (IllegalArgumentException expected) {
-            // do nothing - should get here
+            // expected
         }
 
-        assertTrue("ConnectivityService requestNetwork with MATCH_ALL_REQUESTS_NETWORK_SPECIFIER",
+        class NonParcelableSpecifier extends NetworkSpecifier {
-                execptionCalled);
+            public boolean satisfiedBy(NetworkSpecifier other) { return false; }
+        };
+        class ParcelableSpecifier extends NonParcelableSpecifier implements Parcelable {
+            @Override public int describeContents() { return 0; }
+            @Override public void writeToParcel(Parcel p, int flags) {}
+        }
+        NetworkRequest.Builder builder;
+
+        builder = new NetworkRequest.Builder().addTransportType(TRANSPORT_ETHERNET);
+        try {
+            builder.setNetworkSpecifier(new NonParcelableSpecifier());
+            Parcel parcelW = Parcel.obtain();
+            builder.build().writeToParcel(parcelW, 0);
+            fail("Parceling a non-parcelable specifier did not throw an exception");
+        } catch (Exception e) {
+            // expected
+        }
+
+        builder = new NetworkRequest.Builder().addTransportType(TRANSPORT_ETHERNET);
+        builder.setNetworkSpecifier(new ParcelableSpecifier());
+        NetworkRequest nr = builder.build();
+        assertNotNull(nr);
+
+        try {
+            Parcel parcelW = Parcel.obtain();
+            nr.writeToParcel(parcelW, 0);
+            byte[] bytes = parcelW.marshall();
+            parcelW.recycle();
+
+            Parcel parcelR = Parcel.obtain();
+            parcelR.unmarshall(bytes, 0, bytes.length);
+            parcelR.setDataPosition(0);
+            NetworkRequest rereadNr = NetworkRequest.CREATOR.createFromParcel(parcelR);
+            fail("Unparceling a non-framework NetworkSpecifier did not throw an exception");
+        } catch (Exception e) {
+            // expected
+        }
     }
 
     @SmallTest