Allow localhost HTTP URLs for the capport API

Allowing the capport API to be hosted on localhost makes it easy to
write fast, stable tests for the feature.
This was not possible because:
 - Pre-validation of the URL used Patterns.WEB_URL, which is
  over-restrictive and excludes domain names without TLD
 - NetworkMonitor needs the API to be hosted via HTTPS which is working
   as intended, however relaxing this requirement only for localhost
   (for testing) seems reasonable.

Bug: 156062304
Test: atest CaptivePortalApiTest in associated change
Original-Change: https://android-review.googlesource.com/1309235
Merged-In: I5f2cdd02376785b152e5b9a6e798d797894ea45b
Change-Id: I5f2cdd02376785b152e5b9a6e798d797894ea45b

3 files changed, 54 insertions, 5 deletions

diff --git a/src/android/net/ip/IpClient.java b/src/android/net/ip/IpClient.java
index 4860ff3..018d6ab 100644
--- a/src/android/net/ip/IpClient.java
+++ b/src/android/net/ip/IpClient.java
@@ -64,7 +64,6 @@ import android.text.TextUtils;
 import android.util.LocalLog;
 import android.util.Log;
 import android.util.Pair;
-import android.util.Patterns;
 import android.util.SparseArray;
 
 import androidx.annotation.NonNull;
@@ -86,6 +85,8 @@ import com.android.server.NetworkStackService.NetworkStackServiceManager;
 import java.io.FileDescriptor;
 import java.io.PrintWriter;
 import java.net.InetAddress;
+import java.net.MalformedURLException;
+import java.net.URL;
 import java.nio.BufferUnderflowException;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
@@ -1263,9 +1264,9 @@ public class IpClient extends StateMachine {
             }
 
             final String capportUrl = mDhcpResults.captivePortalApiUrl;
-            // Uri.parse does no syntax check; do a simple regex check to eliminate garbage.
+            // Uri.parse does no syntax check; do a simple check to eliminate garbage.
             // If the URL is still incorrect data fetching will fail later, which is fine.
-            if (capportUrl != null && Patterns.WEB_URL.matcher(capportUrl).matches()) {
+            if (isParseableUrl(capportUrl)) {
                 NetworkInformationShimImpl.newInstance()
                         .setCaptivePortalApiUrl(newLp, Uri.parse(capportUrl));
             }
@@ -1303,6 +1304,19 @@ public class IpClient extends StateMachine {
         return newLp;
     }
 
+    private static boolean isParseableUrl(String url) {
+        // Verify that a URL has a reasonable format that can be parsed as per the URL constructor.
+        // This does not use Patterns.WEB_URL as that pattern excludes URLs without TLDs, such as on
+        // localhost.
+        if (url == null) return false;
+        try {
+            new URL(url);
+            return true;
+        } catch (MalformedURLException e) {
+            return false;
+        }
+    }
+
     private static void addAllReachableDnsServers(
             LinkProperties lp, Iterable<InetAddress> dnses) {
         // TODO: Investigate deleting this reachability check.  We should be
diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java
index 8a1b0c7..fc7f8c9 100755
--- a/src/com/android/server/connectivity/NetworkMonitor.java
+++ b/src/com/android/server/connectivity/NetworkMonitor.java
@@ -2548,7 +2548,12 @@ public class NetworkMonitor extends StateMachine {
             final String apiContent;
             try {
                 final URL url = new URL(mCaptivePortalApiUrl.toString());
-                if (!"https".equals(url.getProtocol())) {
+                // Protocol must be HTTPS
+                // (as per https://www.ietf.org/id/draft-ietf-capport-api-07.txt, #4).
+                // Only allow HTTP on localhost, for testing.
+                final boolean isLocalhostHttp =
+                        "localhost".equals(url.getHost()) && "http".equals(url.getProtocol());
+                if (!"https".equals(url.getProtocol()) && !isLocalhostHttp) {
                     validationLog("Invalid captive portal API protocol: " + url.getProtocol());
                     return null;
                 }
diff --git a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
index f0a7bfb..4b249ce 100644
--- a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
+++ b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
@@ -1253,7 +1253,7 @@ public class NetworkMonitorTest {
     @Test
     public void testIsCaptivePortal_CapportApiInvalidContent() throws Exception {
         assumeTrue(CaptivePortalDataShimImpl.isSupported());
-        setStatus(mHttpsConnection, 204);
+        setSslException(mHttpsConnection);
         setPortal302(mHttpConnection);
         setApiContent(mCapportApiConnection, "{SomeInvalidText");
         runNetworkTest(makeCapportLPs(), CELL_METERED_CAPABILITIES,
@@ -1264,6 +1264,36 @@ public class NetworkMonitorTest {
         verify(mHttpConnection).getResponseCode();
     }
 
+    private void runCapportApiInvalidUrlTest(String url) throws Exception {
+        assumeTrue(CaptivePortalDataShimImpl.isSupported());
+        setSslException(mHttpsConnection);
+        setPortal302(mHttpConnection);
+        final LinkProperties lp = new LinkProperties(TEST_LINK_PROPERTIES);
+        lp.setCaptivePortalApiUrl(Uri.parse(url));
+        runNetworkTest(makeCapportLPs(), CELL_METERED_CAPABILITIES,
+                VALIDATION_RESULT_PORTAL, 0 /* probesSucceeded */,
+                TEST_LOGIN_URL);
+
+        verify(mCallbacks, never()).notifyCaptivePortalDataChanged(any());
+        verify(mCapportApiConnection, never()).getInputStream();
+        verify(mHttpConnection).getResponseCode();
+    }
+
+    @Test
+    public void testIsCaptivePortal_HttpIsInvalidCapportApiScheme() throws Exception {
+        runCapportApiInvalidUrlTest("http://capport.example.com");
+    }
+
+    @Test
+    public void testIsCaptivePortal_FileIsInvalidCapportApiScheme() throws Exception {
+        runCapportApiInvalidUrlTest("file://localhost/myfile");
+    }
+
+    @Test
+    public void testIsCaptivePortal_InvalidUrlFormat() throws Exception {
+        runCapportApiInvalidUrlTest("ThisIsNotAValidUrl");
+    }
+
     @Test @IgnoreUpTo(Build.VERSION_CODES.Q)
     public void testIsCaptivePortal_CapportApiNotSupported() throws Exception {
         // Test that on a R+ device, if NetworkStack was compiled without CaptivePortalData support