From bc1104b4656f561902cb466ed2c96d36447010da Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Tue, 9 May 2017 15:19:01 +0900 Subject: [PATCH] ConnectivityManager: improve argument validation Using Preconditions and dedicated static methods for checking arguments to improve error stack traces without error messages. Test: covered by previously added unit test Bug: 36701874 Change-Id: Id872b2c887a4bca43a8c3644622add1c2ee57c6d --- .../java/android/net/ConnectivityManager.java | 52 +++++++++---------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index c96d19d96c..9a7ce5b30d 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -1466,9 +1466,7 @@ public class ConnectivityManager { // Map from type to transports. final int NOT_FOUND = -1; final int transport = sLegacyTypeToTransport.get(type, NOT_FOUND); - if (transport == NOT_FOUND) { - throw new IllegalArgumentException("unknown legacy type: " + type); - } + Preconditions.checkArgument(transport != NOT_FOUND, "unknown legacy type: " + type); nc.addTransportType(transport); // Map from type to capabilities. @@ -1814,9 +1812,7 @@ public class ConnectivityManager { */ public void removeDefaultNetworkActiveListener(OnNetworkActiveListener l) { INetworkActivityListener rl = mNetworkActivityListeners.get(l); - if (rl == null) { - throw new IllegalArgumentException("Listener not registered: " + l); - } + Preconditions.checkArgument(rl != null, "Listener was not registered."); try { getNetworkManagementService().unregisterNetworkActivityListener(rl); } catch (RemoteException e) { @@ -1873,9 +1869,8 @@ public class ConnectivityManager { /** {@hide} */ public static final void enforceTetherChangePermission(Context context, String callingPkg) { - if (null == context || null == callingPkg) { - throw new IllegalArgumentException("arguments should not be null"); - } + Preconditions.checkNotNull(context, "Context cannot be null"); + Preconditions.checkNotNull(callingPkg, "callingPkg cannot be null"); if (context.getResources().getStringArray( com.android.internal.R.array.config_mobile_hotspot_provision_app).length == 2) { @@ -2773,7 +2768,7 @@ public class ConnectivityManager { } CallbackHandler(Handler handler) { - this(handler.getLooper()); + this(Preconditions.checkNotNull(handler, "Handler cannot be null.").getLooper()); } @Override @@ -2890,7 +2885,7 @@ public class ConnectivityManager { private NetworkRequest sendRequestForNetwork(NetworkCapabilities need, NetworkCallback callback, int timeoutMs, int action, int legacyType, CallbackHandler handler) { - Preconditions.checkArgument(callback != null, "null NetworkCallback"); + checkCallbackNotNull(callback); Preconditions.checkArgument(action == REQUEST || need != null, "null NetworkCapabilities"); final NetworkRequest request; try { @@ -3042,14 +3037,11 @@ public class ConnectivityManager { */ public void requestNetwork(NetworkRequest request, NetworkCallback networkCallback, int timeoutMs) { - if (timeoutMs <= 0) { - throw new IllegalArgumentException("Non-positive timeoutMs: " + timeoutMs); - } + checkTimeout(timeoutMs); int legacyType = inferLegacyTypeForNetworkCapabilities(request.networkCapabilities); requestNetwork(request, networkCallback, timeoutMs, legacyType, getDefaultHandler()); } - /** * Request a network to satisfy a set of {@link android.net.NetworkCapabilities}, limited * by a timeout. @@ -3079,9 +3071,7 @@ public class ConnectivityManager { */ public void requestNetwork(NetworkRequest request, NetworkCallback networkCallback, Handler handler, int timeoutMs) { - if (timeoutMs <= 0) { - throw new IllegalArgumentException("Non-positive timeoutMs"); - } + checkTimeout(timeoutMs); int legacyType = inferLegacyTypeForNetworkCapabilities(request.networkCapabilities); CallbackHandler cbHandler = new CallbackHandler(handler); requestNetwork(request, networkCallback, timeoutMs, legacyType, cbHandler); @@ -3153,7 +3143,7 @@ public class ConnectivityManager { * {@link NetworkCapabilities#NET_CAPABILITY_CAPTIVE_PORTAL}. */ public void requestNetwork(NetworkRequest request, PendingIntent operation) { - checkPendingIntent(operation); + checkPendingIntentNotNull(operation); try { mService.pendingRequestForNetwork(request.networkCapabilities, operation); } catch (RemoteException e) { @@ -3176,7 +3166,7 @@ public class ConnectivityManager { * corresponding NetworkRequest you'd like to remove. Cannot be null. */ public void releaseNetworkRequest(PendingIntent operation) { - checkPendingIntent(operation); + checkPendingIntentNotNull(operation); try { mService.releasePendingNetworkRequest(operation); } catch (RemoteException e) { @@ -3184,10 +3174,16 @@ public class ConnectivityManager { } } - private void checkPendingIntent(PendingIntent intent) { - if (intent == null) { - throw new IllegalArgumentException("PendingIntent cannot be null."); - } + private static void checkPendingIntentNotNull(PendingIntent intent) { + Preconditions.checkNotNull(intent, "PendingIntent cannot be null."); + } + + private static void checkCallbackNotNull(NetworkCallback callback) { + Preconditions.checkNotNull(callback, "null NetworkCallback"); + } + + private static void checkTimeout(int timeoutMs) { + Preconditions.checkArgumentPositive(timeoutMs, "timeoutMs must be strictly positive."); } /** @@ -3257,7 +3253,7 @@ public class ConnectivityManager { * comes from {@link PendingIntent#getBroadcast}. Cannot be null. */ public void registerNetworkCallback(NetworkRequest request, PendingIntent operation) { - checkPendingIntent(operation); + checkPendingIntentNotNull(operation); try { mService.pendingListenForNetwork(request.networkCapabilities, operation); } catch (RemoteException e) { @@ -3301,8 +3297,9 @@ public class ConnectivityManager { // capabilities, this request is guaranteed, at all times, to be // satisfied by the same network, if any, that satisfies the default // request, i.e., the system default network. + NetworkCapabilities nullCapabilities = null; CallbackHandler cbHandler = new CallbackHandler(handler); - sendRequestForNetwork(null, networkCallback, 0, REQUEST, TYPE_NONE, cbHandler); + sendRequestForNetwork(nullCapabilities, networkCallback, 0, REQUEST, TYPE_NONE, cbHandler); } /** @@ -3339,7 +3336,7 @@ public class ConnectivityManager { * @param networkCallback The {@link NetworkCallback} used when making the request. */ public void unregisterNetworkCallback(NetworkCallback networkCallback) { - Preconditions.checkArgument(networkCallback != null, "null NetworkCallback"); + checkCallbackNotNull(networkCallback); final List reqs = new ArrayList<>(); // Find all requests associated to this callback and stop callback triggers immediately. // Callback is reusable immediately. http://b/20701525, http://b/35921499. @@ -3375,6 +3372,7 @@ public class ConnectivityManager { * Cannot be null. */ public void unregisterNetworkCallback(PendingIntent operation) { + checkPendingIntentNotNull(operation); releaseNetworkRequest(operation); }