Report probe URL as redirect on 200 portals
On portals that return HTTP 200 with a login page instead of a redirect,
report the probe URL instead of null as the login URL.
NetworkMonitor already falls back to the probe URL to open the captive
portal app when the redirect URL is empty. This only affects the result
reported to ConnectivityService in handleNetworkTested, which is only
used to transmit to the NetworkAgent in onValidationStatusChanged.
This means that starting from S, NetworkAgents will also see a redirect
URL on 200 portals, instead of a failed validation (null URL) as before.
The change does not affect R and earlier as behavior of
ConnectivityService and NetworkAgents is OEM-specific and may lead to
unexpected results.
Bug: 172048052
Test: atest NetworkStackTests
Original-Change: https://android-review.googlesource.com/1719397
Merged-In: I4c68bfc9ae8aa38d73bc62443ac923f07ee1ed46
Change-Id: I4c68bfc9ae8aa38d73bc62443ac923f07ee1ed46
diff --git a/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeResult.java b/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeResult.java
index 2ba1dcc..8b388ad 100755
--- a/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeResult.java
+++ b/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeResult.java
@@ -103,11 +103,22 @@
}
public boolean isSuccessful() {
- return mHttpResponseCode == SUCCESS_CODE;
+ return isSuccessCode(mHttpResponseCode);
}
public boolean isPortal() {
- return !isSuccessful() && (mHttpResponseCode >= 200) && (mHttpResponseCode <= 399);
+ return isPortalCode(mHttpResponseCode);
+ }
+
+ private static boolean isSuccessCode(int responseCode) {
+ return responseCode == SUCCESS_CODE;
+ }
+
+ /**
+ * @return Whether the specified HTTP return code indicates a captive portal.
+ */
+ public static boolean isPortalCode(int responseCode) {
+ return !isSuccessCode(responseCode) && (responseCode >= 200) && (responseCode <= 399);
}
public boolean isFailed() {
diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java
index 7bc6a49..af21851 100755
--- a/src/com/android/server/connectivity/NetworkMonitor.java
+++ b/src/com/android/server/connectivity/NetworkMonitor.java
@@ -2541,8 +2541,17 @@
final CaptivePortalProbeResult probeResult;
if (probeSpec == null) {
+ if (CaptivePortalProbeResult.isPortalCode(httpResponseCode)
+ && TextUtils.isEmpty(redirectUrl)
+ && ShimUtils.isAtLeastS()) {
+ // If a portal is a non-redirect portal (often portals that return HTTP 200 with a
+ // login page for all HTTP requests), report the probe URL as the login URL starting
+ // from S (b/172048052). This avoids breaking assumptions that
+ // [is a portal] is equivalent to [there is a login URL].
+ redirectUrl = url.toString();
+ }
probeResult = new CaptivePortalProbeResult(httpResponseCode, redirectUrl,
- url.toString(), 1 << probeType);
+ url.toString(), 1 << probeType);
} else {
probeResult = probeSpec.getResult(httpResponseCode, redirectUrl);
}
diff --git a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
index f010007..fa7b89f 100644
--- a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
+++ b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
@@ -155,6 +155,7 @@
import com.android.server.connectivity.nano.DnsEvent;
import com.android.server.connectivity.nano.WifiData;
import com.android.testutils.DevSdkIgnoreRule;
+import com.android.testutils.DevSdkIgnoreRule.IgnoreAfter;
import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo;
import com.android.testutils.HandlerUtils;
@@ -1068,6 +1069,34 @@
runPortalNetworkTest();
}
+ @Test
+ public void testIsCaptivePortal_Http200EmptyResponse() throws Exception {
+ setSslException(mHttpsConnection);
+ setStatus(mHttpConnection, 200);
+ // Invalid if there is no content (can't login to an empty page)
+ runNetworkTest(VALIDATION_RESULT_INVALID, 0 /* probesSucceeded */, null);
+ verify(mCallbacks, never()).showProvisioningNotification(any(), any());
+ }
+
+ private void doCaptivePortal200ResponseTest(String expectedRedirectUrl) throws Exception {
+ setSslException(mHttpsConnection);
+ setStatus(mHttpConnection, 200);
+ doReturn(100L).when(mHttpConnection).getContentLengthLong();
+ // Redirect URL was null before S
+ runNetworkTest(VALIDATION_RESULT_PORTAL, 0 /* probesSucceeded */, expectedRedirectUrl);
+ verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS)).showProvisioningNotification(any(), any());
+ }
+
+ @Test @IgnoreAfter(Build.VERSION_CODES.R)
+ public void testIsCaptivePortal_HttpProbeIs200Portal_R() throws Exception {
+ doCaptivePortal200ResponseTest(null);
+ }
+
+ @Test @IgnoreUpTo(Build.VERSION_CODES.R)
+ public void testIsCaptivePortal_HttpProbeIs200Portal() throws Exception {
+ doCaptivePortal200ResponseTest(TEST_HTTP_URL);
+ }
+
private void setupPrivateIpResponse(String privateAddr) throws Exception {
setSslException(mHttpsConnection);
setPortal302(mHttpConnection);