[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
This commit is contained in:
Etan Cohen
2019-05-21 12:06:04 -07:00
parent ac6f142334
commit b58e366f2d
2 changed files with 34 additions and 13 deletions

View File

@@ -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: {

View File

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