Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4c4bc93b authored by Chalard Jean's avatar Chalard Jean
Browse files

Fix setCapabilities. 

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.

Bug: 79748782
Test: new tests written, old tests pass
Change-Id: Ie46581bdaf9ecc2f14aab44788bbdb27a3fec8c1
parent 86b2581c

core/java/android/net/NetworkCapabilities.java

+18 −10
Original line number Diff line number Diff line
@@ -63,16 +63,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);

        }

    }
@@ -91,6 +82,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.

core/java/android/net/NetworkRequest.java

+1 −2
Original line number Diff line number Diff line
@@ -198,8 +198,7 @@ public class NetworkRequest implements Parcelable { 
         * @hide

         */

        public Builder setCapabilities(NetworkCapabilities nc) {

            mNetworkCapabilities.clearAll();

            mNetworkCapabilities.combineCapabilities(nc);

            mNetworkCapabilities.set(nc);

            return this;

        }

tests/net/java/android/net/NetworkCapabilitiesTest.java

+59 −2
Original line number Diff line number Diff line
@@ -56,6 +56,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() {
@@ -374,6 +375,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();
@@ -400,14 +407,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
@@ -446,4 +469,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);

    }

}