[CM] Fix NPE due to unvalidated callback value

When unregistering callback due to ON_UNAVAILABLE did not check for
a non-null callback.

Bug: 132950880
Test: atest ConnectivityServiceTest
Change-Id: I8f3322963f322e6690f1403681bf66e8b38b35f8

2 files changed, 34 insertions, 13 deletions

diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java
index a69ca99500d6..e73fb46f61e8 100644
--- a/core/java/android/net/ConnectivityManager.java
+++ b/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: {
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 23cfbd4b16ba..1f32bd47b7ab 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/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
-    public void testUnfulfillableNetworkRequest() throws Exception {
+    private void runUnfulfillableNetworkRequest(boolean preUnregister) throws Exception {
         NetworkRequest nr = new NetworkRequest.Builder().addTransportType(
                 NetworkCapabilities.TRANSPORT_WIFI).build();
         final TestNetworkCallback networkCallback = new TestNetworkCallback();
@@ -3855,14 +3864,25 @@ public class ConnectivityServiceTest {
             }
         }
 
-        // 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();
+        if (preUnregister) {
+            mCm.unregisterNetworkCallback(networkCallback);
 
-        // unregister network callback - a no-op, but should not fail
-        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 (since already freed by the
+            // on-unavailable), but should not fail or throw exceptions.
+            mCm.unregisterNetworkCallback(networkCallback);
+        }
 
         testFactory.unregister();
         handlerThread.quit();