ConnectivityManager: allow usage of TYPE_NONE

This patch allows to use TYPE_NONE for the legacy network type variable
of NetworkInfo. This usage is "safe" with respect to legacy APIs using
network types as most of them already returns null or do nothing for
TYPE_NONE.

Of the existing APIs in ConnectivityManager that accept a network type
argument, those which were already returning null or doing nothing for
TYPE_NONE are:
  getNetworkInfo(int)
  getNetworkForType(int)
  stopUsingNetworkFeature(int, String)
  networkCapabilitiesForType(int)
  requestRouteToHostAddress(int, InetAddress)
  reportInetCondition(int, int)
  isNetworkSupported(int)
  getLinkProperties(int)

Only setProvisioningNotificationVisible needs an additional guard
against TYPE_NONE.

Bug: 30088447
Bug: 62844794
Test: runtest frameworks-net
Change-Id: I112596fcd03d3c2cd42a2a84d265adb38e3944bb

diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java
index 02f0f18..55127a8 100644 (file)
--- a/core/java/android/net/ConnectivityManager.java
+++ b/core/java/android/net/ConnectivityManager.java
@@ -579,6 +579,8 @@ public class ConnectivityManager {
     /** {@hide} */
     public static final int MAX_NETWORK_TYPE = TYPE_VPN;
 
+    private static final int MIN_NETWORK_TYPE = TYPE_MOBILE;
+
     /**
      * If you want to set the default network preference,you can directly
      * change the networkAttributes array in framework's config.xml.
@@ -636,7 +638,7 @@ public class ConnectivityManager {
      *             validate a network type.
      */
     public static boolean isNetworkTypeValid(int networkType) {
-        return networkType >= 0 && networkType <= MAX_NETWORK_TYPE;
+        return MIN_NETWORK_TYPE <= networkType && networkType <= MAX_NETWORK_TYPE;
     }
 
     /**
@@ -649,6 +651,8 @@ public class ConnectivityManager {
      */
     public static String getNetworkTypeName(int type) {
         switch (type) {
+          case TYPE_NONE:
+                return "NONE";
             case TYPE_MOBILE:
                 return "MOBILE";
             case TYPE_WIFI:

diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java
index 2dd7f75..76646b8 100644 (file)
--- a/core/java/android/net/NetworkCapabilities.java
+++ b/core/java/android/net/NetworkCapabilities.java
@@ -21,6 +21,7 @@ import android.os.Parcelable;
 
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.BitUtils;
+import com.android.internal.util.Preconditions;
 
 import java.util.Objects;
 
@@ -429,6 +430,11 @@ public final class NetworkCapabilities implements Parcelable {
     /** @hide */
     public static final int MAX_TRANSPORT = TRANSPORT_LOWPAN;
 
+    /** @hide */
+    public static boolean isValidTransport(int transportType) {
+        return (MIN_TRANSPORT <= transportType) && (transportType <= MAX_TRANSPORT);
+    }
+
     private static final String[] TRANSPORT_NAMES = {
         "CELLULAR",
         "WIFI",
@@ -453,9 +459,7 @@ public final class NetworkCapabilities implements Parcelable {
      * @hide
      */
     public NetworkCapabilities addTransportType(int transportType) {
-        if (transportType < MIN_TRANSPORT || transportType > MAX_TRANSPORT) {
-            throw new IllegalArgumentException("TransportType out of range");
-        }
+        checkValidTransportType(transportType);
         mTransportTypes |= 1 << transportType;
         setNetworkSpecifier(mNetworkSpecifier); // used for exception checking
         return this;
@@ -469,9 +473,7 @@ public final class NetworkCapabilities implements Parcelable {
      * @hide
      */
     public NetworkCapabilities removeTransportType(int transportType) {
-        if (transportType < MIN_TRANSPORT || transportType > MAX_TRANSPORT) {
-            throw new IllegalArgumentException("TransportType out of range");
-        }
+        checkValidTransportType(transportType);
         mTransportTypes &= ~(1 << transportType);
         setNetworkSpecifier(mNetworkSpecifier); // used for exception checking
         return this;
@@ -495,10 +497,7 @@ public final class NetworkCapabilities implements Parcelable {
      * @return {@code true} if set on this instance.
      */
     public boolean hasTransport(int transportType) {
-        if (transportType < MIN_TRANSPORT || transportType > MAX_TRANSPORT) {
-            return false;
-        }
-        return ((mTransportTypes & (1 << transportType)) != 0);
+        return isValidTransport(transportType) && ((mTransportTypes & (1 << transportType)) != 0);
     }
 
     private void combineTransportTypes(NetworkCapabilities nc) {
@@ -906,9 +905,14 @@ public final class NetworkCapabilities implements Parcelable {
      * @hide
      */
     public static String transportNameOf(int transport) {
-        if (transport < 0 || TRANSPORT_NAMES.length <= transport) {
+        if (!isValidTransport(transport)) {
             return "UNKNOWN";
         }
         return TRANSPORT_NAMES[transport];
     }
+
+    private static void checkValidTransportType(int transport) {
+        Preconditions.checkArgument(
+                isValidTransport(transport), "Invalid TransportType " + transport);
+    }
 }

diff --git a/core/java/android/net/NetworkInfo.java b/core/java/android/net/NetworkInfo.java
index 42f5feb..84c32be 100644 (file)
--- a/core/java/android/net/NetworkInfo.java
+++ b/core/java/android/net/NetworkInfo.java
@@ -127,7 +127,8 @@ public class NetworkInfo implements Parcelable {
      * @hide
      */
     public NetworkInfo(int type, int subtype, String typeName, String subtypeName) {
-        if (!ConnectivityManager.isNetworkTypeValid(type)) {
+        if (!ConnectivityManager.isNetworkTypeValid(type)
+                && type != ConnectivityManager.TYPE_NONE) {
             throw new IllegalArgumentException("Invalid network type: " + type);
         }
         mNetworkType = type;

diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 5221b5f..eafa88f 100644 (file)
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -3908,6 +3908,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
     public void setProvisioningNotificationVisible(boolean visible, int networkType,
             String action) {
         enforceConnectivityInternalPermission();
+        if (!ConnectivityManager.isNetworkTypeValid(networkType)) {
+            return;
+        }
         final long ident = Binder.clearCallingIdentity();
         try {
             // Concatenate the range of types onto the range of NetIDs.

diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 1418be6..9d4a2b0 100644 (file)
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -19,6 +19,7 @@ package com.android.server;
 import static android.net.ConnectivityManager.CONNECTIVITY_ACTION;
 import static android.net.ConnectivityManager.TYPE_ETHERNET;
 import static android.net.ConnectivityManager.TYPE_MOBILE;
+import static android.net.ConnectivityManager.TYPE_NONE;
 import static android.net.ConnectivityManager.TYPE_WIFI;
 import static android.net.ConnectivityManager.getNetworkTypeName;
 import static android.net.NetworkCapabilities.*;
@@ -113,6 +114,7 @@ import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.BooleanSupplier;
+import java.util.function.Predicate;
 
 /**
  * Tests for {@link ConnectivityService}.
@@ -316,6 +318,9 @@ public class ConnectivityServiceTest extends AndroidTestCase {
                 case TRANSPORT_CELLULAR:
                     mScore = 50;
                     break;
+                case TRANSPORT_WIFI_AWARE:
+                    mScore = 20;
+                    break;
                 default:
                     throw new UnsupportedOperationException("unimplemented network type");
             }
@@ -397,6 +402,15 @@ public class ConnectivityServiceTest extends AndroidTestCase {
          * @param validated Indicate if network should pretend to be validated.
          */
         public void connect(boolean validated) {
+            connect(validated, true);
+        }
+
+        /**
+         * Transition this NetworkAgent to CONNECTED state.
+         * @param validated Indicate if network should pretend to be validated.
+         * @param hasInternet Indicate if network should pretend to have NET_CAPABILITY_INTERNET.
+         */
+        public void connect(boolean validated, boolean hasInternet) {
             assertEquals("MockNetworkAgents can only be connected once",
                     mNetworkInfo.getDetailedState(), DetailedState.IDLE);
             assertFalse(mNetworkCapabilities.hasCapability(NET_CAPABILITY_INTERNET));
@@ -419,7 +433,9 @@ public class ConnectivityServiceTest extends AndroidTestCase {
                 };
                 mCm.registerNetworkCallback(request, callback);
             }
-            addCapability(NET_CAPABILITY_INTERNET);
+            if (hasInternet) {
+                addCapability(NET_CAPABILITY_INTERNET);
+            }
 
             connectWithoutInternet();
 
@@ -773,7 +789,10 @@ public class ConnectivityServiceTest extends AndroidTestCase {
      * Fails if TIMEOUT_MS goes by before {@code conditionVariable} opens.
      */
     static private void waitFor(ConditionVariable conditionVariable) {
-        assertTrue(conditionVariable.block(TIMEOUT_MS));
+        if (conditionVariable.block(TIMEOUT_MS)) {
+            return;
+        }
+        fail("ConditionVariable was blocked for more than " + TIMEOUT_MS + "ms");
     }
 
     @Override
@@ -828,7 +847,7 @@ public class ConnectivityServiceTest extends AndroidTestCase {
             case TRANSPORT_CELLULAR:
                 return TYPE_MOBILE;
             default:
-                throw new IllegalStateException("Unknown transport " + transport);
+                return TYPE_NONE;
         }
     }
 
@@ -839,6 +858,9 @@ public class ConnectivityServiceTest extends AndroidTestCase {
         // Test getActiveNetwork()
         assertNotNull(mCm.getActiveNetwork());
         assertEquals(mCm.getActiveNetwork(), mCm.getActiveNetworkForUid(Process.myUid()));
+        if (!NetworkCapabilities.isValidTransport(transport)) {
+            throw new IllegalStateException("Unknown transport " + transport);
+        }
         switch (transport) {
             case TRANSPORT_WIFI:
                 assertEquals(mCm.getActiveNetwork(), mWiFiNetworkAgent.getNetwork());
@@ -847,7 +869,7 @@ public class ConnectivityServiceTest extends AndroidTestCase {
                 assertEquals(mCm.getActiveNetwork(), mCellNetworkAgent.getNetwork());
                 break;
             default:
-                throw new IllegalStateException("Unknown transport" + transport);
+                break;
         }
         // Test getNetworkInfo(Network)
         assertNotNull(mCm.getNetworkInfo(mCm.getActiveNetwork()));
@@ -1267,7 +1289,26 @@ public class ConnectivityServiceTest extends AndroidTestCase {
             return expectCallback(state, agent, TIMEOUT_MS);
         }
 
-        void expectAvailableCallbacks(MockNetworkAgent agent, boolean expectSuspended, int timeoutMs) {
+        CallbackInfo expectCallbackLike(Predicate<CallbackInfo> fn) {
+            return expectCallbackLike(fn, TIMEOUT_MS);
+        }
+
+        CallbackInfo expectCallbackLike(Predicate<CallbackInfo> fn, int timeoutMs) {
+            int timeLeft = timeoutMs;
+            while (timeLeft > 0) {
+                long start = SystemClock.elapsedRealtime();
+                CallbackInfo info = nextCallback(timeLeft);
+                if (fn.test(info)) {
+                    return info;
+                }
+                timeLeft -= (SystemClock.elapsedRealtime() - start);
+            }
+            fail("Did not receive expected callback after " + timeoutMs + "ms");
+            return null;
+        }
+
+        void expectAvailableCallbacks(
+                MockNetworkAgent agent, boolean expectSuspended, int timeoutMs) {
             expectCallback(CallbackState.AVAILABLE, agent, timeoutMs);
             if (expectSuspended) {
                 expectCallback(CallbackState.SUSPENDED, agent, timeoutMs);
@@ -1824,26 +1865,18 @@ public class ConnectivityServiceTest extends AndroidTestCase {
     @SmallTest
     public void testNoMutableNetworkRequests() throws Exception {
         PendingIntent pendingIntent = PendingIntent.getBroadcast(mContext, 0, new Intent("a"), 0);
-        NetworkRequest.Builder builder = new NetworkRequest.Builder();
-        builder.addCapability(NET_CAPABILITY_VALIDATED);
-        try {
-            mCm.requestNetwork(builder.build(), new NetworkCallback());
-            fail();
-        } catch (IllegalArgumentException expected) {}
-        try {
-            mCm.requestNetwork(builder.build(), pendingIntent);
-            fail();
-        } catch (IllegalArgumentException expected) {}
-        builder = new NetworkRequest.Builder();
-        builder.addCapability(NET_CAPABILITY_CAPTIVE_PORTAL);
-        try {
-            mCm.requestNetwork(builder.build(), new NetworkCallback());
-            fail();
-        } catch (IllegalArgumentException expected) {}
-        try {
-            mCm.requestNetwork(builder.build(), pendingIntent);
-            fail();
-        } catch (IllegalArgumentException expected) {}
+        NetworkRequest request1 = new NetworkRequest.Builder()
+                .addCapability(NET_CAPABILITY_VALIDATED)
+                .build();
+        NetworkRequest request2 = new NetworkRequest.Builder()
+                .addCapability(NET_CAPABILITY_CAPTIVE_PORTAL)
+                .build();
+
+        Class<IllegalArgumentException> expected = IllegalArgumentException.class;
+        assertException(() -> { mCm.requestNetwork(request1, new NetworkCallback()); }, expected);
+        assertException(() -> { mCm.requestNetwork(request1, pendingIntent); }, expected);
+        assertException(() -> { mCm.requestNetwork(request2, new NetworkCallback()); }, expected);
+        assertException(() -> { mCm.requestNetwork(request2, pendingIntent); }, expected);
     }
 
     @SmallTest
@@ -3239,6 +3272,67 @@ public class ConnectivityServiceTest extends AndroidTestCase {
         }
     }
 
+    @SmallTest
+    public void testNetworkInfoOfTypeNone() {
+        ConditionVariable broadcastCV = waitForConnectivityBroadcasts(1);
+
+        verifyNoNetwork();
+        MockNetworkAgent lowpanNetwork = new MockNetworkAgent(TRANSPORT_WIFI_AWARE);
+        assertNull(mCm.getActiveNetworkInfo());
+
+        Network[] allNetworks = mCm.getAllNetworks();
+        assertLength(1, allNetworks);
+        Network network = allNetworks[0];
+        NetworkCapabilities capabilities = mCm.getNetworkCapabilities(network);
+        assertTrue(capabilities.hasTransport(TRANSPORT_WIFI_AWARE));
+
+        final NetworkRequest request =
+                new NetworkRequest.Builder().addTransportType(TRANSPORT_WIFI_AWARE).build();
+        final TestNetworkCallback callback = new TestNetworkCallback();
+        mCm.registerNetworkCallback(request, callback);
+
+        // Bring up lowpan.
+        lowpanNetwork.connect(false, false);
+        callback.expectAvailableCallbacks(lowpanNetwork);
+
+        assertNull(mCm.getActiveNetworkInfo());
+        assertNull(mCm.getActiveNetwork());
+        // TODO: getAllNetworkInfo is dirty and returns a non-empty array rght from the start
+        // of this test. Fix it and uncomment the assert below.
+        //assertEmpty(mCm.getAllNetworkInfo());
+
+        // Disconnect lowpan.
+        lowpanNetwork.disconnect();
+        callback.expectCallback(CallbackState.LOST, lowpanNetwork);
+        mCm.unregisterNetworkCallback(callback);
+
+        verifyNoNetwork();
+        if (broadcastCV.block(10)) {
+            fail("expected no broadcast, but got CONNECTIVITY_ACTION broadcast");
+        }
+    }
+
+    @SmallTest
+    public void testDeprecatedAndUnsupportedOperations() throws Exception {
+        final int TYPE_NONE = ConnectivityManager.TYPE_NONE;
+        assertNull(mCm.getNetworkInfo(TYPE_NONE));
+        assertNull(mCm.getNetworkForType(TYPE_NONE));
+        assertNull(mCm.getLinkProperties(TYPE_NONE));
+        assertFalse(mCm.isNetworkSupported(TYPE_NONE));
+
+        assertException(() -> { mCm.networkCapabilitiesForType(TYPE_NONE); },
+                IllegalArgumentException.class);
+
+        Class<UnsupportedOperationException> unsupported = UnsupportedOperationException.class;
+        assertException(() -> { mCm.startUsingNetworkFeature(TYPE_WIFI, ""); }, unsupported);
+        assertException(() -> { mCm.stopUsingNetworkFeature(TYPE_WIFI, ""); }, unsupported);
+        // TODO: let test context have configuration application target sdk version
+        // and test that pre-M requesting for TYPE_NONE sends back APN_REQUEST_FAILED
+        assertException(() -> { mCm.startUsingNetworkFeature(TYPE_NONE, ""); }, unsupported);
+        assertException(() -> { mCm.stopUsingNetworkFeature(TYPE_NONE, ""); }, unsupported);
+        assertException(() -> { mCm.requestRouteToHostAddress(TYPE_NONE, null); }, unsupported);
+    }
+
     private static <T> void assertEmpty(T[] ts) {
         int length = ts.length;
         assertEquals("expected empty array, but length was " + length, 0, length);
@@ -3249,4 +3343,16 @@ public class ConnectivityServiceTest extends AndroidTestCase {
         assertEquals(String.format("expected array of length %s, but length was %s for %s",
                 expected, length, Arrays.toString(got)), expected, length);
     }
+
+    private static <T> void assertException(Runnable block, Class<T> expected) {
+        try {
+            block.run();
+            fail("Expected exception of type " + expected);
+        } catch (Exception got) {
+            if (!got.getClass().equals(expected)) {
+                fail("Expected exception of type " + expected + " but got " + got);
+            }
+            return;
+        }
+    }
 }