diff --git a/framework-t/src/android/net/INetworkStatsService.aidl b/framework-t/src/android/net/INetworkStatsService.aidl index 01ac10681a..ce57a4a145 100644 --- a/framework-t/src/android/net/INetworkStatsService.aidl +++ b/framework-t/src/android/net/INetworkStatsService.aidl @@ -21,10 +21,10 @@ import android.net.INetworkStatsSession; import android.net.Network; import android.net.NetworkStateSnapshot; import android.net.NetworkStats; -import android.net.NetworkStatsHistory; import android.net.NetworkTemplate; import android.net.UnderlyingNetworkInfo; import android.net.netstats.IUsageCallback; +import android.net.netstats.StatsResult; import android.net.netstats.provider.INetworkStatsProvider; import android.net.netstats.provider.INetworkStatsProviderCallback; import android.os.IBinder; @@ -78,16 +78,13 @@ interface INetworkStatsService { void unregisterUsageRequest(in DataUsageRequest request); /** Get the uid stats information since boot */ - NetworkStats getTypelessUidStats(int uid); + StatsResult getUidStats(int uid); /** Get the iface stats information since boot */ - NetworkStats getTypelessIfaceStats(String iface); + StatsResult getIfaceStats(String iface); /** Get the total network stats information since boot */ - NetworkStats getTypelessTotalStats(); - - /** Get the uid stats information (with specified type) since boot */ - long getUidStats(int uid, int type); + StatsResult getTotalStats(); /** Registers a network stats provider */ INetworkStatsProviderCallback registerNetworkStatsProvider(String tag, diff --git a/framework-t/src/android/net/TrafficStats.java b/framework-t/src/android/net/TrafficStats.java index 3b6a69bb33..ab0aaa0a23 100644 --- a/framework-t/src/android/net/TrafficStats.java +++ b/framework-t/src/android/net/TrafficStats.java @@ -19,6 +19,7 @@ package android.net; import static android.annotation.SystemApi.Client.MODULE_LIBRARIES; import android.annotation.NonNull; +import android.annotation.Nullable; import android.annotation.RequiresPermission; import android.annotation.SuppressLint; import android.annotation.SystemApi; @@ -29,6 +30,7 @@ import android.app.usage.NetworkStatsManager; import android.compat.annotation.UnsupportedAppUsage; import android.content.Context; import android.media.MediaPlayer; +import android.net.netstats.StatsResult; import android.os.Binder; import android.os.Build; import android.os.RemoteException; @@ -40,8 +42,6 @@ import java.io.IOException; import java.net.DatagramSocket; import java.net.Socket; import java.net.SocketException; -import java.util.Iterator; -import java.util.Objects; /** @@ -963,13 +963,13 @@ public class TrafficStats { || android.os.Process.myUid() != uid) { return UNSUPPORTED; } - final NetworkStats stats; + final StatsResult stats; try { - stats = getStatsService().getTypelessUidStats(uid); + stats = getStatsService().getUidStats(uid); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } - return getValueForTypeFromFirstEntry(stats, type); + return getEntryValueForType(stats, type); } /** @hide */ @@ -977,13 +977,13 @@ public class TrafficStats { if (!isEntryValueTypeValid(type)) { return UNSUPPORTED; } - final NetworkStats stats; + final StatsResult stats; try { - stats = getStatsService().getTypelessTotalStats(); + stats = getStatsService().getTotalStats(); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } - return getValueForTypeFromFirstEntry(stats, type); + return getEntryValueForType(stats, type); } /** @hide */ @@ -991,13 +991,13 @@ public class TrafficStats { if (!isEntryValueTypeValid(type)) { return UNSUPPORTED; } - final NetworkStats stats; + final StatsResult stats; try { - stats = getStatsService().getTypelessIfaceStats(iface); + stats = getStatsService().getIfaceStats(iface); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } - return getValueForTypeFromFirstEntry(stats, type); + return getEntryValueForType(stats, type); } /** @@ -1127,18 +1127,18 @@ public class TrafficStats { public static final int TYPE_TX_PACKETS = 3; /** @hide */ - private static long getEntryValueForType(@NonNull NetworkStats.Entry entry, int type) { - Objects.requireNonNull(entry); + private static long getEntryValueForType(@Nullable StatsResult stats, int type) { + if (stats == null) return UNSUPPORTED; if (!isEntryValueTypeValid(type)) return UNSUPPORTED; switch (type) { case TYPE_RX_BYTES: - return entry.getRxBytes(); + return stats.rxBytes; case TYPE_RX_PACKETS: - return entry.getRxPackets(); + return stats.rxPackets; case TYPE_TX_BYTES: - return entry.getTxBytes(); + return stats.txBytes; case TYPE_TX_PACKETS: - return entry.getTxPackets(); + return stats.txPackets; default: throw new IllegalStateException("Bug: Invalid type: " + type + " should not reach here."); @@ -1157,13 +1157,5 @@ public class TrafficStats { return false; } } - - /** @hide */ - public static long getValueForTypeFromFirstEntry(@NonNull NetworkStats stats, int type) { - Objects.requireNonNull(stats); - Iterator iter = stats.iterator(); - if (!iter.hasNext()) return UNSUPPORTED; - return getEntryValueForType(iter.next(), type); - } } diff --git a/framework-t/src/android/net/netstats/StatsResult.aidl b/framework-t/src/android/net/netstats/StatsResult.aidl new file mode 100644 index 0000000000..3f09566104 --- /dev/null +++ b/framework-t/src/android/net/netstats/StatsResult.aidl @@ -0,0 +1,31 @@ +/** + * Copyright (c) 2024, The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.net.netstats; + +/** + * A lightweight class to pass result of TrafficStats#get{Total|Iface|Uid}Stats. + * + * @hide + */ +@JavaDerive(equals=true, toString=true) +@JavaOnlyImmutable +parcelable StatsResult { + long rxBytes; + long rxPackets; + long txBytes; + long txPackets; +} \ No newline at end of file diff --git a/service-t/src/com/android/server/net/NetworkStatsService.java b/service-t/src/com/android/server/net/NetworkStatsService.java index 294a85a8db..7572f0e712 100644 --- a/service-t/src/com/android/server/net/NetworkStatsService.java +++ b/service-t/src/com/android/server/net/NetworkStatsService.java @@ -52,7 +52,6 @@ import static android.net.NetworkTemplate.MATCH_WIFI; import static android.net.TrafficStats.KB_IN_BYTES; import static android.net.TrafficStats.MB_IN_BYTES; import static android.net.TrafficStats.UID_TETHERING; -import static android.net.TrafficStats.getValueForTypeFromFirstEntry; import static android.net.connectivity.ConnectivityCompatChanges.ENABLE_TRAFFICSTATS_RATE_LIMIT_CACHE; import static android.net.netstats.NetworkStatsDataMigrationUtils.PREFIX_UID; import static android.net.netstats.NetworkStatsDataMigrationUtils.PREFIX_UID_TAG; @@ -127,6 +126,7 @@ import android.net.UnderlyingNetworkInfo; import android.net.Uri; import android.net.netstats.IUsageCallback; import android.net.netstats.NetworkStatsDataMigrationUtils; +import android.net.netstats.StatsResult; import android.net.netstats.provider.INetworkStatsProvider; import android.net.netstats.provider.INetworkStatsProviderCallback; import android.net.netstats.provider.NetworkStatsProvider; @@ -2133,18 +2133,12 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } } + @Nullable @Override - public long getUidStats(int uid, int type) { - return getValueForTypeFromFirstEntry(getTypelessUidStats(uid), type); - } - - @NonNull - @Override - public NetworkStats getTypelessUidStats(int uid) { - final NetworkStats stats = new NetworkStats(0, 0); + public StatsResult getUidStats(int uid) { final int callingUid = Binder.getCallingUid(); if (callingUid != android.os.Process.SYSTEM_UID && callingUid != uid) { - return stats; + return null; } final NetworkStats.Entry entry; if (mAlwaysUseTrafficStatsServiceRateLimitCache @@ -2153,10 +2147,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { () -> mDeps.nativeGetUidStat(uid)); } else entry = mDeps.nativeGetUidStat(uid); - if (entry != null) { - stats.insertEntry(entry); - } - return stats; + return getStatsResultFromEntryOrNull(entry); } @Nullable @@ -2173,9 +2164,9 @@ public class NetworkStatsService extends INetworkStatsService.Stub { return entry; } - @NonNull + @Nullable @Override - public NetworkStats getTypelessIfaceStats(@NonNull String iface) { + public StatsResult getIfaceStats(@NonNull String iface) { Objects.requireNonNull(iface); final NetworkStats.Entry entry; @@ -2186,11 +2177,13 @@ public class NetworkStatsService extends INetworkStatsService.Stub { () -> getIfaceStatsInternal(iface)); } else entry = getIfaceStatsInternal(iface); - NetworkStats stats = new NetworkStats(0, 0); - if (entry != null) { - stats.insertEntry(entry); - } - return stats; + return getStatsResultFromEntryOrNull(entry); + } + + @Nullable + private StatsResult getStatsResultFromEntryOrNull(@Nullable NetworkStats.Entry entry) { + if (entry == null) return null; + return new StatsResult(entry.rxBytes, entry.rxPackets, entry.txBytes, entry.txPackets); } @Nullable @@ -2203,9 +2196,9 @@ public class NetworkStatsService extends INetworkStatsService.Stub { return entry; } - @NonNull + @Nullable @Override - public NetworkStats getTypelessTotalStats() { + public StatsResult getTotalStats() { final NetworkStats.Entry entry; if (mAlwaysUseTrafficStatsServiceRateLimitCache || mDeps.isChangeEnabled( @@ -2214,11 +2207,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { IFACE_ALL, UID_ALL, () -> getTotalStatsInternal()); } else entry = getTotalStatsInternal(); - final NetworkStats stats = new NetworkStats(0, 0); - if (entry != null) { - stats.insertEntry(entry); - } - return stats; + return getStatsResultFromEntryOrNull(entry); } @Override diff --git a/tests/cts/net/src/android/net/cts/NetworkStatsBinderTest.java b/tests/cts/net/src/android/net/cts/NetworkStatsBinderTest.java index 1a48983028..10adee0f7f 100644 --- a/tests/cts/net/src/android/net/cts/NetworkStatsBinderTest.java +++ b/tests/cts/net/src/android/net/cts/NetworkStatsBinderTest.java @@ -19,28 +19,28 @@ package android.net.cts; import static android.os.Process.INVALID_UID; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import android.annotation.NonNull; +import android.annotation.Nullable; import android.content.Context; import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.net.INetworkStatsService; import android.net.TrafficStats; +import android.net.connectivity.android.net.netstats.StatsResult; import android.os.Build; import android.os.IBinder; import android.os.Process; import android.os.RemoteException; -import android.test.AndroidTestCase; -import android.util.SparseArray; import androidx.test.InstrumentationRegistry; -import androidx.test.runner.AndroidJUnit4; import com.android.internal.util.CollectionUtils; +import com.android.testutils.ConnectivityModuleTest; import com.android.testutils.DevSdkIgnoreRule; +import com.android.testutils.DevSdkIgnoreRunner; -import org.junit.Before; -import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -48,37 +48,20 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.List; -import java.util.function.Function; import java.util.function.Predicate; -@RunWith(AndroidJUnit4.class) +@ConnectivityModuleTest +@DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.S_V2) // Mainline NetworkStats starts from T. +@RunWith(DevSdkIgnoreRunner.class) public class NetworkStatsBinderTest { - // NOTE: These are shamelessly copied from TrafficStats. - private static final int TYPE_RX_BYTES = 0; - private static final int TYPE_RX_PACKETS = 1; - private static final int TYPE_TX_BYTES = 2; - private static final int TYPE_TX_PACKETS = 3; - - @Rule - public DevSdkIgnoreRule mIgnoreRule = new DevSdkIgnoreRule( - Build.VERSION_CODES.Q /* ignoreClassUpTo */); - - private final SparseArray> mUidStatsQueryOpArray = new SparseArray<>(); - - @Before - public void setUp() throws Exception { - mUidStatsQueryOpArray.put(TYPE_RX_BYTES, uid -> TrafficStats.getUidRxBytes(uid)); - mUidStatsQueryOpArray.put(TYPE_RX_PACKETS, uid -> TrafficStats.getUidRxPackets(uid)); - mUidStatsQueryOpArray.put(TYPE_TX_BYTES, uid -> TrafficStats.getUidTxBytes(uid)); - mUidStatsQueryOpArray.put(TYPE_TX_PACKETS, uid -> TrafficStats.getUidTxPackets(uid)); - } - - private long getUidStatsFromBinder(int uid, int type) throws Exception { - Method getServiceMethod = Class.forName("android.os.ServiceManager") + @Nullable + private StatsResult getUidStatsFromBinder(int uid) throws Exception { + final Method getServiceMethod = Class.forName("android.os.ServiceManager") .getDeclaredMethod("getService", new Class[]{String.class}); - IBinder binder = (IBinder) getServiceMethod.invoke(null, Context.NETWORK_STATS_SERVICE); - INetworkStatsService nss = INetworkStatsService.Stub.asInterface(binder); - return nss.getUidStats(uid, type); + final IBinder binder = (IBinder) getServiceMethod.invoke( + null, Context.NETWORK_STATS_SERVICE); + final INetworkStatsService nss = INetworkStatsService.Stub.asInterface(binder); + return nss.getUidStats(uid); } private int getFirstAppUidThat(@NonNull Predicate predicate) { @@ -108,38 +91,34 @@ public class NetworkStatsBinderTest { if (notMyUid != INVALID_UID) testUidList.add(notMyUid); for (final int uid : testUidList) { - for (int i = 0; i < mUidStatsQueryOpArray.size(); i++) { - final int type = mUidStatsQueryOpArray.keyAt(i); - try { - final long uidStatsFromBinder = getUidStatsFromBinder(uid, type); - final long uidTrafficStats = mUidStatsQueryOpArray.get(type).apply(uid); + try { + final StatsResult uidStatsFromBinder = getUidStatsFromBinder(uid); - // Verify that UNSUPPORTED is returned if the uid is not current app uid. - if (uid != myUid) { - assertEquals(uidStatsFromBinder, TrafficStats.UNSUPPORTED); - } + if (uid != myUid) { + // Verify that null is returned if the uid is not current app uid. + assertNull(uidStatsFromBinder); + } else { // Verify that returned result is the same with the result get from // TrafficStats. - // TODO: If the test is flaky then it should instead assert that the values - // are approximately similar. - assertEquals("uidStats is not matched for query type " + type - + ", uid=" + uid + ", myUid=" + myUid, uidTrafficStats, - uidStatsFromBinder); - } catch (IllegalAccessException e) { - /* Java language access prevents exploitation. */ - return; - } catch (InvocationTargetException e) { - /* Underlying method has been changed. */ - return; - } catch (ClassNotFoundException e) { - /* not vulnerable if hidden API no longer available */ - return; - } catch (NoSuchMethodException e) { - /* not vulnerable if hidden API no longer available */ - return; - } catch (RemoteException e) { - return; + assertEquals(uidStatsFromBinder.rxBytes, TrafficStats.getUidRxBytes(uid)); + assertEquals(uidStatsFromBinder.rxPackets, TrafficStats.getUidRxPackets(uid)); + assertEquals(uidStatsFromBinder.txBytes, TrafficStats.getUidTxBytes(uid)); + assertEquals(uidStatsFromBinder.txPackets, TrafficStats.getUidTxPackets(uid)); } + } catch (IllegalAccessException e) { + /* Java language access prevents exploitation. */ + return; + } catch (InvocationTargetException e) { + /* Underlying method has been changed. */ + return; + } catch (ClassNotFoundException e) { + /* not vulnerable if hidden API no longer available */ + return; + } catch (NoSuchMethodException e) { + /* not vulnerable if hidden API no longer available */ + return; + } catch (RemoteException e) { + return; } } } diff --git a/tests/unit/java/android/net/TrafficStatsTest.kt b/tests/unit/java/android/net/TrafficStatsTest.kt deleted file mode 100644 index c61541e34f..0000000000 --- a/tests/unit/java/android/net/TrafficStatsTest.kt +++ /dev/null @@ -1,46 +0,0 @@ -/* -* Copyright (C) 2024 The Android Open Source Project -* -* Licensed under the Apache License, Version 2.0 (the "License"); -* you may not use this file except in compliance with the License. -* You may obtain a copy of the License at -* -* http://www.apache.org/licenses/LICENSE-2.0 -* -* Unless required by applicable law or agreed to in writing, software -* distributed under the License is distributed on an "AS IS" BASIS, -* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -* See the License for the specific language governing permissions and -* limitations under the License. -*/ - -package android.net - -import android.net.TrafficStats.getValueForTypeFromFirstEntry -import android.net.TrafficStats.TYPE_RX_BYTES -import android.net.TrafficStats.UNSUPPORTED -import android.os.Build -import com.android.testutils.DevSdkIgnoreRule -import com.android.testutils.DevSdkIgnoreRunner -import org.junit.Test -import org.junit.runner.RunWith -import kotlin.test.assertEquals - -private const val TEST_IFACE1 = "test_iface1" - -@RunWith(DevSdkIgnoreRunner::class) -@DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.TIRAMISU) -class TrafficStatsTest { - - @Test - fun testGetValueForTypeFromFirstEntry() { - var stats: NetworkStats = NetworkStats(0, 0) - // empty stats - assertEquals(getValueForTypeFromFirstEntry(stats, TYPE_RX_BYTES), UNSUPPORTED.toLong()) - // invalid type - stats.insertEntry(TEST_IFACE1, 1, 2, 3, 4) - assertEquals(getValueForTypeFromFirstEntry(stats, 1000), UNSUPPORTED.toLong()) - // valid type - assertEquals(getValueForTypeFromFirstEntry(stats, TYPE_RX_BYTES), 1) - } -} \ No newline at end of file diff --git a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java index ef4c44d14e..e6f8d2160f 100644 --- a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java @@ -56,7 +56,6 @@ import static android.net.NetworkTemplate.OEM_MANAGED_YES; import static android.net.TrafficStats.MB_IN_BYTES; import static android.net.TrafficStats.UID_REMOVED; import static android.net.TrafficStats.UID_TETHERING; -import static android.net.TrafficStats.getValueForTypeFromFirstEntry; import static android.net.connectivity.ConnectivityCompatChanges.ENABLE_TRAFFICSTATS_RATE_LIMIT_CACHE; import static android.net.netstats.NetworkStatsDataMigrationUtils.PREFIX_UID; import static android.net.netstats.NetworkStatsDataMigrationUtils.PREFIX_UID_TAG; @@ -128,8 +127,8 @@ import android.net.TelephonyNetworkSpecifier; import android.net.TestNetworkSpecifier; import android.net.TetherStatsParcel; import android.net.TetheringManager; -import android.net.TrafficStats; import android.net.UnderlyingNetworkInfo; +import android.net.netstats.StatsResult; import android.net.netstats.provider.INetworkStatsProviderCallback; import android.net.wifi.WifiInfo; import android.os.DropBoxManager; @@ -209,7 +208,6 @@ import java.util.Objects; import java.util.Set; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.function.Function; /** * Tests for {@link NetworkStatsService}. @@ -2515,22 +2513,18 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { // Assert for 3 different API return values respectively. private void assertTrafficStatsValues(String iface, int uid, long rxBytes, long rxPackets, long txBytes, long txPackets) { - assertTrafficStatsValuesThat(rxBytes, rxPackets, txBytes, txPackets, - (type) -> getValueForTypeFromFirstEntry(mService.getTypelessTotalStats(), type)); - assertTrafficStatsValuesThat(rxBytes, rxPackets, txBytes, txPackets, - (type) -> getValueForTypeFromFirstEntry( - mService.getTypelessIfaceStats(iface), type) - ); - assertTrafficStatsValuesThat(rxBytes, rxPackets, txBytes, txPackets, - (type) -> getValueForTypeFromFirstEntry(mService.getTypelessUidStats(uid), type)); + assertStatsResultEquals(mService.getTotalStats(), rxBytes, rxPackets, txBytes, txPackets); + assertStatsResultEquals(mService.getIfaceStats(iface), rxBytes, rxPackets, txBytes, + txPackets); + assertStatsResultEquals(mService.getUidStats(uid), rxBytes, rxPackets, txBytes, txPackets); } - private void assertTrafficStatsValuesThat(long rxBytes, long rxPackets, long txBytes, - long txPackets, Function fetcher) { - assertEquals(rxBytes, (long) fetcher.apply(TrafficStats.TYPE_RX_BYTES)); - assertEquals(rxPackets, (long) fetcher.apply(TrafficStats.TYPE_RX_PACKETS)); - assertEquals(txBytes, (long) fetcher.apply(TrafficStats.TYPE_TX_BYTES)); - assertEquals(txPackets, (long) fetcher.apply(TrafficStats.TYPE_TX_PACKETS)); + private void assertStatsResultEquals(StatsResult stats, long rxBytes, long rxPackets, + long txBytes, long txPackets) { + assertEquals(rxBytes, stats.rxBytes); + assertEquals(rxPackets, stats.rxPackets); + assertEquals(txBytes, stats.txBytes); + assertEquals(txPackets, stats.txPackets); } private void assertShouldRunComparison(boolean expected, boolean isDebuggable) {