From cbfbb3755a5ad98893bae3536190ab4c5b744c1f Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Wed, 7 Feb 2018 21:17:43 +0900 Subject: [PATCH 1/6] Add missing'}' in javadoc of public API method Bug: 73052508 Test: pure documentation change Change-Id: I92514629da1b000dd3d1165acd8efcdec75b49b9 --- core/java/android/net/ConnectivityManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 11d338d05c..180a4cebea 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -2656,7 +2656,7 @@ public class ConnectivityManager { * A {@code NetworkCallback} is registered by calling * {@link #requestNetwork(NetworkRequest, NetworkCallback)}, * {@link #registerNetworkCallback(NetworkRequest, NetworkCallback)}, - * or {@link #registerDefaultNetworkCallback(NetworkCallback). A {@code NetworkCallback} is + * or {@link #registerDefaultNetworkCallback(NetworkCallback)}. A {@code NetworkCallback} is * unregistered by calling {@link #unregisterNetworkCallback(NetworkCallback)}. * A {@code NetworkCallback} should be registered at most once at any time. * A {@code NetworkCallback} that has been unregistered can be registered again. From 2550e069bc8fcf1d66971d7065fb23b130c6c5bc Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 26 Jan 2018 19:24:40 +0900 Subject: [PATCH 2/6] Send null UIDs to apps instead of single-uid lists. Prior to this change ConnectivityManager used to patch in the UID of the requesting app inside the NetworkCapabilities sent to it. The rationale was that the app may not know what other apps may use the network, so the view it should have of the network should always say the network only applies to that app. But this has an unfortunate side effect : apps can't match the received network against a default NetworkCapabilities. Ostensibly this only applies to the system because all involved calls are @hide, but still : system code would get some NetworkCapabilities, for example using networkCapabilitiesForType, and then try to match the capabilities of an available network using satisfiedByNetworkCapabilities. Because the passed network is declared to only apply to one's own UID and the UIDs of the NetworkCapabilities are set to null meaning "I need this network to apply to all UIDs", the answer will be "false". While this is WAI in a sense, it is very counter-intuitive that code trying to match a network would be required to patch in its own UIDs. There are three ways of fixing this : 1. Require all apps to do the above. It's correct, but it's cumbersome and counterintuitive. Multiple places in existing code needs to be fixed, Tethering is an example. 2. Write the UIDs of the caller in any NetworkCapabilities object that is created. This is not very practical, because it imposes the converse requirement on all NetworkAgents, which would then have to clear the UIDs before they send the capabilities to ConnectivityService. All NetworkAgents need to be fixed. 3. Instead of sending an object with a list of one UID to apps, send a null list. The drawback is that the networks nominally look to apps like they apply to all apps. I argue this does not matter ; what matters is that the UID lists do not leak. Clients just see a null list of UIDs (and third party can't even access them without using reflection). No other changes are required besides this two-line patch. This patch implements 3. I believe it is the saner approach, with both the most intuitive behavior and the best backward compatibility characteristics, as well as the easiest change. This does not encroach on the future plans to make the actual UID list available to apps with NETWORK_SETTINGS. Test: runtest frameworks-net Change-Id: I978d91197668119e051c24e1d04aafe1644a41cf --- .../android/server/ConnectivityService.java | 24 +++++++++++-------- .../server/ConnectivityServiceTest.java | 3 +-- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index bff5c10d4e..dc70347035 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1260,11 +1260,7 @@ public class ConnectivityService extends IConnectivityManager.Stub for (Network network : networks) { nai = getNetworkAgentInfoForNetwork(network); nc = getNetworkCapabilitiesInternal(nai); - // nc is a copy of the capabilities in nai, so it's fine to mutate it - // TODO : don't remove the UIDs when communicating with processes - // that have the NETWORK_SETTINGS permission. if (nc != null) { - nc.setSingleUid(userId); result.put(network, nc); } } @@ -1332,7 +1328,9 @@ public class ConnectivityService extends IConnectivityManager.Stub if (nai != null) { synchronized (nai) { if (nai.networkCapabilities != null) { - return new NetworkCapabilities(nai.networkCapabilities); + // TODO : don't remove the UIDs when communicating with processes + // that have the NETWORK_SETTINGS permission. + return networkCapabilitiesWithoutUids(nai.networkCapabilities); } } } @@ -1345,6 +1343,10 @@ public class ConnectivityService extends IConnectivityManager.Stub return getNetworkCapabilitiesInternal(getNetworkAgentInfoForNetwork(network)); } + private NetworkCapabilities networkCapabilitiesWithoutUids(NetworkCapabilities nc) { + return new NetworkCapabilities(nc).setUids(null); + } + @Override public NetworkState[] getAllNetworkState() { // Require internal since we're handing out IMSI details @@ -1354,6 +1356,10 @@ public class ConnectivityService extends IConnectivityManager.Stub for (Network network : getAllNetworks()) { final NetworkAgentInfo nai = getNetworkAgentInfoForNetwork(network); if (nai != null) { + // TODO (b/73321673) : NetworkState contains a copy of the + // NetworkCapabilities, which may contain UIDs of apps to which the + // network applies. Should the UIDs be cleared so as not to leak or + // interfere ? result.add(nai.getNetworkState()); } } @@ -4909,7 +4915,7 @@ public class ConnectivityService extends IConnectivityManager.Stub releasePendingNetworkRequestWithDelay(pendingIntent); } - private static void callCallbackForRequest(NetworkRequestInfo nri, + private void callCallbackForRequest(NetworkRequestInfo nri, NetworkAgentInfo networkAgent, int notificationType, int arg1) { if (nri.messenger == null) { return; // Default request has no msgr @@ -4927,11 +4933,9 @@ public class ConnectivityService extends IConnectivityManager.Stub break; } case ConnectivityManager.CALLBACK_CAP_CHANGED: { + // networkAgent can't be null as it has been accessed a few lines above. final NetworkCapabilities nc = - new NetworkCapabilities(networkAgent.networkCapabilities); - // TODO : don't remove the UIDs when communicating with processes - // that have the NETWORK_SETTINGS permission. - nc.setSingleUid(nri.mUid); + networkCapabilitiesWithoutUids(networkAgent.networkCapabilities); putParcelable(bundle, nc); break; } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index e7abede4cd..2ec19bf754 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -3682,8 +3682,7 @@ public class ConnectivityServiceTest { vpnNetworkCallback.expectAvailableCallbacksUnvalidated(vpnNetworkAgent); genericNetworkCallback.expectCallback(CallbackState.NETWORK_CAPABILITIES, vpnNetworkAgent); - vpnNetworkCallback.expectCapabilitiesLike( - nc -> nc.appliesToUid(uid) && !nc.appliesToUid(uid + 1), vpnNetworkAgent); + vpnNetworkCallback.expectCapabilitiesLike(nc -> null == nc.getUids(), vpnNetworkAgent); ranges.clear(); vpnNetworkAgent.setUids(ranges); From a23bc9e5013e5efd5d2c7205cd7bd7ece7929e1f Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 30 Jan 2018 22:41:41 +0900 Subject: [PATCH 3/6] Publish FOREGROUND and add NOT_SUSPENDED capabilities. NOT_SUSPENDED and FOREGROUND are capabilities that need to be public so as to reach feature parity with what information can be gotten through the use of CONNECTIVITY_ACTION and synchronous calls to ConnectivityManager. This change makes them public, and wires up the NOT_SUSPENDED capability. This deprecates in effect the old onSuspended and onResumed callbacks, but these have never been public. This also converts the onAvailable path from a multiple binder call design to a simpler, single binder call. This is only for internal convenience Test: runtest frameworks-net Test: cts Test: also manual testing Change-Id: I6ea524bb361ecef0569ea2f9006c1e516378bc25 --- .../java/android/net/ConnectivityManager.java | 40 ++++++++++++++++--- .../java/android/net/NetworkCapabilities.java | 30 ++++++++++---- .../android/server/ConnectivityService.java | 29 +++++++++----- .../server/connectivity/NetworkAgentInfo.java | 11 ++++- .../android/net/ConnectivityManagerTest.java | 11 ++++- .../server/ConnectivityServiceTest.java | 33 ++++++++++++++- 6 files changed, 126 insertions(+), 28 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 11d338d05c..6848ae1ab7 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -2685,6 +2685,32 @@ public class ConnectivityManager { * satisfying the request changes. * * @param network The {@link Network} of the satisfying network. + * @param networkCapabilities The {@link NetworkCapabilities} of the satisfying network. + * @param linkProperties The {@link LinkProperties} of the satisfying network. + * @hide + */ + public void onAvailable(Network network, NetworkCapabilities networkCapabilities, + LinkProperties linkProperties) { + // 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. + onAvailable(network); + if (!networkCapabilities.hasCapability( + NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED)) { + onNetworkSuspended(network); + } + onCapabilitiesChanged(network, networkCapabilities); + onLinkPropertiesChanged(network, linkProperties); + } + + /** + * Called when the framework connects and has declared a new network ready for use. + * This callback may be called more than once if the {@link Network} that is + * satisfying the request changes. This will always immediately be followed by a + * call to {@link #onCapabilitiesChanged(Network, NetworkCapabilities)} then by a + * call to {@link #onLinkPropertiesChanged(Network, LinkProperties)}. + * + * @param network The {@link Network} of the satisfying network. */ public void onAvailable(Network network) {} @@ -2727,7 +2753,8 @@ public class ConnectivityManager { * changes capabilities but still satisfies the stated need. * * @param network The {@link Network} whose capabilities have changed. - * @param networkCapabilities The new {@link android.net.NetworkCapabilities} for this network. + * @param networkCapabilities The new {@link android.net.NetworkCapabilities} for this + * network. */ public void onCapabilitiesChanged(Network network, NetworkCapabilities networkCapabilities) {} @@ -2743,7 +2770,7 @@ public class ConnectivityManager { /** * Called when the network the framework connected to for this request - * goes into {@link NetworkInfo.DetailedState.SUSPENDED}. + * goes into {@link NetworkInfo.State#SUSPENDED}. * This generally means that while the TCP connections are still live, * temporarily network data fails to transfer. Specifically this is used * on cellular networks to mask temporary outages when driving through @@ -2754,9 +2781,8 @@ public class ConnectivityManager { /** * Called when the network the framework connected to for this request - * returns from a {@link NetworkInfo.DetailedState.SUSPENDED} state. - * This should always be preceeded by a matching {@code onNetworkSuspended} - * call. + * returns from a {@link NetworkInfo.State#SUSPENDED} state. This should always be + * preceded by a matching {@link NetworkCallback#onNetworkSuspended} call. * @hide */ public void onNetworkResumed(Network network) {} @@ -2865,7 +2891,9 @@ public class ConnectivityManager { break; } case CALLBACK_AVAILABLE: { - callback.onAvailable(network); + NetworkCapabilities cap = getObject(message, NetworkCapabilities.class); + LinkProperties lp = getObject(message, LinkProperties.class); + callback.onAvailable(network, cap, lp); break; } case CALLBACK_LOSING: { diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index e81ed9a21c..bc4d9555c9 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -116,6 +116,7 @@ public final class NetworkCapabilities implements Parcelable { NET_CAPABILITY_NOT_ROAMING, NET_CAPABILITY_FOREGROUND, NET_CAPABILITY_NOT_CONGESTED, + NET_CAPABILITY_NOT_SUSPENDED, }) public @interface NetCapability { } @@ -239,7 +240,6 @@ public final class NetworkCapabilities implements Parcelable { /** * Indicates that this network is available for use by apps, and not a network that is being * kept up in the background to facilitate fast network switching. - * @hide */ public static final int NET_CAPABILITY_FOREGROUND = 19; @@ -252,8 +252,20 @@ public final class NetworkCapabilities implements Parcelable { */ public static final int NET_CAPABILITY_NOT_CONGESTED = 20; + /** + * Indicates that this network is not currently suspended. + *

+ * When a network is suspended, the network's IP addresses and any connections + * established on the network remain valid, but the network is temporarily unable + * to transfer data. This can happen, for example, if a cellular network experiences + * a temporary loss of signal, such as when driving through a tunnel, etc. + * A network with this capability is not suspended, so is expected to be able to + * transfer data. + */ + public static final int NET_CAPABILITY_NOT_SUSPENDED = 21; + private static final int MIN_NET_CAPABILITY = NET_CAPABILITY_MMS; - private static final int MAX_NET_CAPABILITY = NET_CAPABILITY_NOT_CONGESTED; + private static final int MAX_NET_CAPABILITY = NET_CAPABILITY_NOT_SUSPENDED; /** * Network capabilities that are expected to be mutable, i.e., can change while a particular @@ -262,12 +274,13 @@ public final class NetworkCapabilities implements Parcelable { private static final long MUTABLE_CAPABILITIES = // TRUSTED can change when user explicitly connects to an untrusted network in Settings. // http://b/18206275 - (1 << NET_CAPABILITY_TRUSTED) | - (1 << NET_CAPABILITY_VALIDATED) | - (1 << NET_CAPABILITY_CAPTIVE_PORTAL) | - (1 << NET_CAPABILITY_NOT_ROAMING) | - (1 << NET_CAPABILITY_FOREGROUND) | - (1 << NET_CAPABILITY_NOT_CONGESTED); + (1 << NET_CAPABILITY_TRUSTED) + | (1 << NET_CAPABILITY_VALIDATED) + | (1 << NET_CAPABILITY_CAPTIVE_PORTAL) + | (1 << NET_CAPABILITY_NOT_ROAMING) + | (1 << NET_CAPABILITY_FOREGROUND) + | (1 << NET_CAPABILITY_NOT_CONGESTED) + | (1 << NET_CAPABILITY_NOT_SUSPENDED); /** * Network capabilities that are not allowed in NetworkRequests. This exists because the @@ -1276,6 +1289,7 @@ public final class NetworkCapabilities implements Parcelable { case NET_CAPABILITY_NOT_ROAMING: return "NOT_ROAMING"; case NET_CAPABILITY_FOREGROUND: return "FOREGROUND"; case NET_CAPABILITY_NOT_CONGESTED: return "NOT_CONGESTED"; + case NET_CAPABILITY_NOT_SUSPENDED: return "NOT_SUSPENDED"; default: return Integer.toString(capability); } } diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index dc70347035..fd2ef18d62 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -30,6 +30,7 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_METERED; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING; +import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VPN; import static android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED; import static android.net.NetworkCapabilities.TRANSPORT_VPN; @@ -4503,10 +4504,12 @@ public class ConnectivityService extends IConnectivityManager.Stub lp.ensureDirectlyConnectedRoutes(); // TODO: Instead of passing mDefaultRequest, provide an API to determine whether a Network // satisfies mDefaultRequest. + final NetworkCapabilities nc = new NetworkCapabilities(networkCapabilities); final NetworkAgentInfo nai = new NetworkAgentInfo(messenger, new AsyncChannel(), - new Network(reserveNetId()), new NetworkInfo(networkInfo), lp, - new NetworkCapabilities(networkCapabilities), currentScore, + new Network(reserveNetId()), new NetworkInfo(networkInfo), lp, nc, currentScore, mContext, mTrackerHandler, new NetworkMisc(networkMisc), mDefaultRequest, this); + // Make sure the network capabilities reflect what the agent info says. + nai.networkCapabilities = mixInCapabilities(nai, nc); synchronized (this) { nai.networkMonitor.systemReady = mSystemReady; } @@ -4735,6 +4738,11 @@ public class ConnectivityService extends IConnectivityManager.Stub } else { newNc.addCapability(NET_CAPABILITY_FOREGROUND); } + if (nai.isSuspended()) { + newNc.removeCapability(NET_CAPABILITY_NOT_SUSPENDED); + } else { + newNc.addCapability(NET_CAPABILITY_NOT_SUSPENDED); + } return newNc; } @@ -4928,6 +4936,11 @@ public class ConnectivityService extends IConnectivityManager.Stub putParcelable(bundle, networkAgent.network); } switch (notificationType) { + case ConnectivityManager.CALLBACK_AVAILABLE: { + putParcelable(bundle, new NetworkCapabilities(networkAgent.networkCapabilities)); + putParcelable(bundle, new LinkProperties(networkAgent.linkProperties)); + break; + } case ConnectivityManager.CALLBACK_LOSING: { msg.arg1 = arg1; break; @@ -5468,6 +5481,10 @@ public class ConnectivityService extends IConnectivityManager.Stub if (networkAgent.getCurrentScore() != oldScore) { rematchAllNetworksAndRequests(networkAgent, oldScore); } + updateCapabilities(networkAgent.getCurrentScore(), networkAgent, + networkAgent.networkCapabilities); + // TODO (b/73132094) : remove this call once the few users of onSuspended and + // onResumed have been removed. notifyNetworkCallbacks(networkAgent, (state == NetworkInfo.State.SUSPENDED ? ConnectivityManager.CALLBACK_SUSPENDED : ConnectivityManager.CALLBACK_RESUMED)); @@ -5504,14 +5521,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } callCallbackForRequest(nri, nai, ConnectivityManager.CALLBACK_AVAILABLE, 0); - // Whether a network is currently suspended is also an important - // element of state to be transferred (it would not otherwise be - // delivered by any currently available mechanism). - if (nai.networkInfo.getState() == NetworkInfo.State.SUSPENDED) { - callCallbackForRequest(nri, nai, ConnectivityManager.CALLBACK_SUSPENDED, 0); - } - callCallbackForRequest(nri, nai, ConnectivityManager.CALLBACK_CAP_CHANGED, 0); - callCallbackForRequest(nri, nai, ConnectivityManager.CALLBACK_IP_CHANGED, 0); } private void sendLegacyNetworkBroadcast(NetworkAgentInfo nai, DetailedState state, int type) { diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index 85b70ca0ff..a24f97e535 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -392,6 +392,15 @@ public class NetworkAgentInfo implements Comparable { return !isVPN() && numForegroundNetworkRequests() == 0 && mNumBackgroundNetworkRequests > 0; } + /** + * Returns whether this network is currently suspended. A network is suspended if it is still + * connected but data temporarily fails to transfer. See {@link NetworkInfo.State#SUSPENDED} + * and {@link NetworkCapabilities#NET_CAPABILITY_NOT_SUSPENDED}. + */ + public boolean isSuspended() { + return networkInfo.getState() == NetworkInfo.State.SUSPENDED; + } + // Does this network satisfy request? public boolean satisfies(NetworkRequest request) { return created && @@ -458,7 +467,7 @@ public class NetworkAgentInfo implements Comparable { public NetworkState getNetworkState() { synchronized (this) { - // Network objects are outwardly immutable so there is no point to duplicating. + // Network objects are outwardly immutable so there is no point in duplicating. // Duplicating also precludes sharing socket factories and connection pools. final String subscriberId = (networkMisc != null) ? networkMisc.subscriberId : null; return new NetworkState(new NetworkInfo(networkInfo), diff --git a/tests/net/java/android/net/ConnectivityManagerTest.java b/tests/net/java/android/net/ConnectivityManagerTest.java index cc792cc749..03a617c354 100644 --- a/tests/net/java/android/net/ConnectivityManagerTest.java +++ b/tests/net/java/android/net/ConnectivityManagerTest.java @@ -38,6 +38,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyBoolean; import static org.mockito.Mockito.anyInt; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.timeout; @@ -217,7 +218,8 @@ public class ConnectivityManagerTest { // callback triggers captor.getValue().send(makeMessage(request, ConnectivityManager.CALLBACK_AVAILABLE)); - verify(callback, timeout(500).times(1)).onAvailable(any()); + verify(callback, timeout(500).times(1)).onAvailable(any(Network.class), + any(NetworkCapabilities.class), any(LinkProperties.class)); // unregister callback manager.unregisterNetworkCallback(callback); @@ -244,7 +246,8 @@ public class ConnectivityManagerTest { // callback triggers captor.getValue().send(makeMessage(req1, ConnectivityManager.CALLBACK_AVAILABLE)); - verify(callback, timeout(100).times(1)).onAvailable(any()); + verify(callback, timeout(100).times(1)).onAvailable(any(Network.class), + any(NetworkCapabilities.class), any(LinkProperties.class)); // unregister callback manager.unregisterNetworkCallback(callback); @@ -335,6 +338,10 @@ public class ConnectivityManagerTest { static Message makeMessage(NetworkRequest req, int messageType) { Bundle bundle = new Bundle(); bundle.putParcelable(NetworkRequest.class.getSimpleName(), req); + // Pass default objects as we don't care which get passed here + bundle.putParcelable(Network.class.getSimpleName(), new Network(1)); + bundle.putParcelable(NetworkCapabilities.class.getSimpleName(), new NetworkCapabilities()); + bundle.putParcelable(LinkProperties.class.getSimpleName(), new LinkProperties()); Message msg = Message.obtain(); msg.what = messageType; msg.setData(bundle); diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 2ec19bf754..24639e9e3f 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -35,6 +35,7 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; import static android.net.NetworkCapabilities.NET_CAPABILITY_MMS; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_METERED; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED; +import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VPN; import static android.net.NetworkCapabilities.NET_CAPABILITY_RCS; import static android.net.NetworkCapabilities.NET_CAPABILITY_SUPL; @@ -528,6 +529,11 @@ public class ConnectivityServiceTest { mNetworkAgent.sendNetworkInfo(mNetworkInfo); } + public void resume() { + mNetworkInfo.setDetailedState(DetailedState.CONNECTED, null, null); + mNetworkAgent.sendNetworkInfo(mNetworkInfo); + } + public void disconnect() { mNetworkInfo.setDetailedState(DetailedState.DISCONNECTED, null, null); mNetworkAgent.sendNetworkInfo(mNetworkInfo); @@ -569,6 +575,10 @@ public class ConnectivityServiceTest { assertTrue(mNetworkStatusReceived.block(TIMEOUT_MS)); return mRedirectUrl; } + + public NetworkCapabilities getNetworkCapabilities() { + return mNetworkCapabilities; + } } /** @@ -1273,6 +1283,7 @@ public class ConnectivityServiceTest { NETWORK_CAPABILITIES, LINK_PROPERTIES, SUSPENDED, + RESUMED, LOSING, LOST, UNAVAILABLE @@ -1343,6 +1354,11 @@ public class ConnectivityServiceTest { setLastCallback(CallbackState.SUSPENDED, network, null); } + @Override + public void onNetworkResumed(Network network) { + setLastCallback(CallbackState.RESUMED, network, null); + } + @Override public void onLosing(Network network, int maxMsToLive) { setLastCallback(CallbackState.LOSING, network, maxMsToLive /* autoboxed int */); @@ -2459,16 +2475,31 @@ public class ConnectivityServiceTest { // Suspend the network. mCellNetworkAgent.suspend(); + cellNetworkCallback.expectCapabilitiesWithout(NET_CAPABILITY_NOT_SUSPENDED, + mCellNetworkAgent); cellNetworkCallback.expectCallback(CallbackState.SUSPENDED, mCellNetworkAgent); cellNetworkCallback.assertNoCallback(); // Register a garden variety default network request. - final TestNetworkCallback dfltNetworkCallback = new TestNetworkCallback(); + TestNetworkCallback dfltNetworkCallback = new TestNetworkCallback(); mCm.registerDefaultNetworkCallback(dfltNetworkCallback); // We should get onAvailable(), onCapabilitiesChanged(), onLinkPropertiesChanged(), // as well as onNetworkSuspended() in rapid succession. dfltNetworkCallback.expectAvailableAndSuspendedCallbacks(mCellNetworkAgent, true); dfltNetworkCallback.assertNoCallback(); + mCm.unregisterNetworkCallback(dfltNetworkCallback); + + mCellNetworkAgent.resume(); + cellNetworkCallback.expectCapabilitiesWith(NET_CAPABILITY_NOT_SUSPENDED, + mCellNetworkAgent); + cellNetworkCallback.expectCallback(CallbackState.RESUMED, mCellNetworkAgent); + cellNetworkCallback.assertNoCallback(); + + dfltNetworkCallback = new TestNetworkCallback(); + mCm.registerDefaultNetworkCallback(dfltNetworkCallback); + // This time onNetworkSuspended should not be called. + dfltNetworkCallback.expectAvailableCallbacksValidated(mCellNetworkAgent); + dfltNetworkCallback.assertNoCallback(); mCm.unregisterNetworkCallback(dfltNetworkCallback); mCm.unregisterNetworkCallback(cellNetworkCallback); From 52e239618b1275b7facc88c1910f99d4b5043db5 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Sat, 10 Feb 2018 05:33:50 +0900 Subject: [PATCH 4/6] Deprecate CONNECTIVITY_ACTION. That was its destiny. Use NetworkCallbacks instead. Test: runtest frameworks-net, but this is only doc changes Change-Id: I3d68dbf817de92c66d899a7cc4519c5639e4c049 --- core/java/android/net/ConnectivityManager.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 6848ae1ab7..2e701329db 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -112,8 +112,14 @@ public class ConnectivityManager { *

* For a disconnect event, the boolean extra EXTRA_NO_CONNECTIVITY * is set to {@code true} if there are no connected networks at all. + * + * @deprecated apps should use the more versatile {@link #requestNetwork}, + * {@link #registerNetworkCallback} or {@link #registerDefaultNetworkCallback} + * functions instead for faster and more detailed updates about the network + * changes they care about. */ @SdkConstant(SdkConstantType.BROADCAST_INTENT_ACTION) + @Deprecated public static final String CONNECTIVITY_ACTION = "android.net.conn.CONNECTIVITY_CHANGE"; /** From b7ca6bf4c1fb26588f1ce557e238e5bbae317a8c Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Thu, 15 Feb 2018 18:47:49 -0800 Subject: [PATCH 5/6] Correct bug in IpSecTransformTest testCreateTransformsWithSameConfigEqual used assertFalse rather than assertTrue Bug: 69385347 Test: Passing on walleye Change-Id: I8caa26e184e8bfc3e8acc9061d85c22d27ebf448 --- tests/net/java/android/net/IpSecTransformTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/net/java/android/net/IpSecTransformTest.java b/tests/net/java/android/net/IpSecTransformTest.java index b4342df585..ffd1f063e4 100644 --- a/tests/net/java/android/net/IpSecTransformTest.java +++ b/tests/net/java/android/net/IpSecTransformTest.java @@ -17,6 +17,7 @@ package android.net; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import android.support.test.filters.SmallTest; @@ -56,6 +57,6 @@ public class IpSecTransformTest { IpSecTransform config1 = new IpSecTransform(null, config); IpSecTransform config2 = new IpSecTransform(null, config); - assertFalse(IpSecTransform.equals(config1, config2)); + assertTrue(IpSecTransform.equals(config1, config2)); } } From af8f5f5ebc950ae83e0d65d4d9cd23a06e57ab1f Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 14 Feb 2018 22:29:11 -0700 Subject: [PATCH 6/6] Better handling of NTP-based clocks. Now that we have a nice Clock abstraction, we can use it to represent a clock backed by an NTP fix. (This makes testing logic much easier to write.) We now rely completely on NetworkTimeUpdateService to keep our NTP fix up to date, instead of trying to refresh in the middle of critical paths which could trigger random ANRs. Add internal FallbackClock to make it easier to handle missing NTP fixes. Add internal SimpleClock to let implementers focus on single millis() method. Test: bit FrameworksNetTests:com.android.server.net.NetworkStatsServiceTest Test: bit FrameworksServicesTests:com.android.server.NetworkPolicyManagerServiceTest Bug: 69714690, 72320957 Change-Id: Ic32cdcbe093d08b73b0e4b23d6910b23ea8e1968 Exempt-From-Owner-Approval: approved in previous PS --- .../server/net/NetworkStatsServiceTest.java | 65 ++++--------------- 1 file changed, 12 insertions(+), 53 deletions(-) diff --git a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java index 0f26edb280..b1b05e8b86 100644 --- a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java @@ -87,12 +87,11 @@ import android.os.Looper; import android.os.Message; import android.os.Messenger; import android.os.PowerManager; +import android.os.SimpleClock; import android.support.test.InstrumentationRegistry; -import android.support.test.runner.AndroidJUnit4; import android.support.test.filters.SmallTest; +import android.support.test.runner.AndroidJUnit4; import android.telephony.TelephonyManager; -import android.util.Log; -import android.util.TrustedTime; import com.android.internal.net.VpnInfo; import com.android.internal.util.test.BroadcastInterceptingContext; @@ -111,6 +110,8 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import java.io.File; +import java.time.Clock; +import java.time.ZoneOffset; import java.util.Objects; /** @@ -155,7 +156,6 @@ public class NetworkStatsServiceTest { private File mStatsDir; private @Mock INetworkManagementService mNetManager; - private @Mock TrustedTime mTime; private @Mock NetworkStatsSettings mSettings; private @Mock IConnectivityManager mConnManager; private @Mock IBinder mBinder; @@ -167,6 +167,13 @@ public class NetworkStatsServiceTest { private INetworkStatsSession mSession; private INetworkManagementEventObserver mNetworkObserver; + private final Clock mClock = new SimpleClock(ZoneOffset.UTC) { + @Override + public long millis() { + return currentTimeMillis(); + } + }; + @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); @@ -184,7 +191,7 @@ public class NetworkStatsServiceTest { powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, TAG); mService = new NetworkStatsService( - mServiceContext, mNetManager, mAlarmManager, wakeLock, mTime, + mServiceContext, mNetManager, mAlarmManager, wakeLock, mClock, TelephonyManager.getDefault(), mSettings, new NetworkStatsObservers(), mStatsDir, getBaseDir(mStatsDir)); mHandlerThread = new HandlerThread("HandlerThread"); @@ -196,7 +203,6 @@ public class NetworkStatsServiceTest { mElapsedRealtime = 0L; - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsUidDetail(buildEmptyStats()); expectSystemReady(); @@ -221,7 +227,6 @@ public class NetworkStatsServiceTest { mStatsDir = null; mNetManager = null; - mTime = null; mSettings = null; mConnManager = null; @@ -233,7 +238,6 @@ public class NetworkStatsServiceTest { public void testNetworkStatsWifi() throws Exception { // pretend that wifi network comes online; service should ask about full // network state, and poll any existing interfaces before updating. - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildWifiState()); expectNetworkStatsSummary(buildEmptyStats()); @@ -248,7 +252,6 @@ public class NetworkStatsServiceTest { // modify some number on wifi, and trigger poll event incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 1024L, 1L, 2048L, 2L)); @@ -262,7 +265,6 @@ public class NetworkStatsServiceTest { // and bump forward again, with counters going higher. this is // important, since polling should correctly subtract last snapshot. incrementCurrentTime(DAY_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 4096L, 4L, 8192L, 8L)); @@ -280,7 +282,6 @@ public class NetworkStatsServiceTest { // pretend that wifi network comes online; service should ask about full // network state, and poll any existing interfaces before updating. - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildWifiState()); expectNetworkStatsSummary(buildEmptyStats()); @@ -295,7 +296,6 @@ public class NetworkStatsServiceTest { // modify some number on wifi, and trigger poll event incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 1024L, 8L, 2048L, 16L)); @@ -324,13 +324,11 @@ public class NetworkStatsServiceTest { // graceful shutdown system, which should trigger persist of stats, and // clear any values in memory. - expectCurrentTime(); expectDefaultSettings(); mServiceContext.sendBroadcast(new Intent(Intent.ACTION_SHUTDOWN)); assertStatsFilesExist(true); // boot through serviceReady() again - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsUidDetail(buildEmptyStats()); expectSystemReady(); @@ -358,7 +356,6 @@ public class NetworkStatsServiceTest { // pretend that wifi network comes online; service should ask about full // network state, and poll any existing interfaces before updating. - expectCurrentTime(); expectSettings(0L, HOUR_IN_MILLIS, WEEK_IN_MILLIS); expectNetworkState(buildWifiState()); expectNetworkStatsSummary(buildEmptyStats()); @@ -370,7 +367,6 @@ public class NetworkStatsServiceTest { // modify some number on wifi, and trigger poll event incrementCurrentTime(2 * HOUR_IN_MILLIS); - expectCurrentTime(); expectSettings(0L, HOUR_IN_MILLIS, WEEK_IN_MILLIS); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 512L, 4L, 512L, 4L)); @@ -386,7 +382,6 @@ public class NetworkStatsServiceTest { // now change bucket duration setting and trigger another poll with // exact same values, which should resize existing buckets. - expectCurrentTime(); expectSettings(0L, 30 * MINUTE_IN_MILLIS, WEEK_IN_MILLIS); expectNetworkStatsSummary(buildEmptyStats()); expectNetworkStatsUidDetail(buildEmptyStats()); @@ -403,7 +398,6 @@ public class NetworkStatsServiceTest { @Test public void testUidStatsAcrossNetworks() throws Exception { // pretend first mobile network comes online - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildMobile3gState(IMSI_1)); expectNetworkStatsSummary(buildEmptyStats()); @@ -415,7 +409,6 @@ public class NetworkStatsServiceTest { // create some traffic on first network incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 2048L, 16L, 512L, 4L)); @@ -437,7 +430,6 @@ public class NetworkStatsServiceTest { // now switch networks; this also tests that we're okay with interfaces // disappearing, to verify we don't count backwards. incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildMobile3gState(IMSI_2)); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) @@ -454,7 +446,6 @@ public class NetworkStatsServiceTest { // create traffic on second network incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 2176L, 17L, 1536L, 12L)); @@ -483,7 +474,6 @@ public class NetworkStatsServiceTest { @Test public void testUidRemovedIsMoved() throws Exception { // pretend that network comes online - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildWifiState()); expectNetworkStatsSummary(buildEmptyStats()); @@ -495,7 +485,6 @@ public class NetworkStatsServiceTest { // create some traffic incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 4128L, 258L, 544L, 34L)); @@ -517,7 +506,6 @@ public class NetworkStatsServiceTest { // now pretend two UIDs are uninstalled, which should migrate stats to // special "removed" bucket. - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 4128L, 258L, 544L, 34L)); @@ -545,7 +533,6 @@ public class NetworkStatsServiceTest { @Test public void testUid3g4gCombinedByTemplate() throws Exception { // pretend that network comes online - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildMobile3gState(IMSI_1)); expectNetworkStatsSummary(buildEmptyStats()); @@ -557,7 +544,6 @@ public class NetworkStatsServiceTest { // create some traffic incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 1) @@ -573,7 +559,6 @@ public class NetworkStatsServiceTest { // now switch over to 4g network incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildMobile4gState(TEST_IFACE2)); expectNetworkStatsSummary(buildEmptyStats()); @@ -588,7 +573,6 @@ public class NetworkStatsServiceTest { // create traffic on second network incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 1) @@ -607,7 +591,6 @@ public class NetworkStatsServiceTest { @Test public void testSummaryForAllUid() throws Exception { // pretend that network comes online - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildWifiState()); expectNetworkStatsSummary(buildEmptyStats()); @@ -619,7 +602,6 @@ public class NetworkStatsServiceTest { // create some traffic for two apps incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 1) @@ -637,7 +619,6 @@ public class NetworkStatsServiceTest { // now create more traffic in next hour, but only for one app incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 1) @@ -669,7 +650,6 @@ public class NetworkStatsServiceTest { @Test public void testForegroundBackground() throws Exception { // pretend that network comes online - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildWifiState()); expectNetworkStatsSummary(buildEmptyStats()); @@ -681,7 +661,6 @@ public class NetworkStatsServiceTest { // create some initial traffic incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 1) @@ -697,7 +676,6 @@ public class NetworkStatsServiceTest { // now switch to foreground incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 1) @@ -730,7 +708,6 @@ public class NetworkStatsServiceTest { @Test public void testMetered() throws Exception { // pretend that network comes online - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildWifiState(true /* isMetered */)); expectNetworkStatsSummary(buildEmptyStats()); @@ -742,7 +719,6 @@ public class NetworkStatsServiceTest { // create some initial traffic incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); // Note that all traffic from NetworkManagementService is tagged as METERED_NO, ROAMING_NO @@ -772,7 +748,6 @@ public class NetworkStatsServiceTest { @Test public void testRoaming() throws Exception { // pretend that network comes online - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildMobile3gState(IMSI_1, true /* isRoaming */)); expectNetworkStatsSummary(buildEmptyStats()); @@ -784,7 +759,6 @@ public class NetworkStatsServiceTest { // Create some traffic incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); // Note that all traffic from NetworkManagementService is tagged as METERED_NO and @@ -813,7 +787,6 @@ public class NetworkStatsServiceTest { @Test public void testTethering() throws Exception { // pretend first mobile network comes online - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildMobile3gState(IMSI_1)); expectNetworkStatsSummary(buildEmptyStats()); @@ -825,7 +798,6 @@ public class NetworkStatsServiceTest { // create some tethering traffic incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); // Traffic seen by kernel counters (includes software tethering). @@ -858,7 +830,6 @@ public class NetworkStatsServiceTest { public void testRegisterUsageCallback() throws Exception { // pretend that wifi network comes online; service should ask about full // network state, and poll any existing interfaces before updating. - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildWifiState()); expectNetworkStatsSummary(buildEmptyStats()); @@ -880,7 +851,6 @@ public class NetworkStatsServiceTest { Messenger messenger = new Messenger(latchedHandler); // Force poll - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); expectNetworkStatsUidDetail(buildEmptyStats()); @@ -909,7 +879,6 @@ public class NetworkStatsServiceTest { // modify some number on wifi, and trigger poll event // not enough traffic to call data usage callback incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 1024L, 1L, 2048L, 2L)); @@ -925,7 +894,6 @@ public class NetworkStatsServiceTest { // and bump forward again, with counters going higher. this is // important, since it will trigger the data usage callback incrementCurrentTime(DAY_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 4096000L, 4L, 8192000L, 8L)); @@ -1068,7 +1036,6 @@ public class NetworkStatsServiceTest { private void expectSettings(long persistBytes, long bucketDuration, long deleteAge) throws Exception { when(mSettings.getPollInterval()).thenReturn(HOUR_IN_MILLIS); - when(mSettings.getTimeCacheMaxAge()).thenReturn(DAY_IN_MILLIS); when(mSettings.getSampleEnabled()).thenReturn(true); final Config config = new Config(bucketDuration, deleteAge, deleteAge); @@ -1084,14 +1051,6 @@ public class NetworkStatsServiceTest { when(mSettings.getUidTagPersistBytes(anyLong())).thenReturn(MB_IN_BYTES); } - private void expectCurrentTime() throws Exception { - when(mTime.forceRefresh()).thenReturn(false); - when(mTime.hasCache()).thenReturn(true); - when(mTime.currentTimeMillis()).thenReturn(currentTimeMillis()); - when(mTime.getCacheAge()).thenReturn(0L); - when(mTime.getCacheCertainty()).thenReturn(0L); - } - private void expectBandwidthControlCheck() throws Exception { when(mNetManager.isBandwidthControlEnabled()).thenReturn(true); }