Merge "Unbreak extraInfo values returned to apps." (78cf2f60) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/ConnectivityService.java

+31 −12

Original line number	Diff line number	Diff line
		@@ -1514,11 +1514,11 @@ public class ConnectivityService extends IConnectivityManager.Stub
		// but only exists if an app asks about them or requests them. Ensure the requesting app
		// gets the type it asks for.
		filtered.setType(type);
		final DetailedState state = isNetworkWithCapabilitiesBlocked(nc, uid, ignoreBlocked)
		? DetailedState.BLOCKED
		: filtered.getDetailedState();
		filtered.setDetailedState(getLegacyLockdownState(state),
		"" /* reason /, null / extraInfo */);
		if (isNetworkWithCapabilitiesBlocked(nc, uid, ignoreBlocked)) {
		filtered.setDetailedState(DetailedState.BLOCKED, null /* reason */,
		null /* extraInfo */);
		}
		filterForLegacyLockdown(filtered);
		return filtered;
		}

		@@ -1594,8 +1594,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
		final DetailedState state = isNetworkWithCapabilitiesBlocked(nc, uid, false)
		? DetailedState.BLOCKED
		: DetailedState.DISCONNECTED;
		info.setDetailedState(getLegacyLockdownState(state),
		"" /* reason /, null / extraInfo */);
		info.setDetailedState(state, null /* reason /, null / extraInfo */);
		filterForLegacyLockdown(info);
		return info;
		}

		@@ -2389,9 +2389,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
		mContext.enforceCallingOrSelfPermission(KeepaliveTracker.PERMISSION, "ConnectivityService");
		}

		// Public because it's used by mLockdownTracker.
		public void sendConnectedBroadcast(NetworkInfo info) {
		PermissionUtils.enforceNetworkStackPermission(mContext);
		private void sendConnectedBroadcast(NetworkInfo info) {
		sendGeneralBroadcast(info, CONNECTIVITY_ACTION);
		}

		@@ -5036,8 +5034,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
		// The legacy lockdown VPN always uses the default network.
		// If the VPN's underlying network is no longer the current default network, it means that
		// the default network has just switched, and the VPN is about to disconnect.
		// Report that the VPN is not connected, so when the state of NetworkInfo objects
		// overwritten by getLegacyLockdownState will be set to CONNECTING and not CONNECTED.
		// Report that the VPN is not connected, so the state of NetworkInfo objects overwritten
		// by filterForLegacyLockdown will be set to CONNECTING and not CONNECTED.
		final NetworkAgentInfo defaultNetwork = getDefaultNetwork();
		if (defaultNetwork == null \|\| !defaultNetwork.network.equals(underlying[0])) {
		return null;
		@@ -5046,6 +5044,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
		return nai;
		};

		// TODO: move all callers to filterForLegacyLockdown and delete this method.
		// This likely requires making sendLegacyNetworkBroadcast take a NetworkInfo object instead of
		// just a DetailedState object.
		private DetailedState getLegacyLockdownState(DetailedState origState) {
		if (origState != DetailedState.CONNECTED) {
		return origState;
		@@ -5055,6 +5056,23 @@ public class ConnectivityService extends IConnectivityManager.Stub
		: DetailedState.CONNECTED;
		}

		private void filterForLegacyLockdown(NetworkInfo ni) {
		if (!mLockdownEnabled \|\| !ni.isConnected()) return;
		// The legacy lockdown VPN replaces the state of every network in CONNECTED state with the
		// state of its VPN. This is to ensure that when an underlying network connects, apps will
		// not see a CONNECTIVITY_ACTION broadcast for a network in state CONNECTED until the VPN
		// comes up, at which point there is a new CONNECTIVITY_ACTION broadcast for the underlying
		// network, this time with a state of CONNECTED.
		//
		// Now that the legacy lockdown code lives in ConnectivityService, and no longer has access
		// to the internal state of the Vpn object, always replace the state with CONNECTING. This
		// is not too far off the truth, since an always-on VPN, when not connected, is always
		// trying to reconnect.
		if (getLegacyLockdownNai() == null) {
		ni.setDetailedState(DetailedState.CONNECTING, "", null);
		}
		}

		@Override
		public void setProvisioningNotificationVisible(boolean visible, int networkType,
		String action) {
		@@ -7938,6 +7956,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
		// and is still connected.
		NetworkInfo info = new NetworkInfo(nai.networkInfo);
		info.setType(type);
		filterForLegacyLockdown(info);
		if (state != DetailedState.DISCONNECTED) {
		info.setDetailedState(state, null, info.getExtraInfo());
		sendConnectedBroadcast(info);

tests/net/java/com/android/server/ConnectivityServiceTest.java

+66 −4

Original line number	Diff line number	Diff line
		@@ -1740,11 +1740,29 @@ public class ConnectivityServiceTest {
		return expected;
		}

		private boolean extraInfoInBroadcastHasExpectedNullness(NetworkInfo ni) {
		final DetailedState state = ni.getDetailedState();
		if (state == DetailedState.CONNECTED && ni.getExtraInfo() == null) return false;
		// Expect a null extraInfo if the network is CONNECTING, because a CONNECTIVITY_ACTION
		// broadcast with a state of CONNECTING only happens due to legacy VPN lockdown, which also
		// nulls out extraInfo.
		if (state == DetailedState.CONNECTING && ni.getExtraInfo() != null) return false;
		// Can't make any assertions about DISCONNECTED broadcasts. When a network actually
		// disconnects, disconnectAndDestroyNetwork sets its state to DISCONNECTED and its extraInfo
		// to null. But if the DISCONNECTED broadcast is just simulated by LegacyTypeTracker due to
		// a network switch, extraInfo will likely be populated.
		// This is likely a bug in CS, but likely not one we can fix without impacting apps.
		return true;
		}

		private ExpectedBroadcast expectConnectivityAction(int type, NetworkInfo.DetailedState state) {
		return registerConnectivityBroadcastThat(1, intent ->
		type == intent.getIntExtra(EXTRA_NETWORK_TYPE, -1) && state.equals(
		((NetworkInfo) intent.getParcelableExtra(EXTRA_NETWORK_INFO))
		.getDetailedState()));
		return registerConnectivityBroadcastThat(1, intent -> {
		final int actualType = intent.getIntExtra(EXTRA_NETWORK_TYPE, -1);
		final NetworkInfo ni = intent.getParcelableExtra(EXTRA_NETWORK_INFO);
		return type == actualType
		&& state == ni.getDetailedState()
		&& extraInfoInBroadcastHasExpectedNullness(ni);
		});
		}

		@Test
		@@ -7182,12 +7200,14 @@ public class ConnectivityServiceTest {
		assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());
		assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
		assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
		assertExtraInfoFromCmPresent(mCellNetworkAgent);

		setUidRulesChanged(RULE_REJECT_ALL);
		cellNetworkCallback.expectBlockedStatusCallback(true, mCellNetworkAgent);
		assertNull(mCm.getActiveNetwork());
		assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
		assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
		assertExtraInfoFromCmBlocked(mCellNetworkAgent);

		// ConnectivityService should cache it not to invoke the callback again.
		setUidRulesChanged(RULE_REJECT_METERED);
		@@ -7198,12 +7218,14 @@ public class ConnectivityServiceTest {
		assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());
		assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
		assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
		assertExtraInfoFromCmPresent(mCellNetworkAgent);

		setUidRulesChanged(RULE_REJECT_METERED);
		cellNetworkCallback.expectBlockedStatusCallback(true, mCellNetworkAgent);
		assertNull(mCm.getActiveNetwork());
		assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
		assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
		assertExtraInfoFromCmBlocked(mCellNetworkAgent);

		// Restrict the network based on UID rule and NOT_METERED capability change.
		mCellNetworkAgent.addCapability(NET_CAPABILITY_NOT_METERED);
		@@ -7212,6 +7234,7 @@ public class ConnectivityServiceTest {
		assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());
		assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
		assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
		assertExtraInfoFromCmPresent(mCellNetworkAgent);

		mCellNetworkAgent.removeCapability(NET_CAPABILITY_NOT_METERED);
		cellNetworkCallback.expectCapabilitiesWithout(NET_CAPABILITY_NOT_METERED,
		@@ -7220,12 +7243,14 @@ public class ConnectivityServiceTest {
		assertNull(mCm.getActiveNetwork());
		assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
		assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
		assertExtraInfoFromCmBlocked(mCellNetworkAgent);

		setUidRulesChanged(RULE_ALLOW_METERED);
		cellNetworkCallback.expectBlockedStatusCallback(false, mCellNetworkAgent);
		assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());
		assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
		assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
		assertExtraInfoFromCmPresent(mCellNetworkAgent);

		setUidRulesChanged(RULE_NONE);
		cellNetworkCallback.assertNoCallback();
		@@ -7236,6 +7261,7 @@ public class ConnectivityServiceTest {
		assertNull(mCm.getActiveNetwork());
		assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
		assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
		assertExtraInfoFromCmBlocked(mCellNetworkAgent);
		setRestrictBackgroundChanged(true);
		cellNetworkCallback.assertNoCallback();

		@@ -7243,12 +7269,14 @@ public class ConnectivityServiceTest {
		cellNetworkCallback.expectBlockedStatusCallback(false, mCellNetworkAgent);
		assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
		assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
		assertExtraInfoFromCmPresent(mCellNetworkAgent);

		setRestrictBackgroundChanged(false);
		cellNetworkCallback.assertNoCallback();
		assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());
		assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
		assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
		assertExtraInfoFromCmPresent(mCellNetworkAgent);

		mCm.unregisterNetworkCallback(cellNetworkCallback);
		}
		@@ -7307,6 +7335,15 @@ public class ConnectivityServiceTest {
		assertNotNull(ni);
		assertEquals(type, ni.getType());
		assertEquals(ConnectivityManager.getNetworkTypeName(type), state, ni.getDetailedState());
		if (state == DetailedState.CONNECTED \|\| state == DetailedState.SUSPENDED) {
		assertNotNull(ni.getExtraInfo());
		} else {
		// Technically speaking, a network that's in CONNECTING state will generally have a
		// non-null extraInfo. This doesn't actually happen in this test because it never calls
		// a legacy API while a network is connecting. When a network is in CONNECTING state
		// because of legacy lockdown VPN, its extraInfo is always null.
		assertNull(ni.getExtraInfo());
		}
		}

		private void assertActiveNetworkInfo(int type, DetailedState state) {
		@@ -7316,6 +7353,26 @@ public class ConnectivityServiceTest {
		checkNetworkInfo(mCm.getNetworkInfo(type), type, state);
		}

		private void assertExtraInfoFromCm(TestNetworkAgentWrapper network, boolean present) {
		final NetworkInfo niForNetwork = mCm.getNetworkInfo(network.getNetwork());
		final NetworkInfo niForType = mCm.getNetworkInfo(network.getLegacyType());
		if (present) {
		assertEquals(network.getExtraInfo(), niForNetwork.getExtraInfo());
		assertEquals(network.getExtraInfo(), niForType.getExtraInfo());
		} else {
		assertNull(niForNetwork.getExtraInfo());
		assertNull(niForType.getExtraInfo());
		}
		}

		private void assertExtraInfoFromCmBlocked(TestNetworkAgentWrapper network) {
		assertExtraInfoFromCm(network, false);
		}

		private void assertExtraInfoFromCmPresent(TestNetworkAgentWrapper network) {
		assertExtraInfoFromCm(network, true);
		}

		// Checks that each of the \|agents\| receive a blocked status change callback with the specified
		// \|blocked\| value, in any order. This is needed because when an event affects multiple
		// networks, ConnectivityService does not guarantee the order in which callbacks are fired.
		@@ -7630,6 +7687,7 @@ public class ConnectivityServiceTest {
		assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
		assertNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED);
		assertNetworkInfo(TYPE_VPN, DetailedState.BLOCKED);
		assertExtraInfoFromCmBlocked(mCellNetworkAgent);

		// TODO: it would be nice if we could simply rely on the production code here, and have
		// LockdownVpnTracker start the VPN, have the VPN code register its NetworkAgent with
		@@ -7658,6 +7716,7 @@ public class ConnectivityServiceTest {
		assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
		assertNetworkInfo(TYPE_WIFI, DetailedState.DISCONNECTED);
		assertNetworkInfo(TYPE_VPN, DetailedState.CONNECTED);
		assertExtraInfoFromCmPresent(mCellNetworkAgent);
		assertTrue(vpnNc.hasTransport(TRANSPORT_VPN));
		assertTrue(vpnNc.hasTransport(TRANSPORT_CELLULAR));
		assertFalse(vpnNc.hasTransport(TRANSPORT_WIFI));
		@@ -7700,6 +7759,7 @@ public class ConnectivityServiceTest {
		assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
		assertNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED);
		assertNetworkInfo(TYPE_VPN, DetailedState.BLOCKED);
		assertExtraInfoFromCmBlocked(mWiFiNetworkAgent);

		// The VPN comes up again on wifi.
		b1 = expectConnectivityAction(TYPE_VPN, DetailedState.CONNECTED);
		@@ -7714,6 +7774,7 @@ public class ConnectivityServiceTest {
		assertNetworkInfo(TYPE_MOBILE, DetailedState.DISCONNECTED);
		assertNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED);
		assertNetworkInfo(TYPE_VPN, DetailedState.CONNECTED);
		assertExtraInfoFromCmPresent(mWiFiNetworkAgent);
		vpnNc = mCm.getNetworkCapabilities(mMockVpn.getNetwork());
		assertTrue(vpnNc.hasTransport(TRANSPORT_VPN));
		assertTrue(vpnNc.hasTransport(TRANSPORT_WIFI));
		@@ -7730,6 +7791,7 @@ public class ConnectivityServiceTest {
		assertNetworkInfo(TYPE_MOBILE, DetailedState.DISCONNECTED);
		assertNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED);
		assertNetworkInfo(TYPE_VPN, DetailedState.CONNECTED);
		assertExtraInfoFromCmPresent(mWiFiNetworkAgent);

		b1 = expectConnectivityAction(TYPE_WIFI, DetailedState.DISCONNECTED);
		mWiFiNetworkAgent.disconnect();