commit c6eed076e074ffdcb5a97b5cbb9acf9bc553a006
author Remi NGUYEN VAN <reminv@google.com> Fri May 28 01:30:30 2021 +0000
committer Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> Fri May 28 01:30:30 2021 +0000
tree 54b2d1472e1c1f62134a3a6166049beee0101848
parent ad6d165eb2d84f83b1613236545ad31ebff58ddb
parent 369109536d0fdcd449e91b7dad64ad8a26174c11

Report probe URL as redirect on 200 portals am: a7500731bf am: 369109536d

Original change: https://android-review.googlesource.com/c/platform/packages/modules/NetworkStack/+/1719397

Change-Id: I2b09aea6eb732cb894bdb487b25ea17cb46d72c0

common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeResult.java

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() {

src/com/android/server/connectivity/NetworkMonitor.java

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);
         }

tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java

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);