Merge "No-op cleanup"
diff --git a/Tethering/Android.bp b/Tethering/Android.bp
index 495c9d7..e4ce615 100644
--- a/Tethering/Android.bp
+++ b/Tethering/Android.bp
@@ -33,6 +33,7 @@
"NetworkStackApiStableShims",
"androidx.annotation_annotation",
"modules-utils-build",
+ "modules-utils-statemachine",
"networkstack-client",
"android.hardware.tetheroffload.config-V1.0-java",
"android.hardware.tetheroffload.control-V1.0-java",
@@ -41,7 +42,6 @@
"net-utils-device-common",
"net-utils-device-common-netlink",
"netd-client",
- "NetworkStackApiCurrentShims",
],
libs: [
"framework-connectivity",
diff --git a/Tethering/common/TetheringLib/src/android/net/TetheringManager.java b/Tethering/common/TetheringLib/src/android/net/TetheringManager.java
index c137b49..9e6e34e 100644
--- a/Tethering/common/TetheringLib/src/android/net/TetheringManager.java
+++ b/Tethering/common/TetheringLib/src/android/net/TetheringManager.java
@@ -290,6 +290,23 @@
getConnector(c -> c.registerTetheringEventCallback(mCallback, pkgName));
}
+ /** @hide */
+ @Override
+ protected void finalize() throws Throwable {
+ final String pkgName = mContext.getOpPackageName();
+ Log.i(TAG, "unregisterTetheringEventCallback:" + pkgName);
+ // 1. It's generally not recommended to perform long operations in finalize, but while
+ // unregisterTetheringEventCallback does an IPC, it's a oneway IPC so should not block.
+ // 2. If the connector is not yet connected, TetheringManager is impossible to finalize
+ // because the connector polling thread strong reference the TetheringManager object. So
+ // it's guaranteed that registerTetheringEventCallback was already called before calling
+ // unregisterTetheringEventCallback in finalize.
+ if (mConnector == null) Log.wtf(TAG, "null connector in finalize!");
+ getConnector(c -> c.unregisterTetheringEventCallback(mCallback, pkgName));
+
+ super.finalize();
+ }
+
private void startPollingForConnector() {
new Thread(() -> {
while (true) {
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java
index 0052267..f664d5d 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java
@@ -28,6 +28,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.eq;
@@ -46,6 +47,7 @@
import android.net.TetheringRequestParcel;
import android.net.ip.IpServer;
import android.os.Bundle;
+import android.os.ConditionVariable;
import android.os.Handler;
import android.os.IBinder;
import android.os.ResultReceiver;
@@ -63,7 +65,6 @@
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
-import org.mockito.Matchers;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
@@ -503,38 +504,80 @@
});
}
+ private class ConnectorSupplier<T> implements Supplier<T> {
+ private T mResult = null;
+
+ public void set(T result) {
+ mResult = result;
+ }
+
+ @Override
+ public T get() {
+ return mResult;
+ }
+ }
+
+ private void forceGc() {
+ System.gc();
+ System.runFinalization();
+ System.gc();
+ }
+
@Test
public void testTetheringManagerLeak() throws Exception {
runAsAccessNetworkState((none) -> {
final ArrayList<ITetheringEventCallback> callbacks = new ArrayList<>();
+ final ConditionVariable registeredCv = new ConditionVariable(false);
doAnswer((invocation) -> {
final Object[] args = invocation.getArguments();
callbacks.add((ITetheringEventCallback) args[0]);
+ registeredCv.open();
return null;
- }).when(mTethering).registerTetheringEventCallback(Matchers.anyObject());
+ }).when(mTethering).registerTetheringEventCallback(any());
- final Supplier<IBinder> supplier = () -> mMockConnector.getIBinder();
+ doAnswer((invocation) -> {
+ final Object[] args = invocation.getArguments();
+ callbacks.remove((ITetheringEventCallback) args[0]);
+ return null;
+ }).when(mTethering).unregisterTetheringEventCallback(any());
+
+ final ConnectorSupplier<IBinder> supplier = new ConnectorSupplier<>();
TetheringManager tm = new TetheringManager(mMockConnector.getService(), supplier);
assertNotNull(tm);
- assertEquals("Internal callback is not registered", 1, callbacks.size());
+ assertEquals("Internal callback should not be registered", 0, callbacks.size());
- final WeakReference weakTm = new WeakReference(tm);
+ final WeakReference<TetheringManager> weakTm = new WeakReference(tm);
assertNotNull(weakTm.get());
+ // TetheringManager couldn't be GCed because pollingConnector thread implicitly
+ // reference TetheringManager object.
tm = null;
+ forceGc();
+ assertNotNull(weakTm.get());
+
+ // After getting connector, pollingConnector thread stops and internal callback is
+ // registered.
+ supplier.set(mMockConnector.getIBinder());
+ final long timeout = 500L;
+ if (!registeredCv.block(timeout)) {
+ fail("TetheringManager poll connector fail after " + timeout + " ms");
+ }
+ assertEquals("Internal callback is not registered", 1, callbacks.size());
+ assertNotNull(weakTm.get());
+
final int attempts = 100;
final long waitIntervalMs = 50;
for (int i = 0; i < attempts; i++) {
- System.gc();
- System.runFinalization();
- System.gc();
+ forceGc();
if (weakTm.get() == null) break;
Thread.sleep(waitIntervalMs);
}
assertNull("TetheringManager weak reference still not null after " + attempts
+ " attempts", weakTm.get());
+
+ assertEquals("Internal callback is not unregistered", 0, callbacks.size());
});
}
}
diff --git a/framework/src/android/net/ConnectivityManager.java b/framework/src/android/net/ConnectivityManager.java
index 8309869..c21bcfa 100644
--- a/framework/src/android/net/ConnectivityManager.java
+++ b/framework/src/android/net/ConnectivityManager.java
@@ -46,7 +46,6 @@
import android.net.TetheringManager.StartTetheringCallback;
import android.net.TetheringManager.TetheringEventCallback;
import android.net.TetheringManager.TetheringRequest;
-import android.net.wifi.WifiNetworkSuggestion;
import android.os.Binder;
import android.os.Build;
import android.os.Build.VERSION_CODES;
@@ -3479,7 +3478,8 @@
* {@link NetworkCapabilities#getTransportInfo()}) like {@link android.net.wifi.WifiInfo}
* contain location sensitive information.
* <li> OwnerUid (retrieved via {@link NetworkCapabilities#getOwnerUid()} is location
- * sensitive for wifi suggestor apps (i.e using {@link WifiNetworkSuggestion}).</li>
+ * sensitive for wifi suggestor apps (i.e using
+ * {@link android.net.wifi.WifiNetworkSuggestion WifiNetworkSuggestion}).</li>
* </p>
* <p>
* Note:
diff --git a/framework/src/android/net/NetworkCapabilities.java b/framework/src/android/net/NetworkCapabilities.java
index e9bcd95..75f0129 100644
--- a/framework/src/android/net/NetworkCapabilities.java
+++ b/framework/src/android/net/NetworkCapabilities.java
@@ -27,7 +27,6 @@
import android.annotation.SystemApi;
import android.compat.annotation.UnsupportedAppUsage;
import android.net.ConnectivityManager.NetworkCallback;
-import android.net.wifi.WifiNetworkSuggestion;
import android.os.Build;
import android.os.Parcel;
import android.os.Parcelable;
@@ -1192,14 +1191,14 @@
*
* <p>
* This field will only be populated for VPN and wifi network suggestor apps (i.e using
- * {@link WifiNetworkSuggestion}), and only for the network they own.
- * In the case of wifi network suggestors apps, this field is also location sensitive, so the
- * app needs to hold {@link android.Manifest.permission#ACCESS_FINE_LOCATION} permission. If the
- * app targets SDK version greater than or equal to {@link Build.VERSION_CODES#S}, then they
- * also need to use {@link NetworkCallback#FLAG_INCLUDE_LOCATION_INFO} to get the info in their
- * callback. If the apps targets SDK version equal to {{@link Build.VERSION_CODES#R}, this field
- * will always be included. The app will be blamed for location access if this field is
- * included.
+ * {@link android.net.wifi.WifiNetworkSuggestion WifiNetworkSuggestion}), and only for the
+ * network they own. In the case of wifi network suggestors apps, this field is also location
+ * sensitive, so the app needs to hold {@link android.Manifest.permission#ACCESS_FINE_LOCATION}
+ * permission. If the app targets SDK version greater than or equal to
+ * {@link Build.VERSION_CODES#S}, then they also need to use
+ * {@link NetworkCallback#FLAG_INCLUDE_LOCATION_INFO} to get the info in their callback. If the
+ * apps targets SDK version equal to {{@link Build.VERSION_CODES#R}, this field will always be
+ * included. The app will be blamed for location access if this field is included.
* </p>
*/
public int getOwnerUid() {
@@ -2129,14 +2128,17 @@
sb.append(" SubscriptionIds: ").append(mSubIds);
}
- if (mUnderlyingNetworks != null && mUnderlyingNetworks.size() > 0) {
- sb.append(" Underlying networks: [");
+ sb.append(" UnderlyingNetworks: ");
+ if (mUnderlyingNetworks != null) {
+ sb.append("[");
final StringJoiner joiner = new StringJoiner(",");
for (int i = 0; i < mUnderlyingNetworks.size(); i++) {
joiner.add(mUnderlyingNetworks.get(i).toString());
}
sb.append(joiner.toString());
sb.append("]");
+ } else {
+ sb.append("Null");
}
sb.append("]");
diff --git a/framework/src/android/net/NetworkInfo.java b/framework/src/android/net/NetworkInfo.java
index bb23494..433933f 100644
--- a/framework/src/android/net/NetworkInfo.java
+++ b/framework/src/android/net/NetworkInfo.java
@@ -179,21 +179,19 @@
/** {@hide} */
@UnsupportedAppUsage
- public NetworkInfo(NetworkInfo source) {
- if (source != null) {
- synchronized (source) {
- mNetworkType = source.mNetworkType;
- mSubtype = source.mSubtype;
- mTypeName = source.mTypeName;
- mSubtypeName = source.mSubtypeName;
- mState = source.mState;
- mDetailedState = source.mDetailedState;
- mReason = source.mReason;
- mExtraInfo = source.mExtraInfo;
- mIsFailover = source.mIsFailover;
- mIsAvailable = source.mIsAvailable;
- mIsRoaming = source.mIsRoaming;
- }
+ public NetworkInfo(@NonNull NetworkInfo source) {
+ synchronized (source) {
+ mNetworkType = source.mNetworkType;
+ mSubtype = source.mSubtype;
+ mTypeName = source.mTypeName;
+ mSubtypeName = source.mSubtypeName;
+ mState = source.mState;
+ mDetailedState = source.mDetailedState;
+ mReason = source.mReason;
+ mExtraInfo = source.mExtraInfo;
+ mIsFailover = source.mIsFailover;
+ mIsAvailable = source.mIsAvailable;
+ mIsRoaming = source.mIsRoaming;
}
}
@@ -479,7 +477,7 @@
* @param detailedState the {@link DetailedState}.
* @param reason a {@code String} indicating the reason for the state change,
* if one was supplied. May be {@code null}.
- * @param extraInfo an optional {@code String} providing addditional network state
+ * @param extraInfo an optional {@code String} providing additional network state
* information passed up from the lower networking layers.
* @deprecated Use {@link NetworkCapabilities} instead.
*/
@@ -491,6 +489,11 @@
this.mState = stateMap.get(detailedState);
this.mReason = reason;
this.mExtraInfo = extraInfo;
+ // Catch both the case where detailedState is null and the case where it's some
+ // unknown value
+ if (null == mState) {
+ throw new NullPointerException("Unknown DetailedState : " + detailedState);
+ }
}
}
diff --git a/service/Android.bp b/service/Android.bp
index 911d67f..3ff7a7c 100644
--- a/service/Android.bp
+++ b/service/Android.bp
@@ -67,7 +67,8 @@
static_libs: [
"dnsresolver_aidl_interface-V9-java",
"modules-utils-build",
- "modules-utils-os",
+ "modules-utils-shell-command-handler",
+ "modules-utils-statemachine",
"net-utils-device-common",
"net-utils-device-common-netlink",
"net-utils-framework-common",
diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index 5b9256e..6c2f7ee 100644
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -2032,6 +2032,7 @@
if (!checkSettingsPermission(callerPid, callerUid)) {
newNc.setUids(null);
newNc.setSSID(null);
+ newNc.setUnderlyingNetworks(null);
}
if (newNc.getNetworkSpecifier() != null) {
newNc.setNetworkSpecifier(newNc.getNetworkSpecifier().redact());
@@ -7305,7 +7306,9 @@
boolean suspended = true; // suspended if all underlying are suspended
boolean hadUnderlyingNetworks = false;
+ ArrayList<Network> newUnderlyingNetworks = null;
if (null != underlyingNetworks) {
+ newUnderlyingNetworks = new ArrayList<>();
for (Network underlyingNetwork : underlyingNetworks) {
final NetworkAgentInfo underlying =
getNetworkAgentInfoForNetwork(underlyingNetwork);
@@ -7335,6 +7338,7 @@
// If this network is not suspended, the VPN is not suspended (the VPN
// is able to transfer some data).
suspended &= !underlyingCaps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED);
+ newUnderlyingNetworks.add(underlyingNetwork);
}
}
if (!hadUnderlyingNetworks) {
@@ -7352,6 +7356,7 @@
newNc.setCapability(NET_CAPABILITY_NOT_ROAMING, !roaming);
newNc.setCapability(NET_CAPABILITY_NOT_CONGESTED, !congested);
newNc.setCapability(NET_CAPABILITY_NOT_SUSPENDED, !suspended);
+ newNc.setUnderlyingNetworks(newUnderlyingNetworks);
}
/**
diff --git a/service/src/com/android/server/connectivity/NetworkAgentInfo.java b/service/src/com/android/server/connectivity/NetworkAgentInfo.java
index 6426f86..b7f3ed9 100644
--- a/service/src/com/android/server/connectivity/NetworkAgentInfo.java
+++ b/service/src/com/android/server/connectivity/NetworkAgentInfo.java
@@ -377,6 +377,9 @@
this.creatorUid = creatorUid;
mLingerDurationMs = lingerDurationMs;
mQosCallbackTracker = qosCallbackTracker;
+ declaredUnderlyingNetworks = (nc.getUnderlyingNetworks() != null)
+ ? nc.getUnderlyingNetworks().toArray(new Network[0])
+ : null;
}
private class AgentDeathMonitor implements IBinder.DeathRecipient {
diff --git a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java
index 99cfdc2..594000b 100644
--- a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java
+++ b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java
@@ -714,6 +714,12 @@
.build();
}
+ private boolean hasPrivateDnsValidated(CallbackEntry entry, Network networkForPrivateDns) {
+ if (!networkForPrivateDns.equals(entry.getNetwork())) return false;
+ final NetworkCapabilities nc = ((CallbackEntry.CapabilitiesChanged) entry).getCaps();
+ return !nc.isPrivateDnsBroken() && nc.hasCapability(NET_CAPABILITY_VALIDATED);
+ }
+
@AppModeFull(reason = "WRITE_SECURE_SETTINGS permission can't be granted to instant apps")
@Test @IgnoreUpTo(Build.VERSION_CODES.Q)
public void testIsPrivateDnsBroken() throws InterruptedException {
@@ -727,8 +733,7 @@
mCtsNetUtils.setPrivateDnsStrictMode(goodPrivateDnsServer);
final Network networkForPrivateDns = mCtsNetUtils.ensureWifiConnected();
cb.eventuallyExpect(CallbackEntry.NETWORK_CAPS_UPDATED, NETWORK_CALLBACK_TIMEOUT_MS,
- entry -> (!((CallbackEntry.CapabilitiesChanged) entry).getCaps()
- .isPrivateDnsBroken()) && networkForPrivateDns.equals(entry.getNetwork()));
+ entry -> hasPrivateDnsValidated(entry, networkForPrivateDns));
// Verifying the broken private DNS sever
mCtsNetUtils.setPrivateDnsStrictMode(invalidPrivateDnsServer);
@@ -2829,6 +2834,19 @@
});
}
+ /**
+ * The networks used in this test are real networks and as such they can see seemingly random
+ * updates of their capabilities or link properties as conditions change, e.g. the network
+ * loses validation or IPv4 shows up. Many tests should simply treat these callbacks as
+ * spurious.
+ */
+ private void assertNoCallbackExceptCapOrLpChange(
+ @NonNull final TestableNetworkCallback cb) {
+ cb.assertNoCallbackThat(NO_CALLBACK_TIMEOUT_MS,
+ c -> !(c instanceof CallbackEntry.CapabilitiesChanged
+ || c instanceof CallbackEntry.LinkPropertiesChanged));
+ }
+
@AppModeFull(reason = "Cannot get WifiManager in instant app mode")
@Test
public void testMobileDataPreferredUids() throws Exception {
@@ -2861,8 +2879,7 @@
// CtsNetTestCases uid is not listed in MOBILE_DATA_PREFERRED_UIDS setting, so the
// per-app default network should be same as system default network.
waitForAvailable(systemDefaultCb, wifiNetwork);
- defaultTrackingCb.eventuallyExpect(CallbackEntry.AVAILABLE, NETWORK_CALLBACK_TIMEOUT_MS,
- entry -> wifiNetwork.equals(entry.getNetwork()));
+ waitForAvailable(defaultTrackingCb, wifiNetwork);
// Active network for CtsNetTestCases uid should be wifi now.
assertEquals(wifiNetwork, mCm.getActiveNetwork());
@@ -2872,10 +2889,10 @@
newMobileDataPreferredUids.add(uid);
ConnectivitySettingsManager.setMobileDataPreferredUids(
mContext, newMobileDataPreferredUids);
- defaultTrackingCb.eventuallyExpect(CallbackEntry.AVAILABLE, NETWORK_CALLBACK_TIMEOUT_MS,
- entry -> cellNetwork.equals(entry.getNetwork()));
- // System default network doesn't change.
- systemDefaultCb.assertNoCallback();
+ waitForAvailable(defaultTrackingCb, cellNetwork);
+ // No change for system default network. Expect no callback except CapabilitiesChanged
+ // or LinkPropertiesChanged which may be triggered randomly from wifi network.
+ assertNoCallbackExceptCapOrLpChange(systemDefaultCb);
// Active network for CtsNetTestCases uid should change to cell, too.
assertEquals(cellNetwork, mCm.getActiveNetwork());
@@ -2884,10 +2901,10 @@
newMobileDataPreferredUids.remove(uid);
ConnectivitySettingsManager.setMobileDataPreferredUids(
mContext, newMobileDataPreferredUids);
- defaultTrackingCb.eventuallyExpect(CallbackEntry.AVAILABLE, NETWORK_CALLBACK_TIMEOUT_MS,
- entry -> wifiNetwork.equals(entry.getNetwork()));
- // System default network still doesn't change.
- systemDefaultCb.assertNoCallback();
+ waitForAvailable(defaultTrackingCb, wifiNetwork);
+ // No change for system default network. Expect no callback except CapabilitiesChanged
+ // or LinkPropertiesChanged which may be triggered randomly from wifi network.
+ assertNoCallbackExceptCapOrLpChange(systemDefaultCb);
// Active network for CtsNetTestCases uid should change back to wifi.
assertEquals(wifiNetwork, mCm.getActiveNetwork());
} finally {
diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
index 352a468..c54a11e 100644
--- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
@@ -151,6 +151,7 @@
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeTrue;
import static org.mockito.AdditionalMatchers.aryEq;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyLong;
@@ -319,6 +320,7 @@
import com.android.internal.util.WakeupMessage;
import com.android.internal.util.test.BroadcastInterceptingContext;
import com.android.internal.util.test.FakeSettingsProvider;
+import com.android.modules.utils.build.SdkLevel;
import com.android.net.module.util.ArrayTrackRecord;
import com.android.net.module.util.CollectionUtils;
import com.android.net.module.util.LocationPermissionChecker;
@@ -7252,6 +7254,15 @@
initialCaps.addTransportType(TRANSPORT_VPN);
initialCaps.addCapability(NET_CAPABILITY_INTERNET);
initialCaps.removeCapability(NET_CAPABILITY_NOT_VPN);
+ final ArrayList<Network> emptyUnderlyingNetworks = new ArrayList<Network>();
+ final ArrayList<Network> underlyingNetworksContainMobile = new ArrayList<Network>();
+ underlyingNetworksContainMobile.add(mobile);
+ final ArrayList<Network> underlyingNetworksContainWifi = new ArrayList<Network>();
+ underlyingNetworksContainWifi.add(wifi);
+ final ArrayList<Network> underlyingNetworksContainMobileAndMobile =
+ new ArrayList<Network>();
+ underlyingNetworksContainMobileAndMobile.add(mobile);
+ underlyingNetworksContainMobileAndMobile.add(wifi);
final NetworkCapabilities withNoUnderlying = new NetworkCapabilities();
withNoUnderlying.addCapability(NET_CAPABILITY_INTERNET);
@@ -7260,17 +7271,20 @@
withNoUnderlying.addCapability(NET_CAPABILITY_NOT_SUSPENDED);
withNoUnderlying.addTransportType(TRANSPORT_VPN);
withNoUnderlying.removeCapability(NET_CAPABILITY_NOT_VPN);
+ withNoUnderlying.setUnderlyingNetworks(emptyUnderlyingNetworks);
final NetworkCapabilities withMobileUnderlying = new NetworkCapabilities(withNoUnderlying);
withMobileUnderlying.addTransportType(TRANSPORT_CELLULAR);
withMobileUnderlying.removeCapability(NET_CAPABILITY_NOT_ROAMING);
withMobileUnderlying.removeCapability(NET_CAPABILITY_NOT_SUSPENDED);
withMobileUnderlying.setLinkDownstreamBandwidthKbps(10);
+ withMobileUnderlying.setUnderlyingNetworks(underlyingNetworksContainMobile);
final NetworkCapabilities withWifiUnderlying = new NetworkCapabilities(withNoUnderlying);
withWifiUnderlying.addTransportType(TRANSPORT_WIFI);
withWifiUnderlying.addCapability(NET_CAPABILITY_NOT_METERED);
withWifiUnderlying.setLinkUpstreamBandwidthKbps(20);
+ withWifiUnderlying.setUnderlyingNetworks(underlyingNetworksContainWifi);
final NetworkCapabilities withWifiAndMobileUnderlying =
new NetworkCapabilities(withNoUnderlying);
@@ -7280,6 +7294,7 @@
withWifiAndMobileUnderlying.removeCapability(NET_CAPABILITY_NOT_ROAMING);
withWifiAndMobileUnderlying.setLinkDownstreamBandwidthKbps(10);
withWifiAndMobileUnderlying.setLinkUpstreamBandwidthKbps(20);
+ withWifiAndMobileUnderlying.setUnderlyingNetworks(underlyingNetworksContainMobileAndMobile);
final NetworkCapabilities initialCapsNotMetered = new NetworkCapabilities(initialCaps);
initialCapsNotMetered.addCapability(NET_CAPABILITY_NOT_METERED);
@@ -7287,40 +7302,61 @@
NetworkCapabilities caps = new NetworkCapabilities(initialCaps);
mService.applyUnderlyingCapabilities(new Network[]{}, initialCapsNotMetered, caps);
assertEquals(withNoUnderlying, caps);
+ assertEquals(0, new ArrayList<>(caps.getUnderlyingNetworks()).size());
caps = new NetworkCapabilities(initialCaps);
mService.applyUnderlyingCapabilities(new Network[]{null}, initialCapsNotMetered, caps);
assertEquals(withNoUnderlying, caps);
+ assertEquals(0, new ArrayList<>(caps.getUnderlyingNetworks()).size());
caps = new NetworkCapabilities(initialCaps);
mService.applyUnderlyingCapabilities(new Network[]{mobile}, initialCapsNotMetered, caps);
assertEquals(withMobileUnderlying, caps);
+ assertEquals(1, new ArrayList<>(caps.getUnderlyingNetworks()).size());
+ assertEquals(mobile, new ArrayList<>(caps.getUnderlyingNetworks()).get(0));
+ caps = new NetworkCapabilities(initialCaps);
mService.applyUnderlyingCapabilities(new Network[]{wifi}, initialCapsNotMetered, caps);
assertEquals(withWifiUnderlying, caps);
+ assertEquals(1, new ArrayList<>(caps.getUnderlyingNetworks()).size());
+ assertEquals(wifi, new ArrayList<>(caps.getUnderlyingNetworks()).get(0));
withWifiUnderlying.removeCapability(NET_CAPABILITY_NOT_METERED);
caps = new NetworkCapabilities(initialCaps);
mService.applyUnderlyingCapabilities(new Network[]{wifi}, initialCaps, caps);
assertEquals(withWifiUnderlying, caps);
+ assertEquals(1, new ArrayList<>(caps.getUnderlyingNetworks()).size());
+ assertEquals(wifi, new ArrayList<>(caps.getUnderlyingNetworks()).get(0));
caps = new NetworkCapabilities(initialCaps);
mService.applyUnderlyingCapabilities(new Network[]{mobile, wifi}, initialCaps, caps);
assertEquals(withWifiAndMobileUnderlying, caps);
+ assertEquals(2, new ArrayList<>(caps.getUnderlyingNetworks()).size());
+ assertEquals(mobile, new ArrayList<>(caps.getUnderlyingNetworks()).get(0));
+ assertEquals(wifi, new ArrayList<>(caps.getUnderlyingNetworks()).get(1));
withWifiUnderlying.addCapability(NET_CAPABILITY_NOT_METERED);
caps = new NetworkCapabilities(initialCaps);
mService.applyUnderlyingCapabilities(new Network[]{null, mobile, null, wifi},
initialCapsNotMetered, caps);
assertEquals(withWifiAndMobileUnderlying, caps);
+ assertEquals(2, new ArrayList<>(caps.getUnderlyingNetworks()).size());
+ assertEquals(mobile, new ArrayList<>(caps.getUnderlyingNetworks()).get(0));
+ assertEquals(wifi, new ArrayList<>(caps.getUnderlyingNetworks()).get(1));
caps = new NetworkCapabilities(initialCaps);
mService.applyUnderlyingCapabilities(new Network[]{null, mobile, null, wifi},
initialCapsNotMetered, caps);
assertEquals(withWifiAndMobileUnderlying, caps);
+ assertEquals(2, new ArrayList<>(caps.getUnderlyingNetworks()).size());
+ assertEquals(mobile, new ArrayList<>(caps.getUnderlyingNetworks()).get(0));
+ assertEquals(wifi, new ArrayList<>(caps.getUnderlyingNetworks()).get(1));
+ caps = new NetworkCapabilities(initialCaps);
mService.applyUnderlyingCapabilities(null, initialCapsNotMetered, caps);
assertEquals(withWifiUnderlying, caps);
+ assertEquals(1, new ArrayList<>(caps.getUnderlyingNetworks()).size());
+ assertEquals(wifi, new ArrayList<>(caps.getUnderlyingNetworks()).get(0));
}
@Test
@@ -7329,51 +7365,78 @@
final NetworkRequest request = new NetworkRequest.Builder()
.removeCapability(NET_CAPABILITY_NOT_VPN).build();
- mCm.registerNetworkCallback(request, callback);
+ runAsShell(NETWORK_SETTINGS, () -> {
+ mCm.registerNetworkCallback(request, callback);
- // Bring up a VPN that specifies an underlying network that does not exist yet.
- // Note: it's sort of meaningless for a VPN app to declare a network that doesn't exist yet,
- // (and doing so is difficult without using reflection) but it's good to test that the code
- // behaves approximately correctly.
- mMockVpn.establishForMyUid(false, true, false);
- assertUidRangesUpdatedForMyUid(true);
- final Network wifiNetwork = new Network(mNetIdManager.peekNextNetId());
- mMockVpn.setUnderlyingNetworks(new Network[]{wifiNetwork});
- callback.expectAvailableCallbacksUnvalidated(mMockVpn);
- assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork())
- .hasTransport(TRANSPORT_VPN));
- assertFalse(mCm.getNetworkCapabilities(mMockVpn.getNetwork())
- .hasTransport(TRANSPORT_WIFI));
+ // Bring up a VPN that specifies an underlying network that does not exist yet.
+ // Note: it's sort of meaningless for a VPN app to declare a network that doesn't exist
+ // yet, (and doing so is difficult without using reflection) but it's good to test that
+ // the code behaves approximately correctly.
+ mMockVpn.establishForMyUid(false, true, false);
+ callback.expectAvailableCallbacksUnvalidated(mMockVpn);
+ assertUidRangesUpdatedForMyUid(true);
+ final Network wifiNetwork = new Network(mNetIdManager.peekNextNetId());
+ mMockVpn.setUnderlyingNetworks(new Network[]{wifiNetwork});
+ // onCapabilitiesChanged() should be called because
+ // NetworkCapabilities#mUnderlyingNetworks is updated.
+ CallbackEntry ce = callback.expectCallback(CallbackEntry.NETWORK_CAPS_UPDATED,
+ mMockVpn);
+ final NetworkCapabilities vpnNc1 = ((CallbackEntry.CapabilitiesChanged) ce).getCaps();
+ // Since the wifi network hasn't brought up,
+ // ConnectivityService#applyUnderlyingCapabilities cannot find it. Update
+ // NetworkCapabilities#mUnderlyingNetworks to an empty array, and it will be updated to
+ // the correct underlying networks once the wifi network brings up. But this case
+ // shouldn't happen in reality since no one could get the network which hasn't brought
+ // up. For the empty array of underlying networks, it should be happened for 2 cases,
+ // the first one is that the VPN app declares an empty array for its underlying
+ // networks, the second one is that the underlying networks are torn down.
+ //
+ // It shouldn't be null since the null value means the underlying networks of this
+ // network should follow the default network.
+ final ArrayList<Network> underlyingNetwork = new ArrayList<>();
+ assertEquals(underlyingNetwork, vpnNc1.getUnderlyingNetworks());
+ // Since the wifi network isn't exist, applyUnderlyingCapabilities()
+ assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork())
+ .hasTransport(TRANSPORT_VPN));
+ assertFalse(mCm.getNetworkCapabilities(mMockVpn.getNetwork())
+ .hasTransport(TRANSPORT_WIFI));
- // Make that underlying network connect, and expect to see its capabilities immediately
- // reflected in the VPN's capabilities.
- mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI);
- assertEquals(wifiNetwork, mWiFiNetworkAgent.getNetwork());
- mWiFiNetworkAgent.connect(false);
- // TODO: the callback for the VPN happens before any callbacks are called for the wifi
- // network that has just connected. There appear to be two issues here:
- // 1. The VPN code will accept an underlying network as soon as getNetworkCapabilities() for
- // it returns non-null (which happens very early, during handleRegisterNetworkAgent).
- // This is not correct because that that point the network is not connected and cannot
- // pass any traffic.
- // 2. When a network connects, updateNetworkInfo propagates underlying network capabilities
- // before rematching networks.
- // Given that this scenario can't really happen, this is probably fine for now.
- callback.expectCallback(CallbackEntry.NETWORK_CAPS_UPDATED, mMockVpn);
- callback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
- assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork())
- .hasTransport(TRANSPORT_VPN));
- assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork())
- .hasTransport(TRANSPORT_WIFI));
+ // Make that underlying network connect, and expect to see its capabilities immediately
+ // reflected in the VPN's capabilities.
+ mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI);
+ assertEquals(wifiNetwork, mWiFiNetworkAgent.getNetwork());
+ mWiFiNetworkAgent.connect(false);
+ // TODO: the callback for the VPN happens before any callbacks are called for the wifi
+ // network that has just connected. There appear to be two issues here:
+ // 1. The VPN code will accept an underlying network as soon as getNetworkCapabilities()
+ // for it returns non-null (which happens very early, during
+ // handleRegisterNetworkAgent).
+ // This is not correct because that that point the network is not connected and
+ // cannot pass any traffic.
+ // 2. When a network connects, updateNetworkInfo propagates underlying network
+ // capabilities before rematching networks.
+ // Given that this scenario can't really happen, this is probably fine for now.
+ ce = callback.expectCallback(CallbackEntry.NETWORK_CAPS_UPDATED, mMockVpn);
+ final NetworkCapabilities vpnNc2 = ((CallbackEntry.CapabilitiesChanged) ce).getCaps();
+ // The wifi network is brought up, NetworkCapabilities#mUnderlyingNetworks is updated to
+ // it.
+ underlyingNetwork.add(wifiNetwork);
+ assertEquals(underlyingNetwork, vpnNc2.getUnderlyingNetworks());
+ callback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
+ assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork())
+ .hasTransport(TRANSPORT_VPN));
+ assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork())
+ .hasTransport(TRANSPORT_WIFI));
- // Disconnect the network, and expect to see the VPN capabilities change accordingly.
- mWiFiNetworkAgent.disconnect();
- callback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent);
- callback.expectCapabilitiesThat(mMockVpn, (nc) ->
- nc.getTransportTypes().length == 1 && nc.hasTransport(TRANSPORT_VPN));
+ // Disconnect the network, and expect to see the VPN capabilities change accordingly.
+ mWiFiNetworkAgent.disconnect();
+ callback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent);
+ callback.expectCapabilitiesThat(mMockVpn, (nc) ->
+ nc.getTransportTypes().length == 1 && nc.hasTransport(TRANSPORT_VPN));
- mMockVpn.disconnect();
- mCm.unregisterNetworkCallback(callback);
+ mMockVpn.disconnect();
+ mCm.unregisterNetworkCallback(callback);
+ });
}
private void assertGetNetworkInfoOfGetActiveNetworkIsConnected(boolean expectedConnectivity) {
@@ -10697,6 +10760,14 @@
return fakeNai(wifiNc, info);
}
+ private NetworkAgentInfo fakeVpnNai(NetworkCapabilities nc) {
+ final NetworkCapabilities vpnNc = new NetworkCapabilities.Builder(nc)
+ .addTransportType(TRANSPORT_VPN).build();
+ final NetworkInfo info = new NetworkInfo(TYPE_VPN, 0 /* subtype */,
+ ConnectivityManager.getNetworkTypeName(TYPE_VPN), "" /* subtypeName */);
+ return fakeNai(vpnNc, info);
+ }
+
private NetworkAgentInfo fakeNai(NetworkCapabilities nc, NetworkInfo networkInfo) {
return new NetworkAgentInfo(null, new Network(NET_ID), networkInfo, new LinkProperties(),
nc, new NetworkScore.Builder().setLegacyInt(0).build(),
@@ -10831,6 +10902,36 @@
}
@Test
+ public void testUnderlyingNetworksWillBeSetInNetworkAgentInfoConstructor() throws Exception {
+ assumeTrue(SdkLevel.isAtLeastT());
+ final Network network1 = new Network(100);
+ final Network network2 = new Network(101);
+ final List<Network> underlyingNetworks = new ArrayList<>();
+ final NetworkCapabilities ncWithEmptyUnderlyingNetworks = new NetworkCapabilities.Builder()
+ .setUnderlyingNetworks(underlyingNetworks)
+ .build();
+ final NetworkAgentInfo vpnNaiWithEmptyUnderlyingNetworks =
+ fakeVpnNai(ncWithEmptyUnderlyingNetworks);
+ assertEquals(underlyingNetworks,
+ Arrays.asList(vpnNaiWithEmptyUnderlyingNetworks.declaredUnderlyingNetworks));
+
+ underlyingNetworks.add(network1);
+ underlyingNetworks.add(network2);
+ final NetworkCapabilities ncWithUnderlyingNetworks = new NetworkCapabilities.Builder()
+ .setUnderlyingNetworks(underlyingNetworks)
+ .build();
+ final NetworkAgentInfo vpnNaiWithUnderlyingNetwokrs = fakeVpnNai(ncWithUnderlyingNetworks);
+ assertEquals(underlyingNetworks,
+ Arrays.asList(vpnNaiWithUnderlyingNetwokrs.declaredUnderlyingNetworks));
+
+ final NetworkCapabilities ncWithoutUnderlyingNetworks = new NetworkCapabilities.Builder()
+ .build();
+ final NetworkAgentInfo vpnNaiWithoutUnderlyingNetwokrs =
+ fakeVpnNai(ncWithoutUnderlyingNetworks);
+ assertNull(vpnNaiWithoutUnderlyingNetwokrs.declaredUnderlyingNetworks);
+ }
+
+ @Test
public void testRegisterConnectivityDiagnosticsCallbackCallsOnConnectivityReport()
throws Exception {
// Set up the Network, which leads to a ConnectivityReport being cached for the network.
diff --git a/tests/unit/java/com/android/server/connectivity/Nat464XlatTest.java b/tests/unit/java/com/android/server/connectivity/Nat464XlatTest.java
index f358726..aa4c4e3 100644
--- a/tests/unit/java/com/android/server/connectivity/Nat464XlatTest.java
+++ b/tests/unit/java/com/android/server/connectivity/Nat464XlatTest.java
@@ -109,8 +109,8 @@
mNai.linkProperties = new LinkProperties();
mNai.linkProperties.setInterfaceName(BASE_IFACE);
- mNai.networkInfo = new NetworkInfo(null);
- mNai.networkInfo.setType(ConnectivityManager.TYPE_WIFI);
+ mNai.networkInfo = new NetworkInfo(ConnectivityManager.TYPE_WIFI, 0 /* subtype */,
+ null /* typeName */, null /* subtypeName */);
mNai.networkCapabilities = new NetworkCapabilities();
markNetworkConnected();
when(mNai.connService()).thenReturn(mConnectivity);