Stop using NetworkState internally in ConnectivityService.
NetworkState is used by many methods that take or return legacy
network types. It is used because it contains most of the state
related to a network.
This code is not easy to follow and is more expensive than it
needs to be: most of the methods that construct a NetworkState
only really need one of its members (e.g., LinkProperties, or
NetworkInfo), but constructing a NetworkState requires making
defensive copies of all its other members as well.
- Instead of using NetworkState, use NetworkAgentInfo, which
already contains all the same data.
- Replace calls to getUnfilteredActiveNetworkState with calls to
getNetworkAgentInfoForUid. When getUnfilteredActiveNetworkState
returned NetworkState.EMPTY, return a null nai, which causes
any caller to see return null LinkProperties/NetworkInfo/etc.
- Rename filterNetworkStateForUid to getFilteredNetworkInfo,
because that's the only thing it actually filters.
- Rename getFilteredNetworkInfo to getFilteredNetworkInfoForType
to avoid having two methods with the same name perform two
different operations. That method was only introduced recently
in aosp/1552503, so it's probably fine to rename it.
Bug: 174123988
Test: passes existing ConnectivityServiceTest
Change-Id: Idfb5e149967266a442b268de6f13a521884dbb8f
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index ebd483c..93fab6e 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -1403,7 +1403,7 @@
return null;
}
- private NetworkState getUnfilteredActiveNetworkState(int uid) {
+ private NetworkAgentInfo getNetworkAgentInfoForUid(int uid) {
NetworkAgentInfo nai = getDefaultNetworkForUid(uid);
final Network[] networks = getVpnUnderlyingNetworks(uid);
@@ -1419,12 +1419,7 @@
nai = null;
}
}
-
- if (nai != null) {
- return nai.getNetworkState();
- } else {
- return NetworkState.EMPTY;
- }
+ return nai;
}
/**
@@ -1477,24 +1472,28 @@
"%s %d(%d) on netId %d", action, nri.mUid, requestId, net.getNetId()));
}
- private void filterNetworkInfo(@NonNull NetworkInfo networkInfo,
- @NonNull NetworkCapabilities nc, int uid, boolean ignoreBlocked) {
- if (isNetworkWithCapabilitiesBlocked(nc, uid, ignoreBlocked)) {
- networkInfo.setDetailedState(DetailedState.BLOCKED, null, null);
- }
- networkInfo.setDetailedState(
- getLegacyLockdownState(networkInfo.getDetailedState()),
- "" /* reason */, null /* extraInfo */);
- }
-
/**
- * Apply any relevant filters to {@link NetworkState} for the given UID. For
+ * Apply any relevant filters to the specified {@link NetworkInfo} for the given UID. For
* example, this may mark the network as {@link DetailedState#BLOCKED} based
* on {@link #isNetworkWithCapabilitiesBlocked}.
*/
- private void filterNetworkStateForUid(NetworkState state, int uid, boolean ignoreBlocked) {
- if (state == null || state.networkInfo == null || state.linkProperties == null) return;
- filterNetworkInfo(state.networkInfo, state.networkCapabilities, uid, ignoreBlocked);
+ @NonNull
+ private NetworkInfo filterNetworkInfo(@NonNull NetworkInfo networkInfo, int type,
+ @NonNull NetworkCapabilities nc, int uid, boolean ignoreBlocked) {
+ NetworkInfo filtered = new NetworkInfo(networkInfo);
+ filtered.setType(type);
+ final DetailedState state = isNetworkWithCapabilitiesBlocked(nc, uid, ignoreBlocked)
+ ? DetailedState.BLOCKED
+ : filtered.getDetailedState();
+ filtered.setDetailedState(getLegacyLockdownState(state),
+ "" /* reason */, null /* extraInfo */);
+ return filtered;
+ }
+
+ private NetworkInfo getFilteredNetworkInfo(NetworkAgentInfo nai, int uid,
+ boolean ignoreBlocked) {
+ return filterNetworkInfo(nai.networkInfo, nai.networkInfo.getType(),
+ nai.networkCapabilities, uid, ignoreBlocked);
}
/**
@@ -1508,10 +1507,11 @@
public NetworkInfo getActiveNetworkInfo() {
enforceAccessPermission();
final int uid = mDeps.getCallingUid();
- final NetworkState state = getUnfilteredActiveNetworkState(uid);
- filterNetworkStateForUid(state, uid, false);
- maybeLogBlockedNetworkInfo(state.networkInfo, uid);
- return state.networkInfo;
+ final NetworkAgentInfo nai = getNetworkAgentInfoForUid(uid);
+ if (nai == null) return null;
+ final NetworkInfo networkInfo = getFilteredNetworkInfo(nai, uid, false);
+ maybeLogBlockedNetworkInfo(networkInfo, uid);
+ return networkInfo;
}
@Override
@@ -1546,9 +1546,9 @@
@Override
public NetworkInfo getActiveNetworkInfoForUid(int uid, boolean ignoreBlocked) {
PermissionUtils.enforceNetworkStackPermission(mContext);
- final NetworkState state = getUnfilteredActiveNetworkState(uid);
- filterNetworkStateForUid(state, uid, ignoreBlocked);
- return state.networkInfo;
+ final NetworkAgentInfo nai = getNetworkAgentInfoForUid(uid);
+ if (nai == null) return null;
+ return getFilteredNetworkInfo(nai, uid, ignoreBlocked);
}
/** Returns a NetworkInfo object for a network that doesn't exist. */
@@ -1567,7 +1567,7 @@
return info;
}
- private NetworkInfo getFilteredNetworkInfo(int networkType, int uid) {
+ private NetworkInfo getFilteredNetworkInfoForType(int networkType, int uid) {
if (!mLegacyTypeTracker.isTypeSupported(networkType)) {
return null;
}
@@ -1575,10 +1575,8 @@
if (nai == null) {
return makeFakeNetworkInfo(networkType, uid);
}
- final NetworkInfo info = new NetworkInfo(nai.networkInfo);
- info.setType(networkType);
- filterNetworkInfo(info, nai.networkCapabilities, uid, false);
- return info;
+ return filterNetworkInfo(nai.networkInfo, networkType, nai.networkCapabilities, uid,
+ false);
}
@Override
@@ -1588,27 +1586,23 @@
if (getVpnUnderlyingNetworks(uid) != null) {
// A VPN is active, so we may need to return one of its underlying networks. This
// information is not available in LegacyTypeTracker, so we have to get it from
- // getUnfilteredActiveNetworkState.
- final NetworkState state = getUnfilteredActiveNetworkState(uid);
- if (state.networkInfo != null && state.networkInfo.getType() == networkType) {
- filterNetworkStateForUid(state, uid, false);
- return state.networkInfo;
+ // getNetworkAgentInfoForUid.
+ final NetworkAgentInfo nai = getNetworkAgentInfoForUid(uid);
+ if (nai == null) return null;
+ final NetworkInfo networkInfo = getFilteredNetworkInfo(nai, uid, false);
+ if (networkInfo.getType() == networkType) {
+ return networkInfo;
}
}
- return getFilteredNetworkInfo(networkType, uid);
+ return getFilteredNetworkInfoForType(networkType, uid);
}
@Override
public NetworkInfo getNetworkInfoForUid(Network network, int uid, boolean ignoreBlocked) {
enforceAccessPermission();
final NetworkAgentInfo nai = getNetworkAgentInfoForNetwork(network);
- if (nai != null) {
- final NetworkState state = nai.getNetworkState();
- filterNetworkStateForUid(state, uid, ignoreBlocked);
- return state.networkInfo;
- } else {
- return null;
- }
+ if (nai == null) return null;
+ return getFilteredNetworkInfo(nai, uid, ignoreBlocked);
}
@Override
@@ -1636,10 +1630,10 @@
return null;
}
final int uid = mDeps.getCallingUid();
- if (!isNetworkWithCapabilitiesBlocked(nai.networkCapabilities, uid, false)) {
- return nai.network;
+ if (isNetworkWithCapabilitiesBlocked(nai.networkCapabilities, uid, false)) {
+ return null;
}
- return null;
+ return nai.network;
}
@Override
@@ -1728,9 +1722,9 @@
public LinkProperties getActiveLinkProperties() {
enforceAccessPermission();
final int uid = mDeps.getCallingUid();
- NetworkState state = getUnfilteredActiveNetworkState(uid);
- if (state.linkProperties == null) return null;
- return linkPropertiesRestrictedForCallerPermissions(state.linkProperties,
+ NetworkAgentInfo nai = getNetworkAgentInfoForUid(uid);
+ if (nai == null) return null;
+ return linkPropertiesRestrictedForCallerPermissions(nai.linkProperties,
Binder.getCallingPid(), uid);
}