Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c679ffa4 authored by Remi NGUYEN VAN's avatar Remi NGUYEN VAN Committed by Automerger Merge Worker
Browse files

Allow localhost HTTP URLs for the capport API am: 6bed0e59 am: faa4cf07

Original change: undetermined

Change-Id: I947f8205e2ca2de1dc838d2b27fa23faae83051e
parents 489f4739 faa4cf07
Loading
Loading
Loading
Loading
+17 −3
Original line number Diff line number Diff line
@@ -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
+6 −1
Original line number Diff line number Diff line
@@ -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;
                }
+31 −1
Original line number Diff line number Diff line
@@ -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