From 1b5c01b06f4bec0a14dc1a83f59b629c9182cb41 Mon Sep 17 00:00:00 2001 From: Anthony Stange Date: Thu, 18 Mar 2021 16:30:59 +0000 Subject: [PATCH] Revert "Replace the usage of UidRange" Revert "Add shims for NetworkRequest" Revert submission 1626206-replaceUidRange Reason for revert: Breaking build - b/183106405 Reverted Changes: I0b79c73e8:Add shims for NetworkRequest I4bc0daf5a:Replace the usage of UidRange I4e5aec6ef:Replace the usage of UidRange I107c329d4:Expose uids related APIs in NetworkRequest and Net... Change-Id: I6290429db1c8e787f8138b55b98fd92a74ac6402 --- .../src/android/net/NetworkCapabilities.java | 31 ++-- framework/src/android/net/NetworkRequest.java | 5 +- framework/src/android/net/UidRange.java | 31 ---- .../android/server/ConnectivityService.java | 22 ++- .../android/net/NetworkCapabilitiesTest.java | 173 ++++++++---------- .../android/server/NetworkAgentWrapper.java | 4 +- .../server/ConnectivityServiceTest.java | 60 +++--- .../android/server/connectivity/VpnTest.java | 133 ++++++-------- 8 files changed, 185 insertions(+), 274 deletions(-) diff --git a/framework/src/android/net/NetworkCapabilities.java b/framework/src/android/net/NetworkCapabilities.java index ba9f21b261..058f3c999d 100644 --- a/framework/src/android/net/NetworkCapabilities.java +++ b/framework/src/android/net/NetworkCapabilities.java @@ -32,7 +32,6 @@ import android.os.Parcelable; import android.os.Process; import android.text.TextUtils; import android.util.ArraySet; -import android.util.Range; import android.util.proto.ProtoOutputStream; import com.android.internal.annotations.VisibleForTesting; @@ -154,7 +153,7 @@ public final class NetworkCapabilities implements Parcelable { setTransportInfo(null); } mSignalStrength = nc.mSignalStrength; - mUids = (nc.mUids == null) ? null : new ArraySet<>(nc.mUids); + setUids(nc.mUids); // Will make the defensive copy setAdministratorUids(nc.getAdministratorUids()); mOwnerUid = nc.mOwnerUid; mUnwantedNetworkCapabilities = nc.mUnwantedNetworkCapabilities; @@ -1459,8 +1458,9 @@ public final class NetworkCapabilities implements Parcelable { * @hide */ public @NonNull NetworkCapabilities setSingleUid(int uid) { - mUids = new ArraySet<>(1); - mUids.add(new UidRange(uid, uid)); + final ArraySet identity = new ArraySet<>(1); + identity.add(new UidRange(uid, uid)); + setUids(identity); return this; } @@ -1469,8 +1469,12 @@ public final class NetworkCapabilities implements Parcelable { * This makes a copy of the set so that callers can't modify it after the call. * @hide */ - public @NonNull NetworkCapabilities setUids(@Nullable Set> uids) { - mUids = UidRange.fromIntRanges(uids); + public @NonNull NetworkCapabilities setUids(Set uids) { + if (null == uids) { + mUids = null; + } else { + mUids = new ArraySet<>(uids); + } return this; } @@ -1479,19 +1483,8 @@ public final class NetworkCapabilities implements Parcelable { * This returns a copy of the set so that callers can't modify the original object. * @hide */ - public @Nullable Set> getUids() { - return UidRange.toIntRanges(mUids); - } - - /** - * Get the list of UIDs this network applies to. - * This returns a copy of the set so that callers can't modify the original object. - * @hide - */ - public @Nullable Set getUidRanges() { - if (mUids == null) return null; - - return new ArraySet<>(mUids); + public @Nullable Set getUids() { + return null == mUids ? null : new ArraySet<>(mUids); } /** diff --git a/framework/src/android/net/NetworkRequest.java b/framework/src/android/net/NetworkRequest.java index 4ebbf06c51..dbe3ecc4d7 100644 --- a/framework/src/android/net/NetworkRequest.java +++ b/framework/src/android/net/NetworkRequest.java @@ -45,7 +45,6 @@ import android.os.Parcel; import android.os.Parcelable; import android.os.Process; import android.text.TextUtils; -import android.util.Range; import android.util.proto.ProtoOutputStream; import java.util.Arrays; @@ -278,11 +277,11 @@ public class NetworkRequest implements Parcelable { * Set the watched UIDs for this request. This will be reset and wiped out unless * the calling app holds the CHANGE_NETWORK_STATE permission. * - * @param uids The watched UIDs as a set of {@code Range}, or null for everything. + * @param uids The watched UIDs as a set of UidRanges, or null for everything. * @return The builder to facilitate chaining. * @hide */ - public Builder setUids(@Nullable Set> uids) { + public Builder setUids(Set uids) { mNetworkCapabilities.setUids(uids); return this; } diff --git a/framework/src/android/net/UidRange.java b/framework/src/android/net/UidRange.java index bc67c745c9..26518d32ed 100644 --- a/framework/src/android/net/UidRange.java +++ b/framework/src/android/net/UidRange.java @@ -20,11 +20,8 @@ import android.annotation.Nullable; import android.os.Parcel; import android.os.Parcelable; import android.os.UserHandle; -import android.util.ArraySet; -import android.util.Range; import java.util.Collection; -import java.util.Set; /** * An inclusive range of UIDs. @@ -152,32 +149,4 @@ public final class UidRange implements Parcelable { } return false; } - - /** - * Convert a set of {@code Range} to a set of {@link UidRange}. - */ - @Nullable - public static ArraySet fromIntRanges(@Nullable Set> ranges) { - if (null == ranges) return null; - - final ArraySet uids = new ArraySet<>(); - for (Range range : ranges) { - uids.add(new UidRange(range.getLower(), range.getUpper())); - } - return uids; - } - - /** - * Convert a set of {@link UidRange} to a set of {@code Range}. - */ - @Nullable - public static ArraySet> toIntRanges(@Nullable Set ranges) { - if (null == ranges) return null; - - final ArraySet> uids = new ArraySet<>(); - for (UidRange range : ranges) { - uids.add(new Range(range.start, range.stop)); - } - return uids; - } } diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index e2aa6e76ef..d99da057c3 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1343,7 +1343,7 @@ public class ConnectivityService extends IConnectivityManager.Stub netCap.addCapability(NET_CAPABILITY_INTERNET); netCap.addCapability(NET_CAPABILITY_NOT_VCN_MANAGED); netCap.removeCapability(NET_CAPABILITY_NOT_VPN); - netCap.setUids(UidRange.toIntRanges(Collections.singleton(uids))); + netCap.setUids(Collections.singleton(uids)); return netCap; } @@ -2903,7 +2903,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (0 == defaultRequest.mRequests.size()) { pw.println("none, this should never occur."); } else { - pw.println(defaultRequest.mRequests.get(0).networkCapabilities.getUidRanges()); + pw.println(defaultRequest.mRequests.get(0).networkCapabilities.getUids()); } pw.decreaseIndent(); pw.decreaseIndent(); @@ -5320,8 +5320,9 @@ public class ConnectivityService extends IConnectivityManager.Stub private Set getUids() { // networkCapabilities.getUids() returns a defensive copy. // multilayer requests will all have the same uids so return the first one. - final Set uids = mRequests.get(0).networkCapabilities.getUidRanges(); - return (null == uids) ? new ArraySet<>() : uids; + final Set uids = null == mRequests.get(0).networkCapabilities.getUids() + ? new ArraySet<>() : mRequests.get(0).networkCapabilities.getUids(); + return uids; } NetworkRequestInfo(@NonNull final NetworkRequest r, @Nullable final PendingIntent pi, @@ -6127,7 +6128,7 @@ public class ConnectivityService extends IConnectivityManager.Stub for (final NetworkRequestInfo nri : mDefaultNetworkRequests) { // Currently, all network requests will have the same uids therefore checking the first // one is sufficient. If/when uids are tracked at the nri level, this can change. - final Set uids = nri.mRequests.get(0).networkCapabilities.getUidRanges(); + final Set uids = nri.mRequests.get(0).networkCapabilities.getUids(); if (null == uids) { continue; } @@ -6568,7 +6569,7 @@ public class ConnectivityService extends IConnectivityManager.Stub return; } - final Set ranges = nai.networkCapabilities.getUidRanges(); + final Set ranges = nai.networkCapabilities.getUids(); final int vpnAppUid = nai.networkCapabilities.getOwnerUid(); // TODO: this create a window of opportunity for apps to receive traffic between the time // when the old rules are removed and the time when new rules are added. To fix this, @@ -6933,8 +6934,8 @@ public class ConnectivityService extends IConnectivityManager.Stub private void updateUids(NetworkAgentInfo nai, NetworkCapabilities prevNc, NetworkCapabilities newNc) { - Set prevRanges = null == prevNc ? null : prevNc.getUidRanges(); - Set newRanges = null == newNc ? null : newNc.getUidRanges(); + Set prevRanges = null == prevNc ? null : prevNc.getUids(); + Set newRanges = null == newNc ? null : newNc.getUids(); if (null == prevRanges) prevRanges = new ArraySet<>(); if (null == newRanges) newRanges = new ArraySet<>(); final Set prevRangesCopy = new ArraySet<>(prevRanges); @@ -9265,7 +9266,7 @@ public class ConnectivityService extends IConnectivityManager.Stub final ArrayList nrs = new ArrayList<>(); nrs.add(createNetworkRequest(NetworkRequest.Type.REQUEST, pref.capabilities)); nrs.add(createDefaultRequest()); - setNetworkRequestUids(nrs, UidRange.fromIntRanges(pref.capabilities.getUids())); + setNetworkRequestUids(nrs, pref.capabilities.getUids()); final NetworkRequestInfo nri = new NetworkRequestInfo(nrs); result.add(nri); } @@ -9481,8 +9482,9 @@ public class ConnectivityService extends IConnectivityManager.Stub private static void setNetworkRequestUids(@NonNull final List requests, @NonNull final Set uids) { + final Set ranges = new ArraySet<>(uids); for (final NetworkRequest req : requests) { - req.networkCapabilities.setUids(UidRange.toIntRanges(uids)); + req.networkCapabilities.setUids(ranges); } } diff --git a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java index a7ad695641..0dfec75922 100644 --- a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java +++ b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java @@ -69,7 +69,6 @@ import android.net.wifi.aware.WifiAwareNetworkSpecifier; import android.os.Build; import android.test.suitebuilder.annotation.SmallTest; import android.util.ArraySet; -import android.util.Range; import androidx.test.runner.AndroidJUnit4; @@ -241,93 +240,72 @@ public class NetworkCapabilitiesTest { @Test public void testSetUids() { final NetworkCapabilities netCap = new NetworkCapabilities(); - // Null uids match all UIDs - netCap.setUids(null); - assertTrue(netCap.appliesToUid(10)); - assertTrue(netCap.appliesToUid(200)); + final Set uids = new ArraySet<>(); + uids.add(new UidRange(50, 100)); + uids.add(new UidRange(3000, 4000)); + netCap.setUids(uids); + assertTrue(netCap.appliesToUid(50)); + assertTrue(netCap.appliesToUid(80)); + assertTrue(netCap.appliesToUid(100)); assertTrue(netCap.appliesToUid(3000)); - assertTrue(netCap.appliesToUid(10010)); + assertTrue(netCap.appliesToUid(3001)); + assertFalse(netCap.appliesToUid(10)); + assertFalse(netCap.appliesToUid(25)); + assertFalse(netCap.appliesToUid(49)); + assertFalse(netCap.appliesToUid(101)); + assertFalse(netCap.appliesToUid(2000)); + assertFalse(netCap.appliesToUid(100000)); + assertTrue(netCap.appliesToUidRange(new UidRange(50, 100))); assertTrue(netCap.appliesToUidRange(new UidRange(70, 72))); assertTrue(netCap.appliesToUidRange(new UidRange(3500, 3912))); + assertFalse(netCap.appliesToUidRange(new UidRange(1, 100))); + assertFalse(netCap.appliesToUidRange(new UidRange(49, 100))); + assertFalse(netCap.appliesToUidRange(new UidRange(1, 10))); + assertFalse(netCap.appliesToUidRange(new UidRange(60, 101))); + assertFalse(netCap.appliesToUidRange(new UidRange(60, 3400))); + + NetworkCapabilities netCap2 = new NetworkCapabilities(); + // A new netcap object has null UIDs, so anything will satisfy it. + assertTrue(netCap2.satisfiedByUids(netCap)); + // Still not equal though. + assertFalse(netCap2.equalsUids(netCap)); + netCap2.setUids(uids); + assertTrue(netCap2.satisfiedByUids(netCap)); + assertTrue(netCap.equalsUids(netCap2)); + assertTrue(netCap2.equalsUids(netCap)); + + uids.add(new UidRange(600, 700)); + netCap2.setUids(uids); + assertFalse(netCap2.satisfiedByUids(netCap)); + assertFalse(netCap.appliesToUid(650)); + assertTrue(netCap2.appliesToUid(650)); + netCap.combineCapabilities(netCap2); + assertTrue(netCap2.satisfiedByUids(netCap)); + assertTrue(netCap.appliesToUid(650)); + assertFalse(netCap.appliesToUid(500)); + + assertTrue(new NetworkCapabilities().satisfiedByUids(netCap)); + netCap.combineCapabilities(new NetworkCapabilities()); + assertTrue(netCap.appliesToUid(500)); assertTrue(netCap.appliesToUidRange(new UidRange(1, 100000))); - - if (isAtLeastS()) { - final Set> uids = new ArraySet<>(); - uids.add(uidRange(50, 100)); - uids.add(uidRange(3000, 4000)); - netCap.setUids(uids); - assertTrue(netCap.appliesToUid(50)); - assertTrue(netCap.appliesToUid(80)); - assertTrue(netCap.appliesToUid(100)); - assertTrue(netCap.appliesToUid(3000)); - assertTrue(netCap.appliesToUid(3001)); - assertFalse(netCap.appliesToUid(10)); - assertFalse(netCap.appliesToUid(25)); - assertFalse(netCap.appliesToUid(49)); - assertFalse(netCap.appliesToUid(101)); - assertFalse(netCap.appliesToUid(2000)); - assertFalse(netCap.appliesToUid(100000)); - - assertTrue(netCap.appliesToUidRange(new UidRange(50, 100))); - assertTrue(netCap.appliesToUidRange(new UidRange(70, 72))); - assertTrue(netCap.appliesToUidRange(new UidRange(3500, 3912))); - assertFalse(netCap.appliesToUidRange(new UidRange(1, 100))); - assertFalse(netCap.appliesToUidRange(new UidRange(49, 100))); - assertFalse(netCap.appliesToUidRange(new UidRange(1, 10))); - assertFalse(netCap.appliesToUidRange(new UidRange(60, 101))); - assertFalse(netCap.appliesToUidRange(new UidRange(60, 3400))); - - NetworkCapabilities netCap2 = new NetworkCapabilities(); - // A new netcap object has null UIDs, so anything will satisfy it. - assertTrue(netCap2.satisfiedByUids(netCap)); - // Still not equal though. - assertFalse(netCap2.equalsUids(netCap)); - netCap2.setUids(uids); - assertTrue(netCap2.satisfiedByUids(netCap)); - assertTrue(netCap.equalsUids(netCap2)); - assertTrue(netCap2.equalsUids(netCap)); - - uids.add(uidRange(600, 700)); - netCap2.setUids(uids); - assertFalse(netCap2.satisfiedByUids(netCap)); - assertFalse(netCap.appliesToUid(650)); - assertTrue(netCap2.appliesToUid(650)); - netCap.combineCapabilities(netCap2); - assertTrue(netCap2.satisfiedByUids(netCap)); - assertTrue(netCap.appliesToUid(650)); - assertFalse(netCap.appliesToUid(500)); - - assertTrue(new NetworkCapabilities().satisfiedByUids(netCap)); - netCap.combineCapabilities(new NetworkCapabilities()); - assertTrue(netCap.appliesToUid(500)); - assertTrue(netCap.appliesToUidRange(new UidRange(1, 100000))); - assertFalse(netCap2.appliesToUid(500)); - assertFalse(netCap2.appliesToUidRange(new UidRange(1, 100000))); - assertTrue(new NetworkCapabilities().satisfiedByUids(netCap)); - - // Null uids satisfies everything. - netCap.setUids(null); - assertTrue(netCap2.satisfiedByUids(netCap)); - assertTrue(netCap.satisfiedByUids(netCap2)); - netCap2.setUids(null); - assertTrue(netCap2.satisfiedByUids(netCap)); - assertTrue(netCap.satisfiedByUids(netCap2)); - } + assertFalse(netCap2.appliesToUid(500)); + assertFalse(netCap2.appliesToUidRange(new UidRange(1, 100000))); + assertTrue(new NetworkCapabilities().satisfiedByUids(netCap)); } @Test public void testParcelNetworkCapabilities() { - final Set> uids = new ArraySet<>(); - uids.add(uidRange(50, 100)); - uids.add(uidRange(3000, 4000)); + final Set uids = new ArraySet<>(); + uids.add(new UidRange(50, 100)); + uids.add(new UidRange(3000, 4000)); final NetworkCapabilities netCap = new NetworkCapabilities() .addCapability(NET_CAPABILITY_INTERNET) + .setUids(uids) .addCapability(NET_CAPABILITY_EIMS) .addCapability(NET_CAPABILITY_NOT_METERED); if (isAtLeastS()) { netCap.setSubIds(Set.of(TEST_SUBID1, TEST_SUBID2)); - netCap.setUids(uids); } else if (isAtLeastR()) { netCap.setOwnerUid(123); netCap.setAdministratorUids(new int[] {5, 11}); @@ -562,16 +540,12 @@ public class NetworkCapabilitiesTest { assertFalse(nc1.satisfiedByNetworkCapabilities(nc2)); } - private ArraySet> uidRanges(int from, int to) { - final ArraySet> range = new ArraySet<>(1); - range.add(uidRange(from, to)); + private ArraySet uidRange(int from, int to) { + final ArraySet range = new ArraySet<>(1); + range.add(new UidRange(from, to)); return range; } - private Range uidRange(int from, int to) { - return new Range(from, to); - } - @Test @IgnoreUpTo(Build.VERSION_CODES.Q) public void testSetAdministratorUids() { NetworkCapabilities nc = @@ -627,23 +601,23 @@ public class NetworkCapabilitiesTest { } catch (IllegalStateException expected) {} nc1.setSSID(TEST_SSID); - if (isAtLeastS()) { - nc1.setUids(uidRanges(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(uidRanges(10, 13)); - nc2.setUids(uidRanges(20, 23)); - assertNotEquals(nc1, nc2); - nc1.combineCapabilities(nc2); - assertTrue(nc1.appliesToUid(12)); - assertFalse(nc2.appliesToUid(12)); - assertTrue(nc1.appliesToUid(22)); - assertTrue(nc2.appliesToUid(22)); + 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)); - // Verify the subscription id list can be combined only when they are equal. + // Verify the subscription id list can be combined only when they are equal. + if (isAtLeastS()) { nc1.setSubIds(Set.of(TEST_SUBID1, TEST_SUBID2)); nc2.setSubIds(Set.of(TEST_SUBID2)); assertThrows(IllegalStateException.class, () -> nc2.combineCapabilities(nc1)); @@ -799,11 +773,8 @@ public class NetworkCapabilitiesTest { if (isAtLeastR()) { assertTrue(DIFFERENT_TEST_SSID.equals(nc2.getSsid())); } - if (isAtLeastS()) { - nc1.setUids(uidRanges(10, 13)); - } else { - nc1.setUids(null); - } + + nc1.setUids(uidRange(10, 13)); nc2.set(nc1); // Overwrites, as opposed to combineCapabilities assertEquals(nc1, nc2); diff --git a/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java b/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java index e2d43cbb8e..01d8186c7d 100644 --- a/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java +++ b/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java @@ -44,11 +44,11 @@ import android.net.NetworkProvider; import android.net.NetworkSpecifier; import android.net.QosFilter; import android.net.SocketKeepalive; +import android.net.UidRange; import android.os.ConditionVariable; import android.os.HandlerThread; import android.os.Message; import android.util.Log; -import android.util.Range; import com.android.net.module.util.ArrayTrackRecord; import com.android.server.connectivity.ConnectivityConstants; @@ -222,7 +222,7 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork { mNetworkAgent.sendNetworkCapabilities(mNetworkCapabilities); } - public void setUids(Set> uids) { + public void setUids(Set uids) { mNetworkCapabilities.setUids(uids); mNetworkAgent.sendNetworkCapabilities(mNetworkCapabilities); } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 6cb5305ea1..8b20a51b44 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -266,7 +266,6 @@ import android.text.TextUtils; import android.util.ArraySet; import android.util.Log; import android.util.Pair; -import android.util.Range; import android.util.SparseArray; import androidx.test.InstrumentationRegistry; @@ -1159,7 +1158,7 @@ public class ConnectivityServiceTest { } public void setUids(Set uids) { - mNetworkCapabilities.setUids(UidRange.toIntRanges(uids)); + mNetworkCapabilities.setUids(uids); if (mAgentRegistered) { mMockNetworkAgent.setNetworkCapabilities(mNetworkCapabilities, true); } @@ -1464,8 +1463,6 @@ public class ConnectivityServiceTest { } private static final int PRIMARY_USER = 0; - private static final UidRange PRIMARY_UIDRANGE = - UidRange.createForUser(UserHandle.of(PRIMARY_USER)); private static final int APP1_UID = UserHandle.getUid(PRIMARY_USER, 10100); private static final int APP2_UID = UserHandle.getUid(PRIMARY_USER, 10101); private static final int VPN_UID = UserHandle.getUid(PRIMARY_USER, 10043); @@ -6947,7 +6944,7 @@ public class ConnectivityServiceTest { final int uid = Process.myUid(); NetworkCapabilities nc = mCm.getNetworkCapabilities(mMockVpn.getNetwork()); assertNotNull("nc=" + nc, nc.getUids()); - assertEquals(nc.getUids(), UidRange.toIntRanges(uidRangesForUids(uid))); + assertEquals(nc.getUids(), uidRangesForUids(uid)); assertVpnTransportInfo(nc, VpnManager.TYPE_VPN_SERVICE); // Set an underlying network and expect to see the VPN transports change. @@ -6972,13 +6969,10 @@ public class ConnectivityServiceTest { // Expect that the VPN UID ranges contain both |uid| and the UID range for the newly-added // restricted user. - final UidRange rRange = UidRange.createForUser(UserHandle.of(RESTRICTED_USER)); - final Range restrictUidRange = new Range(rRange.start, rRange.stop); - final Range singleUidRange = new Range(uid, uid); callback.expectCapabilitiesThat(mMockVpn, (caps) -> caps.getUids().size() == 2 - && caps.getUids().contains(singleUidRange) - && caps.getUids().contains(restrictUidRange) + && caps.getUids().contains(new UidRange(uid, uid)) + && caps.getUids().contains(createUidRange(RESTRICTED_USER)) && caps.hasTransport(TRANSPORT_VPN) && caps.hasTransport(TRANSPORT_WIFI)); @@ -6987,8 +6981,8 @@ public class ConnectivityServiceTest { callback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent); callback.expectCapabilitiesThat(mMockVpn, (caps) -> caps.getUids().size() == 2 - && caps.getUids().contains(singleUidRange) - && caps.getUids().contains(restrictUidRange) + && caps.getUids().contains(new UidRange(uid, uid)) + && caps.getUids().contains(createUidRange(RESTRICTED_USER)) && caps.hasTransport(TRANSPORT_VPN) && !caps.hasTransport(TRANSPORT_WIFI)); @@ -7002,7 +6996,7 @@ public class ConnectivityServiceTest { // change made just before that (i.e., loss of TRANSPORT_WIFI) is preserved. callback.expectCapabilitiesThat(mMockVpn, (caps) -> caps.getUids().size() == 1 - && caps.getUids().contains(singleUidRange) + && caps.getUids().contains(new UidRange(uid, uid)) && caps.hasTransport(TRANSPORT_VPN) && !caps.hasTransport(TRANSPORT_WIFI)); } @@ -7660,7 +7654,7 @@ public class ConnectivityServiceTest { assertNotNull(underlying); mMockVpn.setVpnType(VpnManager.TYPE_VPN_LEGACY); // The legacy lockdown VPN only supports userId 0. - final Set ranges = Collections.singleton(PRIMARY_UIDRANGE); + final Set ranges = Collections.singleton(createUidRange(PRIMARY_USER)); mMockVpn.registerAgent(ranges); mMockVpn.setUnderlyingNetworks(new Network[]{underlying}); mMockVpn.connect(true); @@ -8622,7 +8616,7 @@ public class ConnectivityServiceTest { lp.addRoute(new RouteInfo(new IpPrefix(Inet4Address.ANY, 0), null)); lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), RTN_UNREACHABLE)); // The uid range needs to cover the test app so the network is visible to it. - final Set vpnRange = Collections.singleton(PRIMARY_UIDRANGE); + final Set vpnRange = Collections.singleton(createUidRange(PRIMARY_USER)); mMockVpn.establish(lp, VPN_UID, vpnRange); assertVpnUidRangesUpdated(true, vpnRange, VPN_UID); @@ -8650,7 +8644,7 @@ public class ConnectivityServiceTest { lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), null)); lp.addRoute(new RouteInfo(new IpPrefix(Inet4Address.ANY, 0), null)); // The uid range needs to cover the test app so the network is visible to it. - final Set vpnRange = Collections.singleton(PRIMARY_UIDRANGE); + final Set vpnRange = Collections.singleton(createUidRange(PRIMARY_USER)); mMockVpn.establish(lp, Process.SYSTEM_UID, vpnRange); assertVpnUidRangesUpdated(true, vpnRange, Process.SYSTEM_UID); @@ -8666,7 +8660,7 @@ public class ConnectivityServiceTest { lp.addRoute(new RouteInfo(new IpPrefix("192.0.2.0/24"), null, "tun0")); lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), RTN_UNREACHABLE)); // The uid range needs to cover the test app so the network is visible to it. - final Set vpnRange = Collections.singleton(PRIMARY_UIDRANGE); + final Set vpnRange = Collections.singleton(createUidRange(PRIMARY_USER)); mMockVpn.establish(lp, Process.SYSTEM_UID, vpnRange); assertVpnUidRangesUpdated(true, vpnRange, Process.SYSTEM_UID); @@ -8681,7 +8675,7 @@ public class ConnectivityServiceTest { lp.addRoute(new RouteInfo(new IpPrefix(Inet4Address.ANY, 0), null)); lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), null)); // The uid range needs to cover the test app so the network is visible to it. - final Set vpnRange = Collections.singleton(PRIMARY_UIDRANGE); + final Set vpnRange = Collections.singleton(createUidRange(PRIMARY_USER)); mMockVpn.establish(lp, VPN_UID, vpnRange); assertVpnUidRangesUpdated(true, vpnRange, VPN_UID); @@ -8733,7 +8727,7 @@ public class ConnectivityServiceTest { lp.addRoute(new RouteInfo(new IpPrefix(Inet4Address.ANY, 0), RTN_UNREACHABLE)); lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), null)); // The uid range needs to cover the test app so the network is visible to it. - final UidRange vpnRange = PRIMARY_UIDRANGE; + final UidRange vpnRange = createUidRange(PRIMARY_USER); final Set vpnRanges = Collections.singleton(vpnRange); mMockVpn.establish(lp, VPN_UID, vpnRanges); assertVpnUidRangesUpdated(true, vpnRanges, VPN_UID); @@ -9014,7 +9008,7 @@ public class ConnectivityServiceTest { private void setupConnectionOwnerUid(int vpnOwnerUid, @VpnManager.VpnType int vpnType) throws Exception { - final Set vpnRange = Collections.singleton(PRIMARY_UIDRANGE); + final Set vpnRange = Collections.singleton(createUidRange(PRIMARY_USER)); mMockVpn.setVpnType(vpnType); mMockVpn.establish(new LinkProperties(), vpnOwnerUid, vpnRange); assertVpnUidRangesUpdated(true, vpnRange, vpnOwnerUid); @@ -9574,7 +9568,7 @@ public class ConnectivityServiceTest { lp.setInterfaceName("tun0"); lp.addRoute(new RouteInfo(new IpPrefix(Inet4Address.ANY, 0), null)); lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), null)); - final UidRange vpnRange = PRIMARY_UIDRANGE; + final UidRange vpnRange = createUidRange(PRIMARY_USER); Set vpnRanges = Collections.singleton(vpnRange); mMockVpn.establish(lp, VPN_UID, vpnRanges); assertVpnUidRangesUpdated(true, vpnRanges, VPN_UID); @@ -9772,7 +9766,7 @@ public class ConnectivityServiceTest { .thenReturn(hasFeature); } - private Range getNriFirstUidRange( + private UidRange getNriFirstUidRange( @NonNull final ConnectivityService.NetworkRequestInfo nri) { return nri.mRequests.get(0).networkCapabilities.getUids().iterator().next(); } @@ -9955,11 +9949,11 @@ public class ConnectivityServiceTest { pref)); // Sort by uid to access nris by index - nris.sort(Comparator.comparingInt(nri -> getNriFirstUidRange(nri).getLower())); - assertEquals(TEST_PACKAGE_UID, (int) getNriFirstUidRange(nris.get(0)).getLower()); - assertEquals(TEST_PACKAGE_UID, (int) getNriFirstUidRange(nris.get(0)).getUpper()); - assertEquals(testPackageNameUid2, (int) getNriFirstUidRange(nris.get(1)).getLower()); - assertEquals(testPackageNameUid2, (int) getNriFirstUidRange(nris.get(1)).getUpper()); + nris.sort(Comparator.comparingInt(nri -> getNriFirstUidRange(nri).start)); + assertEquals(TEST_PACKAGE_UID, getNriFirstUidRange(nris.get(0)).start); + assertEquals(TEST_PACKAGE_UID, getNriFirstUidRange(nris.get(0)).stop); + assertEquals(testPackageNameUid2, getNriFirstUidRange(nris.get(1)).start); + assertEquals(testPackageNameUid2, getNriFirstUidRange(nris.get(1)).stop); } @Test @@ -9989,17 +9983,17 @@ public class ConnectivityServiceTest { // UIDs for all users and all managed packages should be present. // Two users each with two packages. final int expectedUidSize = 2; - final List> uids = + final List uids = new ArrayList<>(nris.get(0).mRequests.get(0).networkCapabilities.getUids()); assertEquals(expectedUidSize, uids.size()); // Sort by uid to access nris by index - uids.sort(Comparator.comparingInt(uid -> uid.getLower())); + uids.sort(Comparator.comparingInt(uid -> uid.start)); final int secondUserTestPackageUid = UserHandle.getUid(secondUser, TEST_PACKAGE_UID); - assertEquals(TEST_PACKAGE_UID, (int) uids.get(0).getLower()); - assertEquals(TEST_PACKAGE_UID, (int) uids.get(0).getUpper()); - assertEquals(secondUserTestPackageUid, (int) uids.get(1).getLower()); - assertEquals(secondUserTestPackageUid, (int) uids.get(1).getUpper()); + assertEquals(TEST_PACKAGE_UID, uids.get(0).start); + assertEquals(TEST_PACKAGE_UID, uids.get(0).stop); + assertEquals(secondUserTestPackageUid, uids.get(1).start); + assertEquals(secondUserTestPackageUid, uids.get(1).stop); } @Test diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index 6ad4900989..11fcea60d9 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -23,7 +23,6 @@ import static android.content.pm.UserInfo.FLAG_RESTRICTED; import static android.net.ConnectivityManager.NetworkCallback; import static android.net.INetd.IF_STATE_DOWN; import static android.net.INetd.IF_STATE_UP; -import static android.os.UserHandle.PER_USER_RANGE; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; @@ -75,6 +74,7 @@ import android.net.Network; import android.net.NetworkCapabilities; import android.net.NetworkInfo.DetailedState; import android.net.RouteInfo; +import android.net.UidRange; import android.net.UidRangeParcel; import android.net.VpnManager; import android.net.VpnService; @@ -181,7 +181,8 @@ public class VpnTest { mPackages.put(PKGS[i], PKG_UIDS[i]); } } - private static final Range PRI_USER_RANGE = uidRangeForUser(primaryUser.id); + private static final UidRange PRI_USER_RANGE = + UidRange.createForUser(UserHandle.of(primaryUser.id)); @Mock(answer = Answers.RETURNS_DEEP_STUBS) private Context mContext; @Mock private UserManager mUserManager; @@ -259,21 +260,6 @@ public class VpnTest { .thenReturn(tunnelResp); } - private Set> rangeSet(Range ... ranges) { - final Set> range = new ArraySet<>(); - for (Range r : ranges) range.add(r); - - return range; - } - - private static Range uidRangeForUser(int userId) { - return new Range(userId * PER_USER_RANGE, (userId + 1) * PER_USER_RANGE - 1); - } - - private Range uidRange(int start, int stop) { - return new Range(start, stop); - } - @Test public void testRestrictedProfilesAreAddedToVpn() { setMockedUsers(primaryUser, secondaryUser, restrictedProfileA, restrictedProfileB); @@ -282,10 +268,12 @@ public class VpnTest { // Assume the user can have restricted profiles. doReturn(true).when(mUserManager).canHaveRestrictedProfile(); - final Set> ranges = + final Set ranges = vpn.createUserAndRestrictedProfilesRanges(primaryUser.id, null, null); - assertEquals(rangeSet(PRI_USER_RANGE, uidRangeForUser(restrictedProfileA.id)), ranges); + assertEquals(new ArraySet<>(Arrays.asList(new UidRange[] { + PRI_USER_RANGE, UidRange.createForUser(UserHandle.of(restrictedProfileA.id)) + })), ranges); } @Test @@ -293,10 +281,10 @@ public class VpnTest { setMockedUsers(primaryUser, managedProfileA); final Vpn vpn = createVpn(primaryUser.id); - final Set> ranges = vpn.createUserAndRestrictedProfilesRanges(primaryUser.id, + final Set ranges = vpn.createUserAndRestrictedProfilesRanges(primaryUser.id, null, null); - assertEquals(rangeSet(PRI_USER_RANGE), ranges); + assertEquals(new ArraySet<>(Arrays.asList(new UidRange[] { PRI_USER_RANGE })), ranges); } @Test @@ -304,38 +292,35 @@ public class VpnTest { setMockedUsers(primaryUser, restrictedProfileA, managedProfileA); final Vpn vpn = createVpn(primaryUser.id); - final Set> ranges = new ArraySet<>(); + final Set ranges = new ArraySet<>(); vpn.addUserToRanges(ranges, primaryUser.id, null, null); - assertEquals(rangeSet(PRI_USER_RANGE), ranges); + assertEquals(new ArraySet<>(Arrays.asList(new UidRange[] { PRI_USER_RANGE })), ranges); } @Test public void testUidAllowAndDenylist() throws Exception { final Vpn vpn = createVpn(primaryUser.id); - final Range user = PRI_USER_RANGE; - final int userStart = user.getLower(); - final int userStop = user.getUpper(); + final UidRange user = PRI_USER_RANGE; final String[] packages = {PKGS[0], PKGS[1], PKGS[2]}; // Allowed list - final Set> allow = vpn.createUserAndRestrictedProfilesRanges(primaryUser.id, - Arrays.asList(packages), null /* disallowedApplications */); - assertEquals(rangeSet( - uidRange(userStart + PKG_UIDS[0], userStart + PKG_UIDS[0]), - uidRange(userStart + PKG_UIDS[1], userStart + PKG_UIDS[2])), - allow); + final Set allow = vpn.createUserAndRestrictedProfilesRanges(primaryUser.id, + Arrays.asList(packages), null); + assertEquals(new ArraySet<>(Arrays.asList(new UidRange[] { + new UidRange(user.start + PKG_UIDS[0], user.start + PKG_UIDS[0]), + new UidRange(user.start + PKG_UIDS[1], user.start + PKG_UIDS[2]) + })), allow); // Denied list - final Set> disallow = - vpn.createUserAndRestrictedProfilesRanges(primaryUser.id, - null /* allowedApplications */, Arrays.asList(packages)); - assertEquals(rangeSet( - uidRange(userStart, userStart + PKG_UIDS[0] - 1), - uidRange(userStart + PKG_UIDS[0] + 1, userStart + PKG_UIDS[1] - 1), - /* Empty range between UIDS[1] and UIDS[2], should be excluded, */ - uidRange(userStart + PKG_UIDS[2] + 1, userStop)), - disallow); + final Set disallow = vpn.createUserAndRestrictedProfilesRanges(primaryUser.id, + null, Arrays.asList(packages)); + assertEquals(new ArraySet<>(Arrays.asList(new UidRange[] { + new UidRange(user.start, user.start + PKG_UIDS[0] - 1), + new UidRange(user.start + PKG_UIDS[0] + 1, user.start + PKG_UIDS[1] - 1), + /* Empty range between UIDS[1] and UIDS[2], should be excluded, */ + new UidRange(user.start + PKG_UIDS[2] + 1, user.stop) + })), disallow); } @Test @@ -365,86 +350,84 @@ public class VpnTest { @Test public void testLockdownChangingPackage() throws Exception { final Vpn vpn = createVpn(primaryUser.id); - final Range user = PRI_USER_RANGE; - final int userStart = user.getLower(); - final int userStop = user.getUpper(); + final UidRange user = PRI_USER_RANGE; + // Set always-on without lockdown. assertTrue(vpn.setAlwaysOnPackage(PKGS[1], false, null)); // Set always-on with lockdown. assertTrue(vpn.setAlwaysOnPackage(PKGS[1], true, null)); verify(mConnectivityManager).setRequireVpnForUids(true, toRanges(new UidRangeParcel[] { - new UidRangeParcel(userStart, userStart + PKG_UIDS[1] - 1), - new UidRangeParcel(userStart + PKG_UIDS[1] + 1, userStop) + new UidRangeParcel(user.start, user.start + PKG_UIDS[1] - 1), + new UidRangeParcel(user.start + PKG_UIDS[1] + 1, user.stop) })); // Switch to another app. assertTrue(vpn.setAlwaysOnPackage(PKGS[3], true, null)); verify(mConnectivityManager).setRequireVpnForUids(false, toRanges(new UidRangeParcel[] { - new UidRangeParcel(userStart, userStart + PKG_UIDS[1] - 1), - new UidRangeParcel(userStart + PKG_UIDS[1] + 1, userStop) + new UidRangeParcel(user.start, user.start + PKG_UIDS[1] - 1), + new UidRangeParcel(user.start + PKG_UIDS[1] + 1, user.stop) })); verify(mConnectivityManager).setRequireVpnForUids(true, toRanges(new UidRangeParcel[] { - new UidRangeParcel(userStart, userStart + PKG_UIDS[3] - 1), - new UidRangeParcel(userStart + PKG_UIDS[3] + 1, userStop) + new UidRangeParcel(user.start, user.start + PKG_UIDS[3] - 1), + new UidRangeParcel(user.start + PKG_UIDS[3] + 1, user.stop) })); } @Test public void testLockdownAllowlist() throws Exception { final Vpn vpn = createVpn(primaryUser.id); - final Range user = PRI_USER_RANGE; - final int userStart = user.getLower(); - final int userStop = user.getUpper(); + final UidRange user = PRI_USER_RANGE; + // Set always-on with lockdown and allow app PKGS[2] from lockdown. assertTrue(vpn.setAlwaysOnPackage( PKGS[1], true, Collections.singletonList(PKGS[2]))); - verify(mConnectivityManager).setRequireVpnForUids(true, toRanges(new UidRangeParcel[] { - new UidRangeParcel(userStart, userStart + PKG_UIDS[1] - 1), - new UidRangeParcel(userStart + PKG_UIDS[2] + 1, userStop) + verify(mConnectivityManager).setRequireVpnForUids(true, toRanges(new UidRangeParcel[] { + new UidRangeParcel(user.start, user.start + PKG_UIDS[1] - 1), + new UidRangeParcel(user.start + PKG_UIDS[2] + 1, user.stop) })); // Change allowed app list to PKGS[3]. assertTrue(vpn.setAlwaysOnPackage( PKGS[1], true, Collections.singletonList(PKGS[3]))); verify(mConnectivityManager).setRequireVpnForUids(false, toRanges(new UidRangeParcel[] { - new UidRangeParcel(userStart + PKG_UIDS[2] + 1, userStop) + new UidRangeParcel(user.start + PKG_UIDS[2] + 1, user.stop) })); verify(mConnectivityManager).setRequireVpnForUids(true, toRanges(new UidRangeParcel[] { - new UidRangeParcel(userStart + PKG_UIDS[1] + 1, userStart + PKG_UIDS[3] - 1), - new UidRangeParcel(userStart + PKG_UIDS[3] + 1, userStop) + new UidRangeParcel(user.start + PKG_UIDS[1] + 1, user.start + PKG_UIDS[3] - 1), + new UidRangeParcel(user.start + PKG_UIDS[3] + 1, user.stop) })); // Change the VPN app. assertTrue(vpn.setAlwaysOnPackage( PKGS[0], true, Collections.singletonList(PKGS[3]))); verify(mConnectivityManager).setRequireVpnForUids(false, toRanges(new UidRangeParcel[] { - new UidRangeParcel(userStart, userStart + PKG_UIDS[1] - 1), - new UidRangeParcel(userStart + PKG_UIDS[1] + 1, userStart + PKG_UIDS[3] - 1) + new UidRangeParcel(user.start, user.start + PKG_UIDS[1] - 1), + new UidRangeParcel(user.start + PKG_UIDS[1] + 1, user.start + PKG_UIDS[3] - 1) })); verify(mConnectivityManager).setRequireVpnForUids(true, toRanges(new UidRangeParcel[] { - new UidRangeParcel(userStart, userStart + PKG_UIDS[0] - 1), - new UidRangeParcel(userStart + PKG_UIDS[0] + 1, userStart + PKG_UIDS[3] - 1) + new UidRangeParcel(user.start, user.start + PKG_UIDS[0] - 1), + new UidRangeParcel(user.start + PKG_UIDS[0] + 1, user.start + PKG_UIDS[3] - 1) })); // Remove the list of allowed packages. assertTrue(vpn.setAlwaysOnPackage(PKGS[0], true, null)); verify(mConnectivityManager).setRequireVpnForUids(false, toRanges(new UidRangeParcel[] { - new UidRangeParcel(userStart + PKG_UIDS[0] + 1, userStart + PKG_UIDS[3] - 1), - new UidRangeParcel(userStart + PKG_UIDS[3] + 1, userStop) + new UidRangeParcel(user.start + PKG_UIDS[0] + 1, user.start + PKG_UIDS[3] - 1), + new UidRangeParcel(user.start + PKG_UIDS[3] + 1, user.stop) })); verify(mConnectivityManager).setRequireVpnForUids(true, toRanges(new UidRangeParcel[] { - new UidRangeParcel(userStart + PKG_UIDS[0] + 1, userStop), + new UidRangeParcel(user.start + PKG_UIDS[0] + 1, user.stop), })); // Add the list of allowed packages. assertTrue(vpn.setAlwaysOnPackage( PKGS[0], true, Collections.singletonList(PKGS[1]))); verify(mConnectivityManager).setRequireVpnForUids(false, toRanges(new UidRangeParcel[] { - new UidRangeParcel(userStart + PKG_UIDS[0] + 1, userStop) + new UidRangeParcel(user.start + PKG_UIDS[0] + 1, user.stop) })); verify(mConnectivityManager).setRequireVpnForUids(true, toRanges(new UidRangeParcel[] { - new UidRangeParcel(userStart + PKG_UIDS[0] + 1, userStart + PKG_UIDS[1] - 1), - new UidRangeParcel(userStart + PKG_UIDS[1] + 1, userStop) + new UidRangeParcel(user.start + PKG_UIDS[0] + 1, user.start + PKG_UIDS[1] - 1), + new UidRangeParcel(user.start + PKG_UIDS[1] + 1, user.stop) })); // Try allowing a package with a comma, should be rejected. @@ -456,12 +439,12 @@ public class VpnTest { assertTrue(vpn.setAlwaysOnPackage( PKGS[0], true, Arrays.asList("com.foo.app", PKGS[2], "com.bar.app"))); verify(mConnectivityManager).setRequireVpnForUids(false, toRanges(new UidRangeParcel[] { - new UidRangeParcel(userStart + PKG_UIDS[0] + 1, userStart + PKG_UIDS[1] - 1), - new UidRangeParcel(userStart + PKG_UIDS[1] + 1, userStop) + new UidRangeParcel(user.start + PKG_UIDS[0] + 1, user.start + PKG_UIDS[1] - 1), + new UidRangeParcel(user.start + PKG_UIDS[1] + 1, user.stop) })); verify(mConnectivityManager).setRequireVpnForUids(true, toRanges(new UidRangeParcel[] { - new UidRangeParcel(userStart + PKG_UIDS[0] + 1, userStart + PKG_UIDS[2] - 1), - new UidRangeParcel(userStart + PKG_UIDS[2] + 1, userStop) + new UidRangeParcel(user.start + PKG_UIDS[0] + 1, user.start + PKG_UIDS[2] - 1), + new UidRangeParcel(user.start + PKG_UIDS[2] + 1, user.stop) })); } @@ -469,7 +452,7 @@ public class VpnTest { public void testLockdownRuleRepeatability() throws Exception { final Vpn vpn = createVpn(primaryUser.id); final UidRangeParcel[] primaryUserRangeParcel = new UidRangeParcel[] { - new UidRangeParcel(PRI_USER_RANGE.getLower(), PRI_USER_RANGE.getUpper())}; + new UidRangeParcel(PRI_USER_RANGE.start, PRI_USER_RANGE.stop)}; // Given legacy lockdown is already enabled, vpn.setLockdown(true); verify(mConnectivityManager, times(1)).setRequireVpnForUids(true, @@ -501,7 +484,7 @@ public class VpnTest { public void testLockdownRuleReversibility() throws Exception { final Vpn vpn = createVpn(primaryUser.id); final UidRangeParcel[] entireUser = { - new UidRangeParcel(PRI_USER_RANGE.getLower(), PRI_USER_RANGE.getUpper()) + new UidRangeParcel(PRI_USER_RANGE.start, PRI_USER_RANGE.stop) }; final UidRangeParcel[] exceptPkg0 = { new UidRangeParcel(entireUser[0].start, entireUser[0].start + PKG_UIDS[0] - 1),