Snap for 9453251 from 6640702afc95ae0ad30d473311c42888b9216678 to mainline-wifi-release Change-Id: I0ef81a0d854aa6f38fa78ee1868f1853f897fe1b
diff --git a/Android.bp b/Android.bp index 116d6a6..49c011e 100644 --- a/Android.bp +++ b/Android.bp
@@ -45,10 +45,6 @@ default_applicable_licenses: ["Android-Apache-2.0"], } -// In some branches, this may be module_current instead of module_33 if the SDK prebuilts are not -// yet dropped there, meaning module_33 cannot be used yet. -module_33_version = "module_33" - // TODO: remove this default and replace with ConnectivityNextEnableDefaults. This will need to be // done separately in each branch due to merge conflicts. // Defaults to enable/disable java targets that depend on @@ -86,7 +82,7 @@ java_defaults { name: "NetworkStackReleaseApiLevel", defaults:["NetworkStackReleaseTargetSdk"], - sdk_version: module_33_version, + sdk_version: "module_33", libs: [ "framework-connectivity", "framework-connectivity-t", @@ -179,7 +175,7 @@ java_library { name: "NetworkStackApi33Shims", - defaults: ["NetworkStackShimsDefaults", "ConnectivityNextEnableDefaults"], + defaults: ["NetworkStackShimsDefaults"], srcs: [ "apishim/33/**/*.java", ], @@ -193,7 +189,7 @@ "framework-connectivity-t.stubs.module_lib", "framework-tethering", ], - sdk_version: module_33_version, + sdk_version: "module_33", visibility: ["//visibility:private"], } @@ -298,7 +294,6 @@ "net-utils-device-common-ip", "net-utils-device-common-netlink", ], - plugins: ["java_api_finder"], } // The versions of the android library containing network stack code compiled for each SDK variant.
diff --git a/jni/network_stack_utils_jni.cpp b/jni/network_stack_utils_jni.cpp index 5ff0288..20eb051 100644 --- a/jni/network_stack_utils_jni.cpp +++ b/jni/network_stack_utils_jni.cpp
@@ -99,7 +99,9 @@ } } -static void network_stack_utils_attachDhcpFilter(JNIEnv *env, jclass clazz, jobject javaFd) { +static void network_stack_utils_attachDhcpFilter(JNIEnv *env, jclass clazz, jobject javaFd, jboolean dropMF) { + static const __u32 frag_mask = static_cast<__u32>((dropMF ? IP_MF : 0) | IP_OFFMASK); + static sock_filter filter_code[] = { // Check the protocol is UDP. BPF_STMT(BPF_LD | BPF_B | BPF_ABS, kIPv4Protocol), @@ -107,7 +109,7 @@ // Check this is not a fragment. BPF_STMT(BPF_LD | BPF_H | BPF_ABS, kIPv4FlagsOffset), - BPF_JUMP(BPF_JMP | BPF_JSET | BPF_K, IP_MF | IP_OFFMASK, 4, 0), + BPF_JUMP(BPF_JMP | BPF_JSET | BPF_K, frag_mask, 4, 0), // Get the IP header length. BPF_STMT(BPF_LDX | BPF_B | BPF_MSH, kEtherHeaderLen), @@ -251,7 +253,7 @@ static const JNINativeMethod gNetworkStackUtilsMethods[] = { /* name, signature, funcPtr */ { "addArpEntry", "([B[BLjava/lang/String;Ljava/io/FileDescriptor;)V", (void*) network_stack_utils_addArpEntry }, - { "attachDhcpFilter", "(Ljava/io/FileDescriptor;)V", (void*) network_stack_utils_attachDhcpFilter }, + { "attachDhcpFilter", "(Ljava/io/FileDescriptor;Z)V", (void*) network_stack_utils_attachDhcpFilter }, { "attachRaFilter", "(Ljava/io/FileDescriptor;I)V", (void*) network_stack_utils_attachRaFilter }, { "attachControlPacketFilter", "(Ljava/io/FileDescriptor;I)V", (void*) network_stack_utils_attachControlPacketFilter }, };
diff --git a/src/android/net/dhcp/DhcpClient.java b/src/android/net/dhcp/DhcpClient.java index a540cef..e494451 100644 --- a/src/android/net/dhcp/DhcpClient.java +++ b/src/android/net/dhcp/DhcpClient.java
@@ -51,6 +51,7 @@ import static com.android.net.module.util.NetworkStackConstants.IPV4_ADDR_ANY; import static com.android.net.module.util.NetworkStackConstants.IPV4_CONFLICT_ANNOUNCE_NUM; import static com.android.net.module.util.NetworkStackConstants.IPV4_CONFLICT_PROBE_NUM; +import static com.android.networkstack.util.NetworkStackUtils.DHCP_DISABLE_DROP_MF; import static com.android.networkstack.util.NetworkStackUtils.DHCP_INIT_REBOOT_VERSION; import static com.android.networkstack.util.NetworkStackUtils.DHCP_IPV6_ONLY_PREFERRED_VERSION; import static com.android.networkstack.util.NetworkStackUtils.DHCP_IP_CONFLICT_DETECT_VERSION; @@ -692,9 +693,11 @@ @Override protected FileDescriptor createFd() { + boolean dropMF = !DeviceConfigUtils.getDeviceConfigPropertyBoolean( + NAMESPACE_CONNECTIVITY, DHCP_DISABLE_DROP_MF, false); try { mPacketSock = Os.socket(AF_PACKET, SOCK_RAW | SOCK_NONBLOCK, 0 /* protocol */); - NetworkStackUtils.attachDhcpFilter(mPacketSock); + NetworkStackUtils.attachDhcpFilter(mPacketSock, dropMF); final SocketAddress addr = makePacketSocketAddress(ETH_P_IP, mIface.index); Os.bind(mPacketSock, addr); } catch (SocketException | ErrnoException e) {
diff --git a/src/com/android/networkstack/netlink/TcpSocketTracker.java b/src/com/android/networkstack/netlink/TcpSocketTracker.java index 33d3e61..acf716b 100644 --- a/src/com/android/networkstack/netlink/TcpSocketTracker.java +++ b/src/com/android/networkstack/netlink/TcpSocketTracker.java
@@ -39,7 +39,10 @@ import static com.android.net.module.util.netlink.StructNlMsgHdr.NLM_F_DUMP; import static com.android.net.module.util.netlink.StructNlMsgHdr.NLM_F_REQUEST; +import android.content.BroadcastReceiver; import android.content.Context; +import android.content.Intent; +import android.content.IntentFilter; import android.net.INetd; import android.net.MarkMaskParcel; import android.net.Network; @@ -47,6 +50,7 @@ import android.os.AsyncTask; import android.os.Build; import android.os.IBinder; +import android.os.PowerManager; import android.os.RemoteException; import android.os.SystemClock; import android.provider.DeviceConfig; @@ -60,6 +64,7 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; import com.android.net.module.util.DeviceConfigUtils; import com.android.net.module.util.netlink.NetlinkConstants; @@ -128,6 +133,10 @@ private final int mNetworkMask; private int mMinPacketsThreshold = DEFAULT_DATA_STALL_MIN_PACKETS_THRESHOLD; private int mTcpPacketsFailRateThreshold = DEFAULT_TCP_PACKETS_FAIL_PERCENTAGE; + + private final Object mDozeModeLock = new Object(); + @GuardedBy("mDozeModeLock") + private boolean mInDozeMode = false; @VisibleForTesting protected final DeviceConfig.OnPropertiesChangedListener mConfigListener = new DeviceConfig.OnPropertiesChangedListener() { @@ -144,6 +153,19 @@ } }; + final BroadcastReceiver mDeviceIdleReceiver = new BroadcastReceiver() { + @Override + public void onReceive(Context context, Intent intent) { + if (intent == null) return; + + if (PowerManager.ACTION_DEVICE_IDLE_MODE_CHANGED.equals(intent.getAction())) { + final PowerManager powerManager = context.getSystemService(PowerManager.class); + final boolean deviceIdle = powerManager.isDeviceIdleMode(); + setDozeMode(deviceIdle); + } + } + }; + public TcpSocketTracker(@NonNull final Dependencies dps, @NonNull final Network network) { mDependencies = dps; mNetwork = network; @@ -172,6 +194,7 @@ TCP_MONITOR_STATE_FILTER)); } mDependencies.addDeviceConfigChangedListener(mConfigListener); + mDependencies.addDeviceIdleReceiver(mDeviceIdleReceiver); } @Nullable @@ -191,10 +214,17 @@ * Request to send a SockDiag Netlink request. Receive and parse the returned message. This * function is not thread-safe and should only be called from only one thread. * - * @Return if this polling request executes successfully or not. + * @Return if this polling request is sent to kernel and executes successfully or not. */ public boolean pollSocketsInfo() { if (!mDependencies.isTcpInfoParsingSupported()) return false; + // Traffic will be restricted in doze mode. TCP info may not reflect the correct network + // behavior. + // TODO: Traffic may be restricted by other reason. Get the restriction info from bpf in T+. + synchronized (mDozeModeLock) { + if (mInDozeMode) return false; + } + FileDescriptor fd = null; try { @@ -358,6 +388,14 @@ */ public boolean isDataStallSuspected() { if (!mDependencies.isTcpInfoParsingSupported()) return false; + + // Skip checking data stall since the traffic will be restricted and it will not be real + // network stall. + // TODO: Traffic may be restricted by other reason. Get the restriction info from bpf in T+. + synchronized (mDozeModeLock) { + if (mInDozeMode) return false; + } + return (getLatestPacketFailPercentage() >= getTcpPacketsFailRateThreshold()); } @@ -467,7 +505,12 @@ /** Stops monitoring and releases resources. */ public void quit() { + // Do not need to unregister receiver and listener since registration is skipped + // in the constructor. + if (!mDependencies.isTcpInfoParsingSupported()) return; + mDependencies.removeDeviceConfigChangedListener(mConfigListener); + mDependencies.removeBroadcastReceiver(mDeviceIdleReceiver); } /** @@ -564,6 +607,14 @@ } } + private void setDozeMode(boolean isEnabled) { + synchronized (mDozeModeLock) { + if (mInDozeMode == isEnabled) return; + mInDozeMode = isEnabled; + log("Doze mode enabled=" + mInDozeMode); + } + } + /** * Dependencies class for testing. */ @@ -658,5 +709,16 @@ @NonNull final DeviceConfig.OnPropertiesChangedListener listener) { DeviceConfig.removeOnPropertiesChangedListener(listener); } + + /** Add receiver for detecting doze mode change to control TCP detection. */ + public void addDeviceIdleReceiver(@NonNull final BroadcastReceiver receiver) { + mContext.registerReceiver(receiver, + new IntentFilter(PowerManager.ACTION_DEVICE_IDLE_MODE_CHANGED)); + } + + /** Remove broadcast receiver. */ + public void removeBroadcastReceiver(@NonNull final BroadcastReceiver receiver) { + mContext.unregisterReceiver(receiver); + } } }
diff --git a/src/com/android/networkstack/util/NetworkStackUtils.java b/src/com/android/networkstack/util/NetworkStackUtils.java index 670a320..67a67c2 100755 --- a/src/com/android/networkstack/util/NetworkStackUtils.java +++ b/src/com/android/networkstack/util/NetworkStackUtils.java
@@ -166,6 +166,11 @@ public static final String DHCP_RAPID_COMMIT_ENABLED = "dhcp_rapid_commit_enabled"; /** + * Disable dropping DHCP packets with IPv4 MF flag set. + */ + public static final String DHCP_DISABLE_DROP_MF = "dhcp_disable_drop_mf"; + + /** * Minimum module version at which to enable the DHCP INIT-REBOOT state. */ public static final String DHCP_INIT_REBOOT_VERSION = "dhcp_init_reboot_version"; @@ -194,14 +199,6 @@ "dhcp_slow_retransmission_version"; /** - * Minimum module version at which to enable dismissal CaptivePortalLogin app in validated - * network feature. CaptivePortalLogin app will also use validation facilities in - * {@link NetworkMonitor} to perform portal validation if feature is enabled. - */ - public static final String DISMISS_PORTAL_IN_VALIDATED_NETWORK = - "dismiss_portal_in_validated_network"; - - /** * Experiment flag to enable considering DNS probes returning private IP addresses as failed * when attempting to detect captive portals. * @@ -321,7 +318,8 @@ /** * Attaches a socket filter that accepts DHCP packets to the given socket. */ - public static native void attachDhcpFilter(FileDescriptor fd) throws ErrnoException; + public static native void attachDhcpFilter(FileDescriptor fd, boolean dropMF) + throws ErrnoException; /** * Attaches a socket filter that accepts ICMPv6 router advertisements to the given socket.
diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java index d34437f..8e0130e 100755 --- a/src/com/android/server/connectivity/NetworkMonitor.java +++ b/src/com/android/server/connectivity/NetworkMonitor.java
@@ -86,7 +86,6 @@ import static com.android.networkstack.util.NetworkStackUtils.DEFAULT_CAPTIVE_PORTAL_FALLBACK_PROBE_SPECS; import static com.android.networkstack.util.NetworkStackUtils.DEFAULT_CAPTIVE_PORTAL_HTTPS_URLS; import static com.android.networkstack.util.NetworkStackUtils.DEFAULT_CAPTIVE_PORTAL_HTTP_URLS; -import static com.android.networkstack.util.NetworkStackUtils.DISMISS_PORTAL_IN_VALIDATED_NETWORK; import static com.android.networkstack.util.NetworkStackUtils.DNS_PROBE_PRIVATE_IP_NO_INTERNET_VERSION; import android.app.PendingIntent; @@ -1325,10 +1324,7 @@ private boolean useRedirectUrlForPortal() { // It must match the conditions in CaptivePortalLogin in which the redirect URL is not // used to validate that the portal is gone. - final boolean aboveQ = - ShimUtils.isReleaseOrDevelopmentApiAbove(Build.VERSION_CODES.Q); - return aboveQ && mDependencies.isFeatureEnabled(mContext, NAMESPACE_CONNECTIVITY, - DISMISS_PORTAL_IN_VALIDATED_NETWORK, aboveQ /* defaultEnabled */); + return ShimUtils.isReleaseOrDevelopmentApiAbove(Build.VERSION_CODES.Q); } @Override
diff --git a/tests/integration/Android.bp b/tests/integration/Android.bp index 5926fec..7f8a9d3 100644 --- a/tests/integration/Android.bp +++ b/tests/integration/Android.bp
@@ -121,7 +121,7 @@ "root/**/*.kt", ], static_libs: [ - "NetworkStackApiCurrentLib", + "NetworkStackApiStableLib", ], platform_apis: true, test_suites: ["general-tests", "mts-networking"],
diff --git a/tests/integration/common/android/net/ip/IpClientIntegrationTestCommon.java b/tests/integration/common/android/net/ip/IpClientIntegrationTestCommon.java index becb968..50f25e6 100644 --- a/tests/integration/common/android/net/ip/IpClientIntegrationTestCommon.java +++ b/tests/integration/common/android/net/ip/IpClientIntegrationTestCommon.java
@@ -612,8 +612,15 @@ mIsNetlinkEventParseEnabled /* default value */); setUpTapInterface(); - mCb = mock(IIpClientCallbacks.class); + // It turns out that Router Solicitation will also be sent out even after the tap interface + // is brought up, however, we want to wait for RS which is sent due to IPv6 stack is enabled + // in the test code. The early RS might bring kind of race, for example, the IPv6 stack has + // not been enabled when test code sees the RS, then kernel will not process RA even if we + // replies immediately after receiving RS. Always waiting for the first RS show up after + // interface is brought up helps prevent the race. + waitForRouterSolicitation(); + mCb = mock(IIpClientCallbacks.class); if (useNetworkStackSignature()) { setUpMocks(); setUpIpClient();
diff --git a/tests/unit/src/com/android/networkstack/netlink/TcpSocketTrackerTest.java b/tests/unit/src/com/android/networkstack/netlink/TcpSocketTrackerTest.java index 1177f96..043ec49 100644 --- a/tests/unit/src/com/android/networkstack/netlink/TcpSocketTrackerTest.java +++ b/tests/unit/src/com/android/networkstack/netlink/TcpSocketTrackerTest.java
@@ -21,8 +21,6 @@ import static android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY; import static android.system.OsConstants.AF_INET; -import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation; - import static com.android.net.module.util.netlink.NetlinkConstants.SOCKDIAG_MSG_HEADER_SIZE; import static junit.framework.Assert.assertEquals; @@ -35,14 +33,19 @@ import static org.mockito.Mockito.any; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import android.content.BroadcastReceiver; +import android.content.Context; +import android.content.Intent; import android.net.INetd; import android.net.MarkMaskParcel; import android.net.Network; import android.os.Build; +import android.os.PowerManager; import android.util.Log; import android.util.Log.TerribleFailureHandler; @@ -62,6 +65,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -190,6 +194,8 @@ private final Network mNetwork = new Network(TEST_NETID1); private final Network mOtherNetwork = new Network(TEST_NETID2); private TerribleFailureHandler mOldWtfHandler; + @Mock private Context mContext; + @Mock private PowerManager mPowerManager; @Rule public final DevSdkIgnoreRule mIgnoreRule = new DevSdkIgnoreRule(); @@ -211,6 +217,7 @@ when(mNetd.getFwmarkForNetwork(eq(TEST_NETID1))) .thenReturn(makeMarkMaskParcel(NETID_MASK, TEST_NETID1_FWMARK)); + doReturn(mPowerManager).when(mContext).getSystemService(PowerManager.class); } @After @@ -262,7 +269,7 @@ @Test @IgnoreUpTo(Build.VERSION_CODES.Q) // TCP info parsing is not supported on Q public void testPollSocketsInfo() throws Exception { // This test requires shims that provide API 30 access - assumeTrue(ConstantsShim.VERSION >= 30); + assumeTrue(ConstantsShim.VERSION >= Build.VERSION_CODES.R); when(mDependencies.isTcpInfoParsingSupported()).thenReturn(false); final TcpSocketTracker tst = new TcpSocketTracker(mDependencies, mNetwork); assertFalse(tst.pollSocketsInfo()); @@ -283,9 +290,7 @@ assertEquals(-1, tst.getLatestPacketFailPercentage()); assertEquals(0, tst.getSentSinceLastRecv()); - final ByteBuffer tcpBufferV6 = getByteBuffer(TEST_RESPONSE_BYTES); - final ByteBuffer tcpBufferV4 = getByteBuffer(TEST_RESPONSE_BYTES); - doReturn(tcpBufferV6, tcpBufferV4).when(mDependencies).recvMessage(any()); + setupNormalTestTcpInfo(); assertTrue(tst.pollSocketsInfo()); assertEquals(10, tst.getSentSinceLastRecv()); @@ -315,17 +320,60 @@ verify(mDependencies, atLeastOnce()).isTcpInfoParsingSupported(); verifyNoMoreInteractions(mDependencies); + + // Verify that no un-registration for the device configuration listener and broadcast + // receiver if TcpInfo parsing is not supported. + tst.quit(); + verify(mDependencies, never()).removeDeviceConfigChangedListener(any()); + verify(mDependencies, never()).removeBroadcastReceiver(any()); + } + + @Test @IgnoreUpTo(Build.VERSION_CODES.Q) + public void testTcpInfoParsingWithDozeMode() throws Exception { + // This test requires shims that provide API 30 access + assumeTrue(ConstantsShim.VERSION >= Build.VERSION_CODES.R); + + final TcpSocketTracker tst = new TcpSocketTracker(mDependencies, mNetwork); + final ArgumentCaptor<BroadcastReceiver> receiverCaptor = + ArgumentCaptor.forClass(BroadcastReceiver.class); + + verify(mDependencies).addDeviceIdleReceiver(receiverCaptor.capture()); + setupNormalTestTcpInfo(); + assertTrue(tst.pollSocketsInfo()); + + // Lower the threshold. + when(mDependencies.getDeviceConfigPropertyInt(any(), eq(CONFIG_TCP_PACKETS_FAIL_PERCENTAGE), + anyInt())).thenReturn(40); + + // Trigger a config update + tst.mConfigListener.onPropertiesChanged(null /* properties */); + assertEquals(10, tst.getSentSinceLastRecv()); + assertEquals(50, tst.getLatestPacketFailPercentage()); + assertTrue(tst.isDataStallSuspected()); + + // Enable doze mode + doReturn(true).when(mPowerManager).isDeviceIdleMode(); + final BroadcastReceiver receiver = receiverCaptor.getValue(); + receiver.onReceive(mContext, new Intent(PowerManager.ACTION_DEVICE_IDLE_MODE_CHANGED)); + assertFalse(tst.pollSocketsInfo()); + assertFalse(tst.isDataStallSuspected()); + } + + private void setupNormalTestTcpInfo() throws Exception { + final ByteBuffer tcpBufferV6 = getByteBuffer(TEST_RESPONSE_BYTES); + final ByteBuffer tcpBufferV4 = getByteBuffer(TEST_RESPONSE_BYTES); + doReturn(tcpBufferV6, tcpBufferV4).when(mDependencies).recvMessage(any()); } @Test @IgnoreAfter(Build.VERSION_CODES.Q) public void testTcpInfoParsingNotSupportedOnQ() { - assertFalse(new TcpSocketTracker.Dependencies(getInstrumentation().getContext()) + assertFalse(new TcpSocketTracker.Dependencies(mContext) .isTcpInfoParsingSupported()); } @Test @IgnoreUpTo(Build.VERSION_CODES.Q) public void testTcpInfoParsingSupportedFromR() { - assertTrue(new TcpSocketTracker.Dependencies(getInstrumentation().getContext()) + assertTrue(new TcpSocketTracker.Dependencies(mContext) .isTcpInfoParsingSupported()); } @@ -359,11 +407,9 @@ @Test @IgnoreUpTo(Build.VERSION_CODES.Q) // TCP info parsing is not supported on Q public void testPollSocketsInfo_BadFormat() throws Exception { // This test requires shims that provide API 30 access - assumeTrue(ConstantsShim.VERSION >= 30); + assumeTrue(ConstantsShim.VERSION >= Build.VERSION_CODES.R); final TcpSocketTracker tst = new TcpSocketTracker(mDependencies, mNetwork); - final ByteBuffer tcpBufferV6 = getByteBuffer(TEST_RESPONSE_BYTES); - final ByteBuffer tcpBufferV4 = getByteBuffer(TEST_RESPONSE_BYTES); - doReturn(tcpBufferV6, tcpBufferV4).when(mDependencies).recvMessage(any()); + setupNormalTestTcpInfo(); assertTrue(tst.pollSocketsInfo()); assertEquals(10, tst.getSentSinceLastRecv()); assertEquals(50, tst.getLatestPacketFailPercentage()); @@ -383,9 +429,7 @@ when(mNetd.getFwmarkForNetwork(eq(TEST_NETID2))) .thenReturn(makeMarkMaskParcel(NETID_MASK, TEST_NETID2_FWMARK)); final TcpSocketTracker tst = new TcpSocketTracker(mDependencies, mOtherNetwork); - final ByteBuffer tcpBufferV6 = getByteBuffer(TEST_RESPONSE_BYTES); - final ByteBuffer tcpBufferV4 = getByteBuffer(TEST_RESPONSE_BYTES); - doReturn(tcpBufferV6, tcpBufferV4).when(mDependencies).recvMessage(any()); + setupNormalTestTcpInfo(); assertTrue(tst.pollSocketsInfo()); assertEquals(0, tst.getSentSinceLastRecv());
diff --git a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java index ae9eea4..122ec6e 100644 --- a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java +++ b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
@@ -58,7 +58,6 @@ import static com.android.networkstack.util.NetworkStackUtils.CAPTIVE_PORTAL_OTHER_FALLBACK_URLS; import static com.android.networkstack.util.NetworkStackUtils.CAPTIVE_PORTAL_USE_HTTPS; import static com.android.networkstack.util.NetworkStackUtils.DEFAULT_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT; -import static com.android.networkstack.util.NetworkStackUtils.DISMISS_PORTAL_IN_VALIDATED_NETWORK; import static com.android.networkstack.util.NetworkStackUtils.DNS_PROBE_PRIVATE_IP_NO_INTERNET_VERSION; import static com.android.server.connectivity.NetworkMonitor.INITIAL_REEVALUATE_DELAY_MS; import static com.android.server.connectivity.NetworkMonitor.extractCharset; @@ -74,7 +73,6 @@ import static org.junit.Assert.fail; import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; -import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.after; @@ -582,7 +580,6 @@ setConsecutiveDnsTimeoutThreshold(5); mCreatedNetworkMonitors = new HashSet<>(); mRegisteredReceivers = new HashSet<>(); - setDismissPortalInValidatedNetwork(false); } @After @@ -2055,7 +2052,8 @@ null /* redirectUrl */); } - public void setupAndLaunchCaptivePortalApp(final NetworkMonitor nm) throws Exception { + public void setupAndLaunchCaptivePortalApp(final NetworkMonitor nm, String expectedUrl) + throws Exception { setSslException(mHttpsConnection); setPortal302(mHttpConnection); doReturn(TEST_LOGIN_URL).when(mHttpConnection).getHeaderField(eq("location")); @@ -2083,15 +2081,27 @@ assertEquals(TEST_NETID, networkCaptor.getValue().netId); // Portal URL should be detection URL. final String redirectUrl = bundle.getString(ConnectivityManager.EXTRA_CAPTIVE_PORTAL_URL); - assertEquals(TEST_HTTP_URL, redirectUrl); + assertEquals(expectedUrl, redirectUrl); resetCallbacks(); } + @Test - public void testCaptivePortalLogin() throws Exception { + public void testCaptivePortalLogin_beforeR() throws Exception { + assumeFalse(ShimUtils.isAtLeastR()); + testCaptivePortalLogin(TEST_HTTP_URL); + } + + @Test + public void testCaptivePortalLogin_AfterR() throws Exception { + assumeTrue(ShimUtils.isAtLeastR()); + testCaptivePortalLogin(TEST_LOGIN_URL); + } + + private void testCaptivePortalLogin(String expectedUrl) throws Exception { final NetworkMonitor nm = makeMonitor(CELL_METERED_CAPABILITIES); - setupAndLaunchCaptivePortalApp(nm); + setupAndLaunchCaptivePortalApp(nm, expectedUrl); // Have the app report that the captive portal is dismissed, and check that we revalidate. setStatus(mHttpsConnection, 204); @@ -2106,9 +2116,20 @@ } @Test - public void testCaptivePortalUseAsIs() throws Exception { + public void testCaptivePortalUseAsIs_beforeR() throws Exception { + assumeFalse(ShimUtils.isAtLeastR()); + testCaptivePortalUseAsIs(TEST_HTTP_URL); + } + + @Test + public void testCaptivePortalUseAsIs_AfterR() throws Exception { + assumeTrue(ShimUtils.isAtLeastR()); + testCaptivePortalUseAsIs(TEST_LOGIN_URL); + } + + private void testCaptivePortalUseAsIs(String expectedUrl) throws Exception { final NetworkMonitor nm = makeMonitor(CELL_METERED_CAPABILITIES); - setupAndLaunchCaptivePortalApp(nm); + setupAndLaunchCaptivePortalApp(nm, expectedUrl); // The user decides this network is wanted as is, either by encountering an SSL error or // encountering an unknown scheme and then deciding to continue through the browser, or by @@ -2676,7 +2697,6 @@ private void testDismissPortalInValidatedNetworkEnabled(String expectedUrl, String locationUrl) throws Exception { - setDismissPortalInValidatedNetwork(true); setSslException(mHttpsConnection); setPortal302(mHttpConnection); doReturn(locationUrl).when(mHttpConnection).getHeaderField(eq("location")); @@ -2880,9 +2900,6 @@ public void testIsCaptivePortal_FromExternalSource() throws Exception { assumeTrue(CaptivePortalDataShimImpl.isSupported()); assumeTrue(ShimUtils.isAtLeastS()); - doReturn(true).when(mDependencies) - .isFeatureEnabled(any(), eq(NAMESPACE_CONNECTIVITY), - eq(DISMISS_PORTAL_IN_VALIDATED_NETWORK), anyBoolean()); final NetworkMonitor monitor = makeMonitor(WIFI_NOT_METERED_CAPABILITIES); NetworkInformationShim networkShim = NetworkInformationShimImpl.newInstance(); @@ -3058,11 +3075,6 @@ eq(Settings.Global.CAPTIVE_PORTAL_MODE), anyInt()); } - private void setDismissPortalInValidatedNetwork(boolean enabled) { - doReturn(enabled).when(mDependencies).isFeatureEnabled(any(), any(), - eq(DISMISS_PORTAL_IN_VALIDATED_NETWORK), anyBoolean()); - } - private void setDeviceConfig(String key, String value) { doReturn(value).when(mDependencies).getDeviceConfigProperty(eq(NAMESPACE_CONNECTIVITY), eq(key), any() /* defaultValue */);