From 3d67f53b02aa5f0adfc12dbc0e5b5643f1a6c42d Mon Sep 17 00:00:00 2001 From: paulhu Date: Fri, 22 Mar 2019 16:35:06 +0800 Subject: [PATCH] Address leftover comments on 923337 and 930217. - Restrict unprivileged apps to use NetworkRequest.Builder#setSignalStrength. - Remove the "throws NullPointerException" in CaptivePortalProbeSpec constructor. - Remove the null check in LinkProperties. - Add annotataion into all ConnectivityManager.NetworkCallback methods. Change-Id: Id275cac1d6a30d7515cd7b113394f5e8a0179314 Fix: 129097486 Test: atest FrameworksNetTests --- api/current.txt | 10 ++--- api/system-current.txt | 3 +- .../java/android/net/ConnectivityManager.java | 24 ++++++----- core/java/android/net/LinkProperties.java | 8 +--- core/java/android/net/NetworkRequest.java | 5 +++ .../captiveportal/CaptivePortalProbeSpec.java | 3 +- core/res/AndroidManifest.xml | 6 +++ .../android/server/ConnectivityService.java | 25 ++++++++++- .../server/ConnectivityServiceTest.java | 41 +++++++++++++++++++ 9 files changed, 97 insertions(+), 28 deletions(-) diff --git a/api/current.txt b/api/current.txt index 76011b52fd907..00d9f50bb6027 100755 --- a/api/current.txt +++ b/api/current.txt @@ -27193,12 +27193,12 @@ package android.net { public static class ConnectivityManager.NetworkCallback { ctor public ConnectivityManager.NetworkCallback(); - method public void onAvailable(android.net.Network); + method public void onAvailable(@NonNull android.net.Network); method public void onBlockedStatusChanged(@NonNull android.net.Network, boolean); - method public void onCapabilitiesChanged(android.net.Network, android.net.NetworkCapabilities); - method public void onLinkPropertiesChanged(android.net.Network, android.net.LinkProperties); - method public void onLosing(android.net.Network, int); - method public void onLost(android.net.Network); + method public void onCapabilitiesChanged(@NonNull android.net.Network, @NonNull android.net.NetworkCapabilities); + method public void onLinkPropertiesChanged(@NonNull android.net.Network, @NonNull android.net.LinkProperties); + method public void onLosing(@NonNull android.net.Network, int); + method public void onLost(@NonNull android.net.Network); method public void onUnavailable(); } diff --git a/api/system-current.txt b/api/system-current.txt index a2f83fd7783ff..d76e76bae4b93 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -97,6 +97,7 @@ package android { field public static final String MOVE_PACKAGE = "android.permission.MOVE_PACKAGE"; field public static final String NETWORK_SCAN = "android.permission.NETWORK_SCAN"; field public static final String NETWORK_SETUP_WIZARD = "android.permission.NETWORK_SETUP_WIZARD"; + field public static final String NETWORK_SIGNAL_STRENGTH_WAKEUP = "android.permission.NETWORK_SIGNAL_STRENGTH_WAKEUP"; field public static final String NOTIFICATION_DURING_SETUP = "android.permission.NOTIFICATION_DURING_SETUP"; field public static final String NOTIFY_TV_INPUTS = "android.permission.NOTIFY_TV_INPUTS"; field public static final String OBSERVE_APP_USAGE = "android.permission.OBSERVE_APP_USAGE"; @@ -3203,7 +3204,7 @@ package android.net { } public static class NetworkRequest.Builder { - method @NonNull public android.net.NetworkRequest.Builder setSignalStrength(int); + method @NonNull @RequiresPermission(android.Manifest.permission.NETWORK_SIGNAL_STRENGTH_WAKEUP) public android.net.NetworkRequest.Builder setSignalStrength(int); } public class NetworkScoreManager { diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index ae93cf0197762..79560f50b441b 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -3231,7 +3231,7 @@ public class ConnectivityManager { * * @hide */ - public void onPreCheck(Network network) {} + public void onPreCheck(@NonNull Network network) {} /** * Called when the framework connects and has declared a new network ready for use. @@ -3244,8 +3244,9 @@ public class ConnectivityManager { * @param blocked Whether access to the {@link Network} is blocked due to system policy. * @hide */ - public void onAvailable(Network network, NetworkCapabilities networkCapabilities, - LinkProperties linkProperties, boolean blocked) { + public void onAvailable(@NonNull Network network, + @NonNull NetworkCapabilities networkCapabilities, + @NonNull LinkProperties linkProperties, boolean blocked) { // Internally only this method is called when a new network is available, and // it calls the callback in the same way and order that older versions used // to call so as not to change the behavior. @@ -3269,7 +3270,7 @@ public class ConnectivityManager { * * @param network The {@link Network} of the satisfying network. */ - public void onAvailable(Network network) {} + public void onAvailable(@NonNull Network network) {} /** * Called when the network is about to be disconnected. Often paired with an @@ -3285,7 +3286,7 @@ public class ConnectivityManager { * network connected. Note that the network may suffer a * hard loss at any time. */ - public void onLosing(Network network, int maxMsToLive) {} + public void onLosing(@NonNull Network network, int maxMsToLive) {} /** * Called when the framework has a hard loss of the network or when the @@ -3293,7 +3294,7 @@ public class ConnectivityManager { * * @param network The {@link Network} lost. */ - public void onLost(Network network) {} + public void onLost(@NonNull Network network) {} /** * Called if no network is found in the timeout time specified in @@ -3313,8 +3314,8 @@ public class ConnectivityManager { * @param networkCapabilities The new {@link android.net.NetworkCapabilities} for this * network. */ - public void onCapabilitiesChanged(Network network, - NetworkCapabilities networkCapabilities) {} + public void onCapabilitiesChanged(@NonNull Network network, + @NonNull NetworkCapabilities networkCapabilities) {} /** * Called when the network the framework connected to for this request @@ -3323,7 +3324,8 @@ public class ConnectivityManager { * @param network The {@link Network} whose link properties have changed. * @param linkProperties The new {@link LinkProperties} for this network. */ - public void onLinkPropertiesChanged(Network network, LinkProperties linkProperties) {} + public void onLinkPropertiesChanged(@NonNull Network network, + @NonNull LinkProperties linkProperties) {} /** * Called when the network the framework connected to for this request @@ -3334,7 +3336,7 @@ public class ConnectivityManager { * a tunnel, etc. * @hide */ - public void onNetworkSuspended(Network network) {} + public void onNetworkSuspended(@NonNull Network network) {} /** * Called when the network the framework connected to for this request @@ -3342,7 +3344,7 @@ public class ConnectivityManager { * preceded by a matching {@link NetworkCallback#onNetworkSuspended} call. * @hide */ - public void onNetworkResumed(Network network) {} + public void onNetworkResumed(@NonNull Network network) {} /** * Called when access to the specified network is blocked or unblocked. diff --git a/core/java/android/net/LinkProperties.java b/core/java/android/net/LinkProperties.java index 03d6d48bc8f6b..ad67763ac3fb0 100644 --- a/core/java/android/net/LinkProperties.java +++ b/core/java/android/net/LinkProperties.java @@ -316,9 +316,6 @@ public final class LinkProperties implements Parcelable { @SystemApi @TestApi public boolean removeLinkAddress(@NonNull LinkAddress toRemove) { - if (toRemove == null) { - return false; - } int i = findLinkAddressIndex(toRemove); if (i >= 0) { mLinkAddresses.remove(i); @@ -391,10 +388,7 @@ public final class LinkProperties implements Parcelable { @TestApi @SystemApi public boolean removeDnsServer(@NonNull InetAddress dnsServer) { - if (dnsServer != null) { - return mDnses.remove(dnsServer); - } - return false; + return mDnses.remove(dnsServer); } /** diff --git a/core/java/android/net/NetworkRequest.java b/core/java/android/net/NetworkRequest.java index 3a41a079dad04..acafa1354a7fd 100644 --- a/core/java/android/net/NetworkRequest.java +++ b/core/java/android/net/NetworkRequest.java @@ -17,6 +17,7 @@ package android.net; import android.annotation.NonNull; +import android.annotation.RequiresPermission; import android.annotation.SystemApi; import android.annotation.UnsupportedAppUsage; import android.net.NetworkCapabilities.NetCapability; @@ -343,10 +344,14 @@ public class NetworkRequest implements Parcelable { * current value. A value of {@code SIGNAL_STRENGTH_UNSPECIFIED} means no value when * received and has no effect when requesting a callback. * + *

This method requires the caller to hold the + * {@link android.Manifest.permission#NETWORK_SIGNAL_STRENGTH_WAKEUP} permission + * * @param signalStrength the bearer-specific signal strength. * @hide */ @SystemApi + @RequiresPermission(android.Manifest.permission.NETWORK_SIGNAL_STRENGTH_WAKEUP) public @NonNull Builder setSignalStrength(int signalStrength) { mNetworkCapabilities.setSignalStrength(signalStrength); return this; diff --git a/core/java/android/net/captiveportal/CaptivePortalProbeSpec.java b/core/java/android/net/captiveportal/CaptivePortalProbeSpec.java index 6c6a16c4534ea..b354607b88566 100644 --- a/core/java/android/net/captiveportal/CaptivePortalProbeSpec.java +++ b/core/java/android/net/captiveportal/CaptivePortalProbeSpec.java @@ -50,8 +50,7 @@ public abstract class CaptivePortalProbeSpec { private final String mEncodedSpec; private final URL mUrl; - CaptivePortalProbeSpec(@NonNull String encodedSpec, @NonNull URL url) - throws NullPointerException { + CaptivePortalProbeSpec(@NonNull String encodedSpec, @NonNull URL url) { mEncodedSpec = checkNotNull(encodedSpec); mUrl = checkNotNull(url); } diff --git a/core/res/AndroidManifest.xml b/core/res/AndroidManifest.xml index e62df2a189a79..6206caafc75ea 100644 --- a/core/res/AndroidManifest.xml +++ b/core/res/AndroidManifest.xml @@ -1562,6 +1562,12 @@ + + + getSignalStrengthThresholds(NetworkAgentInfo nai) { diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 92a865a3dc588..34dafa7723a8e 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -3022,6 +3022,47 @@ public class ConnectivityServiceTest { } } + @Test + public void testInvalidSignalStrength() { + NetworkRequest r = new NetworkRequest.Builder() + .addCapability(NET_CAPABILITY_INTERNET) + .addTransportType(TRANSPORT_WIFI) + .setSignalStrength(-75) + .build(); + // Registering a NetworkCallback with signal strength but w/o NETWORK_SIGNAL_STRENGTH_WAKEUP + // permission should get SecurityException. + try { + mCm.registerNetworkCallback(r, new NetworkCallback()); + fail("Expected SecurityException filing a callback with signal strength"); + } catch (SecurityException expected) { + // expected + } + + try { + mCm.registerNetworkCallback(r, PendingIntent.getService( + mServiceContext, 0, new Intent(), 0)); + fail("Expected SecurityException filing a callback with signal strength"); + } catch (SecurityException expected) { + // expected + } + + // Requesting a Network with signal strength should get IllegalArgumentException. + try { + mCm.requestNetwork(r, new NetworkCallback()); + fail("Expected IllegalArgumentException filing a request with signal strength"); + } catch (IllegalArgumentException expected) { + // expected + } + + try { + mCm.requestNetwork(r, PendingIntent.getService( + mServiceContext, 0, new Intent(), 0)); + fail("Expected IllegalArgumentException filing a request with signal strength"); + } catch (IllegalArgumentException expected) { + // expected + } + } + @Test public void testRegisterDefaultNetworkCallback() throws Exception { final TestNetworkCallback defaultNetworkCallback = new TestNetworkCallback();