Add default capabilities for empty config
While doing some testing for another change (pa/2844131), I noticed that
an interface in config.xml with no network capabilities listed provided
no internet connection. parseEthernetConfig was updated to provide empty
config capabilities with the default capabilities (which
includes NET_CAPABILITY_INTERNET for non test ifaces).
Flag: BUG_FIX
Test: atest ConnectivityCoverageTests
Test: atest ConnectivityCoverageTests:android.net.connectivity.com.android.server.ethernet.EthernetTrackerTest
Bug: 352523028
Change-Id: I6f73b0554f95c20fd1912a3c8da2b3c64f2ac4c4
diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java
index 4a9410e..eedf427 100644
--- a/service-t/src/com/android/server/ethernet/EthernetTracker.java
+++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java
@@ -51,7 +51,6 @@
import com.android.modules.utils.build.SdkLevel;
import com.android.net.module.util.HandlerUtils;
import com.android.net.module.util.NetdUtils;
-import com.android.net.module.util.PermissionUtils;
import com.android.net.module.util.SharedLog;
import com.android.net.module.util.ip.NetlinkMonitor;
import com.android.net.module.util.netlink.NetlinkConstants;
@@ -624,7 +623,7 @@
nc = mNetworkCapabilities.get(hwAddress);
if (nc == null) {
final boolean isTestIface = iface.matches(TEST_IFACE_REGEXP);
- nc = createDefaultNetworkCapabilities(isTestIface);
+ nc = createDefaultNetworkCapabilities(isTestIface, /* overrideTransport */ null);
}
}
@@ -739,9 +738,13 @@
*/
private void parseEthernetConfig(String configString) {
final EthernetTrackerConfig config = createEthernetTrackerConfig(configString);
- NetworkCapabilities nc = createNetworkCapabilities(
- !TextUtils.isEmpty(config.mCapabilities) /* clear default capabilities */,
- config.mCapabilities, config.mTransport).build();
+ NetworkCapabilities nc;
+ if (TextUtils.isEmpty(config.mCapabilities)) {
+ boolean isTestIface = config.mIface.matches(TEST_IFACE_REGEXP);
+ nc = createDefaultNetworkCapabilities(isTestIface, config.mTransport);
+ } else {
+ nc = createNetworkCapabilities(config.mCapabilities, config.mTransport).build();
+ }
mNetworkCapabilities.put(config.mIface, nc);
if (null != config.mIpConfig) {
@@ -756,15 +759,16 @@
return new EthernetTrackerConfig(configString.split(";", /* limit of tokens */ 4));
}
- private static NetworkCapabilities createDefaultNetworkCapabilities(boolean isTestIface) {
- NetworkCapabilities.Builder builder = createNetworkCapabilities(
- false /* clear default capabilities */, null, null)
- .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED)
- .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED)
- .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING)
- .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_CONGESTED)
- .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED)
- .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED);
+ private static NetworkCapabilities createDefaultNetworkCapabilities(
+ boolean isTestIface, @Nullable String overrideTransport) {
+ NetworkCapabilities.Builder builder =
+ createNetworkCapabilities(/* commaSeparatedCapabilities */ null, overrideTransport)
+ .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED)
+ .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED)
+ .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING)
+ .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_CONGESTED)
+ .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED)
+ .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED);
if (isTestIface) {
builder.addTransportType(NetworkCapabilities.TRANSPORT_TEST);
@@ -778,7 +782,6 @@
/**
* Parses a static list of network capabilities
*
- * @param clearDefaultCapabilities Indicates whether or not to clear any default capabilities
* @param commaSeparatedCapabilities A comma separated string list of integer encoded
* NetworkCapability.NET_CAPABILITY_* values
* @param overrideTransport A string representing a single integer encoded override transport
@@ -788,12 +791,12 @@
*/
@VisibleForTesting
static NetworkCapabilities.Builder createNetworkCapabilities(
- boolean clearDefaultCapabilities, @Nullable String commaSeparatedCapabilities,
- @Nullable String overrideTransport) {
+ @Nullable String commaSeparatedCapabilities, @Nullable String overrideTransport) {
- final NetworkCapabilities.Builder builder = clearDefaultCapabilities
- ? NetworkCapabilities.Builder.withoutDefaultCapabilities()
- : new NetworkCapabilities.Builder();
+ final NetworkCapabilities.Builder builder =
+ TextUtils.isEmpty(commaSeparatedCapabilities)
+ ? new NetworkCapabilities.Builder()
+ : NetworkCapabilities.Builder.withoutDefaultCapabilities();
// Determine the transport type. If someone has tried to define an override transport then
// attempt to add it. Since we can only have one override, all errors with it will
diff --git a/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java b/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java
index e6aba22..b7cfaf9 100644
--- a/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java
+++ b/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java
@@ -166,9 +166,10 @@
EthernetTracker.parseStaticIpConfiguration(configAsString));
}
- private NetworkCapabilities.Builder makeEthernetCapabilitiesBuilder(boolean clearAll) {
+ private NetworkCapabilities.Builder makeEthernetCapabilitiesBuilder(boolean clearDefaults) {
final NetworkCapabilities.Builder builder =
- clearAll ? NetworkCapabilities.Builder.withoutDefaultCapabilities()
+ clearDefaults
+ ? NetworkCapabilities.Builder.withoutDefaultCapabilities()
: new NetworkCapabilities.Builder();
return builder.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING)
.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_CONGESTED)
@@ -176,21 +177,20 @@
}
/**
- * Test: Attempt to create a capabilties with various valid sets of capabilities/transports
+ * Test: Attempt to create a capabilities with various valid sets of capabilities/transports
*/
@Test
public void createNetworkCapabilities() {
-
// Particularly common expected results
- NetworkCapabilities defaultEthernetCleared =
- makeEthernetCapabilitiesBuilder(true /* clearAll */)
+ NetworkCapabilities defaultCapabilities =
+ makeEthernetCapabilitiesBuilder(false /* clearDefaults */)
.setLinkUpstreamBandwidthKbps(100000)
.setLinkDownstreamBandwidthKbps(100000)
.addTransportType(NetworkCapabilities.TRANSPORT_ETHERNET)
.build();
NetworkCapabilities ethernetClearedWithCommonCaps =
- makeEthernetCapabilitiesBuilder(true /* clearAll */)
+ makeEthernetCapabilitiesBuilder(true /* clearDefaults */)
.setLinkUpstreamBandwidthKbps(100000)
.setLinkDownstreamBandwidthKbps(100000)
.addTransportType(NetworkCapabilities.TRANSPORT_ETHERNET)
@@ -200,89 +200,71 @@
.addCapability(15)
.build();
- // Empty capabilities and transports lists with a "please clear defaults" should
- // yield an empty capabilities set with TRANPORT_ETHERNET
- assertParsedNetworkCapabilities(defaultEthernetCleared, true, "", "");
+ // Empty capabilities and transports should return the default capabilities set
+ // with TRANSPORT_ETHERNET
+ assertParsedNetworkCapabilities(defaultCapabilities, "", "");
- // Empty capabilities and transports without the clear defaults flag should return the
- // default capabilities set with TRANSPORT_ETHERNET
- assertParsedNetworkCapabilities(
- makeEthernetCapabilitiesBuilder(false /* clearAll */)
- .setLinkUpstreamBandwidthKbps(100000)
- .setLinkDownstreamBandwidthKbps(100000)
- .addTransportType(NetworkCapabilities.TRANSPORT_ETHERNET)
- .build(),
- false, "", "");
-
- // A list of capabilities without the clear defaults flag should return the default
- // capabilities, mixed with the desired capabilities, and TRANSPORT_ETHERNET
- assertParsedNetworkCapabilities(
- makeEthernetCapabilitiesBuilder(false /* clearAll */)
- .setLinkUpstreamBandwidthKbps(100000)
- .setLinkDownstreamBandwidthKbps(100000)
- .addTransportType(NetworkCapabilities.TRANSPORT_ETHERNET)
- .addCapability(11)
- .addCapability(12)
- .build(),
- false, "11,12", "");
-
- // Adding a list of capabilities with a clear defaults will leave exactly those capabilities
- // with a default TRANSPORT_ETHERNET since no overrides are specified
- assertParsedNetworkCapabilities(ethernetClearedWithCommonCaps, true, "12,13,14,15", "");
+ // Adding a list of capabilities will leave exactly those capabilities with a default
+ // TRANSPORT_ETHERNET since no overrides are specified
+ assertParsedNetworkCapabilities(ethernetClearedWithCommonCaps, "12,13,14,15", "");
// Adding any invalid capabilities to the list will cause them to be ignored
- assertParsedNetworkCapabilities(ethernetClearedWithCommonCaps, true, "12,13,14,15,65,73", "");
- assertParsedNetworkCapabilities(ethernetClearedWithCommonCaps, true, "12,13,14,15,abcdefg", "");
+ assertParsedNetworkCapabilities(ethernetClearedWithCommonCaps, "12,13,14,15,65,73", "");
+ assertParsedNetworkCapabilities(ethernetClearedWithCommonCaps, "12,13,14,15,abcdefg", "");
// Adding a valid override transport will remove the default TRANSPORT_ETHERNET transport
- // and apply only the override to the capabiltities object
+ // and apply only the override to the capabilities object
assertParsedNetworkCapabilities(
- makeEthernetCapabilitiesBuilder(true /* clearAll */)
+ makeEthernetCapabilitiesBuilder(false /* clearDefaults */)
.setLinkUpstreamBandwidthKbps(100000)
.setLinkDownstreamBandwidthKbps(100000)
.addTransportType(0)
.build(),
- true, "", "0");
+ "",
+ "0");
assertParsedNetworkCapabilities(
- makeEthernetCapabilitiesBuilder(true /* clearAll */)
+ makeEthernetCapabilitiesBuilder(false /* clearDefaults */)
.setLinkUpstreamBandwidthKbps(100000)
.setLinkDownstreamBandwidthKbps(100000)
.addTransportType(1)
.build(),
- true, "", "1");
+ "",
+ "1");
assertParsedNetworkCapabilities(
- makeEthernetCapabilitiesBuilder(true /* clearAll */)
+ makeEthernetCapabilitiesBuilder(false /* clearDefaults */)
.setLinkUpstreamBandwidthKbps(100000)
.setLinkDownstreamBandwidthKbps(100000)
.addTransportType(2)
.build(),
- true, "", "2");
+ "",
+ "2");
assertParsedNetworkCapabilities(
- makeEthernetCapabilitiesBuilder(true /* clearAll */)
+ makeEthernetCapabilitiesBuilder(false /* clearDefaults */)
.setLinkUpstreamBandwidthKbps(100000)
.setLinkDownstreamBandwidthKbps(100000)
.addTransportType(3)
.build(),
- true, "", "3");
+ "",
+ "3");
- // "4" is TRANSPORT_VPN, which is unsupported. Should default back to TRANPORT_ETHERNET
- assertParsedNetworkCapabilities(defaultEthernetCleared, true, "", "4");
+ // "4" is TRANSPORT_VPN, which is unsupported. Should default back to TRANSPORT_ETHERNET
+ assertParsedNetworkCapabilities(defaultCapabilities, "", "4");
// "5" is TRANSPORT_WIFI_AWARE, which is currently supported due to no legacy TYPE_NONE
// conversion. When that becomes available, this test must be updated
- assertParsedNetworkCapabilities(defaultEthernetCleared, true, "", "5");
+ assertParsedNetworkCapabilities(defaultCapabilities, "", "5");
// "6" is TRANSPORT_LOWPAN, which is currently supported due to no legacy TYPE_NONE
// conversion. When that becomes available, this test must be updated
- assertParsedNetworkCapabilities(defaultEthernetCleared, true, "", "6");
+ assertParsedNetworkCapabilities(defaultCapabilities, "", "6");
// Adding an invalid override transport will leave the transport as TRANSPORT_ETHERNET
- assertParsedNetworkCapabilities(defaultEthernetCleared,true, "", "100");
- assertParsedNetworkCapabilities(defaultEthernetCleared, true, "", "abcdefg");
+ assertParsedNetworkCapabilities(defaultCapabilities, "", "100");
+ assertParsedNetworkCapabilities(defaultCapabilities, "", "abcdefg");
// Ensure the adding of both capabilities and transports work
assertParsedNetworkCapabilities(
- makeEthernetCapabilitiesBuilder(true /* clearAll */)
+ makeEthernetCapabilitiesBuilder(true /* clearDefaults */)
.setLinkUpstreamBandwidthKbps(100000)
.setLinkDownstreamBandwidthKbps(100000)
.addCapability(12)
@@ -291,17 +273,21 @@
.addCapability(15)
.addTransportType(3)
.build(),
- true, "12,13,14,15", "3");
+ "12,13,14,15",
+ "3");
// Ensure order does not matter for capability list
- assertParsedNetworkCapabilities(ethernetClearedWithCommonCaps, true, "13,12,15,14", "");
+ assertParsedNetworkCapabilities(ethernetClearedWithCommonCaps, "13,12,15,14", "");
}
- private void assertParsedNetworkCapabilities(NetworkCapabilities expectedNetworkCapabilities,
- boolean clearCapabilties, String configCapabiltiies,String configTransports) {
- assertEquals(expectedNetworkCapabilities,
- EthernetTracker.createNetworkCapabilities(clearCapabilties, configCapabiltiies,
- configTransports).build());
+ private void assertParsedNetworkCapabilities(
+ NetworkCapabilities expectedNetworkCapabilities,
+ String configCapabiltiies,
+ String configTransports) {
+ assertEquals(
+ expectedNetworkCapabilities,
+ EthernetTracker.createNetworkCapabilities(configCapabiltiies, configTransports)
+ .build());
}
@Test