Merge "Add tests for NetworkMonitor isCaptivePortal" am: 97ff63812c am: aebb44fd8e
am: 238f479b66
Change-Id: I0f93d8a475a949e61e9f2b8e8116c241ae68faaf
diff --git a/services/core/java/com/android/server/connectivity/NetworkMonitor.java b/services/core/java/com/android/server/connectivity/NetworkMonitor.java
index 59beef2..843ba2e 100644
--- a/services/core/java/com/android/server/connectivity/NetworkMonitor.java
+++ b/services/core/java/com/android/server/connectivity/NetworkMonitor.java
@@ -261,7 +261,7 @@
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 @@
// 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,
- NetworkMonitorSettings settings) {
+ NetworkRequest defaultRequest, IpConnectivityLog logger, Dependencies deps) {
// Add suffix indicating which NetworkMonitor we're talking about.
super(TAG + networkAgentInfo.name());
@@ -323,9 +324,9 @@
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 @@
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 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(
- NetworkMonitorSettings settings, Context context) {
- return settings.getSetting(
- context, Settings.Global.CAPTIVE_PORTAL_HTTP_URL, DEFAULT_HTTP_URL);
+ public static String getCaptivePortalServerHttpUrl(Dependencies deps, Context context) {
+ return deps.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 @@
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 @@
}
private String getCaptivePortalUserAgent() {
- return mSettings.getSetting(mContext,
+ return mDependencies.getSetting(mContext,
Settings.Global.CAPTIVE_PORTAL_USER_AGENT, DEFAULT_USER_AGENT);
}
@@ -965,7 +965,7 @@
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 @@
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 @@
}
@VisibleForTesting
- public interface NetworkMonitorSettings {
- int getSetting(Context context, String symbol, int defaultValue);
- String getSetting(Context context, String symbol, String defaultValue);
+ public static class Dependencies {
+ public Network getNetwork(NetworkAgentInfo networkAgentInfo) {
+ 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 @@
final String value = Settings.Global.getString(context.getContentResolver(), symbol);
return value != null ? value : defaultValue;
}
+
+ public static final Dependencies DEFAULT = new Dependencies();
}
}
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 142c88b..e3db7e8 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -880,7 +880,7 @@
NetworkAgentInfo networkAgentInfo, NetworkRequest defaultRequest,
IpConnectivityLog log) {
super(context, handler, networkAgentInfo, defaultRequest, log,
- NetworkMonitor.NetworkMonitorSettings.DEFAULT);
+ NetworkMonitor.Dependencies.DEFAULT);
connectivityHandler = handler;
}
diff --git a/tests/net/java/com/android/server/connectivity/NetworkMonitorTest.java b/tests/net/java/com/android/server/connectivity/NetworkMonitorTest.java
index 27a897d..b017130 100644
--- a/tests/net/java/com/android/server/connectivity/NetworkMonitorTest.java
+++ b/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.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;
- @Mock Handler mHandler;
- @Mock IpConnectivityLog mLogger;
- @Mock NetworkAgentInfo mAgent;
- @Mock NetworkMonitor.NetworkMonitorSettings mSettings;
- @Mock NetworkRequest mRequest;
- @Mock TelephonyManager mTelephony;
- @Mock WifiManager mWifi;
+ private static final String TEST_HTTP_URL = "http://www.google.com/gen_204";
+ private static final String TEST_HTTPS_URL = "https://www.google.com/gen_204";
+ private static final String TEST_FALLBACK_URL = "http://fallback.google.com/gen_204";
+ private static final String TEST_OTHER_FALLBACK_URL = "http://otherfallback.google.com/gen_204";
@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(new Network(TEST_ID));
+ 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(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() {
- NetworkMonitor monitor = makeMonitor();
+ public void testIsCaptivePortal_HttpProbeIsPortal() throws IOException {
+ 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);
}
}