Merge "Add tests for NetworkMonitor isCaptivePortal"

services/core/java/com/android/server/connectivity/NetworkMonitor.java

@@ -261,7 +261,7 @@ public class NetworkMonitor extends StateMachine {
     private final WifiManager mWifiManager;
     private final NetworkRequest mDefaultRequest;
     private final IpConnectivityLog mMetricsLog;
-    private final NetworkMonitorSettings mSettings;
+    private final Dependencies mDependencies;
 
     // Configuration values for captive portal detection probes.
     private final String mCaptivePortalUserAgent;
@@ -301,18 +301,19 @@ public class NetworkMonitor extends StateMachine {
     // This variable is set before transitioning to the mCaptivePortalState.
     private CaptivePortalProbeResult mLastPortalProbeResult = CaptivePortalProbeResult.FAILED;
 
+    // Random generator to select fallback URL index
+    private final Random mRandom;
     private int mNextFallbackUrlIndex = 0;
 
     public NetworkMonitor(Context context, Handler handler, NetworkAgentInfo networkAgentInfo,
             NetworkRequest defaultRequest) {
         this(context, handler, networkAgentInfo, defaultRequest, new IpConnectivityLog(),
-                NetworkMonitorSettings.DEFAULT);
+                Dependencies.DEFAULT);
     }
 
     @VisibleForTesting
     protected NetworkMonitor(Context context, Handler handler, NetworkAgentInfo networkAgentInfo,
-            NetworkRequest defaultRequest, IpConnectivityLog logger,
+            NetworkRequest defaultRequest, IpConnectivityLog logger, Dependencies deps) {
-            NetworkMonitorSettings settings) {
         // Add suffix indicating which NetworkMonitor we're talking about.
         super(TAG + networkAgentInfo.name());
 
@@ -323,9 +324,9 @@ public class NetworkMonitor extends StateMachine {
         mContext = context;
         mMetricsLog = logger;
         mConnectivityServiceHandler = handler;
-        mSettings = settings;
+        mDependencies = deps;
         mNetworkAgentInfo = networkAgentInfo;
-        mNetwork = new OneAddressPerFamilyNetwork(networkAgentInfo.network());
+        mNetwork = deps.getNetwork(networkAgentInfo);
         mNetId = mNetwork.netId;
         mTelephonyManager = (TelephonyManager) context.getSystemService(Context.TELEPHONY_SERVICE);
         mWifiManager = (WifiManager) context.getSystemService(Context.WIFI_SERVICE);
@@ -343,9 +344,10 @@ public class NetworkMonitor extends StateMachine {
         mUseHttps = getUseHttpsValidation();
         mCaptivePortalUserAgent = getCaptivePortalUserAgent();
         mCaptivePortalHttpsUrl = makeURL(getCaptivePortalServerHttpsUrl());
-        mCaptivePortalHttpUrl = makeURL(getCaptivePortalServerHttpUrl(settings, context));
+        mCaptivePortalHttpUrl = makeURL(getCaptivePortalServerHttpUrl(deps, context));
         mCaptivePortalFallbackUrls = makeCaptivePortalFallbackUrls();
         mCaptivePortalFallbackSpecs = makeCaptivePortalFallbackProbeSpecs();
+        mRandom = deps.getRandom();
 
         start();
     }
@@ -883,40 +885,38 @@ public class NetworkMonitor extends StateMachine {
     public boolean getIsCaptivePortalCheckEnabled() {
         String symbol = Settings.Global.CAPTIVE_PORTAL_MODE;
         int defaultValue = Settings.Global.CAPTIVE_PORTAL_MODE_PROMPT;
-        int mode = mSettings.getSetting(mContext, symbol, defaultValue);
+        int mode = mDependencies.getSetting(mContext, symbol, defaultValue);
         return mode != Settings.Global.CAPTIVE_PORTAL_MODE_IGNORE;
     }
 
     public boolean getUseHttpsValidation() {
-        return mSettings.getSetting(mContext, Settings.Global.CAPTIVE_PORTAL_USE_HTTPS, 1) == 1;
+        return mDependencies.getSetting(mContext, Settings.Global.CAPTIVE_PORTAL_USE_HTTPS, 1) == 1;
     }
 
     public boolean getWifiScansAlwaysAvailableDisabled() {
-        return mSettings.getSetting(mContext, Settings.Global.WIFI_SCAN_ALWAYS_AVAILABLE, 0) == 0;
+        return mDependencies.getSetting(mContext, Settings.Global.WIFI_SCAN_ALWAYS_AVAILABLE, 0) == 0;
     }
 
     private String getCaptivePortalServerHttpsUrl() {
-        return mSettings.getSetting(mContext,
+        return mDependencies.getSetting(mContext,
                 Settings.Global.CAPTIVE_PORTAL_HTTPS_URL, DEFAULT_HTTPS_URL);
     }
 
     // Static for direct access by ConnectivityService
     public static String getCaptivePortalServerHttpUrl(Context context) {
-        return getCaptivePortalServerHttpUrl(NetworkMonitorSettings.DEFAULT, context);
+        return getCaptivePortalServerHttpUrl(Dependencies.DEFAULT, context);
     }
 
-    public static String getCaptivePortalServerHttpUrl(
+    public static String getCaptivePortalServerHttpUrl(Dependencies deps, Context context) {
-            NetworkMonitorSettings settings, Context context) {
+        return deps.getSetting(context, Settings.Global.CAPTIVE_PORTAL_HTTP_URL, DEFAULT_HTTP_URL);
-        return settings.getSetting(
-                context, Settings.Global.CAPTIVE_PORTAL_HTTP_URL, DEFAULT_HTTP_URL);
     }
 
     private URL[] makeCaptivePortalFallbackUrls() {
         try {
             String separator = ",";
-            String firstUrl = mSettings.getSetting(mContext,
+            String firstUrl = mDependencies.getSetting(mContext,
                     Settings.Global.CAPTIVE_PORTAL_FALLBACK_URL, DEFAULT_FALLBACK_URL);
-            String joinedUrls = firstUrl + separator + mSettings.getSetting(mContext,
+            String joinedUrls = firstUrl + separator + mDependencies.getSetting(mContext,
                     Settings.Global.CAPTIVE_PORTAL_OTHER_FALLBACK_URLS,
                     DEFAULT_OTHER_FALLBACK_URLS);
             List<URL> urls = new ArrayList<>();
@@ -940,7 +940,7 @@ public class NetworkMonitor extends StateMachine {
 
     private CaptivePortalProbeSpec[] makeCaptivePortalFallbackProbeSpecs() {
         try {
-            final String settingsValue = mSettings.getSetting(
+            final String settingsValue = mDependencies.getSetting(
                     mContext, Settings.Global.CAPTIVE_PORTAL_FALLBACK_PROBE_SPECS, null);
             // Probe specs only used if configured in settings
             if (TextUtils.isEmpty(settingsValue)) {
@@ -956,7 +956,7 @@ public class NetworkMonitor extends StateMachine {
     }
 
     private String getCaptivePortalUserAgent() {
-        return mSettings.getSetting(mContext,
+        return mDependencies.getSetting(mContext,
                 Settings.Global.CAPTIVE_PORTAL_USER_AGENT, DEFAULT_USER_AGENT);
     }
 
@@ -965,7 +965,7 @@ public class NetworkMonitor extends StateMachine {
             return null;
         }
         int idx = Math.abs(mNextFallbackUrlIndex) % mCaptivePortalFallbackUrls.length;
-        mNextFallbackUrlIndex += new Random().nextInt(); // randomely change url without memory.
+        mNextFallbackUrlIndex += mRandom.nextInt(); // randomly change url without memory.
         return mCaptivePortalFallbackUrls[idx];
     }
 
@@ -974,7 +974,7 @@ public class NetworkMonitor extends StateMachine {
             return null;
         }
         // Randomly change spec without memory. Also randomize the first attempt.
-        final int idx = Math.abs(new Random().nextInt()) % mCaptivePortalFallbackSpecs.length;
+        final int idx = Math.abs(mRandom.nextInt()) % mCaptivePortalFallbackSpecs.length;
         return mCaptivePortalFallbackSpecs[idx];
     }
 
@@ -1392,15 +1392,15 @@ public class NetworkMonitor extends StateMachine {
     }
 
     @VisibleForTesting
-    public interface NetworkMonitorSettings {
+    public static class Dependencies {
-        int getSetting(Context context, String symbol, int defaultValue);
+        public Network getNetwork(NetworkAgentInfo networkAgentInfo) {
-        String getSetting(Context context, String symbol, String defaultValue);
+            return new OneAddressPerFamilyNetwork(networkAgentInfo.network());
+        }
 
-        static NetworkMonitorSettings DEFAULT = new DefaultNetworkMonitorSettings();
+        public Random getRandom() {
-    }
+            return new Random();
+        }
 
-    @VisibleForTesting
-    public static class DefaultNetworkMonitorSettings implements NetworkMonitorSettings {
         public int getSetting(Context context, String symbol, int defaultValue) {
             return Settings.Global.getInt(context.getContentResolver(), symbol, defaultValue);
         }
@@ -1409,5 +1409,7 @@ public class NetworkMonitor extends StateMachine {
             final String value = Settings.Global.getString(context.getContentResolver(), symbol);
             return value != null ? value : defaultValue;
         }
+
+        public static final Dependencies DEFAULT = new Dependencies();
     }
 }

tests/net/java/com/android/server/ConnectivityServiceTest.java

@@ -880,7 +880,7 @@ public class ConnectivityServiceTest {
                 NetworkAgentInfo networkAgentInfo, NetworkRequest defaultRequest,
                 IpConnectivityLog log) {
             super(context, handler, networkAgentInfo, defaultRequest, log,
-                    NetworkMonitor.NetworkMonitorSettings.DEFAULT);
+                    NetworkMonitor.Dependencies.DEFAULT);
             connectivityHandler = handler;
         }
 

tests/net/java/com/android/server/connectivity/NetworkMonitorTest.java

@@ -16,22 +16,35 @@
 
 package com.android.server.connectivity;
 
-import static org.junit.Assert.assertEquals;
+import static junit.framework.Assert.assertFalse;
+
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.anyInt;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import android.content.Context;
+import android.net.ConnectivityManager;
+import android.net.LinkProperties;
 import android.net.Network;
+import android.net.NetworkCapabilities;
+import android.net.NetworkInfo;
 import android.net.NetworkRequest;
+import android.net.captiveportal.CaptivePortalProbeResult;
 import android.net.metrics.IpConnectivityLog;
 import android.net.wifi.WifiManager;
 import android.os.Handler;
+import android.provider.Settings;
 import android.support.test.filters.SmallTest;
 import android.support.test.runner.AndroidJUnit4;
 import android.telephony.TelephonyManager;
+import android.util.ArrayMap;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -39,38 +52,273 @@ import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
+import java.io.IOException;
+import java.net.HttpURLConnection;
+import java.net.InetAddress;
+import java.net.URL;
+import java.util.Random;
+
+import javax.net.ssl.SSLHandshakeException;
+
 
 @RunWith(AndroidJUnit4.class)
 @SmallTest
 public class NetworkMonitorTest {
+    private static final String LOCATION_HEADER = "location";
 
-    static final int TEST_ID = 60; // should be less than min netid 100
+    private @Mock Context mContext;
+    private @Mock Handler mHandler;
+    private @Mock IpConnectivityLog mLogger;
+    private @Mock NetworkAgentInfo mAgent;
+    private @Mock NetworkInfo mNetworkInfo;
+    private @Mock NetworkRequest mRequest;
+    private @Mock TelephonyManager mTelephony;
+    private @Mock WifiManager mWifi;
+    private @Mock Network mNetwork;
+    private @Mock HttpURLConnection mHttpConnection;
+    private @Mock HttpURLConnection mHttpsConnection;
+    private @Mock HttpURLConnection mFallbackConnection;
+    private @Mock HttpURLConnection mOtherFallbackConnection;
+    private @Mock Random mRandom;
+    private @Mock NetworkMonitor.Dependencies mDependencies;
 
-    @Mock Context mContext;
+    private static final String TEST_HTTP_URL = "http://www.google.com/gen_204";
-    @Mock Handler mHandler;
+    private static final String TEST_HTTPS_URL = "https://www.google.com/gen_204";
-    @Mock IpConnectivityLog mLogger;
+    private static final String TEST_FALLBACK_URL = "http://fallback.google.com/gen_204";
-    @Mock NetworkAgentInfo mAgent;
+    private static final String TEST_OTHER_FALLBACK_URL = "http://otherfallback.google.com/gen_204";
-    @Mock NetworkMonitor.NetworkMonitorSettings mSettings;
-    @Mock NetworkRequest mRequest;
-    @Mock TelephonyManager mTelephony;
-    @Mock WifiManager mWifi;
 
     @Before
-    public void setUp() {
+    public void setUp() throws IOException {
         MockitoAnnotations.initMocks(this);
+        mAgent.linkProperties = new LinkProperties();
+        mAgent.networkCapabilities = new NetworkCapabilities()
+                .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR);
+        mAgent.networkInfo = mNetworkInfo;
+
+        when(mAgent.network()).thenReturn(mNetwork);
+        when(mDependencies.getNetwork(any())).thenReturn(mNetwork);
+        when(mDependencies.getRandom()).thenReturn(mRandom);
+        when(mDependencies.getSetting(any(), eq(Settings.Global.CAPTIVE_PORTAL_MODE), anyInt()))
+                .thenReturn(Settings.Global.CAPTIVE_PORTAL_MODE_PROMPT);
+        when(mDependencies.getSetting(any(), eq(Settings.Global.CAPTIVE_PORTAL_USE_HTTPS),
+                anyInt())).thenReturn(1);
+        when(mDependencies.getSetting(any(), eq(Settings.Global.CAPTIVE_PORTAL_HTTP_URL),
+                anyString())).thenReturn(TEST_HTTP_URL);
+        when(mDependencies.getSetting(any(), eq(Settings.Global.CAPTIVE_PORTAL_HTTPS_URL),
+                anyString())).thenReturn(TEST_HTTPS_URL);
 
-        when(mAgent.network()).thenReturn(new Network(TEST_ID));
         when(mContext.getSystemService(Context.TELEPHONY_SERVICE)).thenReturn(mTelephony);
         when(mContext.getSystemService(Context.WIFI_SERVICE)).thenReturn(mWifi);
+
+        when(mNetworkInfo.getType()).thenReturn(ConnectivityManager.TYPE_WIFI);
+        setFallbackUrl(TEST_FALLBACK_URL);
+        setOtherFallbackUrls(TEST_OTHER_FALLBACK_URL);
+        setFallbackSpecs(null); // Test with no fallback spec by default
+        when(mRandom.nextInt()).thenReturn(0);
+
+        when(mNetwork.openConnection(any())).then((invocation) -> {
+            URL url = invocation.getArgument(0);
+            switch(url.toString()) {
+                case TEST_HTTP_URL:
+                    return mHttpConnection;
+                case TEST_HTTPS_URL:
+                    return mHttpsConnection;
+                case TEST_FALLBACK_URL:
+                    return mFallbackConnection;
+                case TEST_OTHER_FALLBACK_URL:
+                    return mOtherFallbackConnection;
+                default:
+                    fail("URL not mocked: " + url.toString());
+                    return null;
+            }
+        });
+        when(mHttpConnection.getRequestProperties()).thenReturn(new ArrayMap<>());
+        when(mHttpsConnection.getRequestProperties()).thenReturn(new ArrayMap<>());
+        when(mNetwork.getAllByName(any())).thenReturn(new InetAddress[] {
+            InetAddress.parseNumericAddress("192.168.0.0")
+        });
     }
 
     NetworkMonitor makeMonitor() {
-        return new NetworkMonitor(mContext, mHandler, mAgent, mRequest, mLogger, mSettings);
+        return new NetworkMonitor(
+                mContext, mHandler, mAgent, mRequest, mLogger, mDependencies);
     }
 
     @Test
-    public void testCreatingNetworkMonitor() {
+    public void testIsCaptivePortal_HttpProbeIsPortal() throws IOException {
-        NetworkMonitor monitor = makeMonitor();
+        setSslException(mHttpsConnection);
+        setPortal302(mHttpConnection);
+
+        assertPortal(makeMonitor().isCaptivePortal());
+    }
+
+    @Test
+    public void testIsCaptivePortal_HttpsProbeIsNotPortal() throws IOException {
+        setStatus(mHttpsConnection, 204);
+        setStatus(mHttpConnection, 500);
+
+        assertNotPortal(makeMonitor().isCaptivePortal());
+    }
+
+    @Test
+    public void testIsCaptivePortal_HttpsProbeFailedHttpSuccessNotUsed() throws IOException {
+        setSslException(mHttpsConnection);
+        // Even if HTTP returns a 204, do not use the result unless HTTPS succeeded
+        setStatus(mHttpConnection, 204);
+        setStatus(mFallbackConnection, 500);
+
+        assertFailed(makeMonitor().isCaptivePortal());
+    }
+
+    @Test
+    public void testIsCaptivePortal_FallbackProbeIsPortal() throws IOException {
+        setSslException(mHttpsConnection);
+        setStatus(mHttpConnection, 500);
+        setPortal302(mFallbackConnection);
+
+        assertPortal(makeMonitor().isCaptivePortal());
+    }
+
+    @Test
+    public void testIsCaptivePortal_FallbackProbeIsNotPortal() throws IOException {
+        setSslException(mHttpsConnection);
+        setStatus(mHttpConnection, 500);
+        setStatus(mFallbackConnection, 204);
+
+        // Fallback probe did not see portal, HTTPS failed -> inconclusive
+        assertFailed(makeMonitor().isCaptivePortal());
+    }
+
+    @Test
+    public void testIsCaptivePortal_OtherFallbackProbeIsPortal() throws IOException {
+        // Set all fallback probes but one to invalid URLs to verify they are being skipped
+        setFallbackUrl(TEST_FALLBACK_URL);
+        setOtherFallbackUrls(TEST_FALLBACK_URL + "," + TEST_OTHER_FALLBACK_URL);
+
+        setSslException(mHttpsConnection);
+        setStatus(mHttpConnection, 500);
+        setStatus(mFallbackConnection, 500);
+        setPortal302(mOtherFallbackConnection);
+
+        // TEST_OTHER_FALLBACK_URL is third
+        when(mRandom.nextInt()).thenReturn(2);
+
+        final NetworkMonitor monitor = makeMonitor();
+
+        // First check always uses the first fallback URL: inconclusive
+        assertFailed(monitor.isCaptivePortal());
+        verify(mFallbackConnection, times(1)).getResponseCode();
+        verify(mOtherFallbackConnection, never()).getResponseCode();
+
+        // Second check uses the URL chosen by Random
+        assertPortal(monitor.isCaptivePortal());
+        verify(mOtherFallbackConnection, times(1)).getResponseCode();
+    }
+
+    @Test
+    public void testIsCaptivePortal_AllProbesFailed() throws IOException {
+        setSslException(mHttpsConnection);
+        setStatus(mHttpConnection, 500);
+        setStatus(mFallbackConnection, 404);
+
+        assertFailed(makeMonitor().isCaptivePortal());
+        verify(mFallbackConnection, times(1)).getResponseCode();
+        verify(mOtherFallbackConnection, never()).getResponseCode();
+    }
+
+    @Test
+    public void testIsCaptivePortal_InvalidUrlSkipped() throws IOException {
+        setFallbackUrl("invalid");
+        setOtherFallbackUrls("otherinvalid," + TEST_OTHER_FALLBACK_URL + ",yetanotherinvalid");
+
+        setSslException(mHttpsConnection);
+        setStatus(mHttpConnection, 500);
+        setPortal302(mOtherFallbackConnection);
+
+        assertPortal(makeMonitor().isCaptivePortal());
+        verify(mOtherFallbackConnection, times(1)).getResponseCode();
+        verify(mFallbackConnection, never()).getResponseCode();
+    }
+
+    private void setupFallbackSpec() throws IOException {
+        setFallbackSpecs("http://example.com@@/@@204@@/@@"
+                + "@@,@@"
+                + TEST_OTHER_FALLBACK_URL + "@@/@@30[12]@@/@@https://(www\\.)?google.com/?.*");
+
+        setSslException(mHttpsConnection);
+        setStatus(mHttpConnection, 500);
+
+        // Use the 2nd fallback spec
+        when(mRandom.nextInt()).thenReturn(1);
+    }
+
+    @Test
+    public void testIsCaptivePortal_FallbackSpecIsNotPortal() throws IOException {
+        setupFallbackSpec();
+        set302(mOtherFallbackConnection, "https://www.google.com/test?q=3");
+
+        // HTTPS failed, fallback spec did not see a portal -> inconclusive
+        assertFailed(makeMonitor().isCaptivePortal());
+        verify(mOtherFallbackConnection, times(1)).getResponseCode();
+        verify(mFallbackConnection, never()).getResponseCode();
+    }
+
+    @Test
+    public void testIsCaptivePortal_FallbackSpecIsPortal() throws IOException {
+        setupFallbackSpec();
+        set302(mOtherFallbackConnection, "http://login.portal.example.com");
+
+        assertPortal(makeMonitor().isCaptivePortal());
+    }
+
+    private void setFallbackUrl(String url) {
+        when(mDependencies.getSetting(any(),
+                eq(Settings.Global.CAPTIVE_PORTAL_FALLBACK_URL), any())).thenReturn(url);
+    }
+
+    private void setOtherFallbackUrls(String urls) {
+        when(mDependencies.getSetting(any(),
+                eq(Settings.Global.CAPTIVE_PORTAL_OTHER_FALLBACK_URLS), any())).thenReturn(urls);
+    }
+
+    private void setFallbackSpecs(String specs) {
+        when(mDependencies.getSetting(any(),
+                eq(Settings.Global.CAPTIVE_PORTAL_FALLBACK_PROBE_SPECS), any())).thenReturn(specs);
+    }
+
+    private void assertPortal(CaptivePortalProbeResult result) {
+        assertTrue(result.isPortal());
+        assertFalse(result.isFailed());
+        assertFalse(result.isSuccessful());
+    }
+
+    private void assertNotPortal(CaptivePortalProbeResult result) {
+        assertFalse(result.isPortal());
+        assertFalse(result.isFailed());
+        assertTrue(result.isSuccessful());
+    }
+
+    private void assertFailed(CaptivePortalProbeResult result) {
+        assertFalse(result.isPortal());
+        assertTrue(result.isFailed());
+        assertFalse(result.isSuccessful());
+    }
+
+    private void setSslException(HttpURLConnection connection) throws IOException {
+        when(connection.getResponseCode()).thenThrow(new SSLHandshakeException("Invalid cert"));
+    }
+
+    private void set302(HttpURLConnection connection, String location) throws IOException {
+        setStatus(connection, 302);
+        when(connection.getHeaderField(LOCATION_HEADER)).thenReturn(location);
+    }
+
+    private void setPortal302(HttpURLConnection connection) throws IOException {
+        set302(connection, "http://login.example.com");
+    }
+
+    private void setStatus(HttpURLConnection connection, int status) throws IOException {
+        when(connection.getResponseCode()).thenReturn(status);
     }
 }