Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4a62d1de authored by Etan Cohen's avatar Etan Cohen
Browse files

[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
parent 0d55bb07

core/java/android/net/ConnectivityManager.java

+5 −4
Original line number Original line Diff line number Diff line
@@ -3449,6 +3449,11 @@ public class ConnectivityManager { 
            final NetworkCallback callback;

            final NetworkCallback callback;

            synchronized (sCallbacks) {

            synchronized (sCallbacks) {

                callback = sCallbacks.get(request);

                callback = sCallbacks.get(request);

                if (callback == null) {

                    Log.w(TAG,

                            "callback not found for " + getCallbackName(message.what) + " message");

                    return;

                }

                if (message.what == CALLBACK_UNAVAIL) {

                if (message.what == CALLBACK_UNAVAIL) {

                    sCallbacks.remove(request);

                    sCallbacks.remove(request);

                    callback.networkRequest = ALREADY_UNREGISTERED;

                    callback.networkRequest = ALREADY_UNREGISTERED;
@@ -3457,10 +3462,6 @@ public class ConnectivityManager { 
            if (DBG) {

            if (DBG) {

                Log.d(TAG, getCallbackName(message.what) + " for network " + network);

                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) {

            switch (message.what) {

                case CALLBACK_PRECHECK: {

                case CALLBACK_PRECHECK: {

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

+29 −9
Original line number Original line Diff line number Diff line
@@ -3816,11 +3816,20 @@ public class ConnectivityServiceTest { 
        networkCallback.assertNoCallback();

        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.

     * 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(

        NetworkRequest nr = new NetworkRequest.Builder().addTransportType(

                NetworkCapabilities.TRANSPORT_WIFI).build();

                NetworkCapabilities.TRANSPORT_WIFI).build();

        final TestNetworkCallback networkCallback = new TestNetworkCallback();

        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!

            // Simulate the factory releasing the request as unfulfillable and expect onUnavailable!

            testFactory.expectRemoveRequests(1);

            testFactory.expectRemoveRequests(1);

            testFactory.triggerUnfulfillable(requests.get(newRequestId));

            testFactory.triggerUnfulfillable(requests.get(newRequestId));



            networkCallback.expectCallback(CallbackState.UNAVAILABLE, null);

            networkCallback.expectCallback(CallbackState.UNAVAILABLE, null);

            testFactory.waitForRequests();

            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);

            mCm.unregisterNetworkCallback(networkCallback);

        }





        testFactory.unregister();

        testFactory.unregister();

        handlerThread.quit();

        handlerThread.quit();