LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix setCapabilities.

Browse Source 
P introduced setSSID, UIDs and unwanted capabilities.
None of these exhibit commutative behavior through combineCapabilities
because their semantics don't allow it. Therefore
NetworkRequest.setCapabilities() is badly broken around any of
these. Look at the comments in the new tests to realize the
extent of the damage.

Clean cherry-pick of ag/4083952

Bug: 79748782
Test: new tests written, old tests pass
Change-Id: Iafe074126132a82af37f4bf056c4a7b8d56bdc83
Merged-In: Ia5bebf8a233775367bbf1b788870528934ecbcfb
Merged-In: I13d7782a6c0c7b1f94137995bbb0d257a58d89c1
This commit is contained in:
Chalard Jean
2018-05-18 23:48:49 +09:00
parent d082c6c128
commit 52a6d547b7
3 changed files with 78 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
core/java/android/net/NetworkCapabilities.java
View File

@@ -61,16 +61,7 @@ public final class NetworkCapabilities implements Parcelable {
public NetworkCapabilities(NetworkCapabilities nc) {
if (nc != null) {
mNetworkCapabilities = nc.mNetworkCapabilities;
mTransportTypes = nc.mTransportTypes;
mLinkUpBandwidthKbps = nc.mLinkUpBandwidthKbps;
mLinkDownBandwidthKbps = nc.mLinkDownBandwidthKbps;
mNetworkSpecifier = nc.mNetworkSpecifier;
mSignalStrength = nc.mSignalStrength;
mUids = nc.mUids;
mEstablishingVpnAppUid = nc.mEstablishingVpnAppUid;
mUnwantedNetworkCapabilities = nc.mUnwantedNetworkCapabilities;
mSSID = nc.mSSID;
set(nc);
}
}
@@ -89,6 +80,23 @@ public final class NetworkCapabilities implements Parcelable {
mSSID = null;
}
/**
* Set all contents of this object to the contents of a NetworkCapabilities.
* @hide
*/
public void set(NetworkCapabilities nc) {
mNetworkCapabilities = nc.mNetworkCapabilities;
mTransportTypes = nc.mTransportTypes;
mLinkUpBandwidthKbps = nc.mLinkUpBandwidthKbps;
mLinkDownBandwidthKbps = nc.mLinkDownBandwidthKbps;
mNetworkSpecifier = nc.mNetworkSpecifier;
mSignalStrength = nc.mSignalStrength;
setUids(nc.mUids); // Will make the defensive copy
mEstablishingVpnAppUid = nc.mEstablishingVpnAppUid;
mUnwantedNetworkCapabilities = nc.mUnwantedNetworkCapabilities;
mSSID = nc.mSSID;
}
/**
* Represents the network's capabilities. If any are specified they will be satisfied
* by any Network that matches all of them.

3
core/java/android/net/NetworkRequest.java
View File

@@ -197,8 +197,7 @@ public class NetworkRequest implements Parcelable {
* @hide
*/
public Builder setCapabilities(NetworkCapabilities nc) {
mNetworkCapabilities.clearAll();
mNetworkCapabilities.combineCapabilities(nc);
mNetworkCapabilities.set(nc);
return this;
}

61
tests/net/java/android/net/NetworkCapabilitiesTest.java
View File

@@ -57,6 +57,7 @@ import java.util.Set;
@SmallTest
public class NetworkCapabilitiesTest {
private static final String TEST_SSID = "TEST_SSID";
private static final String DIFFERENT_TEST_SSID = "DIFFERENT_TEST_SSID";
@Test
public void testMaybeMarkCapabilitiesRestricted() {
@@ -382,6 +383,12 @@ public class NetworkCapabilitiesTest {
assertFalse(nc1.satisfiedByNetworkCapabilities(nc2));
}
private ArraySet<UidRange> uidRange(int from, int to) {
final ArraySet<UidRange> range = new ArraySet<>(1);
range.add(new UidRange(from, to));
return range;
}
@Test
public void testCombineCapabilities() {
NetworkCapabilities nc1 = new NetworkCapabilities();
@@ -408,14 +415,30 @@ public class NetworkCapabilitiesTest {
nc2.combineCapabilities(nc1);
assertTrue(TEST_SSID.equals(nc2.getSSID()));
// Because they now have the same SSID, the folllowing call should not throw
// Because they now have the same SSID, the following call should not throw
nc2.combineCapabilities(nc1);
nc1.setSSID("different " + TEST_SSID);
nc1.setSSID(DIFFERENT_TEST_SSID);
try {
nc2.combineCapabilities(nc1);
fail("Expected IllegalStateException: can't combine different SSIDs");
} catch (IllegalStateException expected) {}
nc1.setSSID(TEST_SSID);
nc1.setUids(uidRange(10, 13));
assertNotEquals(nc1, nc2);
nc2.combineCapabilities(nc1); // Everything + 10~13 is still everything.
assertNotEquals(nc1, nc2);
nc1.combineCapabilities(nc2); // 10~13 + everything is everything.
assertEquals(nc1, nc2);
nc1.setUids(uidRange(10, 13));
nc2.setUids(uidRange(20, 23));
assertNotEquals(nc1, nc2);
nc1.combineCapabilities(nc2);
assertTrue(nc1.appliesToUid(12));
assertFalse(nc2.appliesToUid(12));
assertTrue(nc1.appliesToUid(22));
assertTrue(nc2.appliesToUid(22));
}
@Test
@@ -454,4 +477,38 @@ public class NetworkCapabilitiesTest {
p.setDataPosition(0);
assertEquals(NetworkCapabilities.CREATOR.createFromParcel(p), netCap);
}
@Test
public void testSet() {
NetworkCapabilities nc1 = new NetworkCapabilities();
NetworkCapabilities nc2 = new NetworkCapabilities();
nc1.addUnwantedCapability(NET_CAPABILITY_CAPTIVE_PORTAL);
nc1.addCapability(NET_CAPABILITY_NOT_ROAMING);
assertNotEquals(nc1, nc2);
nc2.set(nc1);
assertEquals(nc1, nc2);
assertTrue(nc2.hasCapability(NET_CAPABILITY_NOT_ROAMING));
assertTrue(nc2.hasUnwantedCapability(NET_CAPABILITY_CAPTIVE_PORTAL));
// This will effectively move NOT_ROAMING capability from required to unwanted for nc1.
nc1.addUnwantedCapability(NET_CAPABILITY_NOT_ROAMING);
nc1.setSSID(TEST_SSID);
nc2.set(nc1);
assertEquals(nc1, nc2);
// Contrary to combineCapabilities, set() will have removed the NOT_ROAMING capability
// from nc2.
assertFalse(nc2.hasCapability(NET_CAPABILITY_NOT_ROAMING));
assertTrue(nc2.hasUnwantedCapability(NET_CAPABILITY_NOT_ROAMING));
assertTrue(TEST_SSID.equals(nc2.getSSID()));
nc1.setSSID(DIFFERENT_TEST_SSID);
nc2.set(nc1);
assertEquals(nc1, nc2);
assertTrue(DIFFERENT_TEST_SSID.equals(nc2.getSSID()));
nc1.setUids(uidRange(10, 13));
nc2.set(nc1); // Overwrites, as opposed to combineCapabilities
assertEquals(nc1, nc2);
}
}