diff --git a/service/src/com/android/server/connectivity/FullScore.java b/service/src/com/android/server/connectivity/FullScore.java index 799f46b524..b13ba9328a 100644 --- a/service/src/com/android/server/connectivity/FullScore.java +++ b/service/src/com/android/server/connectivity/FullScore.java @@ -29,6 +29,7 @@ import android.net.NetworkAgentConfig; import android.net.NetworkCapabilities; import android.net.NetworkScore; import android.net.NetworkScore.KeepConnectedReason; +import android.util.Log; import android.util.SparseArray; import com.android.internal.annotations.VisibleForTesting; @@ -46,6 +47,8 @@ import java.util.StringJoiner; * they are handling a score that had the CS-managed bits set. */ public class FullScore { + private static final String TAG = FullScore.class.getSimpleName(); + // This will be removed soon. Do *NOT* depend on it for any new code that is not part of // a migration. private final int mLegacyInt; @@ -126,7 +129,15 @@ public class FullScore { @VisibleForTesting static @NonNull String policyNameOf(final int policy) { final String name = sMessageNames.get(policy); - if (name == null) throw new IllegalArgumentException("Unknown policy: " + policy); + if (name == null) { + // Don't throw here because name might be null due to proguard stripping out the + // POLICY_* constants, potentially causing a crash only on user builds because proguard + // does not run on userdebug builds. + // TODO: make MessageUtils safer by not returning the array and instead storing it + // internally and providing a getter (that does not throw) for individual values. + Log.wtf(TAG, "Unknown policy: " + policy); + return Integer.toString(policy); + } return name.substring("POLICY_".length()); } diff --git a/tests/unit/java/com/android/server/connectivity/FullScoreTest.kt b/tests/unit/java/com/android/server/connectivity/FullScoreTest.kt index e7f6245547..c03a9cde70 100644 --- a/tests/unit/java/com/android/server/connectivity/FullScoreTest.kt +++ b/tests/unit/java/com/android/server/connectivity/FullScoreTest.kt @@ -22,6 +22,7 @@ import android.net.NetworkScore.KEEP_CONNECTED_NONE import android.os.Build import android.text.TextUtils import android.util.ArraySet +import android.util.Log import androidx.test.filters.SmallTest import com.android.server.connectivity.FullScore.MAX_CS_MANAGED_POLICY import com.android.server.connectivity.FullScore.POLICY_ACCEPT_UNVALIDATED @@ -32,11 +33,12 @@ import com.android.server.connectivity.FullScore.POLICY_IS_VALIDATED import com.android.server.connectivity.FullScore.POLICY_IS_VPN import com.android.testutils.DevSdkIgnoreRule import com.android.testutils.DevSdkIgnoreRunner +import org.junit.After +import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import kotlin.reflect.full.staticProperties import kotlin.test.assertEquals -import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertTrue @@ -63,6 +65,23 @@ class FullScoreTest { return mixInScore(nc, nac, validated, false /* yieldToBadWifi */, destroyed) } + private val TAG = this::class.simpleName + + private var wtfHandler: Log.TerribleFailureHandler? = null + + @Before + fun setUp() { + // policyNameOf will call Log.wtf if passed an invalid policy. + wtfHandler = Log.setWtfHandler() { tagString, what, system -> + Log.d(TAG, "WTF captured, ignoring: $tagString $what") + } + } + + @After + fun tearDown() { + Log.setWtfHandler(wtfHandler) + } + @Test fun testGetLegacyInt() { val ns = FullScore(50, 0L /* policy */, KEEP_CONNECTED_NONE) @@ -101,10 +120,9 @@ class FullScoreTest { assertFalse(foundNames.contains(name)) foundNames.add(name) } - assertFailsWith { - FullScore.policyNameOf(MAX_CS_MANAGED_POLICY + 1) - } assertEquals("IS_UNMETERED", FullScore.policyNameOf(POLICY_IS_UNMETERED)) + val invalidPolicy = MAX_CS_MANAGED_POLICY + 1 + assertEquals(Integer.toString(invalidPolicy), FullScore.policyNameOf(invalidPolicy)) } fun getAllPolicies() = Regex("POLICY_.*").let { nameRegex ->