diff --git a/framework/Android.bp b/framework/Android.bp index 5e7262a945..d31f74fe0d 100644 --- a/framework/Android.bp +++ b/framework/Android.bp @@ -80,6 +80,9 @@ java_sdk_library { "framework-wifi.stubs.module_lib", "net-utils-device-common", ], + static_libs: [ + "modules-utils-build", + ], libs: [ "unsupportedappusage", ], diff --git a/framework/jarjar-rules.txt b/framework/jarjar-rules.txt index 2e5848cb10..ae6b9c3337 100644 --- a/framework/jarjar-rules.txt +++ b/framework/jarjar-rules.txt @@ -1,2 +1,3 @@ rule com.android.net.module.util.** android.net.connectivity.framework.util.@1 +rule com.android.modules.utils.** android.net.connectivity.framework.modules.utils.@1 rule android.net.NetworkFactory* android.net.connectivity.framework.NetworkFactory@1 diff --git a/framework/src/android/net/NetworkInfo.java b/framework/src/android/net/NetworkInfo.java index 433933f841..b7ec519d2c 100644 --- a/framework/src/android/net/NetworkInfo.java +++ b/framework/src/android/net/NetworkInfo.java @@ -24,6 +24,7 @@ import android.os.Parcelable; import android.text.TextUtils; import com.android.internal.annotations.VisibleForTesting; +import com.android.modules.utils.build.SdkLevel; import java.util.EnumMap; @@ -180,6 +181,10 @@ public class NetworkInfo implements Parcelable { /** {@hide} */ @UnsupportedAppUsage public NetworkInfo(@NonNull NetworkInfo source) { + // S- didn't use to crash when passing null. This plants a timebomb where mState and + // some other fields are null, but there may be existing code that relies on this behavior + // and doesn't trip the timebomb, so on SdkLevel < T, keep the old behavior. b/145972387 + if (null == source && !SdkLevel.isAtLeastT()) return; synchronized (source) { mNetworkType = source.mNetworkType; mSubtype = source.mSubtype; @@ -490,8 +495,9 @@ public class NetworkInfo implements Parcelable { this.mReason = reason; this.mExtraInfo = extraInfo; // Catch both the case where detailedState is null and the case where it's some - // unknown value - if (null == mState) { + // unknown value. This is clearly incorrect usage, but S- didn't use to crash (at + // least immediately) so keep the old behavior on older frameworks for safety. + if (null == mState && SdkLevel.isAtLeastT()) { throw new NullPointerException("Unknown DetailedState : " + detailedState); } } diff --git a/tests/cts/net/src/android/net/cts/NetworkInfoTest.kt b/tests/cts/net/src/android/net/cts/NetworkInfoTest.kt index fa15e8f82c..d6120f8769 100644 --- a/tests/cts/net/src/android/net/cts/NetworkInfoTest.kt +++ b/tests/cts/net/src/android/net/cts/NetworkInfoTest.kt @@ -26,16 +26,19 @@ import android.telephony.TelephonyManager import androidx.test.filters.SmallTest import androidx.test.platform.app.InstrumentationRegistry import androidx.test.runner.AndroidJUnit4 +import com.android.modules.utils.build.SdkLevel import com.android.testutils.DevSdkIgnoreRule import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo import org.junit.Assert.assertEquals import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertTrue -import org.junit.Assert.fail import org.junit.Rule import org.junit.runner.RunWith import org.junit.Test +import kotlin.reflect.jvm.isAccessible +import kotlin.test.assertFails +import kotlin.test.assertFailsWith const val TYPE_MOBILE = ConnectivityManager.TYPE_MOBILE const val TYPE_WIFI = ConnectivityManager.TYPE_WIFI @@ -97,12 +100,16 @@ class NetworkInfoTest { assertNull(networkInfo.reason) assertNull(networkInfo.extraInfo) - try { + assertFailsWith { NetworkInfo(ConnectivityManager.MAX_NETWORK_TYPE + 1, TelephonyManager.NETWORK_TYPE_LTE, MOBILE_TYPE_NAME, LTE_SUBTYPE_NAME) - fail("Unexpected behavior. Network type is invalid.") - } catch (e: IllegalArgumentException) { - // Expected behavior. + } + + if (SdkLevel.isAtLeastT()) { + assertFailsWith { NetworkInfo(null) } + } else { + // Doesn't immediately crash on S- + NetworkInfo(null) } } @@ -118,5 +125,29 @@ class NetworkInfoTest { assertEquals(State.CONNECTED, networkInfo.state) assertEquals(reason, networkInfo.reason) assertEquals(extraReason, networkInfo.extraInfo) + + // Create an incorrect enum value by calling the default constructor of the enum + val constructor = DetailedState::class.java.declaredConstructors.first { + it.parameters.size == 2 + } + constructor.isAccessible = true + val incorrectDetailedState = constructor.newInstance("any", 200) as DetailedState + if (SdkLevel.isAtLeastT()) { + assertFailsWith { + NetworkInfo(null) + } + assertFailsWith { + networkInfo.setDetailedState(null, "reason", "extraInfo") + } + // This actually throws ArrayOutOfBoundsException because of the implementation of + // EnumMap, but that's an implementation detail so accept any crash. + assertFails { + networkInfo.setDetailedState(incorrectDetailedState, "reason", "extraInfo") + } + } else { + // Doesn't immediately crash on S- + NetworkInfo(null) + networkInfo.setDetailedState(null, "reason", "extraInfo") + } } -} +} \ No newline at end of file