From e08de4d322d2526fbd3c9207506226a315eb63f8 Mon Sep 17 00:00:00 2001 From: Motomu Utsumi Date: Wed, 28 Feb 2024 15:58:42 +0900 Subject: [PATCH] Fix wrong request_count in connectivityStateSample and add tests This CL updates CSTest to use mocked appOpsManager to bypass checkPackage enforcement so that test can file network request as SYSTEM_UID Bug: 314627716 Test: atest ConnectivitySampleMetricsTest Change-Id: I0a5f3b1bf2c343644e6f5e2a4ec1b0460e2645c4 --- .../android/server/ConnectivityService.java | 2 +- .../metrics/ConnectivitySampleMetricsTest.kt | 73 ++++++++++++++++++- .../server/connectivityservice/base/CSTest.kt | 20 ++++- 3 files changed, 90 insertions(+), 5 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 30b14b2bf4..e7db52cb42 100755 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -2607,7 +2607,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // Not the system, so it's an app requesting on its own behalf. type = RequestType.RT_APP.getNumber(); } - countPerType.put(type, countPerType.get(type, 0)); + countPerType.put(type, countPerType.get(type, 0) + 1); } for (int i = countPerType.size() - 1; i >= 0; --i) { final RequestCountForType.Builder r = RequestCountForType.newBuilder(); diff --git a/tests/unit/java/com/android/metrics/ConnectivitySampleMetricsTest.kt b/tests/unit/java/com/android/metrics/ConnectivitySampleMetricsTest.kt index 53baee1c89..8a9286fb6a 100644 --- a/tests/unit/java/com/android/metrics/ConnectivitySampleMetricsTest.kt +++ b/tests/unit/java/com/android/metrics/ConnectivitySampleMetricsTest.kt @@ -1,5 +1,6 @@ package com.android.metrics +import android.net.ConnectivityThread import android.net.NetworkCapabilities import android.net.NetworkCapabilities.CONNECTIVITY_MANAGED_CAPABILITIES import android.net.NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL @@ -15,13 +16,20 @@ import android.net.NetworkCapabilities.NET_ENTERPRISE_ID_1 import android.net.NetworkCapabilities.NET_ENTERPRISE_ID_3 import android.net.NetworkCapabilities.TRANSPORT_CELLULAR import android.net.NetworkCapabilities.TRANSPORT_WIFI +import android.net.NetworkRequest import android.net.NetworkScore import android.net.NetworkScore.KEEP_CONNECTED_FOR_TEST import android.net.NetworkScore.POLICY_EXITING import android.net.NetworkScore.POLICY_TRANSPORT_PRIMARY import android.os.Build import android.os.Handler +import android.os.Process +import android.os.Process.SYSTEM_UID import android.stats.connectivity.MeteredState +import android.stats.connectivity.RequestType +import android.stats.connectivity.RequestType.RT_APP +import android.stats.connectivity.RequestType.RT_SYSTEM +import android.stats.connectivity.RequestType.RT_SYSTEM_ON_BEHALF_OF_APP import android.stats.connectivity.ValidatedState import androidx.test.filters.SmallTest import com.android.net.module.util.BitUtils @@ -31,11 +39,13 @@ import com.android.server.connectivity.FullScore import com.android.server.connectivity.FullScore.POLICY_IS_UNMETERED import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo import com.android.testutils.DevSdkIgnoreRunner -import org.junit.Test -import org.junit.runner.RunWith +import com.android.testutils.TestableNetworkCallback import java.util.concurrent.CompletableFuture import kotlin.test.assertEquals import kotlin.test.fail +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith private fun Handler.onHandler(f: () -> T): T { val future = CompletableFuture() @@ -80,7 +90,7 @@ private val NetworkCapabilities.validatedState get() = when { @IgnoreUpTo(Build.VERSION_CODES.TIRAMISU) class ConnectivitySampleMetricsTest : CSTest() { @Test - fun testSampleConnectivityState() { + fun testSampleConnectivityState_Network() { val wifi1Caps = NetworkCapabilities.Builder() .addTransportType(TRANSPORT_WIFI) .addCapability(NET_CAPABILITY_NOT_METERED) @@ -179,4 +189,61 @@ class ConnectivitySampleMetricsTest : CSTest() { "expected ${expectedWifi2Policies.toPolicyString()}, " + "found ${foundWifi2.scorePolicies.toPolicyString()}") } + + private fun fileNetworkRequest(requestType: RequestType, requestCount: Int, uid: Int? = null) { + if (uid != null) { + deps.setCallingUid(uid) + } + try { + repeat(requestCount) { + when (requestType) { + RT_APP, RT_SYSTEM -> cm.requestNetwork( + NetworkRequest.Builder().build(), + TestableNetworkCallback() + ) + + RT_SYSTEM_ON_BEHALF_OF_APP -> cm.registerDefaultNetworkCallbackForUid( + Process.myUid(), + TestableNetworkCallback(), + Handler(ConnectivityThread.getInstanceLooper())) + + else -> fail("invalid requestType: " + requestType) + } + } + } finally { + deps.unmockCallingUid() + } + } + + + @Test + fun testSampleConnectivityState_NetworkRequest() { + val requestCount = 5 + fileNetworkRequest(RT_APP, requestCount); + fileNetworkRequest(RT_SYSTEM, requestCount, SYSTEM_UID); + fileNetworkRequest(RT_SYSTEM_ON_BEHALF_OF_APP, requestCount, SYSTEM_UID); + + val stats = csHandler.onHandler { service.sampleConnectivityState() } + + assertEquals(3, stats.networkRequestCount.requestCountForTypeList.size) + val appRequest = stats.networkRequestCount.requestCountForTypeList.find { + it.requestType == RT_APP + } ?: fail("Can't find RT_APP request") + val systemRequest = stats.networkRequestCount.requestCountForTypeList.find { + it.requestType == RT_SYSTEM + } ?: fail("Can't find RT_SYSTEM request") + val systemOnBehalfOfAppRequest = stats.networkRequestCount.requestCountForTypeList.find { + it.requestType == RT_SYSTEM_ON_BEHALF_OF_APP + } ?: fail("Can't find RT_SYSTEM_ON_BEHALF_OF_APP request") + + // Verify request count is equal or larger than the number of request this test filed + // since ConnectivityService internally files network requests + assertTrue("Unexpected RT_APP count, expected >= $requestCount, " + + "found ${appRequest.requestCount}", appRequest.requestCount >= requestCount) + assertTrue("Unexpected RT_SYSTEM count, expected >= $requestCount, " + + "found ${systemRequest.requestCount}", systemRequest.requestCount >= requestCount) + assertTrue("Unexpected RT_SYSTEM_ON_BEHALF_OF_APP count, expected >= $requestCount, " + + "found ${systemOnBehalfOfAppRequest.requestCount}", + systemOnBehalfOfAppRequest.requestCount >= requestCount) + } } diff --git a/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt b/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt index 3b83c41e9b..3ed1597310 100644 --- a/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt @@ -17,6 +17,7 @@ package com.android.server import android.app.AlarmManager +import android.app.AppOpsManager import android.content.BroadcastReceiver import android.content.Context import android.content.Intent @@ -75,8 +76,8 @@ import com.android.testutils.waitForIdle import java.util.concurrent.Executors import java.util.concurrent.LinkedBlockingQueue import java.util.concurrent.TimeUnit -import java.util.function.Consumer import java.util.function.BiConsumer +import java.util.function.Consumer import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.fail @@ -103,6 +104,8 @@ internal const val VERSION_U = 4 internal const val VERSION_V = 5 internal const val VERSION_MAX = VERSION_V +internal const val CALLING_UID_UNMOCKED = Process.INVALID_UID + private fun NetworkCapabilities.getLegacyType() = when (transportTypes.getOrElse(0) { TRANSPORT_WIFI }) { TRANSPORT_BLUETOOTH -> ConnectivityManager.TYPE_BLUETOOTH @@ -176,6 +179,7 @@ open class CSTest { val systemConfigManager = makeMockSystemConfigManager() val batteryStats = mock() val batteryManager = BatteryStatsManager(batteryStats) + val appOpsManager = mock() val telephonyManager = mock().also { doReturn(true).`when`(it).isDataCapable() } @@ -298,6 +302,19 @@ open class CSTest { override fun isAtLeastT() = if (isSdkUnmocked) super.isAtLeastT() else sdkLevel >= VERSION_T override fun isAtLeastU() = if (isSdkUnmocked) super.isAtLeastU() else sdkLevel >= VERSION_U override fun isAtLeastV() = if (isSdkUnmocked) super.isAtLeastV() else sdkLevel >= VERSION_V + + private var callingUid = CALLING_UID_UNMOCKED + + fun unmockCallingUid() { + setCallingUid(CALLING_UID_UNMOCKED) + } + + fun setCallingUid(callingUid: Int) { + visibleOnHandlerThread(csHandler) { this.callingUid = callingUid } + } + + override fun getCallingUid() = + if (callingUid == CALLING_UID_UNMOCKED) super.getCallingUid() else callingUid } inner class CSContext(base: Context) : BroadcastInterceptingContext(base) { @@ -398,6 +415,7 @@ open class CSTest { Context.TELEPHONY_SERVICE -> telephonyManager Context.BATTERY_STATS_SERVICE -> batteryManager Context.STATS_MANAGER -> null // Stats manager is final and can't be mocked + Context.APP_OPS_SERVICE -> appOpsManager else -> super.getSystemService(serviceName) }