Merge "[CM] Fix NPE due to unvalidated callback value" am: 43c1536c6b

am: 51ddc176ab

Change-Id: Idf439c1f6badd715776eaaced1a781b066ba6d37

core/java/android/net/ConnectivityManager.java

@@ -3449,6 +3449,11 @@ public class ConnectivityManager {
             final NetworkCallback callback;
             synchronized (sCallbacks) {
                 callback = sCallbacks.get(request);
+                if (callback == null) {
+                    Log.w(TAG,
+                            "callback not found for " + getCallbackName(message.what) + " message");
+                    return;
+                }
                 if (message.what == CALLBACK_UNAVAIL) {
                     sCallbacks.remove(request);
                     callback.networkRequest = ALREADY_UNREGISTERED;
@@ -3457,10 +3462,6 @@ public class ConnectivityManager {
             if (DBG) {
                 Log.d(TAG, getCallbackName(message.what) + " for network " + network);
             }
-            if (callback == null) {
-                Log.w(TAG, "callback not found for " + getCallbackName(message.what) + " message");
-                return;
-            }
 
             switch (message.what) {
                 case CALLBACK_PRECHECK: {

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

@@ -3816,11 +3816,20 @@ public class ConnectivityServiceTest {
         networkCallback.assertNoCallback();
     }
 
+    @Test
+    public void testUnfulfillableNetworkRequest() throws Exception {
+        runUnfulfillableNetworkRequest(false);
+    }
+
+    @Test
+    public void testUnfulfillableNetworkRequestAfterUnregister() throws Exception {
+        runUnfulfillableNetworkRequest(true);
+    }
+
     /**
      * Validate the callback flow for a factory releasing a request as unfulfillable.
      */
-    @Test
+    private void runUnfulfillableNetworkRequest(boolean preUnregister) throws Exception {
-    public void testUnfulfillableNetworkRequest() throws Exception {
         NetworkRequest nr = new NetworkRequest.Builder().addTransportType(
                 NetworkCapabilities.TRANSPORT_WIFI).build();
         final TestNetworkCallback networkCallback = new TestNetworkCallback();
@@ -3855,14 +3864,25 @@ public class ConnectivityServiceTest {
             }
         }
 
+        if (preUnregister) {
+            mCm.unregisterNetworkCallback(networkCallback);
+
+            // Simulate the factory releasing the request as unfulfillable: no-op since
+            // the callback has already been unregistered (but a test that no exceptions are
+            // thrown).
+            testFactory.triggerUnfulfillable(requests.get(newRequestId));
+        } else {
             // Simulate the factory releasing the request as unfulfillable and expect onUnavailable!
             testFactory.expectRemoveRequests(1);
             testFactory.triggerUnfulfillable(requests.get(newRequestId));
+
             networkCallback.expectCallback(CallbackState.UNAVAILABLE, null);
             testFactory.waitForRequests();
 
-        // unregister network callback - a no-op, but should not fail
+            // unregister network callback - a no-op (since already freed by the
+            // on-unavailable), but should not fail or throw exceptions.
             mCm.unregisterNetworkCallback(networkCallback);
+        }
 
         testFactory.unregister();
         handlerThread.quit();